From 45a3bd7777660f5559cde6d85c2a43e75c988bee Mon Sep 17 00:00:00 2001 From: Zipeng WU Date: Tue, 12 Jul 2022 16:48:42 +0200 Subject: [PATCH] SONAR-15798 Quality Profile administration should have limited privileges on projects --- .../qualityprofile/ws/AddProjectAction.java | 8 +++----- .../qualityprofile/ws/DeleteAction.java | 2 +- .../qualityprofile/ws/QProfileWsSupport.java | 20 ++++++++++++++----- .../ws/RemoveProjectAction.java | 6 +++--- .../qualityprofile/ws/SearchAction.java | 4 ++-- .../qualityprofile/ws/search-example.json | 4 ++-- .../ws/AddProjectActionTest.java | 5 +++-- .../qualityprofile/ws/DeleteActionTest.java | 5 +++-- .../ws/RemoveProjectActionTest.java | 19 ++++++++++++++++-- .../qualityprofile/ws/SearchActionTest.java | 4 ++-- 10 files changed, 51 insertions(+), 26 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/AddProjectAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/AddProjectAction.java index d5366352372..0ee10c415a4 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/AddProjectAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/AddProjectAction.java @@ -87,7 +87,7 @@ public class AddProjectAction implements QProfileWsAction { ProjectDto project = loadProject(dbSession, request); QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromName(request)); - checkPermissions(dbSession, profile, project); + checkPermissions(profile, project); QProfileDto currentProfile = dbClient.qualityProfileDao().selectAssociatedToProjectAndLanguage(dbSession, project, profile.getLanguage()); QProfileDto deactivatedProfile = null; @@ -112,15 +112,13 @@ public class AddProjectAction implements QProfileWsAction { response.noContent(); } - private ProjectDto loadProject(DbSession dbSession, Request request) { String projectKey = request.mandatoryParam(PARAM_PROJECT); return componentFinder.getProjectByKey(dbSession, projectKey); } - private void checkPermissions(DbSession dbSession, QProfileDto profile, ProjectDto project) { - if (wsSupport.canEdit(dbSession, profile) - || userSession.hasProjectPermission(UserRole.ADMIN, project)) { + private void checkPermissions(QProfileDto profile, ProjectDto project) { + if (wsSupport.canAdministrate(profile) || userSession.hasProjectPermission(UserRole.ADMIN, project)) { return; } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/DeleteAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/DeleteAction.java index 468a6b7a3ab..1d184113819 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/DeleteAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/DeleteAction.java @@ -77,7 +77,7 @@ public class DeleteAction implements QProfileWsAction { try (DbSession dbSession = dbClient.openSession(false)) { QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromName(request)); - wsSupport.checkCanEdit(dbSession, profile); + wsSupport.checkCanAdministrate(profile); Collection descendants = selectDescendants(dbSession, profile); ensureNoneIsMarkedAsDefault(dbSession, profile, descendants); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java index bb4bd5129e4..59ed4c78e5f 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java @@ -91,19 +91,22 @@ public class QProfileWsSupport { } boolean canEdit(DbSession dbSession, QProfileDto profile) { - if (profile.isBuiltIn() || !userSession.isLoggedIn()) { - return false; - } - if (userSession.hasPermission(GlobalPermission.ADMINISTER_QUALITY_PROFILES)) { + if (canAdministrate(profile)) { return true; } - UserDto user = dbClient.userDao().selectByLogin(dbSession, userSession.getLogin()); checkState(user != null, "User from session does not exist"); return dbClient.qProfileEditUsersDao().exists(dbSession, profile, user) || dbClient.qProfileEditGroupsDao().exists(dbSession, profile, userSession.getGroups()); } + boolean canAdministrate(QProfileDto profile) { + if (profile.isBuiltIn() || !userSession.isLoggedIn()) { + return false; + } + return userSession.hasPermission(GlobalPermission.ADMINISTER_QUALITY_PROFILES); + } + public void checkCanEdit(DbSession dbSession, QProfileDto profile) { checkNotBuiltIn(profile); if (!canEdit(dbSession, profile)) { @@ -111,6 +114,13 @@ public class QProfileWsSupport { } } + public void checkCanAdministrate(QProfileDto profile) { + checkNotBuiltIn(profile); + if (!canAdministrate(profile)) { + throw insufficientPrivilegesException(); + } + } + void checkNotBuiltIn(QProfileDto profile) { checkRequest(!profile.isBuiltIn(), "Operation forbidden for built-in Quality Profile '%s' with language '%s'", profile.getName(), profile.getLanguage()); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/RemoveProjectAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/RemoveProjectAction.java index 4ab8d7cf527..f1683a430ab 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/RemoveProjectAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/RemoveProjectAction.java @@ -85,7 +85,7 @@ public class RemoveProjectAction implements QProfileWsAction { try (DbSession dbSession = dbClient.openSession(false)) { ProjectDto project = loadProject(dbSession, request); QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromName(request)); - checkPermissions(dbSession, profile, project); + checkPermissions(profile, project); dbClient.qualityProfileDao().deleteProjectProfileAssociation(dbSession, project, profile); dbSession.commit(); @@ -109,8 +109,8 @@ public class RemoveProjectAction implements QProfileWsAction { return componentFinder.getProjectByKey(dbSession, projectKey); } - private void checkPermissions(DbSession dbSession, QProfileDto profile, ProjectDto project) { - if (wsSupport.canEdit(dbSession, profile) || userSession.hasProjectPermission(UserRole.ADMIN, project)) { + private void checkPermissions(QProfileDto profile, ProjectDto project) { + if (wsSupport.canAdministrate(profile) || userSession.hasProjectPermission(UserRole.ADMIN, project)) { return; } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/SearchAction.java index 4c696b13ff8..99ce822f259 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/SearchAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/ws/SearchAction.java @@ -265,8 +265,8 @@ public class SearchAction implements QProfileWsAction { .setEdit(!profile.isBuiltIn() && (isGlobalQProfileAdmin || data.isEditable(profile))) .setSetAsDefault(!isDefault && isGlobalQProfileAdmin) .setCopy(isGlobalQProfileAdmin) - .setDelete(!isDefault && !profile.isBuiltIn() && (isGlobalQProfileAdmin || data.isEditable(profile))) - .setAssociateProjects(!isDefault && (isGlobalQProfileAdmin || data.isEditable(profile)))); + .setDelete(!isDefault && !profile.isBuiltIn() && isGlobalQProfileAdmin) + .setAssociateProjects(!isDefault && isGlobalQProfileAdmin)); } return response.build(); } diff --git a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/search-example.json b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/search-example.json index eb13119b881..18555bcd1fa 100644 --- a/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/search-example.json +++ b/server/sonar-webserver-webapi/src/main/resources/org/sonar/server/qualityprofile/ws/search-example.json @@ -40,8 +40,8 @@ "edit": true, "setAsDefault": false, "copy": false, - "delete": true, - "associateProjects": true + "delete": false, + "associateProjects": false } }, { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/AddProjectActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/AddProjectActionTest.java index 52a39342278..a31d95fc0f9 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/AddProjectActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/AddProjectActionTest.java @@ -28,6 +28,7 @@ import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; +import org.sonar.db.permission.GlobalPermission; import org.sonar.db.project.ProjectDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.user.UserDto; @@ -96,12 +97,12 @@ public class AddProjectActionTest { } @Test - public void as_qprofile_editor() { + public void as_qprofile_editor_and_global_admin() { UserDto user = db.users().insertUser(); QProfileDto qualityProfile = db.qualityProfiles().insert(qp -> qp.setLanguage(LANGUAGE_1)); db.qualityProfiles().addUserPermission(qualityProfile, user); ProjectDto project = db.components().insertPrivateProjectDto(); - userSession.logIn(user); + userSession.logIn(user).addPermission(GlobalPermission.ADMINISTER_QUALITY_PROFILES); call(project, qualityProfile); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/DeleteActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/DeleteActionTest.java index cd153c77bf1..4d16a9ff660 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/DeleteActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/DeleteActionTest.java @@ -30,6 +30,7 @@ import org.sonar.core.util.UuidFactoryFast; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.permission.GlobalPermission; import org.sonar.db.project.ProjectDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.user.UserDto; @@ -93,11 +94,11 @@ public class DeleteActionTest { } @Test - public void as_qprofile_editor() { + public void as_qprofile_editor_and_global_admin() { QProfileDto profile = createProfile(); UserDto user = db.users().insertUser(); db.qualityProfiles().addUserPermission(profile, user); - userSession.logIn(user); + userSession.logIn(user).addPermission(GlobalPermission.ADMINISTER_QUALITY_PROFILES); TestResponse response = ws.newRequest() .setMethod("POST") diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/RemoveProjectActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/RemoveProjectActionTest.java index 0fd260ed919..45683a0bb18 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/RemoveProjectActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/RemoveProjectActionTest.java @@ -31,6 +31,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ResourceTypesRule; +import org.sonar.db.permission.GlobalPermission; import org.sonar.db.project.ProjectDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.user.UserDto; @@ -132,13 +133,13 @@ public class RemoveProjectActionTest { } @Test - public void as_qprofile_editor() { + public void as_qprofile_editor_and_global_admin() { ProjectDto project = db.components().insertPrivateProjectDto(); QProfileDto profile = db.qualityProfiles().insert(p -> p.setLanguage(LANGUAGE_1)); db.qualityProfiles().associateWithProject(project, profile); UserDto user = db.users().insertUser(); db.qualityProfiles().addUserPermission(profile, user); - userSession.logIn(user); + userSession.logIn(user).addPermission(GlobalPermission.ADMINISTER_QUALITY_PROFILES); call(project, profile); @@ -146,6 +147,20 @@ public class RemoveProjectActionTest { verify(qualityProfileChangeEventService).publishRuleActivationToSonarLintClients(project, null, profile); } + @Test + public void as_qprofile_editor_fail_if_not_project_nor_global_admin() { + ProjectDto project = db.components().insertPrivateProjectDto(); + QProfileDto profile = db.qualityProfiles().insert(p -> p.setLanguage(LANGUAGE_1)); + db.qualityProfiles().associateWithProject(project, profile); + UserDto user = db.users().insertUser(); + db.qualityProfiles().addUserPermission(profile, user); + userSession.logIn(user); + + assertThatThrownBy(() -> call(project, profile)) + .isInstanceOf(ForbiddenException.class) + .hasMessage("Insufficient privileges"); + } + @Test public void fail_if_not_enough_permissions() { userSession.logIn(db.users().insertUser()); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/SearchActionTest.java index 1af11399c92..cef65850221 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/SearchActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/qualityprofile/ws/SearchActionTest.java @@ -287,9 +287,9 @@ public class SearchActionTest { .extracting(QualityProfile::getKey, qp -> qp.getActions().getEdit(), qp -> qp.getActions().getCopy(), qp -> qp.getActions().getSetAsDefault(), qp -> qp.getActions().getDelete(), qp -> qp.getActions().getAssociateProjects()) .containsExactlyInAnyOrder( - tuple(profile1.getKee(), true, false, false, true, true), + tuple(profile1.getKee(), true, false, false, false, false), tuple(profile2.getKee(), false, false, false, false, false), - tuple(profile3.getKee(), true, false, false, true, true), + tuple(profile3.getKee(), true, false, false, false, false), tuple(builtInProfile.getKee(), false, false, false, false, false)); assertThat(result.getActions().getCreate()).isFalse(); } -- 2.39.5