diff --git a/scm-webapp/src/main/java/sonia/scm/security/PermissionAssigner.java b/scm-webapp/src/main/java/sonia/scm/security/PermissionAssigner.java index 2821460e55..b7874add69 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/PermissionAssigner.java +++ b/scm-webapp/src/main/java/sonia/scm/security/PermissionAssigner.java @@ -17,6 +17,7 @@ public class PermissionAssigner { } public Collection getAvailablePermissions() { + PermissionPermissions.read().check(); return securitySystem.getAvailablePermissions(); } @@ -37,6 +38,7 @@ public class PermissionAssigner { } private Set readPermissions(Predicate predicate) { + PermissionPermissions.read().check(); return securitySystem.getPermissions(predicate) .stream() .map(AssignedPermission::getPermission) @@ -54,6 +56,7 @@ public class PermissionAssigner { } private void adaptPermissions(String id, boolean groupPermission, Collection permissions, Collection existingPermissions) { + PermissionPermissions.assign().check(); List toRemove = existingPermissions.stream() .filter(p -> !permissions.contains(p.getPermission())) .collect(Collectors.toList()); diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultSecuritySystemTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultSecuritySystemTest.java index e9b4a0ae00..457bb96ae4 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultSecuritySystemTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultSecuritySystemTest.java @@ -221,22 +221,6 @@ public class DefaultSecuritySystemTest extends AbstractTestBase securitySystem.deletePermission(sap); } - /** - * Method description - * - */ - @Test(expected = UnauthorizedException.class) - public void testUnauthorizedGetPermission() - { - setAdminSubject(); - - createPermission("trillian", false, - "repository:*:READ"); - - setUserSubject(); - securitySystem.getPermissions(p -> true); - } - /** * Method description * diff --git a/scm-webapp/src/test/java/sonia/scm/security/PermissionAssignerTest.java b/scm-webapp/src/test/java/sonia/scm/security/PermissionAssignerTest.java index f698f24bfb..a5eb32b594 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PermissionAssignerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PermissionAssignerTest.java @@ -2,10 +2,12 @@ package sonia.scm.security; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; +import org.apache.shiro.authz.UnauthorizedException; import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import sonia.scm.plugin.PluginLoader; import sonia.scm.store.InMemoryConfigurationEntryStoreFactory; import sonia.scm.util.ClassLoaders; @@ -22,6 +24,9 @@ public class PermissionAssignerTest { @Rule public ShiroRule shiroRule = new ShiroRule(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + private DefaultSecuritySystem securitySystem; private PermissionAssigner permissionAssigner; @@ -32,10 +37,14 @@ public class PermissionAssignerTest { securitySystem = new DefaultSecuritySystem(new InMemoryConfigurationEntryStoreFactory(), pluginLoader); - securitySystem.addPermission(new AssignedPermission("1", "perm:read:1")); - securitySystem.addPermission(new AssignedPermission("1", "perm:read:2")); - securitySystem.addPermission(new AssignedPermission("2", "perm:read:2")); - securitySystem.addPermission(new AssignedPermission("1", true, "perm:read:2")); + try { + securitySystem.addPermission(new AssignedPermission("1", "perm:read:1")); + securitySystem.addPermission(new AssignedPermission("1", "perm:read:2")); + securitySystem.addPermission(new AssignedPermission("2", "perm:read:2")); + securitySystem.addPermission(new AssignedPermission("1", true, "perm:read:2")); + } catch (UnauthorizedException e) { + // ignore for tests with limited privileges + } permissionAssigner = new PermissionAssigner(securitySystem); } @@ -46,6 +55,21 @@ public class PermissionAssignerTest { Assertions.assertThat(permissionDescriptors).hasSize(2); } + @Test + public void shouldFindGroupPermissions() { + Collection permissionDescriptors = permissionAssigner.readPermissionsForUser("1"); + + Assertions.assertThat(permissionDescriptors).hasSize(2); + } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldNotReadUserPermissionsForUnprivilegedUser() { + expectedException.expect(UnauthorizedException.class); + + permissionAssigner.readPermissionsForUser("1"); + } + @Test public void shouldOverwriteUserPermissions() { permissionAssigner.setPermissionsForUser("2", asList(new PermissionDescriptor("perm:read:3"), new PermissionDescriptor("perm:read:4"))); @@ -54,4 +78,12 @@ public class PermissionAssignerTest { Assertions.assertThat(permissionDescriptors).hasSize(2); } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldNotOverwriteUserPermissionsForUnprivilegedUser() { + expectedException.expect(UnauthorizedException.class); + + permissionAssigner.setPermissionsForUser("2", asList(new PermissionDescriptor("perm:read:3"), new PermissionDescriptor("perm:read:4"))); + } }