From db20ac93dcde28cbabf2679c9462a959caed462c Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 8 Feb 2017 21:17:53 +0100 Subject: [PATCH] SONAR-8761 organizations mgmt WS must check if org feature is enabled --- .../server/organization/ws/CreateAction.java | 3 +- .../server/organization/ws/DeleteAction.java | 14 +- .../ws/OrganizationsWsSupport.java | 10 ++ .../server/organization/ws/UpdateAction.java | 18 ++- .../organization/ws/CreateActionTest.java | 140 +++++++++++------- .../organization/ws/DeleteActionTest.java | 76 ++++++---- .../ws/EnableSupportActionTest.java | 7 +- .../organization/ws/UpdateActionTest.java | 100 +++++++++---- .../src/test/java/org/sonar/db/DbTester.java | 5 + 9 files changed, 246 insertions(+), 127 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java index 57dc62b0a8d..5fb1b26d71c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/CreateAction.java @@ -64,7 +64,7 @@ public class CreateAction implements OrganizationsAction { WebService.NewAction action = context.createAction(ACTION) .setPost(true) .setDescription("Create an organization.
" + - "Requires 'Administer System' permission unless any logged in user is allowed to create an organization (see appropriate setting).") + "Requires 'Administer System' permission unless any logged in user is allowed to create an organization (see appropriate setting). Organization feature must be enabled.") .setResponseExample(getClass().getResource("example-create.json")) .setInternal(true) .setSince("6.2") @@ -97,6 +97,7 @@ public class CreateAction implements OrganizationsAction { String avatar = wsSupport.getAndCheckAvatar(request); try (DbSession dbSession = dbClient.openSession(false)) { + wsSupport.checkFeatureEnabled(dbSession); OrganizationDto organization = organizationCreation.create( dbSession, userSession.getUserId().longValue(), diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java index e0aeb4a699c..f6eb82c613f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java @@ -44,13 +44,15 @@ public class DeleteAction implements OrganizationsAction { private final DbClient dbClient; private final DefaultOrganizationProvider defaultOrganizationProvider; private final ComponentCleanerService componentCleanerService; + private final OrganizationsWsSupport support; public DeleteAction(UserSession userSession, DbClient dbClient, DefaultOrganizationProvider defaultOrganizationProvider, - ComponentCleanerService componentCleanerService) { + ComponentCleanerService componentCleanerService, OrganizationsWsSupport support) { this.userSession = userSession; this.dbClient = dbClient; this.defaultOrganizationProvider = defaultOrganizationProvider; this.componentCleanerService = componentCleanerService; + this.support = support; } @Override @@ -58,7 +60,7 @@ public class DeleteAction implements OrganizationsAction { WebService.NewAction action = context.createAction(ACTION) .setPost(true) .setDescription("Delete an organization.
" + - "Require 'Administer System' permission on the specified organization.") + "Require 'Administer System' permission on the specified organization. Organization feature must be enabled.") .setInternal(true) .setSince("6.2") .setHandler(this); @@ -73,10 +75,12 @@ public class DeleteAction implements OrganizationsAction { public void handle(Request request, Response response) throws Exception { userSession.checkLoggedIn(); - String key = request.mandatoryParam(PARAM_KEY); - preventDeletionOfDefaultOrganization(key, defaultOrganizationProvider.get()); - try (DbSession dbSession = dbClient.openSession(false)) { + support.checkFeatureEnabled(dbSession); + + String key = request.mandatoryParam(PARAM_KEY); + preventDeletionOfDefaultOrganization(key, defaultOrganizationProvider.get()); + OrganizationDto organizationDto = checkFoundWithOptional( dbClient.organizationDao().selectByKey(dbSession, key), "Organization with key '%s' not found", diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java index 90c5f6e5c39..38a9fe68117 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/OrganizationsWsSupport.java @@ -26,6 +26,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.organization.OrganizationValidation; import org.sonar.server.property.InternalProperties; import org.sonarqube.ws.Organizations; @@ -123,4 +124,13 @@ public class OrganizationsWsSupport { Optional value = dbClient.internalPropertiesDao().selectByKey(dbSession, InternalProperties.ORGANIZATION_ENABLED); return value.isPresent() && Boolean.parseBoolean(value.get()); } + + /** + * Ensures that the organization feature is enabled, otherwise throws {@link BadRequestException} + */ + void checkFeatureEnabled(DbSession dbSession) { + if (!isFeatureEnabled(dbSession)) { + throw new BadRequestException("Organizations feature is not enabled"); + } + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java index 2c67c71c814..3fb4c4237a2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/UpdateAction.java @@ -59,7 +59,7 @@ public class UpdateAction implements OrganizationsAction { WebService.NewAction action = context.createAction(ACTION) .setPost(true) .setDescription("Update an organization.
" + - "Require 'Administer System' permission.") + "Require 'Administer System' permission. Organization feature must be enabled.") .setInternal(true) .setSince("6.2") .setHandler(this); @@ -76,15 +76,17 @@ public class UpdateAction implements OrganizationsAction { public void handle(Request request, Response response) throws Exception { userSession.checkLoggedIn(); - String key = request.mandatoryParam(PARAM_KEY); + try (DbSession dbSession = dbClient.openSession(false)) { + wsSupport.checkFeatureEnabled(dbSession); - UpdateOrganizationRequest updateRequest = new UpdateOrganizationRequest( - request.getParam(PARAM_NAME, (rqt, paramKey) -> wsSupport.getAndCheckName(rqt)), - request.getParam(PARAM_DESCRIPTION, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckDescription(rqt))), - request.getParam(PARAM_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckUrl(rqt))), - request.getParam(PARAM_AVATAR_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckAvatar(rqt)))); + String key = request.mandatoryParam(PARAM_KEY); + + UpdateOrganizationRequest updateRequest = new UpdateOrganizationRequest( + request.getParam(PARAM_NAME, (rqt, paramKey) -> wsSupport.getAndCheckName(rqt)), + request.getParam(PARAM_DESCRIPTION, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckDescription(rqt))), + request.getParam(PARAM_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckUrl(rqt))), + request.getParam(PARAM_AVATAR_URL, (rqt, paramKey) -> emptyAsNull(wsSupport.getAndCheckAvatar(rqt)))); - try (DbSession dbSession = dbClient.openSession(false)) { OrganizationDto dto = getDto(dbSession, key); userSession.checkOrganizationPermission(dto.getUuid(), SYSTEM_ADMIN); diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java index 70f69075edf..be2136eaf55 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java @@ -47,6 +47,7 @@ import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserMembershipDto; import org.sonar.db.user.UserMembershipQuery; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.OrganizationCreation; @@ -84,7 +85,6 @@ public class CreateActionTest { private DbClient dbClient = dbTester.getDbClient(); private DbSession dbSession = dbTester.getSession(); - private Settings settings = new MapSettings() .setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, false); private UuidFactory uuidFactory = mock(UuidFactory.class); @@ -99,7 +99,7 @@ public class CreateActionTest { assertThat(action.key()).isEqualTo("create"); assertThat(action.isPost()).isTrue(); assertThat(action.description()).isEqualTo("Create an organization.
" + - "Requires 'Administer System' permission unless any logged in user is allowed to create an organization (see appropriate setting)."); + "Requires 'Administer System' permission unless any logged in user is allowed to create an organization (see appropriate setting). Organization feature must be enabled."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); assertThat(action.handler()).isEqualTo(underTest); @@ -129,7 +129,7 @@ public class CreateActionTest { @Test public void verify_response_example() throws URISyntaxException, IOException { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(Uuids.UUID_EXAMPLE_01, SOME_DATE); String response = executeJsonRequest("Foo Company", "foo-company", "The Foo company produces quality software for Bar.", "https://www.foo.com", "https://www.foo.com/foo.png"); @@ -138,7 +138,10 @@ public class CreateActionTest { } @Test - public void request_fails_if_user_is_not_logged_in() { + public void request_fails_if_user_is_not_logged_in_and_logged_in_users_cannot_create_organizations() { + enableOrganizations(); + userSession.anonymous(); + expectedException.expect(ForbiddenException.class); expectedException.expectMessage("Insufficient privileges"); @@ -146,7 +149,20 @@ public class CreateActionTest { } @Test - public void request_fails_if_user_is_not_root_and_logged_in_user_can_not_create_organizations() { + public void request_fails_if_user_is_not_logged_in_and_logged_in_users_can_create_organizations() { + enableOrganizations(); + userSession.anonymous(); + settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); + + expectedException.expect(UnauthorizedException.class); + expectedException.expectMessage("Authentication is required"); + + executeRequest("name"); + } + + @Test + public void request_fails_if_user_is_not_root_and_logged_in_users_cannot_create_organizations() { + enableOrganizations(); userSession.logIn(); expectedException.expect(ForbiddenException.class); @@ -156,36 +172,27 @@ public class CreateActionTest { } @Test - public void request_succeeds_if_user_is_root_and_logged_in_user_can_not_create_organizations() { - makeUserRoot(); + public void request_succeeds_if_user_is_root_and_logged_in_users_cannot_create_organizations() { + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); } @Test - public void request_succeeds_if_user_is_root_and_logged_in_user_can_create_organizations() { + public void request_succeeds_if_user_is_root_and_logged_in_users_can_create_organizations() { + enableOrganizationsAndLogInAsRoot(); settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); - makeUserRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); } @Test - public void request_fails_if_user_is_not_logged_in_and_logged_in_user_can_create_organizations() { - settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); - - expectedException.expect(UnauthorizedException.class); - expectedException.expectMessage("Authentication is required"); - - executeRequest("name"); - } - - @Test - public void request_succeeds_if_user_is_not_root_and_logged_in_user_can_create_organizations() { - settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); + public void request_succeeds_if_user_is_not_root_and_logged_in_users_can_create_organizations() { + enableOrganizations(); userSession.logIn(); + settings.setProperty(ORGANIZATIONS_ANYONE_CAN_CREATE, true); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); verifyResponseAndDb(executeRequest("foo"), SOME_UUID, "foo", "foo", SOME_DATE); @@ -193,7 +200,7 @@ public class CreateActionTest { @Test public void request_fails_if_name_param_is_missing() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'name' parameter is missing"); @@ -203,7 +210,7 @@ public class CreateActionTest { @Test public void request_fails_if_name_is_one_char_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Name 'a' must be at least 2 chars long"); @@ -213,7 +220,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_name_is_two_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); verifyResponseAndDb(executeRequest("ab"), SOME_UUID, "ab", "ab", SOME_DATE); @@ -221,7 +228,7 @@ public class CreateActionTest { @Test public void request_fails_if_name_is_65_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Name '" + STRING_65_CHARS_LONG + "' must be at most 64 chars long"); @@ -231,7 +238,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_name_is_64_char_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String name = STRING_65_CHARS_LONG.substring(0, 64); @@ -241,7 +248,7 @@ public class CreateActionTest { @Test public void request_fails_if_key_one_char_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Key 'a' must be at least 2 chars long"); @@ -251,7 +258,7 @@ public class CreateActionTest { @Test public void request_fails_if_key_is_33_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); String key = STRING_65_CHARS_LONG.substring(0, 33); @@ -263,7 +270,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_key_is_2_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); verifyResponseAndDb(executeRequest("foo", "ab"), SOME_UUID, "foo", "ab", SOME_DATE); @@ -271,7 +278,7 @@ public class CreateActionTest { @Test public void requests_succeeds_if_key_is_32_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String key = STRING_65_CHARS_LONG.substring(0, 32); @@ -281,28 +288,28 @@ public class CreateActionTest { @Test public void requests_fails_if_key_contains_non_ascii_chars_but_dash() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); requestFailsWithInvalidCharInKey("ab@"); } @Test public void request_fails_if_key_starts_with_a_dash() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); requestFailsWithInvalidCharInKey("-ab"); } @Test public void request_fails_if_key_ends_with_a_dash() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); requestFailsWithInvalidCharInKey("ab-"); } @Test public void request_fails_if_key_contains_space() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); requestFailsWithInvalidCharInKey("a b"); } @@ -316,19 +323,18 @@ public class CreateActionTest { @Test public void request_fails_if_key_is_specified_and_already_exists_in_DB() { - makeUserRoot(); - String key = "the-key"; - insertOrganization(key); + enableOrganizationsAndLogInAsRoot(); + OrganizationDto org = insertOrganization("the-key"); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Key '" + key + "' is already used. Specify another one."); + expectedException.expectMessage("Key '" + org.getKey() + "' is already used. Specify another one."); - executeRequest("foo", key); + executeRequest("foo", org.getKey()); } @Test public void request_fails_if_key_computed_from_name_already_exists_in_DB() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); String key = STRING_65_CHARS_LONG.substring(0, 32); insertOrganization(key); @@ -342,7 +348,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_description_url_and_avatar_are_not_specified() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); CreateWsResponse response = executeRequest("foo", "bar", null, null, null); @@ -351,7 +357,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_description_url_and_avatar_are_specified() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); CreateWsResponse response = executeRequest("foo", "bar", "moo", "doo", "boo"); @@ -360,7 +366,7 @@ public class CreateActionTest { @Test public void request_succeeds_to_generate_key_from_name_more_then_32_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String name = STRING_65_CHARS_LONG.substring(0, 33); @@ -371,7 +377,7 @@ public class CreateActionTest { @Test public void request_generates_key_ignoring_multiple_following_spaces() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String name = "ab cd"; @@ -382,7 +388,7 @@ public class CreateActionTest { @Test public void request_fails_if_description_is_257_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Description '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -392,7 +398,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_description_is_256_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String description = STRING_257_CHARS_LONG.substring(0, 256); @@ -402,7 +408,7 @@ public class CreateActionTest { @Test public void request_fails_if_url_is_257_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Url '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -412,7 +418,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_url_is_256_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String url = STRING_257_CHARS_LONG.substring(0, 256); @@ -422,7 +428,7 @@ public class CreateActionTest { @Test public void request_fails_if_avatar_is_257_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Avatar '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -432,7 +438,7 @@ public class CreateActionTest { @Test public void request_succeeds_if_avatar_is_256_chars_long() { - makeUserRoot(); + enableOrganizationsAndLogInAsRoot(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); String avatar = STRING_257_CHARS_LONG.substring(0, 256); @@ -442,6 +448,7 @@ public class CreateActionTest { @Test public void request_creates_owners_group_with_all_permissions_for_new_organization_and_add_current_user_to_it() { + enableOrganizations(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().makeRoot(dbTester.users().insertUser()); userSession.logIn(user).setRoot(); @@ -466,6 +473,7 @@ public class CreateActionTest { @Test public void request_creates_default_template_for_owner_group_and_anyone() { + enableOrganizations(); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); UserDto user = dbTester.users().makeRoot(dbTester.users().insertUser()); userSession.logIn(user).setRoot(); @@ -485,11 +493,16 @@ public class CreateActionTest { .containsOnly( tuple(ownersGroup.getId(), UserRole.ADMIN), tuple(ownersGroup.getId(), UserRole.ISSUE_ADMIN), tuple(0L, UserRole.USER), tuple(0L, UserRole.CODEVIEWER)); - } - private void makeUserRoot() { - userSession.logIn().setRoot(); + @Test + public void request_fails_with_BadRequestException_if_organization_feature_is_disabled() { + logInAsRoot(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Organizations feature is not enabled"); + + executeJsonRequest("Foo Company", "foo-company", "The Foo company produces quality software for Bar.", "https://www.foo.com", "https://www.foo.com/foo.png"); } private void mockForSuccessfulInsert(String uuid, long now) { @@ -572,13 +585,28 @@ public class CreateActionTest { assertThat(dto.getUpdatedAt()).isEqualTo(createdAt); } - private void insertOrganization(String key) { - dbClient.organizationDao().insert(dbTester.getSession(), new OrganizationDto() + private OrganizationDto insertOrganization(String key) { + OrganizationDto dto = new OrganizationDto() .setUuid(key + "_uuid") .setKey(key) .setName(key + "_name") .setCreatedAt((long) key.hashCode()) - .setUpdatedAt((long) key.hashCode())); + .setUpdatedAt((long) key.hashCode()); + dbClient.organizationDao().insert(dbTester.getSession(), dto); dbTester.commit(); + return dto; + } + + private void enableOrganizations() { + dbTester.enableOrganizations(); + } + + private void logInAsRoot() { + userSession.logIn().setRoot(); + } + + private void enableOrganizationsAndLogInAsRoot() { + enableOrganizations(); + logInAsRoot(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java index 2777d314004..83260c6eb70 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java @@ -36,9 +36,11 @@ import org.sonar.db.permission.template.PermissionTemplateDto; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.server.component.ComponentCleanerService; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.organization.OrganizationValidationImpl; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsActionTester; @@ -63,7 +65,8 @@ public class DeleteActionTest { private DbClient dbClient = dbTester.getDbClient(); private DbSession session = dbTester.getSession(); private ComponentCleanerService componentCleanerService = mock(ComponentCleanerService.class); - private DeleteAction underTest = new DeleteAction(userSession, dbTester.getDbClient(), TestDefaultOrganizationProvider.from(dbTester), componentCleanerService); + private OrganizationsWsSupport support = new OrganizationsWsSupport(new OrganizationValidationImpl(), dbClient); + private DeleteAction underTest = new DeleteAction(userSession, dbTester.getDbClient(), TestDefaultOrganizationProvider.from(dbTester), componentCleanerService, support); private WsActionTester wsTester = new WsActionTester(underTest); @Test @@ -72,7 +75,7 @@ public class DeleteActionTest { assertThat(action.key()).isEqualTo("delete"); assertThat(action.isPost()).isTrue(); assertThat(action.description()).isEqualTo("Delete an organization.
" + - "Require 'Administer System' permission on the specified organization."); + "Require 'Administer System' permission on the specified organization. Organization feature must be enabled."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); assertThat(action.handler()).isEqualTo(underTest); @@ -85,8 +88,20 @@ public class DeleteActionTest { .matches(param -> "Organization key".equals(param.description())); } + @Test + public void request_fails_with_organization_feature_is_disabled() { + userSession.logIn(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(""); + + wsTester.newRequest().execute(); + } + @Test public void request_fails_with_UnauthorizedException_if_user_is_not_logged_in() { + enableOrganizations(); + expectedException.expect(UnauthorizedException.class); expectedException.expectMessage("Authentication is required"); @@ -96,18 +111,17 @@ public class DeleteActionTest { @Test public void request_fails_with_IAE_if_key_param_is_missing() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'key' parameter is missing"); - wsTester.newRequest() - .execute(); + wsTester.newRequest().execute(); } @Test public void request_fails_with_IAE_if_key_is_the_one_of_default_organization() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Default Organization can't be deleted"); @@ -117,7 +131,7 @@ public class DeleteActionTest { @Test public void request_fails_with_NotFoundException_if_organization_with_specified_key_does_not_exist() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(NotFoundException.class); expectedException.expectMessage("Organization with key 'foo' not found"); @@ -126,7 +140,8 @@ public class DeleteActionTest { } @Test - public void request_fails_with_ForbiddenException_when_user_has_no_System_Administer_permission_for_non_guarded_organization() { + public void request_fails_with_ForbiddenException_when_user_is_not_root_and_is_not_administrator_of_specified_organization() { + enableOrganizations(); OrganizationDto organization = dbTester.organizations().insert(); userSession.logIn(); @@ -137,9 +152,10 @@ public class DeleteActionTest { } @Test - public void request_fails_with_ForbiddenException_when_user_does_not_have_System_Administer_permission_on_specified_non_guarded_organization() { + public void request_fails_with_ForbiddenException_when_user_is_not_root_and_is_administrator_of_other_organization() { + enableOrganizations(); OrganizationDto organization = dbTester.organizations().insert(); - userSession.logIn().addOrganizationPermission(dbTester.getDefaultOrganization().getUuid(), SYSTEM_ADMIN); + logInAsAdministrator(dbTester.getDefaultOrganization()); expectedException.expect(ForbiddenException.class); expectedException.expectMessage("Insufficient privileges"); @@ -148,9 +164,10 @@ public class DeleteActionTest { } @Test - public void request_deletes_specified_non_guarded_organization_if_exists_and_user_has_Admin_permission_on_it() { + public void request_deletes_specified_organization_if_exists_and_user_is_administrator_of_it() { + enableOrganizations(); OrganizationDto organization = dbTester.organizations().insert(); - userSession.logIn().addOrganizationPermission(organization.getUuid(), SYSTEM_ADMIN); + logInAsAdministrator(organization); sendRequest(organization); @@ -158,18 +175,8 @@ public class DeleteActionTest { } @Test - public void request_fails_with_ForbiddenException_when_user_has_System_Administer_permission_on_specified_guarded_organization() { - OrganizationDto organization = dbTester.organizations().insert(dto -> dto.setGuarded(true)); - userSession.logIn().addOrganizationPermission(organization.getUuid(), SYSTEM_ADMIN); - - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); - - sendRequest(organization); - } - - @Test - public void request_deletes_specified_non_guarded_organization_if_exists_and_user_is_root() { + public void request_deletes_specified_organization_if_exists_and_user_is_root() { + enableOrganizations(); OrganizationDto organization = dbTester.organizations().insert(); userSession.logIn().setRoot(); @@ -190,7 +197,7 @@ public class DeleteActionTest { @Test public void request_also_deletes_components_of_specified_organization() { - userSession.logIn().setRoot(); + enableOrganizationsAndLogInAsRoot(); OrganizationDto organization = dbTester.organizations().insert(); ComponentDto project = dbTester.components().insertProject(organization); @@ -213,7 +220,7 @@ public class DeleteActionTest { @Test public void request_also_deletes_permissions_templates_and_permissions_and_groups_of_specified_organization() { - userSession.logIn().setRoot(); + enableOrganizationsAndLogInAsRoot(); OrganizationDto org = dbTester.organizations().insert(); OrganizationDto otherOrg = dbTester.organizations().insert(); @@ -278,4 +285,21 @@ public class DeleteActionTest { .setParam(PARAM_KEY, organizationKey) .execute(); } + + private void enableOrganizations() { + dbTester.enableOrganizations(); + } + + private void logInAsRoot() { + userSession.logIn().setRoot(); + } + + private void logInAsAdministrator(OrganizationDto organization) { + userSession.logIn().addOrganizationPermission(organization.getUuid(), SYSTEM_ADMIN); + } + + private void enableOrganizationsAndLogInAsRoot() { + enableOrganizations(); + logInAsRoot(); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/EnableSupportActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/EnableSupportActionTest.java index 52e65907acd..ef1d7e13127 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/EnableSupportActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/EnableSupportActionTest.java @@ -32,7 +32,6 @@ import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.OrganizationValidationImpl; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.property.InternalProperties; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsActionTester; @@ -57,13 +56,17 @@ public class EnableSupportActionTest { @Test public void enabling_support_saves_internal_property_and_flags_caller_as_root() { UserDto user = db.users().insertUser(); + UserDto otherUser = db.users().insertUser(); + db.properties().verifyInternal("organization.enabled", null); db.rootFlag().verify(user.getLogin(), false); + db.rootFlag().verify(otherUser.getLogin(), false); logInAsSystemAdministrator(user.getLogin()); call(); - assertThat(db.getDbClient().internalPropertiesDao().selectByKey(db.getSession(), InternalProperties.ORGANIZATION_ENABLED)).hasValue("true"); + db.properties().verifyInternal("organization.enabled", "true"); db.rootFlag().verify(user.getLogin(), true); + db.rootFlag().verify(otherUser.getLogin(), false); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java index 4339eec14ac..a06d1352191 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/UpdateActionTest.java @@ -28,6 +28,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; import org.sonar.db.DbTester; import org.sonar.db.organization.OrganizationDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.organization.OrganizationValidationImpl; @@ -68,7 +69,7 @@ public class UpdateActionTest { assertThat(action.key()).isEqualTo("update"); assertThat(action.isPost()).isTrue(); assertThat(action.description()).isEqualTo("Update an organization.
" + - "Require 'Administer System' permission."); + "Require 'Administer System' permission. Organization feature must be enabled."); assertThat(action.isInternal()).isTrue(); assertThat(action.since()).isEqualTo("6.2"); assertThat(action.handler()).isEqualTo(underTest); @@ -97,17 +98,46 @@ public class UpdateActionTest { .matches(param -> param.description() != null); } + @Test + public void request_fails_with_organization_feature_is_disabled() { + userSession.logIn(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(""); + + wsTester.newRequest().execute(); + } + + @Test + public void request_succeeds_if_user_is_root() { + enableOrganizationsAndLogInAsRoot(); + OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); + + verifyResponseAndDb(executeKeyRequest(dto.getKey(), "ab"), dto, "ab", DATE_2); + } + + @Test + public void request_succeeds_if_user_is_administrator_of_specified_organization() { + enableOrganizations(); + OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); + logInAsAdministrator(dto); + + verifyResponseAndDb(executeKeyRequest(dto.getKey(), "ab"), dto, "ab", DATE_2); + } + @Test public void request_fails_with_UnauthorizedException_when_user_is_not_logged_in() { + enableOrganizations(); + expectedException.expect(UnauthorizedException.class); expectedException.expectMessage("Authentication is required"); - wsTester.newRequest() - .execute(); + wsTester.newRequest().execute(); } @Test - public void request_fails_if_user_does_not_have_any_SYSTEM_ADMIN_permission() { + public void request_fails_if_user_is_not_root_and_is_not_organization_administrator() { + enableOrganizations(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); userSession.logIn(); @@ -118,20 +148,20 @@ public class UpdateActionTest { } @Test - public void request_fails_if_user_has_SYSTEM_ADMIN_permission_on_other_organization() { - OrganizationDto dto = dbTester.organizations().insert(); - userSession.addOrganizationPermission(dbTester.getDefaultOrganization().getUuid(), SYSTEM_ADMIN); - userSession.logIn(); + public void request_fails_if_user_is_administrator_of_another_organization() { + enableOrganizations(); + OrganizationDto org = dbTester.organizations().insert(); + logInAsAdministrator(dbTester.getDefaultOrganization()); expectedException.expect(ForbiddenException.class); expectedException.expectMessage("Insufficient privileges"); - executeKeyRequest(dto.getKey(), "name"); + executeKeyRequest(org.getKey(), "name"); } @Test public void request_fails_if_key_is_missing() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The 'key' parameter is missing"); @@ -140,16 +170,16 @@ public class UpdateActionTest { } @Test - public void request_with_only_key_param_does_not_fail_and_updates_only_updateAt_field() { + public void request_with_only_key_param_succeeds_and_updates_only_updateAt_field() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); verifyResponseAndDb(executeKeyRequest(dto.getKey(), null), dto, dto.getName(), DATE_2); } @Test public void request_fails_if_name_is_one_char_long() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Name 'a' must be at least 2 chars long"); @@ -159,15 +189,15 @@ public class UpdateActionTest { @Test public void request_succeeds_if_name_is_two_chars_long() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); verifyResponseAndDb(executeKeyRequest(dto.getKey(), "ab"), dto, "ab", DATE_2); } @Test public void request_fails_if_name_is_65_chars_long() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Name '" + STRING_65_CHARS_LONG + "' must be at most 64 chars long"); @@ -177,8 +207,8 @@ public class UpdateActionTest { @Test public void request_succeeds_if_name_is_64_char_long() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); String name = STRING_65_CHARS_LONG.substring(0, 64); @@ -187,8 +217,8 @@ public class UpdateActionTest { @Test public void request_succeeds_if_description_url_and_avatar_are_not_specified() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, null); verifyResponseAndDb(response, dto, "bar", DATE_2); @@ -196,8 +226,8 @@ public class UpdateActionTest { @Test public void request_succeeds_if_description_url_and_avatar_are_specified() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", "moo", "doo", "boo"); verifyResponseAndDb(response, dto, "bar", "moo", "doo", "boo", DATE_2); @@ -205,7 +235,7 @@ public class UpdateActionTest { @Test public void request_fails_if_description_is_257_chars_long() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Description '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -215,8 +245,8 @@ public class UpdateActionTest { @Test public void request_succeeds_if_description_is_256_chars_long() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); String description = STRING_257_CHARS_LONG.substring(0, 256); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", description, null, null); @@ -225,7 +255,7 @@ public class UpdateActionTest { @Test public void request_fails_if_url_is_257_chars_long() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Url '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -235,9 +265,9 @@ public class UpdateActionTest { @Test public void request_succeeds_if_url_is_256_chars_long() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); String url = STRING_257_CHARS_LONG.substring(0, 256); - giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, url, null); verifyResponseAndDb(response, dto, "bar", dto.getDescription(), url, dto.getAvatarUrl(), DATE_2); @@ -245,7 +275,7 @@ public class UpdateActionTest { @Test public void request_fails_if_avatar_is_257_chars_long() { - userSession.logIn(); + enableOrganizationsAndLogInAsRoot(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Avatar '" + STRING_257_CHARS_LONG + "' must be at most 256 chars long"); @@ -255,8 +285,8 @@ public class UpdateActionTest { @Test public void request_succeeds_if_avatar_is_256_chars_long() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); String avatar = STRING_257_CHARS_LONG.substring(0, 256); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bar", null, null, avatar); @@ -265,17 +295,13 @@ public class UpdateActionTest { @Test public void request_removes_optional_parameters_when_associated_parameter_are_empty() { + enableOrganizationsAndLogInAsRoot(); OrganizationDto dto = mockForSuccessfulUpdate(DATE_1, DATE_2); - giveUserSystemAdminPermission(dto); Organizations.UpdateWsResponse response = executeKeyRequest(dto.getKey(), "bla", "", "", ""); verifyResponseAndDb(response, dto, "bla", null, null, null, DATE_2); } - private void giveUserSystemAdminPermission(OrganizationDto organizationDto) { - userSession.logIn().addOrganizationPermission(organizationDto.getUuid(), SYSTEM_ADMIN); - } - private OrganizationDto mockForSuccessfulUpdate(long createdAt, long nextNow) { OrganizationDto dto = insertOrganization(createdAt); when(system2.now()).thenReturn(nextNow); @@ -352,4 +378,20 @@ public class UpdateActionTest { assertThat(newDto.getUpdatedAt()).isEqualTo(updateAt); } + private void enableOrganizations() { + dbTester.enableOrganizations(); + } + + private void logInAsRoot() { + userSession.logIn().setRoot(); + } + + private void logInAsAdministrator(OrganizationDto organizationDto) { + userSession.logIn().addOrganizationPermission(organizationDto.getUuid(), SYSTEM_ADMIN); + } + + private void enableOrganizationsAndLogInAsRoot() { + enableOrganizations(); + logInAsRoot(); + } } diff --git a/sonar-db/src/test/java/org/sonar/db/DbTester.java b/sonar-db/src/test/java/org/sonar/db/DbTester.java index 17651fa8bd5..61a0c277b1a 100644 --- a/sonar-db/src/test/java/org/sonar/db/DbTester.java +++ b/sonar-db/src/test/java/org/sonar/db/DbTester.java @@ -177,6 +177,11 @@ public class DbTester extends ExternalResource { return this; } + public DbTester enableOrganizations() { + properties().insertInternal("organization.enabled", "true"); + return this; + } + @Override protected void before() throws Throwable { db.start(); -- 2.39.5