From: Simon Brandhof Date: Tue, 31 Jan 2017 21:29:28 +0000 (+0100) Subject: SONAR-8716 drop UserSession#hasComponentPermission(permission, componentKey) X-Git-Tag: 6.3-RC1~260 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bc641282c50e5aab8939c475b733169f0b75b341;p=sonarqube.git SONAR-8716 drop UserSession#hasComponentPermission(permission, componentKey) --- diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java index b2ed1cfc861..285b787f83d 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java @@ -121,11 +121,6 @@ public class CeUserSession implements UserSession { return notImplementedBooleanMethod(); } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - return notImplementedBooleanMethod(); - } - @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { return notImplementedBooleanMethod(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionPrivilegeChecker.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionPrivilegeChecker.java index db8c4a6fd31..5474d58056c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionPrivilegeChecker.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionPrivilegeChecker.java @@ -37,13 +37,9 @@ public class PermissionPrivilegeChecker { .checkOrganizationPermission(organizationUuid, SYSTEM_ADMIN); } - /** - * @deprecated does not support organizations. Replaced by {@link #checkProjectAdmin(UserSession, String, Optional)} - */ - @Deprecated - public static void checkProjectAdminUserByComponentKey(UserSession userSession, @Nullable String componentKey) { + public static void checkProjectAdminUserByComponentUuid(UserSession userSession, @Nullable String componentUuid) { userSession.checkLoggedIn(); - if (componentKey == null || !userSession.hasComponentPermission(UserRole.ADMIN, componentKey)) { + if (componentUuid == null || !userSession.hasComponentUuidPermission(UserRole.ADMIN, componentUuid)) { userSession.checkPermission(SYSTEM_ADMIN); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java index cac9319c624..398c6c3486b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateService.java @@ -31,12 +31,10 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.resources.Qualifiers; import org.sonar.api.server.ServerSide; import org.sonar.core.component.ComponentKeys; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.util.stream.Collectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; -import org.sonar.db.component.ResourceDto; import org.sonar.db.organization.DefaultTemplates; import org.sonar.db.permission.GroupPermissionDto; import org.sonar.db.permission.UserPermissionDto; @@ -54,8 +52,6 @@ import static java.lang.String.format; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.sonar.api.security.DefaultGroups.isAnyone; -import static org.sonar.server.permission.PermissionPrivilegeChecker.checkProjectAdminUserByComponentKey; -import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; @ServerSide public class PermissionTemplateService { @@ -72,26 +68,7 @@ public class PermissionTemplateService { this.userSession = userSession; this.defaultTemplatesResolver = defaultTemplatesResolver; } - - /** - * @deprecated replaced by {@link #applyDefault(DbSession, String, ComponentDto, Long)}, which does not - * verify that user is authorized to administrate the component. - */ - @Deprecated - public void applyDefaultPermissionTemplate(DbSession session, String organizationUuid, String componentKey) { - ComponentDto component = checkFoundWithOptional(dbClient.componentDao().selectByKey(session, componentKey), "Component key '%s' not found", componentKey); - ResourceDto provisioned = dbClient.resourceDao().selectProvisionedProject(session, componentKey); - if (provisioned == null) { - checkProjectAdminUserByComponentKey(userSession, componentKey); - } else { - userSession.checkPermission(GlobalPermissions.PROVISIONING); - } - - Integer currentUserId = userSession.getUserId(); - Long userId = Qualifiers.PROJECT.equals(component.qualifier()) && currentUserId != null ? currentUserId.longValue() : null; - applyDefault(session, organizationUuid, component, userId); - } - + public boolean wouldUserHavePermissionWithDefaultTemplate(DbSession dbSession, String organizationUuid, @Nullable Long userId, String permission, @Nullable String branch, String projectKey, String qualifier) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/project/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/project/ws/DeleteAction.java index 607e13722c8..989fb51d2ff 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/project/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/project/ws/DeleteAction.java @@ -19,7 +19,6 @@ */ package org.sonar.server.project.ws; -import javax.annotation.Nullable; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; @@ -75,30 +74,23 @@ public class DeleteAction implements ProjectsWsAction { @Override public void handle(Request request, Response response) throws Exception { + // fail-fast if not logged in + userSession.checkLoggedIn(); String uuid = request.param(PARAM_ID); String key = request.param(PARAM_KEY); - checkPermissions(uuid, key); try (DbSession dbSession = dbClient.openSession(false)) { ComponentDto project = componentFinder.getByUuidOrKey(dbSession, uuid, key, ID_AND_KEY); + checkPermission(project); componentCleanerService.delete(dbSession, project); } response.noContent(); } - private void checkPermissions(@Nullable String uuid, @Nullable String key) { - if (missPermissionsBasedOnUuid(uuid) || missPermissionsBasedOnKey(key)) { - userSession.checkLoggedIn().checkPermission(GlobalPermissions.SYSTEM_ADMIN); + private void checkPermission(ComponentDto project) { + if (!userSession.hasComponentPermission(UserRole.ADMIN, project)) { + userSession.checkOrganizationPermission(project.getOrganizationUuid(), GlobalPermissions.SYSTEM_ADMIN); } } - - private boolean missPermissionsBasedOnKey(@Nullable String key) { - return key != null && !userSession.hasComponentPermission(UserRole.ADMIN, key) && !userSession.hasPermission(GlobalPermissions.SYSTEM_ADMIN); - } - - private boolean missPermissionsBasedOnUuid(@Nullable String uuid) { - return uuid != null && !userSession.hasComponentUuidPermission(UserRole.ADMIN, uuid) && !userSession.hasPermission(GlobalPermissions.SYSTEM_ADMIN); - } - } 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 d5242f0af6b..a2aac204a78 100644 --- 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 @@ -25,8 +25,8 @@ import java.util.List; import java.util.Set; import org.sonar.api.security.DefaultGroups; import org.sonar.core.permission.GlobalPermissions; -import org.sonar.db.user.GroupDto; import org.sonar.db.component.ComponentDto; +import org.sonar.db.user.GroupDto; /** * Allow code to be executed with the highest privileges possible, as if executed by a {@link GlobalPermissions#SYSTEM_ADMIN} account. @@ -124,11 +124,6 @@ public final class DoPrivileged { return true; } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - return true; - } - @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { 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 1923dbd19e3..bc866fa866f 100644 --- 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 @@ -49,8 +49,6 @@ import static java.util.Objects.requireNonNull; * Implementation of {@link UserSession} used in web server */ public class ServerUserSession extends AbstractUserSession { - private Map projectKeyByComponentKey = newHashMap(); - @CheckForNull private final UserDto userDto; private final DbClient dbClient; @@ -61,7 +59,6 @@ public class ServerUserSession extends AbstractUserSession { private SetMultimap projectUuidByPermission = HashMultimap.create(); private SetMultimap permissionsByOrganizationUuid; private Map projectUuidByComponentUuid = newHashMap(); - private List projectPermissionsCheckedByKey = new ArrayList<>(); private List projectPermissionsCheckedByUuid = new ArrayList<>(); private ServerUserSession(DbClient dbClient, @Nullable UserDto userDto) { @@ -164,44 +161,6 @@ public class ServerUserSession extends AbstractUserSession { return globalPermissions; } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - if (isRoot() || hasPermission(permission)) { - return true; - } - - String projectKey = projectKeyByComponentKey.get(componentKey); - if (projectKey == null) { - ResourceDto project = resourceDao.getRootProjectByComponentKey(componentKey); - if (project == null) { - return false; - } - projectKey = project.getKey(); - } - boolean hasComponentPermission = hasProjectPermission(permission, projectKey); - if (hasComponentPermission) { - projectKeyByComponentKey.put(componentKey, projectKey); - return true; - } - return false; - } - - private boolean hasProjectPermission(String permission, String projectKey) { - if (isRoot()) { - return true; - } - if (!projectPermissionsCheckedByKey.contains(permission)) { - try (DbSession dbSession = dbClient.openSession(false)) { - Collection projectKeys = dbClient.authorizationDao().selectAuthorizedRootProjectsKeys(dbSession, getUserId(), permission); - for (String key : projectKeys) { - projectKeyByPermission.put(permission, key); - } - projectPermissionsCheckedByKey.add(permission); - } - } - return projectKeyByPermission.get(permission).contains(projectKey); - } - @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { if (isRoot() || hasPermission(permission)) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java index 34607091b75..97761aebe56 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java @@ -137,11 +137,6 @@ public class ThreadLocalUserSession implements UserSession { return get().hasComponentPermission(permission, component); } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - return get().hasComponentPermission(permission, componentKey); - } - @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { return get().hasComponentUuidPermission(permission, componentUuid); diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java index 772293bfe6b..9425b43fbf9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java @@ -135,15 +135,6 @@ public interface UserSession { */ boolean hasComponentPermission(String permission, ComponentDto component); - /** - * Does the user have the given permission for a component key ? - * - * First, check if the user has the global permission (even if the component doesn't exist) - * If not, check is the user has the permission on the project of the component - * If the component doesn't exist, return false - */ - boolean hasComponentPermission(String permission, String componentKey); - /** * Does the user have the given project permission for a component uuid ? diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java index 9f0a8f4500d..9e95d53f8c6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java @@ -34,6 +34,7 @@ import org.sonar.db.component.ComponentDto; import org.sonar.server.component.ComponentCleanerService; import org.sonar.server.component.ComponentFinder; import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; @@ -76,7 +77,7 @@ public class DeleteActionTest { } @Test - public void global_admin_deletes_project_by_uuid() throws Exception { + public void global_admin_deletes_project_by_id() throws Exception { ComponentDto project = componentDbTester.insertProject(); userSessionRule.login().setGlobalPermissions(UserRole.ADMIN); @@ -115,7 +116,7 @@ public class DeleteActionTest { @Test public void project_administrator_deletes_the_project_by_key() throws Exception { ComponentDto project = componentDbTester.insertProject(); - userSessionRule.login().addProjectPermissions(UserRole.ADMIN, project.key()); + userSessionRule.login().addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); call(newRequest().setParam(PARAM_KEY, project.key())); @@ -123,11 +124,23 @@ public class DeleteActionTest { } @Test - public void fail_if_insufficient_privileges() throws Exception { - userSessionRule.login().setGlobalPermissions(UserRole.CODEVIEWER, UserRole.ISSUE_ADMIN, UserRole.USER); + public void return_403_if_not_project_admin_nor_org_admin() throws Exception { + ComponentDto project = componentDbTester.insertProject(); + + userSessionRule.login().addProjectUuidPermissions(project.uuid(), UserRole.CODEVIEWER, UserRole.ISSUE_ADMIN, UserRole.USER); expectedException.expect(ForbiddenException.class); - call(newRequest().setParam(PARAM_ID, "whatever-the-uuid")); + call(newRequest().setParam(PARAM_ID, project.uuid())); + } + + @Test + public void return_401_if_not_logged_in() throws Exception { + ComponentDto project = componentDbTester.insertProject(); + + userSessionRule.anonymous(); + expectedException.expect(UnauthorizedException.class); + + call(newRequest().setParam(PARAM_ID, project.uuid())); } private WsTester.TestRequest newRequest() { 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 6d5fcb00e82..e60f4624d40 100644 --- 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 @@ -45,7 +45,6 @@ public abstract class AbstractMockUserSession private Map projectUuidByComponentUuid = newHashMap(); private List projectPermissionsCheckedByKey = newArrayList(); private List projectPermissionsCheckedByUuid = newArrayList(); - private Map projectKeyByComponentKey = newHashMap(); protected AbstractMockUserSession(Class clazz) { this.clazz = clazz; @@ -75,9 +74,6 @@ public abstract class AbstractMockUserSession public T addProjectPermissions(String projectPermission, String... projectKeys) { this.projectPermissionsCheckedByKey.add(projectPermission); this.projectKeyByPermission.putAll(projectPermission, newArrayList(projectKeys)); - for (String projectKey : projectKeys) { - this.projectKeyByComponentKey.put(projectKey, projectKey); - } return clazz.cast(this); } @@ -95,7 +91,6 @@ public abstract class AbstractMockUserSession */ @Deprecated public T addComponentPermission(String projectPermission, String projectKey, String componentKey) { - this.projectKeyByComponentKey.put(componentKey, projectKey); addProjectPermissions(projectPermission, projectKey); return clazz.cast(this); } @@ -116,17 +111,7 @@ public abstract class AbstractMockUserSession return hasComponentUuidPermission(permission, component.projectUuid()); } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - String projectKey = projectKeyByComponentKey.get(componentKey); - return hasPermission(permission) || (projectKey != null && hasProjectPermission(permission, projectKey)); - } - - private boolean hasProjectPermission(String permission, String projectKey) { - return projectPermissionsCheckedByKey.contains(permission) && projectKeyByPermission.get(permission).contains(projectKey); - } - - @Override + @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { String projectUuid = projectUuidByComponentUuid.get(componentUuid); return hasPermission(permission) || (projectUuid != null && hasProjectPermissionByUuid(permission, projectUuid)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java index 6a6333bec99..f09ad6c98b5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java @@ -261,11 +261,6 @@ public class UserSessionRule implements TestRule, UserSession { return currentUserSession.hasComponentPermission(permission, component); } - @Override - public boolean hasComponentPermission(String permission, String componentKey) { - return currentUserSession.hasComponentPermission(permission, componentKey); - } - @Override public boolean hasComponentUuidPermission(String permission, String componentUuid) { return currentUserSession.hasComponentUuidPermission(permission, componentUuid); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/DoPrivilegedTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/DoPrivilegedTest.java index 484351094e7..907da4fd757 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/DoPrivilegedTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/DoPrivilegedTest.java @@ -21,6 +21,7 @@ package org.sonar.server.user; import org.junit.Before; import org.junit.Test; +import org.sonar.db.component.ComponentDto; import org.sonar.server.tester.MockUserSession; import static org.assertj.core.api.Assertions.assertThat; @@ -47,14 +48,14 @@ public class DoPrivilegedTest { // verify the session used inside Privileged task assertThat(catcher.userSession.isLoggedIn()).isFalse(); assertThat(catcher.userSession.hasPermission("any permission")).isTrue(); - assertThat(catcher.userSession.hasComponentPermission("any permission", "any project")).isTrue(); + assertThat(catcher.userSession.hasComponentPermission("any permission", new ComponentDto())).isTrue(); // verify session in place after task is done assertThat(threadLocalUserSession.get()).isSameAs(session); } @Test - public void should_lose_privileges_on_exception() { + public void should_loose_privileges_on_exception() { UserSessionCatcherTask catcher = new UserSessionCatcherTask() { @Override protected void doPrivileged() { @@ -73,7 +74,7 @@ public class DoPrivilegedTest { // verify the session used inside Privileged task assertThat(catcher.userSession.isLoggedIn()).isFalse(); assertThat(catcher.userSession.hasPermission("any permission")).isTrue(); - assertThat(catcher.userSession.hasComponentPermission("any permission", "any project")).isTrue(); + assertThat(catcher.userSession.hasComponentPermission("any permission", new ComponentDto())).isTrue(); } } 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 963568ff6f6..3055af6d5a3 100644 --- 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 @@ -179,16 +179,6 @@ public class ServerUserSessionTest { assertThat(underTest.checkPermission("whatever!")).isSameAs(underTest); } - @Test - public void has_component_permission() { - addProjectPermissions(project, UserRole.USER); - UserSession session = newUserSession(userDto); - - assertThat(session.hasComponentPermission(UserRole.USER, FILE_KEY)).isTrue(); - assertThat(session.hasComponentPermission(UserRole.CODEVIEWER, FILE_KEY)).isFalse(); - assertThat(session.hasComponentPermission(UserRole.ADMIN, FILE_KEY)).isFalse(); - } - @Test public void hasComponentUuidPermission_returns_true_if_user_has_project_permission_for_given_uuid_in_db() { addProjectPermissions(project, UserRole.USER); @@ -209,16 +199,6 @@ public class ServerUserSessionTest { assertThat(underTest.hasComponentUuidPermission("whatever", "who cares?")).isTrue(); } - @Test - public void hasComponentPermission_returns_true_if_user_has_global_permission_in_db() { - addGlobalPermissions(UserRole.USER); - UserSession session = newUserSession(userDto); - - assertThat(session.hasComponentPermission(UserRole.USER, FILE_KEY)).isTrue(); - assertThat(session.hasComponentPermission(UserRole.CODEVIEWER, FILE_KEY)).isFalse(); - assertThat(session.hasComponentPermission(UserRole.ADMIN, FILE_KEY)).isFalse(); - } - @Test public void has_component_uuid_permission_with_only_global_permission() { addGlobalPermissions(UserRole.USER); @@ -274,16 +254,6 @@ public class ServerUserSessionTest { assertThat(session.hasPermission(GlobalPermissions.QUALITY_GATE_ADMIN)).isFalse(); } - @Test - public void has_project_permission_for_anonymous() throws Exception { - addAnyonePermissions(organization, project, UserRole.USER); - UserSession session = newAnonymousSession(); - - assertThat(session.hasComponentPermission(UserRole.USER, FILE_KEY)).isTrue(); - assertThat(session.hasComponentPermission(UserRole.CODEVIEWER, FILE_KEY)).isFalse(); - assertThat(session.hasComponentPermission(UserRole.ADMIN, FILE_KEY)).isFalse(); - } - @Test public void checkOrganizationPermission_fails_with_ForbiddenException_when_user_has_no_permissions_on_organization() { expectInsufficientPrivilegesForbiddenException();