SONAR-8761 root has permissions on all organizations and components

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java
index c4dc9e856d705c30a6f6305fc1f0ecadbcee6cab..ceed194d26b5439e078643526b57be8a5b7168dd 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/AbstractUserSession.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.user;
 
+import java.util.Optional;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.UnauthorizedException;
@@ -29,16 +30,15 @@ public abstract class AbstractUserSession implements UserSession {
   private static final String AUTHENTICATION_IS_REQUIRED_MESSAGE = "Authentication is required";
 
   @Override
-  public UserSession checkLoggedIn() {
+  public final UserSession checkLoggedIn() {
     if (!isLoggedIn()) {
       throw new UnauthorizedException(AUTHENTICATION_IS_REQUIRED_MESSAGE);
     }
     return this;
   }
 
-
   @Override
-  public UserSession checkIsRoot() {
+  public final UserSession checkIsRoot() {
     if (!isRoot()) {
       throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
     }
@@ -46,10 +46,14 @@ public abstract class AbstractUserSession implements UserSession {
   }
 
   @Override
-  public UserSession checkOrganizationPermission(String organizationUuid, String permission) {
-    if (isRoot()) {
-      return this;
-    }
+  public final boolean hasOrganizationPermission(String organizationUuid, String permission) {
+    return isRoot() || hasOrganizationPermissionImpl(organizationUuid, permission);
+  }
+
+  protected abstract boolean hasOrganizationPermissionImpl(String organizationUuid, String permission);
+
+  @Override
+  public final UserSession checkOrganizationPermission(String organizationUuid, String permission) {
     if (!hasOrganizationPermission(organizationUuid, permission)) {
       throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
     }
@@ -57,12 +61,27 @@ public abstract class AbstractUserSession implements UserSession {
   }
 
   @Override
-  public boolean hasComponentPermission(String permission, ComponentDto component) {
-    return hasComponentUuidPermission(permission, component.projectUuid());
+  public final boolean hasComponentPermission(String permission, ComponentDto component) {
+    return isRoot() || hasProjectUuidPermission(permission, component.projectUuid());
   }
 
   @Override
-  public UserSession checkComponentPermission(String projectPermission, ComponentDto component) {
+  public final boolean hasComponentUuidPermission(String permission, String componentUuid) {
+    if (isRoot()) {
+      return true;
+    }
+    Optional<String> projectUuid = componentUuidToProjectUuid(componentUuid);
+    return projectUuid
+      .map(s -> hasProjectUuidPermission(permission, s))
+      .orElse(false);
+  }
+
+  protected abstract Optional<String> componentUuidToProjectUuid(String componentUuid);
+
+  protected abstract boolean hasProjectUuidPermission(String permission, String projectUuid);
+
+  @Override
+  public final UserSession checkComponentPermission(String projectPermission, ComponentDto component) {
     if (!hasComponentPermission(projectPermission, component)) {
       throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
     }
@@ -70,7 +89,7 @@ public abstract class AbstractUserSession implements UserSession {
   }
 
   @Override
-  public UserSession checkComponentUuidPermission(String permission, String componentUuid) {
+  public final UserSession checkComponentUuidPermission(String permission, String componentUuid) {
     if (!hasComponentUuidPermission(permission, componentUuid)) {
       throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
     }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java b/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
index e911fc3247886701196f30eeca38a94bb649aaed..dfed5a8ee7630c37556374523ef14faad530e37d 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
@@ -21,8 +21,8 @@ package org.sonar.server.user;
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Optional;
 import org.sonar.core.permission.GlobalPermissions;
-import org.sonar.db.component.ComponentDto;
 import org.sonar.db.user.GroupDto;
 
 /**
@@ -97,17 +97,18 @@ public final class DoPrivileged {
       }
 
       @Override
-      public boolean hasOrganizationPermission(String organizationUuid, String permission) {
+      protected boolean hasOrganizationPermissionImpl(String organizationUuid, String permission) {
         return true;
       }
 
       @Override
-      public boolean hasComponentPermission(String permission, ComponentDto component) {
-        return true;
+      protected Optional<String> componentUuidToProjectUuid(String componentUuid) {
+        // always root so unused
+        throw new UnsupportedOperationException();
       }
 
       @Override
-      public boolean hasComponentUuidPermission(String permission, String componentUuid) {
+      protected boolean hasProjectUuidPermission(String permission, String projectUuid) {
         return true;
       }
     }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
index f5176bf863ba6845452ee87498ae0780aad59682..8b8224fa08456b71c167744ff77c441b61d72a38 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.server.user;
 
-import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.HashMultimap;
@@ -29,6 +28,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -112,7 +112,7 @@ public class ServerUserSession extends AbstractUserSession {
   }
 
   @Override
-  public boolean hasOrganizationPermission(String organizationUuid, String permission) {
+  protected boolean hasOrganizationPermissionImpl(String organizationUuid, String permission) {
     if (permissionsByOrganizationUuid == null) {
       permissionsByOrganizationUuid = HashMultimap.create();
     }
@@ -136,33 +136,27 @@ public class ServerUserSession extends AbstractUserSession {
   }
 
   @Override
-  public boolean hasComponentUuidPermission(String permission, String componentUuid) {
-    if (isRoot()) {
-      return true;
-    }
-
+  protected Optional<String> componentUuidToProjectUuid(String componentUuid) {
     String projectUuid = projectUuidByComponentUuid.get(componentUuid);
-    if (projectUuid == null) {
-      try (DbSession dbSession = dbClient.openSession(false)) {
-        Optional<ComponentDto> component = dbClient.componentDao().selectByUuid(dbSession, componentUuid);
-        if (!component.isPresent()) {
-          return false;
-        }
-        projectUuid = component.get().projectUuid();
-      }
+    if (projectUuid != null) {
+      return Optional.of(projectUuid);
     }
-    boolean hasComponentPermission = hasProjectPermissionByUuid(permission, projectUuid);
-    if (hasComponentPermission) {
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      com.google.common.base.Optional<ComponentDto> component = dbClient.componentDao().selectByUuid(dbSession, componentUuid);
+      if (!component.isPresent()) {
+        return Optional.empty();
+      }
+      projectUuid = component.get().projectUuid();
       projectUuidByComponentUuid.put(componentUuid, projectUuid);
-      return true;
+      return Optional.of(projectUuid);
     }
-    return false;
   }
 
-  // To keep private
-  private boolean hasProjectPermissionByUuid(String permission, String projectUuid) {
+  @Override
+  protected boolean hasProjectUuidPermission(String permission, String projectUuid) {
     if (!projectPermissionsCheckedByUuid.contains(permission)) {
       try (DbSession dbSession = dbClient.openSession(false)) {
+        // FIXME do not load all the authorized projects. It can be huge.
         Collection<String> projectUuids = dbClient.authorizationDao().selectAuthorizedRootProjectsUuids(dbSession, getUserId(), permission);
         addProjectPermission(permission, projectUuids);
       }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/AbstractMockUserSession.java b/server/sonar-server/src/test/java/org/sonar/server/tester/AbstractMockUserSession.java
index 1a4fa0fbc648321e253cfd838c818ee76ba94bd1..641319a053e139e6a432c0d036bf9d5735fc48d7 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/tester/AbstractMockUserSession.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/tester/AbstractMockUserSession.java
@@ -22,7 +22,7 @@ package org.sonar.server.tester;
 import com.google.common.collect.HashMultimap;
 import java.util.List;
 import java.util.Map;
-import org.sonar.db.component.ComponentDto;
+import java.util.Optional;
 import org.sonar.server.user.AbstractUserSession;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -55,26 +55,18 @@ public abstract class AbstractMockUserSession<T extends AbstractMockUserSession>
   }
 
   @Override
-  public boolean hasComponentPermission(String permission, ComponentDto component) {
-    return isRoot() || hasComponentUuidPermission(permission, component.projectUuid());
+  protected boolean hasOrganizationPermissionImpl(String organizationUuid, String permission) {
+    return permissionsByOrganizationUuid.get(organizationUuid).contains(permission);
   }
 
   @Override
-  public boolean hasComponentUuidPermission(String permission, String componentUuid) {
-    if (isRoot()) {
-      return true;
-    }
-    String projectUuid = projectUuidByComponentUuid.get(componentUuid);
-    return projectUuid != null && hasProjectPermissionByUuid(permission, projectUuid);
-  }
-
-  private boolean hasProjectPermissionByUuid(String permission, String projectUuid) {
-    return projectPermissionsCheckedByUuid.contains(permission) && projectUuidByPermission.get(permission).contains(projectUuid);
+  protected Optional<String> componentUuidToProjectUuid(String componentUuid) {
+    return Optional.ofNullable(projectUuidByComponentUuid.get(componentUuid));
   }
 
   @Override
-  public boolean hasOrganizationPermission(String organizationUuid, String permission) {
-    return isRoot() || permissionsByOrganizationUuid.get(organizationUuid).contains(permission);
+  protected boolean hasProjectUuidPermission(String permission, String projectUuid) {
+    return projectPermissionsCheckedByUuid.contains(permission) && projectUuidByPermission.get(permission).contains(projectUuid);
   }
 
   public T addOrganizationPermission(String organizationUuid, String permission) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
index f57130e1f3db47fddf030b4a2512a8ecc802dfcf..3e61b16544472baf7cc9f5b27eb7f2a22f51bdab 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
@@ -26,7 +26,6 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.System2;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
@@ -37,6 +36,8 @@ import org.sonar.db.user.UserDto;
 import org.sonar.server.exceptions.ForbiddenException;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.core.permission.GlobalPermissions.PROVISIONING;
+import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
 import static org.sonar.db.user.UserTesting.newUserDto;
 import static org.sonar.server.user.ServerUserSession.createForAnonymous;
 import static org.sonar.server.user.ServerUserSession.createForUser;
@@ -183,34 +184,53 @@ public class ServerUserSessionTest {
   }
 
   @Test
-  public void checkOrganizationPermission_fails_with_ForbiddenException_when_user_has_no_permissions_on_organization() {
+  public void checkOrganizationPermission_throws_ForbiddenException_when_user_doesnt_have_the_specified_permission_on_organization() {
+    OrganizationDto org = db.organizations().insert();
+    db.users().insertUser(NON_ROOT_USER_DTO);
+
     expectInsufficientPrivilegesForbiddenException();
 
-    newUserSession(NON_ROOT_USER_DTO).checkOrganizationPermission("org-uuid", "perm1");
+    newUserSession(NON_ROOT_USER_DTO).checkOrganizationPermission(org.getUuid(), PROVISIONING);
+  }
+
+  @Test
+  public void checkOrganizationPermission_succeeds_when_user_has_the_specified_permission_on_organization() {
+    OrganizationDto org = db.organizations().insert();
+    db.users().insertUser(NON_ROOT_USER_DTO);
+    db.users().insertPermissionOnUser(org, NON_ROOT_USER_DTO, PROVISIONING);
+
+    newUserSession(NON_ROOT_USER_DTO).checkOrganizationPermission(org.getUuid(), PROVISIONING);
+  }
+
+  @Test
+  public void checkOrganizationPermission_succeeds_when_user_is_root() {
+    OrganizationDto org = db.organizations().insert();
+
+    newUserSession(ROOT_USER_DTO).checkOrganizationPermission(org.getUuid(), PROVISIONING);
   }
 
   @Test
   public void hasOrganizationPermission_for_logged_in_user() {
     OrganizationDto org = db.organizations().insert();
     ComponentDto project = db.components().insertProject(org);
-    db.users().insertPermissionOnUser(org, userDto, GlobalPermissions.PROVISIONING);
+    db.users().insertPermissionOnUser(org, userDto, PROVISIONING);
     db.users().insertProjectPermissionOnUser(userDto, UserRole.ADMIN, project);
 
     UserSession session = newUserSession(userDto);
-    assertThat(session.hasOrganizationPermission(org.getUuid(), GlobalPermissions.PROVISIONING)).isTrue();
-    assertThat(session.hasOrganizationPermission(org.getUuid(), GlobalPermissions.SYSTEM_ADMIN)).isFalse();
-    assertThat(session.hasOrganizationPermission("another-org", GlobalPermissions.PROVISIONING)).isFalse();
+    assertThat(session.hasOrganizationPermission(org.getUuid(), PROVISIONING)).isTrue();
+    assertThat(session.hasOrganizationPermission(org.getUuid(), SYSTEM_ADMIN)).isFalse();
+    assertThat(session.hasOrganizationPermission("another-org", PROVISIONING)).isFalse();
   }
 
   @Test
-  public void hasOrganizationPermission_for_anonymous_user() {
+  public void test_hasOrganizationPermission_for_anonymous_user() {
     OrganizationDto org = db.organizations().insert();
-    db.users().insertPermissionOnAnyone(org, GlobalPermissions.PROVISIONING);
+    db.users().insertPermissionOnAnyone(org, PROVISIONING);
 
     UserSession session = newAnonymousSession();
-    assertThat(session.hasOrganizationPermission(org.getUuid(), GlobalPermissions.PROVISIONING)).isTrue();
-    assertThat(session.hasOrganizationPermission(org.getUuid(), GlobalPermissions.SYSTEM_ADMIN)).isFalse();
-    assertThat(session.hasOrganizationPermission("another-org", GlobalPermissions.PROVISIONING)).isFalse();
+    assertThat(session.hasOrganizationPermission(org.getUuid(), PROVISIONING)).isTrue();
+    assertThat(session.hasOrganizationPermission(org.getUuid(), SYSTEM_ADMIN)).isFalse();
+    assertThat(session.hasOrganizationPermission("another-org", PROVISIONING)).isFalse();
   }
 
   private ServerUserSession newUserSession(UserDto userDto) {