From 45ca558101867f4bc087828a34cb00de3896c0aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 6 May 2019 16:01:01 +0200 Subject: [PATCH] Get verbs from repository roles --- .../DefaultAuthorizationCollector.java | 46 ++++++++----- .../DefaultAuthorizationCollectorTest.java | 64 ++++++++++++++++--- 2 files changed, 86 insertions(+), 24 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java index f4efd3a307..f07cdce900 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -52,7 +52,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; -import sonia.scm.config.ScmConfiguration; import sonia.scm.group.GroupNames; import sonia.scm.group.GroupPermissions; import sonia.scm.plugin.Extension; @@ -64,7 +63,6 @@ import sonia.scm.user.UserPermissions; import sonia.scm.util.Util; import java.util.Collection; -import java.util.Set; //~--- JDK imports ------------------------------------------------------------ @@ -90,18 +88,19 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector /** * Constructs ... - * - * @param cacheManager + * @param cacheManager * @param repositoryDAO * @param securitySystem + * @param repositoryPermissionProvider */ @Inject public DefaultAuthorizationCollector(CacheManager cacheManager, - RepositoryDAO repositoryDAO, SecuritySystem securitySystem) + RepositoryDAO repositoryDAO, SecuritySystem securitySystem, RepositoryPermissionProvider repositoryPermissionProvider) { this.cache = cacheManager.getCache(CACHE_NAME); this.repositoryDAO = repositoryDAO; this.securitySystem = securitySystem; + this.repositoryPermissionProvider = repositoryPermissionProvider; } //~--- methods -------------------------------------------------------------- @@ -201,16 +200,8 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector for (RepositoryPermission permission : repositoryPermissions) { hasPermission = isUserPermitted(user, groups, permission); - if (hasPermission && !permission.getVerbs().isEmpty()) - { - String perm = "repository:" + String.join(",", permission.getVerbs()) + ":" + repository.getId(); - if (logger.isTraceEnabled()) - { - logger.trace("add repository permission {} for user {} at repository {}", - perm, user.getName(), repository.getName()); - } - - builder.add(perm); + if (hasPermission) { + addRepositoryPermission(builder, repository, user, hasPermission, permission); } } @@ -226,6 +217,29 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector } } + private void addRepositoryPermission(Builder builder, Repository repository, User user, boolean hasPermission, RepositoryPermission permission) { + Collection verbs = getVerbs(permission); + if (!verbs.isEmpty()) + { + String perm = "repository:" + String.join(",", verbs) + ":" + repository.getId(); + if (logger.isTraceEnabled()) + { + logger.trace("add repository permission {} for user {} at repository {}", + perm, user.getName(), repository.getName()); + } + + builder.add(perm); + } + } + + private Collection getVerbs(RepositoryPermission permission) { + return permission.getRole() == null? permission.getVerbs(): getVerbsForRole(permission.getRole()); + } + + private Collection getVerbsForRole(String roleName) { + return repositoryPermissionProvider.availableRoles().stream().filter(role -> roleName.equals(role.getName())).findFirst().orElseThrow(() -> new RuntimeException()).getVerbs(); + } + private AuthorizationInfo createAuthorizationInfo(User user, GroupNames groups) { Builder builder = ImmutableSet.builder(); @@ -353,4 +367,6 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector /** security system */ private final SecuritySystem securitySystem; + + private final RepositoryPermissionProvider repositoryPermissionProvider; } diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java index dc62119209..bf49c12005 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java @@ -33,10 +33,10 @@ package sonia.scm.security; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.SimpleAuthorizationInfo; +import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; import org.hamcrest.Matchers; @@ -49,11 +49,11 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; -import sonia.scm.config.ScmConfiguration; import sonia.scm.group.GroupNames; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryDAO; import sonia.scm.repository.RepositoryPermission; +import sonia.scm.repository.RepositoryRole; import sonia.scm.repository.RepositoryTestData; import sonia.scm.user.User; import sonia.scm.user.UserTestData; @@ -90,6 +90,9 @@ public class DefaultAuthorizationCollectorTest { @Mock private SecuritySystem securitySystem; + @Mock + private RepositoryPermissionProvider repositoryPermissionProvider; + private DefaultAuthorizationCollector collector; @Rule @@ -101,11 +104,11 @@ public class DefaultAuthorizationCollectorTest { @Before public void setUp(){ when(cacheManager.getCache(Mockito.any(String.class))).thenReturn(cache); - collector = new DefaultAuthorizationCollector(cacheManager, repositoryDAO, securitySystem); + collector = new DefaultAuthorizationCollector(cacheManager, repositoryDAO, securitySystem, repositoryPermissionProvider); } /** - * Tests {@link AuthorizationCollector#collect()} without user role. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} without user role. */ @Test @SubjectAware @@ -118,7 +121,7 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#collect()} from cache. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} from cache. */ @Test @SubjectAware( @@ -134,7 +137,7 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#collect()} with cache. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} with cache. */ @Test @SubjectAware( @@ -148,7 +151,7 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#collect()} without permissions. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} without permissions. */ @Test @SubjectAware( @@ -165,7 +168,7 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#collect()} with repository permissions. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} with repository permissions. */ @Test @SubjectAware( @@ -191,7 +194,50 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#collect()} with global permissions. + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} with repository roles. + */ + @Test + @SubjectAware( + configuration = "classpath:sonia/scm/shiro-001.ini" + ) + public void testCollectWithRepositoryRolePermissions() { + when(repositoryPermissionProvider.availableRoles()).thenReturn( + asList( + new RepositoryRole("user role", asList("user"), "xml"), + new RepositoryRole("group role", asList("group"), "xml"), + new RepositoryRole("system role", asList("system"), "system") + )); + + String group = "heart-of-gold-crew"; + authenticate(UserTestData.createTrillian(), group); + Repository heartOfGold = RepositoryTestData.createHeartOfGold(); + heartOfGold.setId("one"); + heartOfGold.setPermissions(Lists.newArrayList( + new RepositoryPermission("trillian", "user role", false), + new RepositoryPermission("trillian", "system role", false) + )); + Repository puzzle42 = RepositoryTestData.create42Puzzle(); + puzzle42.setId("two"); + RepositoryPermission permission = new RepositoryPermission(group, "group role", true); + puzzle42.setPermissions(Lists.newArrayList(permission)); + when(repositoryDAO.getAll()).thenReturn(Lists.newArrayList(heartOfGold, puzzle42)); + + // execute and assert + AuthorizationInfo authInfo = collector.collect(); + assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER)); + assertThat(authInfo.getObjectPermissions(), nullValue()); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder( + "user:autocomplete", + "group:autocomplete", + "user:changePassword:trillian", + "repository:user:one", + "repository:system:one", + "repository:group:two", + "user:read:trillian")); + } + + /** + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} with global permissions. */ @Test @SubjectAware(