From 86cbb7d1c0ba6bc7de42ba216230f91e23ff1ae1 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 28 Oct 2013 20:50:25 +0100 Subject: [PATCH] SONAR-4819 A project admin can no more manage permissions on his project --- .../permission/InternalPermissionService.java | 31 ++++++- .../InternalPermissionTemplateService.java | 45 ++++++++--- .../permission/PermissionTemplateUpdater.java | 18 ++++- .../org/sonar/server/user/UserSession.java | 34 +++++++- .../app/controllers/roles_controller.rb | 29 ++++++- .../server/issue/IssueCommentServiceTest.java | 2 +- .../InternalPermissionServiceTest.java | 81 ++++++++++++++++--- ...InternalPermissionTemplateServiceTest.java | 31 ++++++- .../PermissionTemplateUpdaterTest.java | 4 +- .../server/user/DefaultUserServiceTest.java | 8 +- .../sonar/server/user/MockUserSession.java | 14 +++- .../sonar/server/user/UserSessionTest.java | 53 +++++++++--- 12 files changed, 292 insertions(+), 58 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java index 09c359ac04a..b07f8b2308e 100644 --- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java +++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java @@ -24,6 +24,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.ServerComponent; import org.sonar.api.security.DefaultGroups; +import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.permission.PermissionFacade; @@ -34,10 +35,10 @@ import org.sonar.core.user.GroupDto; import org.sonar.core.user.UserDao; import org.sonar.core.user.UserDto; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.user.UserSession; import javax.annotation.Nullable; - import java.util.List; import java.util.Map; @@ -86,7 +87,7 @@ public class InternalPermissionService implements ServerComponent { ResourceDto provisioned = resourceDao.selectProvisionedProject(componentKey); if (provisioned == null) { - UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + checkProjectAdminPermission(component.getId()); } else { UserSession.get().checkGlobalPermission(GlobalPermissions.PROVISIONING); } @@ -96,9 +97,18 @@ public class InternalPermissionService implements ServerComponent { public void applyPermissionTemplate(Map params) { UserSession.get().checkLoggedIn(); - UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + ApplyPermissionTemplateQuery query = ApplyPermissionTemplateQuery.buildFromParams(params); query.validate(); + + // If only one project is selected, check user has admin permission on it, otherwise we are in the case of a bulk change and only system admin has permission to do it + if (query.getSelectedComponents().size() == 1) { + checkProjectAdminPermission(Long.parseLong(query.getSelectedComponents().get(0))); + } else { + checkProjectAdminPermission(null); + UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + } + for (String component : query.getSelectedComponents()) { permissionFacade.applyPermissionTemplate(query.getTemplateKey(), Long.parseLong(component)); } @@ -106,7 +116,6 @@ public class InternalPermissionService implements ServerComponent { private void changePermission(String permissionChange, Map params) { UserSession.get().checkLoggedIn(); - UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); PermissionChangeQuery permissionChangeQuery = PermissionChangeQuery.buildFromParams(params); permissionChangeQuery.validate(); applyPermissionChange(permissionChange, permissionChangeQuery); @@ -122,6 +131,8 @@ public class InternalPermissionService implements ServerComponent { private void applyGroupPermissionChange(String operation, PermissionChangeQuery permissionChangeQuery) { Long componentId = getComponentId(permissionChangeQuery.component()); + checkProjectAdminPermission(componentId); + List existingPermissions = permissionFacade.selectGroupPermissions(permissionChangeQuery.group(), componentId); if (shouldSkipPermissionChange(operation, existingPermissions, permissionChangeQuery)) { LOG.info("Skipping permission change '{} {}' for group {} as it matches the current permission scheme", @@ -138,6 +149,8 @@ public class InternalPermissionService implements ServerComponent { private void applyUserPermissionChange(String operation, PermissionChangeQuery permissionChangeQuery) { Long componentId = getComponentId(permissionChangeQuery.component()); + checkProjectAdminPermission(componentId); + List existingPermissions = permissionFacade.selectUserPermissions(permissionChangeQuery.user(), componentId); if (shouldSkipPermissionChange(operation, existingPermissions, permissionChangeQuery)) { LOG.info("Skipping permission change '{} {}' for user {} as it matches the current permission scheme", @@ -190,4 +203,14 @@ public class InternalPermissionService implements ServerComponent { throw new BadRequestException(String.format(NOT_FOUND_FORMAT, objectType, objectKey)); } } + + private void checkProjectAdminPermission(@Nullable Long componentId) { + if (componentId == null) { + UserSession.get().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + } else { + if (!UserSession.get().hasGlobalPermission(GlobalPermissions.SYSTEM_ADMIN) && !UserSession.get().hasProjectPermission(UserRole.ADMIN, componentId)) { + throw new ForbiddenException("Insufficient privileges"); + } + } + } } diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java index 063544a921e..e7835f53809 100644 --- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java +++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java @@ -27,13 +27,15 @@ import org.slf4j.LoggerFactory; import org.sonar.api.ServerComponent; import org.sonar.core.permission.PermissionTemplateDao; import org.sonar.core.permission.PermissionTemplateDto; +import org.sonar.core.resource.ResourceDao; +import org.sonar.core.resource.ResourceDto; +import org.sonar.core.resource.ResourceQuery; import org.sonar.core.user.UserDao; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ServerErrorException; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - import java.util.List; /** @@ -45,24 +47,30 @@ public class InternalPermissionTemplateService implements ServerComponent { private final PermissionTemplateDao permissionTemplateDao; private final UserDao userDao; + private final ResourceDao resourceDao; - public InternalPermissionTemplateService(PermissionTemplateDao permissionTemplateDao, UserDao userDao) { + public InternalPermissionTemplateService(PermissionTemplateDao permissionTemplateDao, UserDao userDao, ResourceDao resourceDao) { this.permissionTemplateDao = permissionTemplateDao; this.userDao = userDao; + this.resourceDao = resourceDao; } @CheckForNull public PermissionTemplate selectPermissionTemplate(String templateName) { - PermissionTemplateUpdater.checkUserCredentials(); + PermissionTemplateUpdater.checkSystemAdminUser(); PermissionTemplateDto permissionTemplateDto = permissionTemplateDao.selectPermissionTemplate(templateName); return PermissionTemplate.create(permissionTemplateDto); } public List selectAllPermissionTemplates() { - PermissionTemplateUpdater.checkUserCredentials(); + return selectAllPermissionTemplates(null); + } + + public List selectAllPermissionTemplates(@Nullable String componentKey) { + PermissionTemplateUpdater.checkProjectAdminUser(getComponentId(componentKey)); List permissionTemplates = Lists.newArrayList(); List permissionTemplateDtos = permissionTemplateDao.selectAllPermissionTemplates(); - if(permissionTemplateDtos != null) { + if (permissionTemplateDtos != null) { for (PermissionTemplateDto permissionTemplateDto : permissionTemplateDtos) { permissionTemplates.add(PermissionTemplate.create(permissionTemplateDto)); } @@ -71,10 +79,10 @@ public class InternalPermissionTemplateService implements ServerComponent { } public PermissionTemplate createPermissionTemplate(String name, @Nullable String description) { - PermissionTemplateUpdater.checkUserCredentials(); + PermissionTemplateUpdater.checkSystemAdminUser(); validateTemplateName(null, name); PermissionTemplateDto permissionTemplateDto = permissionTemplateDao.createPermissionTemplate(name, description); - if(permissionTemplateDto.getId() == null) { + if (permissionTemplateDto.getId() == null) { String errorMsg = "Template creation failed"; LOG.error(errorMsg); throw new ServerErrorException(errorMsg); @@ -83,13 +91,13 @@ public class InternalPermissionTemplateService implements ServerComponent { } public void updatePermissionTemplate(Long templateId, String newName, @Nullable String newDescription) { - PermissionTemplateUpdater.checkUserCredentials(); + PermissionTemplateUpdater.checkSystemAdminUser(); validateTemplateName(templateId, newName); permissionTemplateDao.updatePermissionTemplate(templateId, newName, newDescription); } public void deletePermissionTemplate(Long templateId) { - PermissionTemplateUpdater.checkUserCredentials(); + PermissionTemplateUpdater.checkSystemAdminUser(); permissionTemplateDao.deletePermissionTemplate(templateId); } @@ -138,18 +146,31 @@ public class InternalPermissionTemplateService implements ServerComponent { } private void validateTemplateName(Long templateId, String templateName) { - if(StringUtils.isNullOrEmpty(templateName)) { + if (StringUtils.isNullOrEmpty(templateName)) { String errorMsg = "Name can't be blank"; throw new BadRequestException(errorMsg); } List existingTemplates = permissionTemplateDao.selectAllPermissionTemplates(); - if(existingTemplates != null) { + if (existingTemplates != null) { for (PermissionTemplateDto existingTemplate : existingTemplates) { - if((templateId == null || !existingTemplate.getId().equals(templateId)) && (existingTemplate.getName().equals(templateName))) { + if ((templateId == null || !existingTemplate.getId().equals(templateId)) && (existingTemplate.getName().equals(templateName))) { String errorMsg = "A template with that name already exists"; throw new BadRequestException(errorMsg); } } } } + + @Nullable + private Long getComponentId(String componentKey) { + if (componentKey == null) { + return null; + } else { + ResourceDto resourceDto = resourceDao.getResource(ResourceQuery.create().setKey(componentKey)); + if (resourceDto == null) { + throw new BadRequestException("Project does not exists."); + } + return resourceDto.getId(); + } + } } diff --git a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java index bcd4cff759d..bdaf6376ff7 100644 --- a/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java +++ b/sonar-server/src/main/java/org/sonar/server/permission/PermissionTemplateUpdater.java @@ -30,8 +30,10 @@ import org.sonar.core.user.GroupDto; import org.sonar.core.user.UserDao; import org.sonar.core.user.UserDto; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.user.UserSession; +import javax.annotation.Nullable; import java.util.List; abstract class PermissionTemplateUpdater { @@ -51,7 +53,7 @@ abstract class PermissionTemplateUpdater { } void executeUpdate() { - checkUserCredentials(); + checkSystemAdminUser(); Long templateId = getTemplateId(templateName); validatePermission(permission); doExecute(templateId, permission); @@ -78,10 +80,20 @@ abstract class PermissionTemplateUpdater { return groupDto.getId(); } - static void checkUserCredentials() { + static void checkSystemAdminUser() { + checkProjectAdminUser(null); + } + + static void checkProjectAdminUser(@Nullable Long projectId) { UserSession currentSession = UserSession.get(); currentSession.checkLoggedIn(); - currentSession.checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + if (projectId == null) { + currentSession.checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + } else { + if (!currentSession.hasGlobalPermission(GlobalPermissions.SYSTEM_ADMIN) && !currentSession.hasProjectPermission(UserRole.ADMIN, projectId)) { + throw new ForbiddenException("Insufficient privileges"); + } + } } private void validatePermission(String permission) { diff --git a/sonar-server/src/main/java/org/sonar/server/user/UserSession.java b/sonar-server/src/main/java/org/sonar/server/user/UserSession.java index a0814b9834c..6dd4938c837 100644 --- a/sonar-server/src/main/java/org/sonar/server/user/UserSession.java +++ b/sonar-server/src/main/java/org/sonar/server/user/UserSession.java @@ -21,6 +21,7 @@ package org.sonar.server.user; import com.google.common.base.Objects; import com.google.common.base.Strings; +import com.google.common.collect.HashMultimap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.core.permission.GlobalPermissions; @@ -31,11 +32,13 @@ import org.sonar.server.platform.Platform; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Locale; +import static com.google.common.collect.Lists.newArrayList; + /** * Part of the current HTTP session */ @@ -50,6 +53,9 @@ public class UserSession { private Locale locale = Locale.ENGLISH; List globalPermissions = null; + HashMultimap projectIdByPermission = HashMultimap.create(); + List projectPermissions = newArrayList(); + UserSession() { } @@ -94,7 +100,7 @@ public class UserSession { } /** - * Ensures that user implies the specified permission. If not a {@link org.sonar.server.exceptions.ForbiddenException} is thrown. + * Ensures that user implies the specified global permission. If not a {@link org.sonar.server.exceptions.ForbiddenException} is thrown. */ public UserSession checkGlobalPermission(String globalPermission) { if (!hasGlobalPermission(globalPermission)) { @@ -125,6 +131,30 @@ public class UserSession { return globalPermissions; } + /** + * Ensures that user implies the specified project permission. If not a {@link org.sonar.server.exceptions.ForbiddenException} is thrown. + */ + public UserSession checkProjectPermission(String projectPermission, Long componentId) { + if (!hasProjectPermission(projectPermission, componentId)) { + throw new ForbiddenException("Insufficient privileges"); + } + return this; + } + + /** + * Does the user have the given project permission ? + */ + public boolean hasProjectPermission(String permission, Long componentId) { + if (!projectPermissions.contains(permission)) { + Collection projectIds = authorizationDao().selectAuthorizedRootProjectsIds(userId, permission); + for (Long id : projectIds) { + projectIdByPermission.put(permission, id); + } + projectPermissions.add(permission); + } + return projectIdByPermission.get(permission).contains(componentId); + } + AuthorizationDao authorizationDao() { return Platform.component(AuthorizationDao.class); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/roles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/roles_controller.rb index 0b744b89716..0a3d13ebba1 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/roles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/roles_controller.rb @@ -22,16 +22,18 @@ class RolesController < ApplicationController SECTION=Navigation::SECTION_CONFIGURATION - before_filter :admin_required verify :method => :post, :only => [:set_users, :set_groups], :redirect_to => {:action => 'global'} # GET /roles/global def global + access_denied unless has_role?(:admin) end # GET /roles/projects def projects + access_denied unless has_role?(:admin) + params['pageSize'] = 25 params['qualifiers'] ||= 'TRK' @query_result = Internal.component_api.findWithUncompleteProjects(params) @@ -52,13 +54,16 @@ class RolesController < ApplicationController # GET /roles/edit_users[?resource=] def edit_users - @project=Project.by_key(params[:resource]) if params[:resource].present? + @project = Project.by_key(params[:resource]) if params[:resource].present? + check_project_admin @role = params[:role] render :partial => 'edit_users' end # POST /roles/set_users?users=&role=[&resource=] def set_users + @project = Project.by_key(params[:resource]) if params[:resource].present? + check_project_admin bad_request('Missing role') if params[:role].blank? UserRole.grant_users(params[:users], params[:role], params[:resource]) render :text => '', :status => 200 @@ -66,13 +71,16 @@ class RolesController < ApplicationController # GET /roles/edit_groups[?resource=] def edit_groups - @project=Project.by_key(params[:resource]) if params[:resource].present? + @project = Project.by_key(params[:resource]) if params[:resource].present? + check_project_admin @role = params[:role] render :partial => 'edit_groups' end # POST /roles/set_groups?users=&role=[&resource=] def set_groups + @project = Project.by_key(params[:resource]) if params[:resource].present? + check_project_admin bad_request('Missing role') if params[:role].blank? GroupRole.grant_groups(params[:groups], params[:role], params[:resource]) render :text => '', :status => 200 @@ -80,13 +88,19 @@ class RolesController < ApplicationController # GET /roles/apply_template_form?criteria def apply_template_form - @permission_templates = Internal.permission_templates.selectAllPermissionTemplates().sort_by {|t| t.name.downcase}.collect {|pt| [pt.name, pt.key]} @names = params[:names] @keys = params[:keys] @qualifiers = params[:qualifiers] || 'TRK' @results_count = params[:results_count].to_i || 0 @components = params[:components] + if @components && @components.size == 1 + project = Project.by_key(@components.first) + @permission_templates = Internal.permission_templates.selectAllPermissionTemplates(project.key).sort_by {|t| t.name.downcase}.collect {|pt| [pt.name, pt.key]} + else + @permission_templates = Internal.permission_templates.selectAllPermissionTemplates().sort_by {|t| t.name.downcase}.collect {|pt| [pt.name, pt.key]} + end + render :partial => 'apply_template_form' end @@ -106,4 +120,11 @@ class RolesController < ApplicationController redirect_to :action => 'projects' end + + private + + def check_project_admin + access_denied unless has_role?(:admin) || has_role?(:admin, @project) + end + end diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java index b97f6dab68d..b1f097daa08 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java @@ -65,7 +65,7 @@ public class IssueCommentServiceTest { @Before public void setUpUser() { - MockUserSession.set().setLogin("admin").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); } @Before diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java index ad5dc4f13dd..99f28326c18 100644 --- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java @@ -28,6 +28,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.resources.Qualifiers; import org.sonar.api.security.DefaultGroups; +import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.permission.PermissionFacade; @@ -49,10 +50,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; public class InternalPermissionServiceTest { @@ -67,16 +65,15 @@ public class InternalPermissionServiceTest { @Before public void setUpCommonStubbing() { - MockUserSession.set().setLogin("admin").setPermissions(GlobalPermissions.SYSTEM_ADMIN); - userDao = mock(UserDao.class); when(userDao.selectActiveUserByLogin("user")).thenReturn(new UserDto().setId(2L).setLogin("user").setActive(true)); when(userDao.selectGroupByName("group")).thenReturn(new GroupDto().setId(2L).setName("group")); resourceDao = mock(ResourceDao.class); - permissionFacade = mock(PermissionFacade.class); + MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); + service = new InternalPermissionService(userDao, resourceDao, permissionFacade); } @@ -104,6 +101,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams("user", null, "org.sample.Sample", "user"); setUpComponentUserPermissions("user", 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.addPermission(params); @@ -127,6 +125,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams("user", null, "org.sample.Sample", "codeviewer"); setUpComponentUserPermissions("user", 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.removePermission(params); @@ -150,6 +149,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams(null, "group", "org.sample.Sample", "user"); setUpGlobalGroupPermissions("group", "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.addPermission(params); @@ -171,6 +171,7 @@ public class InternalPermissionServiceTest { new ResourceDto().setId(10L).setKey("org.sample.Sample")); params = buildPermissionChangeParams(null, DefaultGroups.ANYONE, "org.sample.Sample", "user"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.addPermission(params); @@ -194,6 +195,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams(null, "group", "org.sample.Sample", "codeviewer"); setUpComponentGroupPermissions("group", 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.removePermission(params); @@ -217,6 +219,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams(null, DefaultGroups.ANYONE, "org.sample.Sample", "codeviewer"); setUpComponentGroupPermissions(DefaultGroups.ANYONE, 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.removePermission(params); @@ -240,6 +243,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams("user", null, "org.sample.Sample", "codeviewer"); setUpComponentUserPermissions("user", 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.addPermission(params); @@ -263,6 +267,7 @@ public class InternalPermissionServiceTest { params = buildPermissionChangeParams(null, "group", "org.sample.Sample", "codeviewer"); setUpComponentGroupPermissions("group", 10L, "codeviewer"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); service.addPermission(params); @@ -311,10 +316,24 @@ public class InternalPermissionServiceTest { } @Test - public void fail_on_insufficient_rights() throws Exception { + public void fail_on_insufficient_global_rights() throws Exception { try { params = buildPermissionChangeParams("user", null, GlobalPermissions.QUALITY_PROFILE_ADMIN); - MockUserSession.set().setLogin("unauthorized").setPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); + MockUserSession.set().setLogin("unauthorized").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); + service.addPermission(params); + } catch (Exception e) { + assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("Insufficient privileges"); + } + } + + @Test + public void fail_on_insufficient_project_rights() throws Exception { + try { + when(resourceDao.getResource(any(ResourceQuery.class))).thenReturn( + new ResourceDto().setId(10L).setKey("org.sample.Sample")); + params = buildPermissionChangeParams(null, DefaultGroups.ANYONE, "org.sample.Sample", "user"); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN); + service.addPermission(params); } catch (Exception e) { assertThat(e).isInstanceOf(ForbiddenException.class).hasMessage("Insufficient privileges"); @@ -332,7 +351,7 @@ public class InternalPermissionServiceTest { } @Test - public void apply_permission_template() throws Exception { + public void apply_permission_template_on_many_projects() throws Exception { params = Maps.newHashMap(); params.put("template_key", "my_template_key"); params.put("components", "1,2,3"); @@ -344,6 +363,45 @@ public class InternalPermissionServiceTest { verify(permissionFacade).applyPermissionTemplate("my_template_key", 3L); } + @Test(expected = ForbiddenException.class) + public void apply_permission_template_on_many_projects_without_permission() { + MockUserSession.set().setLogin("admin").setGlobalPermissions(); + + params = Maps.newHashMap(); + params.put("template_key", "my_template_key"); + params.put("components", "1,2,3"); + + service.applyPermissionTemplate(params); + + verify(permissionFacade, never()).applyPermissionTemplate(anyString(), anyLong()); + } + + @Test + public void apply_permission_template_on_one_project() throws Exception { + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 1L); + + params = Maps.newHashMap(); + params.put("template_key", "my_template_key"); + params.put("components", "1"); + + service.applyPermissionTemplate(params); + + verify(permissionFacade).applyPermissionTemplate("my_template_key", 1L); + } + + @Test(expected = ForbiddenException.class) + public void apply_permission_template_on_one_project_without_permission() { + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN); + + params = Maps.newHashMap(); + params.put("template_key", "my_template_key"); + params.put("components", "1"); + + service.applyPermissionTemplate(params); + + verify(permissionFacade).applyPermissionTemplate("my_template_key", 1L); + } + @Test public void apply_default_permission_template_on_standard_project() { final String componentKey = "component"; @@ -353,6 +411,7 @@ public class InternalPermissionServiceTest { ComponentDto mockComponent = mock(ComponentDto.class); when(mockComponent.getId()).thenReturn(componentId); when(mockComponent.qualifier()).thenReturn(qualifier); + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 1234L); when(resourceDao.findByKey(componentKey)).thenReturn(mockComponent); service.applyDefaultPermissionTemplate(componentKey); @@ -377,7 +436,7 @@ public class InternalPermissionServiceTest { @Test public void apply_default_permission_template_on_provisioned_project_with_permission() { - MockUserSession.set().setLogin("provisioning").setPermissions(GlobalPermissions.PROVISIONING); + MockUserSession.set().setLogin("provisioning").setGlobalPermissions(GlobalPermissions.PROVISIONING); final String componentKey = "component"; final long componentId = 1234l; final String qualifier = Qualifiers.PROJECT; diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java index 2899855e3d2..1c3a2dea77c 100644 --- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java @@ -27,6 +27,9 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.web.UserRole; import org.sonar.core.permission.*; +import org.sonar.core.resource.ResourceDao; +import org.sonar.core.resource.ResourceDto; +import org.sonar.core.resource.ResourceQuery; import org.sonar.core.user.GroupDto; import org.sonar.core.user.UserDao; import org.sonar.core.user.UserDto; @@ -48,6 +51,8 @@ public class InternalPermissionTemplateServiceTest { private PermissionTemplateDao permissionTemplateDao; private UserDao userDao; + private ResourceDao resourceDao; + private InternalPermissionTemplateService permissionTemplateService; @Rule @@ -55,10 +60,11 @@ public class InternalPermissionTemplateServiceTest { @Before public void setUp() { - MockUserSession.set().setLogin("admin").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); permissionTemplateDao = mock(PermissionTemplateDao.class); userDao = mock(UserDao.class); - permissionTemplateService = new InternalPermissionTemplateService(permissionTemplateDao, userDao); + resourceDao = mock(ResourceDao.class); + permissionTemplateService = new InternalPermissionTemplateService(permissionTemplateDao, userDao, resourceDao); } @Test @@ -150,6 +156,27 @@ public class InternalPermissionTemplateServiceTest { assertThat(templates).onProperty("description").containsOnly("template1", "template2"); } + @Test + public void should_retrieve_all_permission_templates_from_project() throws Exception { + MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, 10L); + + PermissionTemplateDto template1 = + new PermissionTemplateDto().setId(1L).setName("template1").setDescription("template1"); + PermissionTemplateDto template2 = + new PermissionTemplateDto().setId(2L).setName("template2").setDescription("template2"); + when(permissionTemplateDao.selectAllPermissionTemplates()).thenReturn(Lists.newArrayList(template1, template2)); + + when(resourceDao.getResource(any(ResourceQuery.class))).thenReturn( + new ResourceDto().setId(10L).setKey("org.sample.Sample")); + + List templates = permissionTemplateService.selectAllPermissionTemplates("org.sample.Sample"); + + assertThat(templates).hasSize(2); + assertThat(templates).onProperty("id").containsOnly(1L, 2L); + assertThat(templates).onProperty("name").containsOnly("template1", "template2"); + assertThat(templates).onProperty("description").containsOnly("template1", "template2"); + } + @Test public void should_update_permission_template() throws Exception { diff --git a/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java b/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java index e04e6090c97..f9803ecb41a 100644 --- a/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java +++ b/sonar-server/src/test/java/org/sonar/server/permission/PermissionTemplateUpdaterTest.java @@ -50,7 +50,7 @@ public class PermissionTemplateUpdaterTest { @Before public void setUpCommonMocks() { - MockUserSession.set().setLogin("admin").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); userDao = mock(UserDao.class); stub(userDao.selectActiveUserByLogin("user")).toReturn(DEFAULT_USER); stub(userDao.selectGroupByName("group")).toReturn(DEFAULT_GROUP); @@ -128,7 +128,7 @@ public class PermissionTemplateUpdaterTest { expected.expect(ForbiddenException.class); expected.expectMessage("Insufficient privileges"); - MockUserSession.set().setLogin("user").setPermissions(GlobalPermissions.SCAN_EXECUTION); + MockUserSession.set().setLogin("user").setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION); PermissionTemplateUpdater updater = new PermissionTemplateUpdater(null, null, null, null, null) { @Override diff --git a/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java b/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java index fa5c1e4bc23..7508930b1d4 100644 --- a/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java @@ -81,7 +81,7 @@ public class DefaultUserServiceTest { @Test public void self_deactivation_is_not_possible() throws Exception { try { - MockUserSession.set().setLogin("simon").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("simon").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); service.deactivate("simon"); fail(); } catch (BadRequestException e) { @@ -93,7 +93,7 @@ public class DefaultUserServiceTest { @Test public void user_deactivation_requires_admin_permission() throws Exception { try { - MockUserSession.set().setLogin("simon").setPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); + MockUserSession.set().setLogin("simon").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); service.deactivate("julien"); fail(); } catch (ForbiddenException e) { @@ -103,14 +103,14 @@ public class DefaultUserServiceTest { @Test public void deactivate_user() throws Exception { - MockUserSession.set().setLogin("simon").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("simon").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); service.deactivate("julien"); verify(dao).deactivateUserByLogin("julien"); } @Test public void fail_to_deactivate_when_blank_login() throws Exception { - MockUserSession.set().setLogin("simon").setPermissions(GlobalPermissions.SYSTEM_ADMIN); + MockUserSession.set().setLogin("simon").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); try { service.deactivate(""); fail(); diff --git a/sonar-server/src/test/java/org/sonar/server/user/MockUserSession.java b/sonar-server/src/test/java/org/sonar/server/user/MockUserSession.java index bffebab5da8..99dd143f4c3 100644 --- a/sonar-server/src/test/java/org/sonar/server/user/MockUserSession.java +++ b/sonar-server/src/test/java/org/sonar/server/user/MockUserSession.java @@ -19,16 +19,20 @@ */ package org.sonar.server.user; -import javax.annotation.Nullable; +import com.google.common.collect.HashMultimap; +import javax.annotation.Nullable; import java.util.Arrays; import java.util.Collections; import java.util.Locale; +import static com.google.common.collect.Lists.newArrayList; + public class MockUserSession extends UserSession { private MockUserSession() { globalPermissions = Collections.emptyList(); + projectIdByPermission = HashMultimap.create(); } public static MockUserSession set() { @@ -56,8 +60,14 @@ public class MockUserSession extends UserSession { return this; } - public MockUserSession setPermissions(String... globalPermissions) { + public MockUserSession setGlobalPermissions(String... globalPermissions) { this.globalPermissions = Arrays.asList(globalPermissions); return this; } + + public MockUserSession addProjectPermissions(String projectPermission, Long... projectIds) { + this.projectPermissions.add(projectPermission); + this.projectIdByPermission.putAll(projectPermission, newArrayList(projectIds)); + return this; + } } diff --git a/sonar-server/src/test/java/org/sonar/server/user/UserSessionTest.java b/sonar-server/src/test/java/org/sonar/server/user/UserSessionTest.java index d1709584ca4..56a033783f5 100644 --- a/sonar-server/src/test/java/org/sonar/server/user/UserSessionTest.java +++ b/sonar-server/src/test/java/org/sonar/server/user/UserSessionTest.java @@ -20,6 +20,7 @@ package org.sonar.server.user; import org.junit.Test; +import org.sonar.api.web.UserRole; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.user.AuthorizationDao; import org.sonar.server.exceptions.ForbiddenException; @@ -27,6 +28,7 @@ import org.sonar.server.exceptions.ForbiddenException; import java.util.Arrays; import java.util.Locale; +import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,7 +49,7 @@ public class UserSessionTest { } @Test - public void getSession() throws Exception { + public void get_session() throws Exception { UserSession.set(new UserSession().setUserId(123).setLogin("karadoc").setLocale(Locale.FRENCH)); UserSession session = UserSession.get(); @@ -59,7 +61,14 @@ public class UserSessionTest { } @Test - public void hasPermission() throws Exception { + public void login_should_not_be_empty() throws Exception { + UserSession session = new UserSession().setLogin(""); + assertThat(session.login()).isNull(); + assertThat(session.isLoggedIn()).isFalse(); + } + + @Test + public void has_global_permission() throws Exception { AuthorizationDao authorizationDao = mock(AuthorizationDao.class); UserSession session = new SpyUserSession("marius", authorizationDao); @@ -71,14 +80,7 @@ public class UserSessionTest { } @Test - public void login_should_not_be_empty() throws Exception { - UserSession session = new UserSession().setLogin(""); - assertThat(session.login()).isNull(); - assertThat(session.isLoggedIn()).isFalse(); - } - - @Test - public void checkPermission_ok() throws Exception { + public void check_global_Permission_ok() throws Exception { AuthorizationDao authorizationDao = mock(AuthorizationDao.class); UserSession session = new SpyUserSession("marius", authorizationDao); @@ -88,7 +90,7 @@ public class UserSessionTest { } @Test(expected = ForbiddenException.class) - public void checkPermission_ko() throws Exception { + public void check_global_Permission_ko() throws Exception { AuthorizationDao authorizationDao = mock(AuthorizationDao.class); UserSession session = new SpyUserSession("marius", authorizationDao); @@ -97,6 +99,35 @@ public class UserSessionTest { session.checkGlobalPermission(GlobalPermissions.DASHBOARD_SHARING); } + @Test + public void has_project_permission() throws Exception { + AuthorizationDao authorizationDao = mock(AuthorizationDao.class); + UserSession session = new SpyUserSession("marius", authorizationDao).setUserId(1); + when(authorizationDao.selectAuthorizedRootProjectsIds(1, UserRole.USER)).thenReturn(newArrayList(10L)); + + assertThat(session.hasProjectPermission(UserRole.USER, 10L)).isTrue(); + assertThat(session.hasProjectPermission(UserRole.CODEVIEWER, 10L)).isFalse(); + assertThat(session.hasProjectPermission(UserRole.ADMIN, 10L)).isFalse(); + } + + @Test + public void check_project_permission_ok() throws Exception { + AuthorizationDao authorizationDao = mock(AuthorizationDao.class); + UserSession session = new SpyUserSession("marius", authorizationDao).setUserId(1); + when(authorizationDao.selectAuthorizedRootProjectsIds(1, UserRole.USER)).thenReturn(newArrayList(10L)); + + session.checkProjectPermission(UserRole.USER, 10L); + } + + @Test(expected = ForbiddenException.class) + public void check_project_permission_ko() throws Exception { + AuthorizationDao authorizationDao = mock(AuthorizationDao.class); + UserSession session = new SpyUserSession("marius", authorizationDao).setUserId(1); + when(authorizationDao.selectAuthorizedRootProjectsIds(1, UserRole.USER)).thenReturn(newArrayList(11L)); + + session.checkProjectPermission(UserRole.USER, 10L); + } + static class SpyUserSession extends UserSession { private AuthorizationDao authorizationDao; -- 2.39.5