From: Teryk Bellahsene Date: Fri, 23 Jun 2017 09:33:37 +0000 (+0200) Subject: SONAR-9448 Sanitize api/qualityprofiles/rename X-Git-Tag: 6.5-M2~86 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=9a14b55abf2e2bc660b239f3da823f0b42193572;p=sonarqube.git SONAR-9448 Sanitize api/qualityprofiles/rename --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RenameAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RenameAction.java index f6b43612c1e..01bb9e6debe 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RenameAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RenameAction.java @@ -19,14 +19,11 @@ */ package org.sonar.server.qualityprofile.ws; -import com.google.common.annotations.VisibleForTesting; -import java.util.Objects; import org.apache.commons.lang.StringUtils; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.NewAction; -import org.sonar.core.util.Uuids; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; @@ -36,13 +33,14 @@ import org.sonar.server.user.UserSession; import static java.lang.String.format; import static java.util.Optional.ofNullable; +import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; import static org.sonar.server.ws.WsUtils.checkRequest; +import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_NAME; +import static org.sonarqube.ws.client.qualityprofile.QualityProfileWsParameters.PARAM_PROFILE; public class RenameAction implements QProfileWsAction { - private static final String PARAM_PROFILE_NAME = "name"; - private static final String PARAM_PROFILE_KEY = "key"; private static final int MAXIMUM_NAME_LENGTH = 100; private final DbClient dbClient; @@ -58,34 +56,35 @@ public class RenameAction implements QProfileWsAction { @Override public void define(WebService.NewController controller) { NewAction setDefault = controller.createAction("rename") - .setSince("5.2") - .setDescription("Rename a quality profile. Require Administer Quality Profiles permission.") - .setPost(true) - .setHandler(this); - - setDefault.createParam(PARAM_PROFILE_NAME) - .setDescription("The new name for the quality profile.") - .setExampleValue("My Sonar way") - .setRequired(true); - - setDefault.createParam(PARAM_PROFILE_KEY) - .setDescription("The key of a quality profile.") - .setExampleValue(Uuids.UUID_EXAMPLE_01) - .setRequired(true); + .setSince("5.2") + .setDescription("Rename a quality profile.
" + + "Requires to be logged in and the 'Administer Quality Profiles' permission.") + .setPost(true) + .setHandler(this); + + setDefault.createParam(PARAM_NAME) + .setDescription("New quality profile name") + .setExampleValue("My Sonar way") + .setRequired(true); + + setDefault.createParam(PARAM_PROFILE) + .setDescription("Quality profile key") + .setDeprecatedKey("key", "6.5") + .setExampleValue(UUID_EXAMPLE_01) + .setRequired(true); } @Override public void handle(Request request, Response response) throws Exception { - String newName = request.mandatoryParam(PARAM_PROFILE_NAME); - String profileKey = request.mandatoryParam(PARAM_PROFILE_KEY); + String newName = request.mandatoryParam(PARAM_NAME); + String profileKey = request.mandatoryParam(PARAM_PROFILE); doHandle(newName, profileKey); response.noContent(); } - @VisibleForTesting - void doHandle(String newName, String profileKey) { + private void doHandle(String newName, String profileKey) { checkRequest(StringUtils.isNotBlank(newName), "Name must be set"); - checkRequest(newName.length() <= MAXIMUM_NAME_LENGTH, String.format("Name is too long (>%d characters)", MAXIMUM_NAME_LENGTH)); + checkRequest(newName.length() <= MAXIMUM_NAME_LENGTH, "Name is too long (>%d characters)", MAXIMUM_NAME_LENGTH); userSession.checkLoggedIn(); try (DbSession dbSession = dbClient.openSession(false)) { @@ -95,19 +94,21 @@ public class RenameAction implements QProfileWsAction { userSession.checkPermission(ADMINISTER_QUALITY_PROFILES, organizationUuid); wsSupport.checkNotBuiltInt(qualityProfile); - if (!Objects.equals(newName, qualityProfile.getName())) { - OrganizationDto organization = dbClient.organizationDao().selectByUuid(dbSession, organizationUuid) - .orElseThrow(() -> new IllegalStateException("No organization found for uuid " + organizationUuid)); - String language = qualityProfile.getLanguage(); - ofNullable(dbClient.qualityProfileDao().selectByNameAndLanguage(dbSession, organization, newName, language)) - .ifPresent(found -> { - throw BadRequestException.create(format("Quality profile already exists: %s", newName)); - }); - - qualityProfile.setName(newName); - dbClient.qualityProfileDao().update(dbSession, qualityProfile); - dbSession.commit(); + if (newName.equals(qualityProfile.getName())) { + return; } + + OrganizationDto organization = dbClient.organizationDao().selectByUuid(dbSession, organizationUuid) + .orElseThrow(() -> new IllegalStateException("No organization found for uuid " + organizationUuid)); + String language = qualityProfile.getLanguage(); + ofNullable(dbClient.qualityProfileDao().selectByNameAndLanguage(dbSession, organization, newName, language)) + .ifPresent(found -> { + throw BadRequestException.create(format("Quality profile already exists: %s", newName)); + }); + + qualityProfile.setName(newName); + dbClient.qualityProfileDao().update(dbSession, qualityProfile); + dbSession.commit(); } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java index 1c547efa587..628e409af4b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java @@ -183,13 +183,4 @@ public class QProfilesWsTest { "organization", "profile", "language", "profileName"); assertThat(inheritance.responseExampleAsString()).isNotEmpty(); } - - @Test - public void define_rename_action() { - WebService.Action rename = controller.action("rename"); - assertThat(rename).isNotNull(); - assertThat(rename.isPost()).isTrue(); - assertThat(rename.params()).hasSize(2).extracting("key").containsOnly( - "key", "name"); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/RenameActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/RenameActionTest.java index 43e8616f6c1..2208956d275 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/RenameActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/RenameActionTest.java @@ -19,10 +19,13 @@ */ package org.sonar.server.qualityprofile.ws; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.server.ws.WebService.Param; import org.sonar.api.utils.System2; import org.sonar.db.DbClient; import org.sonar.db.DbTester; @@ -34,12 +37,12 @@ import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.tester.UserSessionRule; -import org.sonar.server.ws.WsTester; +import org.sonar.server.ws.TestRequest; +import org.sonar.server.ws.WsActionTester; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; +import static org.sonar.core.util.Protobuf.setNullable; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; public class RenameActionTest { @@ -51,26 +54,22 @@ public class RenameActionTest { @Rule public ExpectedException expectedException = ExpectedException.none(); + private DbClient dbClient = db.getDbClient(); + + private WsActionTester ws; + private String xoo1Key = "xoo1"; private String xoo2Key = "xoo2"; - private DbClient dbClient; - private WsTester tester; private OrganizationDto organization; - private RenameAction underTest; - private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); @Before public void setUp() { - dbClient = db.getDbClient(); TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSessionRule, defaultOrganizationProvider); - underTest = new RenameAction( - dbClient, - userSessionRule, wsSupport); - tester = new WsTester(new QProfilesWs( - underTest)); - organization = db.organizations().insert(); + RenameAction underTest = new RenameAction(dbClient, userSessionRule, wsSupport); + ws = new WsActionTester(underTest); + organization = db.organizations().insert(); createProfiles(); } @@ -79,7 +78,7 @@ public class RenameActionTest { logInAsQProfileAdministrator(); String qualityProfileKey = createNewValidQualityProfileKey(); - underTest.doHandle("the new name", qualityProfileKey); + call(qualityProfileKey, "the new name"); QProfileDto reloaded = db.getDbClient().qualityProfileDao().selectByUuid(db.getSession(), qualityProfileKey); assertThat(reloaded.getName()).isEqualTo("the new name"); @@ -106,7 +105,7 @@ public class RenameActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Quality profile already exists: Invalid, duplicated name"); - underTest.doHandle("Invalid, duplicated name", qualityProfileKey1); + call(qualityProfileKey1, "Invalid, duplicated name"); } @Test @@ -130,22 +129,20 @@ public class RenameActionTest { db.qualityProfiles().insert(qualityProfile2); String qualityProfileKey2 = qualityProfile2.getKee(); - underTest.doHandle("Duplicated name", qualityProfileKey1); + call(qualityProfileKey1, "Duplicated name"); QProfileDto reloaded = db.getDbClient().qualityProfileDao().selectByUuid(db.getSession(), qualityProfileKey1); assertThat(reloaded.getName()).isEqualTo("Duplicated name"); } @Test - public void fail_if_parameter_key_is_missing() throws Exception { + public void fail_if_parameter_profile_is_missing() throws Exception { logInAsQProfileAdministrator(); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("The 'key' parameter is missing"); + expectedException.expectMessage("The 'profile' parameter is missing"); - tester.newPostRequest("api/qualityprofiles", "rename") - .setParam("name", "Other Sonar Way") - .execute(); + call(null, "Other Sonar Way"); } @Test @@ -155,9 +152,7 @@ public class RenameActionTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'name' parameter is missing"); - tester.newPostRequest("api/qualityprofiles", "rename") - .setParam("key", "sonar-way-xoo1-13245") - .execute(); + call("sonar-way-xoo1-13245", null); } @Test @@ -175,7 +170,7 @@ public class RenameActionTest { expectedException.expect(ForbiddenException.class); expectedException.expectMessage("Insufficient privileges"); - underTest.doHandle("Hey look I am not quality profile admin!", qualityProfileKey); + call(qualityProfileKey, "Hey look I am not quality profile admin!"); } @Test @@ -183,10 +178,7 @@ public class RenameActionTest { expectedException.expect(UnauthorizedException.class); expectedException.expectMessage("Authentication is required"); - tester.newPostRequest("api/qualityprofiles", "rename") - .setParam("key", "sonar-way-xoo1-13245") - .setParam("name", "Not logged in") - .execute(); + call("sonar-way-xoo1-13245", "Not logged in"); } @Test @@ -196,10 +188,7 @@ public class RenameActionTest { expectedException.expect(NotFoundException.class); expectedException.expectMessage("Quality Profile with key 'polop' does not exist"); - tester.newPostRequest("api/qualityprofiles", "rename") - .setParam("key", "polop") - .setParam("name", "Uh oh, I don't know this profile") - .execute(); + call("polop", "Uh oh, I don't know this profile"); } @Test @@ -209,7 +198,7 @@ public class RenameActionTest { expectedException.expect(BadRequestException.class); - underTest.doHandle("the new name", qualityProfileKey); + call(qualityProfileKey, "the new name"); } @Test @@ -218,13 +207,13 @@ public class RenameActionTest { String qualityProfileKey = createNewValidQualityProfileKey(); String a100charName = "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"; - underTest.doHandle(a100charName, qualityProfileKey); + call(qualityProfileKey, a100charName); expectedException.expect(BadRequestException.class); expectedException.expectMessage("Name is too long (>100 characters)"); String a101charName = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901"; - underTest.doHandle(a101charName, qualityProfileKey); + call(qualityProfileKey, a101charName); } @Test @@ -234,7 +223,7 @@ public class RenameActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Name must be set"); - underTest.doHandle(" ", qualityProfileKey); + call(qualityProfileKey, " "); } @Test @@ -244,7 +233,18 @@ public class RenameActionTest { expectedException.expect(NotFoundException.class); expectedException.expectMessage("Quality Profile with key 'unknown' does not exist"); - underTest.doHandle("the new name", "unknown"); + call("unknown", "the new name"); + } + + @Test + public void definition() { + WebService.Action definition = ws.getDef(); + + assertThat(definition.key()).isEqualTo("rename"); + assertThat(definition.isPost()).isTrue(); + assertThat(definition.params()).extracting(Param::key).containsExactlyInAnyOrder("profile", "name"); + Param profile = definition.param("profile"); + assertThat(profile.deprecatedKey()).isEqualTo("key"); } private String createNewValidQualityProfileKey() { @@ -255,14 +255,11 @@ public class RenameActionTest { } private void createProfiles() { - db.qualityProfiles().insert(organization, p -> - p.setKee("sonar-way-xoo1-12345").setLanguage(xoo1Key).setName("Sonar way")); + db.qualityProfiles().insert(organization, p -> p.setKee("sonar-way-xoo1-12345").setLanguage(xoo1Key).setName("Sonar way")); - QProfileDto parentXoo2 = db.qualityProfiles().insert(organization, p -> - p.setKee("sonar-way-xoo2-23456").setLanguage(xoo2Key).setName("Sonar way")); + QProfileDto parentXoo2 = db.qualityProfiles().insert(organization, p -> p.setKee("sonar-way-xoo2-23456").setLanguage(xoo2Key).setName("Sonar way")); - db.qualityProfiles().insert(organization, p -> - p.setKee("my-sonar-way-xoo2-34567").setLanguage(xoo2Key).setName("My Sonar way").setParentKee(parentXoo2.getKee())); + db.qualityProfiles().insert(organization, p -> p.setKee("my-sonar-way-xoo2-34567").setLanguage(xoo2Key).setName("My Sonar way").setParentKee(parentXoo2.getKee())); } private void logInAsQProfileAdministrator() { @@ -270,4 +267,14 @@ public class RenameActionTest { .logIn() .addPermission(ADMINISTER_QUALITY_PROFILES, organization); } + + private void call(@Nullable String key, @Nullable String name) { + TestRequest request = ws.newRequest() + .setMethod("POST"); + + setNullable(key, k -> request.setParam("profile", k)); + setNullable(name, n -> request.setParam("name", n)); + + request.execute(); + } } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualityprofile/QualityProfileWsParameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualityprofile/QualityProfileWsParameters.java index 59f30a4ceb0..5ce75182da8 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualityprofile/QualityProfileWsParameters.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualityprofile/QualityProfileWsParameters.java @@ -22,12 +22,12 @@ package org.sonarqube.ws.client.qualityprofile; public class QualityProfileWsParameters { public static final String CONTROLLER_QUALITY_PROFILES = "api/qualityprofiles"; - public interface RestoreActionParameters { + String PARAM_BACKUP = "backup"; } - public static final String ACTION_ACTIVATE_RULE = "activate_rule"; + public static final String ACTION_ACTIVATE_RULES = "activate_rules"; public static final String ACTION_ADD_PROJECT = "add_project"; public static final String ACTION_CHANGE_PARENT = "change_parent"; @@ -40,11 +40,12 @@ public class QualityProfileWsParameters { public static final String ACTION_RESTORE = "restore"; public static final String ACTION_SEARCH = "search"; public static final String ACTION_SET_DEFAULT = "set_default"; - public static final String PARAM_DEFAULTS = "defaults"; + public static final String PARAM_FROM_KEY = "fromKey"; public static final String PARAM_ORGANIZATION = "organization"; public static final String PARAM_LANGUAGE = "language"; + public static final String PARAM_NAME = "name"; public static final String PARAM_PARAMS = "params"; public static final String PARAM_PARENT_NAME = "parentName"; public static final String PARAM_PARENT_PROFILE = "parentProfile";