Fix permission checks

scm-webapp/src/main/java/sonia/scm/security/PermissionAssigner.java

@@ -17,6 +17,7 @@ public class PermissionAssigner {
   }
 
   public Collection<PermissionDescriptor> getAvailablePermissions() {
+    PermissionPermissions.read().check();
     return securitySystem.getAvailablePermissions();
   }
 
@@ -37,6 +38,7 @@ public class PermissionAssigner {
   }
 
   private Set<PermissionDescriptor> readPermissions(Predicate<AssignedPermission> 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<PermissionDescriptor> permissions, Collection<AssignedPermission> existingPermissions) {
+    PermissionPermissions.assign().check();
     List<AssignedPermission> toRemove = existingPermissions.stream()
       .filter(p -> !permissions.contains(p.getPermission()))
       .collect(Collectors.toList());

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
    *

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"));
+    try {
-    securitySystem.addPermission(new AssignedPermission("1", "perm:read:2"));
+      securitySystem.addPermission(new AssignedPermission("1", "perm:read:1"));
-    securitySystem.addPermission(new AssignedPermission("2", "perm:read:2"));
+      securitySystem.addPermission(new AssignedPermission("1", "perm:read:2"));
-    securitySystem.addPermission(new AssignedPermission("1", true, "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<PermissionDescriptor> 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")));
+  }
 }