From 0e2f2db424b45802ee59aecbdd2148285f6630c4 Mon Sep 17 00:00:00 2001 From: Nolwenn Cadic <98824442+Nolwenn-cadic-sonarsource@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:06:29 +0100 Subject: [PATCH] SONAR-23594 Fix SSF-606 --- .../common/user/service/UserServiceIT.java | 24 +++++++++++++++++++ .../management/ManagedInstanceChecker.java | 4 ++++ .../common/user/service/UserService.java | 9 +++++++ .../ManagedInstanceCheckerTest.java | 20 ++++++++++++++++ .../controller/DefaultUserController.java | 1 + 5 files changed, 58 insertions(+) diff --git a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java index 58b649e1fa4..14233509ae1 100644 --- a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java +++ b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java @@ -19,6 +19,9 @@ */ package org.sonar.server.common.user.service; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Collection; @@ -30,6 +33,7 @@ import java.util.stream.IntStream; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.server.authentication.IdentityProvider; import org.sonar.api.utils.DateUtils; @@ -87,6 +91,7 @@ import static org.mockito.Mockito.when; import static org.sonar.db.property.PropertyTesting.newUserPropertyDto; import static org.sonar.db.user.UserTesting.newUserDto; +@RunWith(DataProviderRunner.class) public class UserServiceIT { private static final UsersSearchRequest SEARCH_REQUEST = getBuilderWithDefaultsPageSize().build(); @@ -839,6 +844,25 @@ public class UserServiceIT { assertThat(updatedUser.getExternalId()).isEqualTo("prov_id"); assertThat(updatedUser.getExternalLogin()).isEqualTo("prov_login"); } + @DataProvider + public static Object[][] updateUserProvider() { + return new Object[][]{ + {new UpdateUser().setName("new name")}, + {new UpdateUser().setEmail("newEmail@test.com")}, + {new UpdateUser().setName("new name").setEmail("newEmail@test.com")}}; + } + + @Test + @UseDataProvider("updateUserProvider") + public void updateUser_whenIncorrectPayloadForManagedInstance_shouldThrow(UpdateUser updateUser) { + doThrow(BadRequestException.create("message")).when(managedInstanceChecker).throwIfInstanceIsManaged(any()); + + UserDto userDto = db.users().insertUser(); + + assertThatThrownBy(() -> userService.updateUser(userDto.getUuid(), updateUser)) + .isInstanceOf(BadRequestException.class) + .hasMessage("message"); + } private void assertUserWithFilter(Function query, String userLogin, boolean isExpectedToBeThere) { diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/management/ManagedInstanceChecker.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/management/ManagedInstanceChecker.java index dad6f4ac6d2..6d007a93c3f 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/management/ManagedInstanceChecker.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/management/ManagedInstanceChecker.java @@ -41,6 +41,10 @@ public class ManagedInstanceChecker { BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), INSTANCE_EXCEPTION_MESSAGE); } + public void throwIfInstanceIsManaged(String errorMessage) { + BadRequestException.checkRequest(!managedInstanceService.isInstanceExternallyManaged(), errorMessage); + } + public void throwIfProjectIsManaged(DbSession dbSession, String projectUuid) { BadRequestException.checkRequest(!managedProjectService.isProjectManaged(dbSession, projectUuid), PROJECT_EXCEPTION_MESSAGE); } diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java index ed83c9673d0..a3291090e3f 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java @@ -222,6 +222,7 @@ public class UserService { public UserInformation updateUser(String uuid, UpdateUser updateUser) { try (DbSession dbSession = dbClient.openSession(false)) { throwIfInvalidChangeOfExternalProvider(updateUser); + throwIfManagedInstanceAndNameOrEmailUpdated(updateUser); UserDto userDto = findUserOrThrow(uuid, dbSession); userUpdater.updateAndCommit(dbSession, userDto, updateUser, u -> { }); @@ -247,6 +248,14 @@ public class UserService { Optional.ofNullable(updateUser.externalIdentityProvider()).ifPresent(this::assertProviderIsSupported); } + private void throwIfManagedInstanceAndNameOrEmailUpdated(UpdateUser updateUser) { + boolean isNameChanged = updateUser.isNameChanged(); + boolean isEmailDefined = updateUser.isEmailChanged(); + if (isNameChanged || isEmailDefined) { + managedInstanceChecker.throwIfInstanceIsManaged("User name and email cannot be updated when the instance is externally managed"); + } + } + private void assertProviderIsSupported(String newExternalProvider) { List allowedIdentityProviders = getAvailableIdentityProviders(); diff --git a/server/sonar-webserver-common/src/test/java/com/sonar/server/common/management/ManagedInstanceCheckerTest.java b/server/sonar-webserver-common/src/test/java/com/sonar/server/common/management/ManagedInstanceCheckerTest.java index 83b3c732707..092ac87b501 100644 --- a/server/sonar-webserver-common/src/test/java/com/sonar/server/common/management/ManagedInstanceCheckerTest.java +++ b/server/sonar-webserver-common/src/test/java/com/sonar/server/common/management/ManagedInstanceCheckerTest.java @@ -62,6 +62,26 @@ public class ManagedInstanceCheckerTest { .hasMessage(INSTANCE_EXCEPTION_MESSAGE); } + @Test + public void throwIfInstanceIsManaged_whenCustomErrorMessage_shouldThrowWithCustomError() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(true); + + String customErrorMessage = "Custom error message"; + + assertThatThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged(customErrorMessage)) + .isInstanceOf(BadRequestException.class) + .hasMessage(customErrorMessage); + } + + @Test + public void throwIfInstanceIsManaged_whenCustomErrorMessageAndInstanceManaged_shouldNotThrow() { + when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(false); + + String customErrorMessage = "Custom error message"; + + assertThatNoException().isThrownBy(() -> managedInstanceChecker.throwIfInstanceIsManaged(customErrorMessage)); + } + @Test public void throwIfInstanceIsManaged_whenInstanceNotExternallyManaged_shouldNotThrow() { when(managedInstanceService.isInstanceExternallyManaged()).thenReturn(false); diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/user/controller/DefaultUserController.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/user/controller/DefaultUserController.java index c6da3fe0422..859fc39e369 100644 --- a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/user/controller/DefaultUserController.java +++ b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/user/controller/DefaultUserController.java @@ -117,6 +117,7 @@ public class DefaultUserController implements UserController { @Override public UserRestResponse updateUser(String id, UserUpdateRestRequest updateRequest) { userSession.checkLoggedIn().checkIsSystemAdministrator(); + UpdateUser update = toUpdateUser(updateRequest); UserInformation updatedUser = userService.updateUser(id, update); return usersSearchResponseGenerator.toRestUser(updatedUser); -- 2.39.5