From c000b67698c71061f0776b63b63cdb77e1569851 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Thu, 6 Aug 2015 11:14:02 +0200 Subject: [PATCH] SONAR-6483 WS permissions/add_group add by group id or group name & rely on PermissionUpdater --- .../server/permission/PermissionService.java | 16 +- .../server/permission/PermissionUpdater.java | 220 ++++++++++++++++++ .../server/permission/ws/AddGroupAction.java | 63 ++++- .../server/permission/ws/AddUserAction.java | 12 +- .../platformlevel/PlatformLevel4.java | 2 + .../IssueBulkChangeServiceMediumTest.java | 4 +- .../issue/IssueCommentServiceMediumTest.java | 4 +- .../server/issue/IssueServiceMediumTest.java | 4 +- .../ws/SearchActionComponentsMediumTest.java | 4 +- .../issue/ws/SearchActionMediumTest.java | 4 +- .../permission/ws/AddGroupActionTest.java | 40 +++- .../permission/ws/AddUserActionTest.java | 10 +- 12 files changed, 342 insertions(+), 41 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/permission/PermissionUpdater.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java index 28ad37a2955..867a553dc9a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java @@ -47,11 +47,10 @@ import org.sonar.server.user.UserSession; @ServerSide public class PermissionService { - - private enum Operation { ADD, REMOVE; } + private static final String OBJECT_TYPE_USER = "User"; private static final String OBJECT_TYPE_GROUP = "Group"; private static final String NOT_FOUND_FORMAT = "%s %s does not exist"; @@ -62,6 +61,7 @@ public class PermissionService { private final IssueAuthorizationIndexer issueAuthorizationIndexer; private final UserSession userSession; private final ComponentFinder componentFinder; + public PermissionService(DbClient dbClient, PermissionRepository permissionRepository, PermissionFinder finder, IssueAuthorizationIndexer issueAuthorizationIndexer, UserSession userSession, ComponentFinder componentFinder) { this.dbClient = dbClient; @@ -92,9 +92,19 @@ public class PermissionService { * To be used only by jruby webapp */ public void addPermission(Map params) { - addPermission(PermissionChange.buildFromParams(params)); + PermissionChange change = PermissionChange.buildFromParams(params); + DbSession session = dbClient.openSession(false); + try { + applyChange(Operation.ADD, change, session); + } finally { + dbClient.closeSession(session); + } } + /** + * @deprecated since 5.2 use PermissionUpdate.addPermission instead + */ + @Deprecated public void addPermission(PermissionChange change) { DbSession session = dbClient.openSession(false); try { diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionUpdater.java new file mode 100644 index 00000000000..159269f1da9 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionUpdater.java @@ -0,0 +1,220 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.permission; + +import java.util.List; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import org.sonar.api.security.DefaultGroups; +import org.sonar.api.web.UserRole; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.permission.PermissionRepository; +import org.sonar.db.user.GroupDto; +import org.sonar.db.user.UserDto; +import org.sonar.server.component.ComponentFinder; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.issue.index.IssueAuthorizationIndexer; +import org.sonar.server.user.UserSession; + +public class PermissionUpdater { + + private enum Operation { + ADD, REMOVE + } + + private static final String OBJECT_TYPE_USER = "User"; + private static final String OBJECT_TYPE_GROUP = "Group"; + private static final String NOT_FOUND_FORMAT = "%s %s does not exist"; + + private final DbClient dbClient; + private final PermissionRepository permissionRepository; + private final IssueAuthorizationIndexer issueAuthorizationIndexer; + private final UserSession userSession; + private final ComponentFinder componentFinder; + + public PermissionUpdater(DbClient dbClient, PermissionRepository permissionRepository, + IssueAuthorizationIndexer issueAuthorizationIndexer, UserSession userSession, ComponentFinder componentFinder) { + this.dbClient = dbClient; + this.permissionRepository = permissionRepository; + this.issueAuthorizationIndexer = issueAuthorizationIndexer; + this.userSession = userSession; + this.componentFinder = componentFinder; + } + + public static List globalPermissions() { + return GlobalPermissions.ALL; + } + + public void addPermission(PermissionChange change) { + DbSession session = dbClient.openSession(false); + try { + applyChange(Operation.ADD, change, session); + } finally { + dbClient.closeSession(session); + } + } + + public void removePermission(PermissionChange change) { + DbSession session = dbClient.openSession(false); + try { + applyChange(Operation.REMOVE, change, session); + } finally { + session.close(); + } + } + + private void applyChange(Operation operation, PermissionChange change, DbSession session) { + userSession.checkLoggedIn(); + change.validate(); + boolean changed; + if (change.user() != null) { + changed = applyChangeOnUser(session, operation, change); + } else { + changed = applyChangeOnGroup(session, operation, change); + } + if (changed) { + session.commit(); + if (change.component() != null) { + indexProjectPermissions(); + } + } + } + + private boolean applyChangeOnGroup(DbSession session, Operation operation, PermissionChange permissionChange) { + Long componentId = getComponentId(session, permissionChange.component()); + checkProjectAdminPermission(permissionChange.component()); + + List existingPermissions = dbClient.roleDao().selectGroupPermissions(session, permissionChange.group(), componentId); + if (shouldSkipPermissionChange(operation, existingPermissions, permissionChange)) { + return false; + } + + Long targetedGroup = getTargetedGroup(session, permissionChange.group()); + String permission = permissionChange.permission(); + if (Operation.ADD == operation) { + checkNotAnyoneAndAdmin(permission, permissionChange.group()); + permissionRepository.insertGroupPermission(componentId, targetedGroup, permission, session); + } else { + checkAdminUsersExistOutsideTheRemovedGroup(session, permissionChange, targetedGroup); + permissionRepository.deleteGroupPermission(componentId, targetedGroup, permission, session); + } + return true; + } + + private void checkNotAnyoneAndAdmin(String permission, String group) { + if (GlobalPermissions.SYSTEM_ADMIN.equals(permission) + && DefaultGroups.isAnyone(group)) { + throw new BadRequestException(String.format("It is not possible to add the '%s' permission to the '%s' group.", permission, group)); + } + } + + private boolean applyChangeOnUser(DbSession session, Operation operation, PermissionChange permissionChange) { + Long componentId = getComponentId(session, permissionChange.component()); + checkProjectAdminPermission(permissionChange.component()); + + List existingPermissions = dbClient.roleDao().selectUserPermissions(session, permissionChange.user(), componentId); + if (shouldSkipPermissionChange(operation, existingPermissions, permissionChange)) { + return false; + } + + Long targetedUser = getTargetedUser(session, permissionChange.user()); + if (Operation.ADD == operation) { + permissionRepository.insertUserPermission(componentId, targetedUser, permissionChange.permission(), session); + } else { + checkOtherAdminUsersExist(session, permissionChange); + permissionRepository.deleteUserPermission(componentId, targetedUser, permissionChange.permission(), session); + } + return true; + + } + + private void checkOtherAdminUsersExist(DbSession session, PermissionChange permissionChange) { + if (GlobalPermissions.SYSTEM_ADMIN.equals(permissionChange.permission()) + && dbClient.roleDao().countUserPermissions(session, permissionChange.permission(), null) <= 1) { + throw new BadRequestException(String.format("Last user with '%s' permission. Permission cannot be removed.", GlobalPermissions.SYSTEM_ADMIN)); + } + } + + private void checkAdminUsersExistOutsideTheRemovedGroup(DbSession session, PermissionChange permissionChange, @Nullable Long groupIdToExclude) { + if (GlobalPermissions.SYSTEM_ADMIN.equals(permissionChange.permission()) + && groupIdToExclude != null + && dbClient.roleDao().countUserPermissions(session, permissionChange.permission(), groupIdToExclude) <= 0) { + throw new BadRequestException(String.format("Last group with '%s' permission. Permission cannot be removed.", GlobalPermissions.SYSTEM_ADMIN)); + } + } + + private Long getTargetedUser(DbSession session, String userLogin) { + UserDto user = dbClient.userDao().selectActiveUserByLogin(session, userLogin); + badRequestIfNullResult(user, OBJECT_TYPE_USER, userLogin); + return user.getId(); + } + + @Nullable + private Long getTargetedGroup(DbSession session, String group) { + if (DefaultGroups.isAnyone(group)) { + return null; + } else { + GroupDto groupDto = dbClient.userDao().selectGroupByName(group, session); + badRequestIfNullResult(groupDto, OBJECT_TYPE_GROUP, group); + return groupDto.getId(); + } + } + + private boolean shouldSkipPermissionChange(Operation operation, List existingPermissions, PermissionChange permissionChange) { + return (Operation.ADD == operation && existingPermissions.contains(permissionChange.permission())) || + (Operation.REMOVE == operation && !existingPermissions.contains(permissionChange.permission())); + } + + @CheckForNull + private Long getComponentId(DbSession session, @Nullable String componentKey) { + if (componentKey == null) { + return null; + } else { + ComponentDto component = componentFinder.getByKey(session, componentKey); + return component.getId(); + } + } + + private static Object badRequestIfNullResult(@Nullable Object component, String objectType, String objectKey) { + if (component == null) { + throw new BadRequestException(String.format(NOT_FOUND_FORMAT, objectType, objectKey)); + } + return component; + } + + private void checkProjectAdminPermission(@Nullable String projectKey) { + if (projectKey == null) { + userSession.checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + } else { + if (!userSession.hasGlobalPermission(GlobalPermissions.SYSTEM_ADMIN) && !userSession.hasProjectPermission(UserRole.ADMIN, projectKey)) { + throw new ForbiddenException("Insufficient privileges"); + } + } + } + + private void indexProjectPermissions() { + issueAuthorizationIndexer.index(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java index 30f7bfc2a44..9d4749bccf5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddGroupAction.java @@ -20,29 +20,40 @@ package org.sonar.server.permission.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; import org.sonar.core.permission.GlobalPermissions; -import org.sonar.server.permission.PermissionService; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.user.GroupDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; public class AddGroupAction implements PermissionsWsAction { public static final String ACTION = "add_group"; public static final String PARAM_PERMISSION = "permission"; public static final String PARAM_GROUP_NAME = "groupName"; + public static final String PARAM_GROUP_ID = "groupId"; - private final PermissionService permissionService; + private final PermissionUpdater permissionUpdater; + private final DbClient dbClient; - public AddGroupAction(PermissionService permissionService) { - this.permissionService = permissionService; + public AddGroupAction(PermissionUpdater permissionUpdater, DbClient dbClient) { + this.permissionUpdater = permissionUpdater; + this.dbClient = dbClient; } @Override public void define(WebService.NewController context) { WebService.NewAction action = context.createAction(ACTION) - .setDescription("Add permission to a group.
Requires 'Administer System' permission.") + .setDescription("Add permission to a group.
" + + "The group name or group id must be provided.
" + + "Requires 'Administer System' permission.") .setSince("5.2") .setPost(true) .setHandler(this); @@ -53,21 +64,55 @@ public class AddGroupAction implements PermissionsWsAction { .setPossibleValues(GlobalPermissions.ALL); action.createParam(PARAM_GROUP_NAME) - .setRequired(true) .setDescription("Group name or 'anyone' (whatever the case)") .setExampleValue("sonar-administrators"); + + action.createParam(PARAM_GROUP_ID) + .setDescription("Group ID") + .setExampleValue("42"); } @Override public void handle(Request request, Response response) throws Exception { String permission = request.mandatoryParam(PARAM_PERMISSION); - String groupName = request.mandatoryParam(PARAM_GROUP_NAME); - permissionService.addPermission( + String groupNameParam = request.param(PARAM_GROUP_NAME); + Long groupId = request.paramAsLong(PARAM_GROUP_ID); + + String groupName = searchName(groupNameParam, groupId); + + permissionUpdater.addPermission( new PermissionChange() .setPermission(permission) .setGroup(groupName) - ); + ); response.noContent(); } + + private String searchName(@Nullable String groupNameParam, @Nullable Long groupId) { + checkParameters(groupNameParam, groupId); + if (groupNameParam != null) { + return groupNameParam; + } + + DbSession dbSession = dbClient.openSession(false); + try { + GroupDto group = dbClient.groupDao().selectById(dbSession, groupId); + if (group == null) { + throw new NotFoundException(String.format("Group with id '%d' not found", groupId)); + } + + return group.getName(); + } finally { + dbClient.closeSession(dbSession); + } + } + + private void checkParameters(@Nullable String groupName, @Nullable Long groupId) { + if (groupName != null ^ groupId != null) { + return; + } + + throw new BadRequestException("Group name or group id must be provided, not both"); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java index db164279b8d..39cd5f0a370 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/AddUserAction.java @@ -24,8 +24,8 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.core.permission.GlobalPermissions; -import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; public class AddUserAction implements PermissionsWsAction { @@ -33,10 +33,10 @@ public class AddUserAction implements PermissionsWsAction { public static final String PARAM_PERMISSION = "permission"; public static final String PARAM_USER_LOGIN = "login"; - private final PermissionService permissionService; + private final PermissionUpdater permissionUpdater; - public AddUserAction(PermissionService permissionService) { - this.permissionService = permissionService; + public AddUserAction(PermissionUpdater permissionUpdater) { + this.permissionUpdater = permissionUpdater; } @Override @@ -62,11 +62,11 @@ public class AddUserAction implements PermissionsWsAction { public void handle(Request request, Response response) throws Exception { String permission = request.mandatoryParam(PARAM_PERMISSION); String userLogin = request.mandatoryParam(PARAM_USER_LOGIN); - permissionService.addPermission( + permissionUpdater.addPermission( new PermissionChange() .setPermission(permission) .setUser(userLogin) - ); + ); response.noContent(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java index e86eb3208d5..efed066e65e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java @@ -172,6 +172,7 @@ import org.sonar.server.notification.email.EmailNotificationChannel; import org.sonar.server.permission.PermissionFinder; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionTemplateService; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.permission.ws.PermissionsWsModule; import org.sonar.server.platform.BackendCleanup; import org.sonar.server.platform.SettingsChangeNotifier; @@ -553,6 +554,7 @@ public class PlatformLevel4 extends PlatformLevel { // permissions PermissionRepository.class, PermissionService.class, + PermissionUpdater.class, PermissionTemplateService.class, PermissionFinder.class, PermissionsWsModule.class, diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java index 1c05f9e3cc1..7cc491d8d36 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceMediumTest.java @@ -47,8 +47,8 @@ import org.sonar.db.user.UserDto; import org.sonar.server.component.ComponentTesting; import org.sonar.server.component.SnapshotTesting; import org.sonar.server.issue.index.IssueIndexer; -import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; @@ -98,7 +98,7 @@ public class IssueBulkChangeServiceMediumTest { // project can be seen by anyone session.commit(); userSessionRule.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); - tester.get(PermissionService.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); + tester.get(PermissionUpdater.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); userSession = userSessionRule.login("john") .addProjectPermissions(UserRole.USER, project.key()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java index 2834e980860..892b35285fb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java @@ -46,8 +46,8 @@ import org.sonar.db.rule.RuleTesting; import org.sonar.server.component.ComponentTesting; import org.sonar.server.component.SnapshotTesting; import org.sonar.server.issue.index.IssueIndexer; -import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.search.IndexClient; import org.sonar.server.tester.ServerTester; @@ -95,7 +95,7 @@ public class IssueCommentServiceMediumTest { // project can be seen by anyone session.commit(); userSessionRule.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); - tester.get(PermissionService.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); + tester.get(PermissionUpdater.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); userSessionRule.login("gandalf"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java index 52bd7cdd062..eeb9de0cda8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java @@ -63,7 +63,7 @@ import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.issue.index.IssueIndexDefinition; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.permission.PermissionChange; -import org.sonar.server.permission.PermissionService; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.source.index.FileSourcesUpdaterHelper; import org.sonar.server.source.index.SourceLineIndexer; @@ -587,7 +587,7 @@ public class IssueServiceMediumTest { session.commit(); // project can be seen by group "anyone" - tester.get(PermissionService.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); + tester.get(PermissionUpdater.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); userSessionRule.login(); return project; diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java index 90274dd1f73..15c32756fde 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionComponentsMediumTest.java @@ -44,7 +44,7 @@ import org.sonar.server.issue.IssueTesting; import org.sonar.server.issue.filter.IssueFilterParameters; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.permission.PermissionChange; -import org.sonar.server.permission.PermissionService; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; @@ -552,7 +552,7 @@ public class SearchActionComponentsMediumTest { private void setAnyoneProjectPermission(ComponentDto project, String permission) { userSessionRule.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); - tester.get(PermissionService.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(permission)); + tester.get(PermissionUpdater.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(permission)); } private IssueDto insertIssue(IssueDto issue) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionMediumTest.java index 683b94a983c..8526c45dd9a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionMediumTest.java @@ -53,7 +53,7 @@ import org.sonar.server.issue.IssueTesting; import org.sonar.server.issue.filter.IssueFilterParameters; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.permission.PermissionChange; -import org.sonar.server.permission.PermissionService; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.search.QueryContext; import org.sonar.server.tester.ServerTester; @@ -663,7 +663,7 @@ public class SearchActionMediumTest { private void setDefaultProjectPermission(ComponentDto project) { // project can be seen by anyone and by code viewer userSessionRule.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); - tester.get(PermissionService.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); + tester.get(PermissionUpdater.class).addPermission(new PermissionChange().setComponentKey(project.getKey()).setGroup(DefaultGroups.ANYONE).setPermission(UserRole.USER)); userSessionRule.login(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddGroupActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddGroupActionTest.java index c24989ba181..26745d5185b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddGroupActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddGroupActionTest.java @@ -23,15 +23,19 @@ package org.sonar.server.permission.ws; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.experimental.categories.Category; import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; import org.sonar.api.utils.System2; import org.sonar.db.DbTester; +import org.sonar.db.user.GroupDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ServerException; -import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; +import org.sonar.test.DbTests; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -39,6 +43,7 @@ import static org.mockito.Mockito.verify; import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; import static org.sonar.server.permission.ws.AddGroupAction.ACTION; +@Category(DbTests.class) public class AddGroupActionTest { UserSessionRule userSession = UserSessionRule.standalone(); WsTester ws; @@ -46,14 +51,15 @@ public class AddGroupActionTest { public DbTester db = DbTester.create(System2.INSTANCE); @Rule public ExpectedException expectedException = ExpectedException.none(); - private PermissionService permissionService; + private PermissionUpdater permissionUpdater; @Before public void setUp() { - permissionService = mock(PermissionService.class); + permissionUpdater = mock(PermissionUpdater.class); ws = new WsTester(new PermissionsWs( - new AddGroupAction(permissionService))); + new AddGroupAction(permissionUpdater, db.getDbClient()))); userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN); + } @Test @@ -64,14 +70,31 @@ public class AddGroupActionTest { .execute(); ArgumentCaptor permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class); - verify(permissionService).addPermission(permissionChangeCaptor.capture()); + verify(permissionUpdater).addPermission(permissionChangeCaptor.capture()); PermissionChange permissionChange = permissionChangeCaptor.getValue(); assertThat(permissionChange.group()).isEqualTo("sonar-administrators"); assertThat(permissionChange.permission()).isEqualTo(SYSTEM_ADMIN); } @Test - public void get_request_are_not_authorized() throws Exception { + public void search_with_group_id() throws Exception { + GroupDto group = db.getDbClient().groupDao().insert(db.getSession(), new GroupDto() + .setName("sonar-administrators")); + db.getSession().commit(); + + ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION) + .setParam(AddGroupAction.PARAM_GROUP_ID, group.getId().toString()) + .setParam(AddGroupAction.PARAM_PERMISSION, SYSTEM_ADMIN) + .execute(); + + ArgumentCaptor permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class); + verify(permissionUpdater).addPermission(permissionChangeCaptor.capture()); + PermissionChange permissionChange = permissionChangeCaptor.getValue(); + assertThat(permissionChange.group()).isEqualTo("sonar-administrators"); + } + + @Test + public void fail_when_get_request() throws Exception { expectedException.expect(ServerException.class); ws.newGetRequest(PermissionsWs.ENDPOINT, ACTION) @@ -81,8 +104,9 @@ public class AddGroupActionTest { } @Test - public void fail_when_group_name_is_missing() throws Exception { - expectedException.expect(IllegalArgumentException.class); + public void fail_when_group_name_and_group_id_are_missing() throws Exception { + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Group name or group id must be provided, not both"); ws.newPostRequest(PermissionsWs.ENDPOINT, ACTION) .setParam(AddGroupAction.PARAM_PERMISSION, SYSTEM_ADMIN) diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddUserActionTest.java index 22f82007879..ee7aac66774 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddUserActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/AddUserActionTest.java @@ -28,8 +28,8 @@ import org.mockito.ArgumentCaptor; import org.sonar.api.utils.System2; import org.sonar.db.DbTester; import org.sonar.server.exceptions.ServerException; -import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionChange; +import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; @@ -46,13 +46,13 @@ public class AddUserActionTest { public DbTester db = DbTester.create(System2.INSTANCE); @Rule public ExpectedException expectedException = ExpectedException.none(); - private PermissionService permissionService; + private PermissionUpdater permissionUpdater; @Before public void setUp() { - permissionService = mock(PermissionService.class); + permissionUpdater = mock(PermissionUpdater.class); ws = new WsTester(new PermissionsWs( - new AddUserAction(permissionService))); + new AddUserAction(permissionUpdater))); userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN); } @@ -64,7 +64,7 @@ public class AddUserActionTest { .execute(); ArgumentCaptor permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class); - verify(permissionService).addPermission(permissionChangeCaptor.capture()); + verify(permissionUpdater).addPermission(permissionChangeCaptor.capture()); PermissionChange permissionChange = permissionChangeCaptor.getValue(); assertThat(permissionChange.user()).isEqualTo("ray.bradbury"); assertThat(permissionChange.permission()).isEqualTo(SYSTEM_ADMIN); -- 2.39.5