From 26bdd39e055c5f2c8d468cee20b02d6901733414 Mon Sep 17 00:00:00 2001 From: Nolwenn Cadic <98824442+Nolwenn-cadic-sonarsource@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:32:01 +0100 Subject: [PATCH] SONAR-23594 Fix SSF-605 --- .../user/ws/ChangePasswordActionIT.java | 32 ++++++++++++++++++- .../server/user/ws/ChangePasswordAction.java | 13 ++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/ChangePasswordActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/ChangePasswordActionIT.java index 7f493a4d841..4439dc5ca8a 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/ChangePasswordActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/user/ws/ChangePasswordActionIT.java @@ -40,6 +40,8 @@ import org.sonar.db.user.SessionTokenDto; import org.sonar.db.user.UserDto; import org.sonar.server.authentication.CredentialsLocalAuthentication; import org.sonar.server.authentication.JwtHttpHandler; +import org.sonar.server.common.management.ManagedInstanceChecker; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.tester.UserSessionRule; @@ -55,6 +57,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -88,7 +91,9 @@ public class ChangePasswordActionIT { private final JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class); - private final ChangePasswordAction underTest = new ChangePasswordAction(db.getDbClient(), userUpdater, userSessionRule, localAuthentication, jwtHttpHandler); + private final ManagedInstanceChecker managedInstanceChecker = mock(ManagedInstanceChecker.class); + + private final ChangePasswordAction underTest = new ChangePasswordAction(db.getDbClient(), userUpdater, userSessionRule, localAuthentication, jwtHttpHandler, managedInstanceChecker); private ServletOutputStream responseOutputStream; @Before @@ -248,6 +253,7 @@ public class ChangePasswordActionIT { executeTest(user.getLogin(), "I dunno", NEW_PASSWORD); verify(response).setStatus(HTTP_BAD_REQUEST); + assertThat(responseOutputStream).hasToString("{\"result\":\"Password cannot be changed when external authentication is used\"}"); } @Test @@ -262,6 +268,30 @@ public class ChangePasswordActionIT { verifyNoInteractions(jwtHttpHandler); } + @Test + public void changePassword_whenInstanceIsManagedAndUserUpdate_shouldThrow() { + doThrow(BadRequestException.create("Operation not allowed when the instance is externally managed.")).when(managedInstanceChecker).throwIfInstanceIsManaged(); + + UserTestData user = createLocalUser(OLD_PASSWORD); + userSessionRule.logIn(user.userDto()); + executeTest(user.getLogin(), OLD_PASSWORD, NEW_PASSWORD); + verify(response).setStatus(HTTP_BAD_REQUEST); + assertThat(responseOutputStream).hasToString("{\"result\":\"Operation not allowed when the instance is externally managed.\"}"); + } + + @Test + public void changePassword_whenInstanceIsManagedAndAdminUpdate_shouldThrow() { + doThrow(BadRequestException.create("Operation not allowed when the instance is externally managed.")).when(managedInstanceChecker).throwIfInstanceIsManaged(); + + UserDto admin = db.users().insertUser(); + userSessionRule.logIn(admin).setSystemAdministrator(); + UserDto user = db.users().insertUser(u -> u.setLocal(false)); + + executeTest(user.getLogin(), OLD_PASSWORD, NEW_PASSWORD); + verify(response).setStatus(HTTP_BAD_REQUEST); + assertThat(responseOutputStream).hasToString("{\"result\":\"Operation not allowed when the instance is externally managed.\"}"); + } + @Test public void verify_definition() { String controllerKey = "foo"; diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java index b2862f86cfa..2ab8ac9ccf2 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java @@ -25,12 +25,12 @@ import com.google.gson.stream.JsonWriter; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.server.http.HttpRequest; import org.sonar.api.server.http.HttpResponse; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.WebService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.sonar.api.web.FilterChain; import org.sonar.api.web.HttpFilter; import org.sonar.api.web.UrlPattern; @@ -41,6 +41,7 @@ import org.sonar.server.authentication.CredentialsLocalAuthentication; import org.sonar.server.authentication.JwtHttpHandler; import org.sonar.server.authentication.event.AuthenticationEvent; import org.sonar.server.authentication.event.AuthenticationException; +import org.sonar.server.common.management.ManagedInstanceChecker; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.UpdateUser; @@ -73,14 +74,16 @@ public class ChangePasswordAction extends HttpFilter implements BaseUsersWsActio private final UserSession userSession; private final CredentialsLocalAuthentication localAuthentication; private final JwtHttpHandler jwtHttpHandler; + private final ManagedInstanceChecker managedInstanceChecker; public ChangePasswordAction(DbClient dbClient, UserUpdater userUpdater, UserSession userSession, CredentialsLocalAuthentication localAuthentication, - JwtHttpHandler jwtHttpHandler) { + JwtHttpHandler jwtHttpHandler, ManagedInstanceChecker managedInstanceChecker) { this.dbClient = dbClient; this.userUpdater = userUpdater; this.userSession = userSession; this.localAuthentication = localAuthentication; this.jwtHttpHandler = jwtHttpHandler; + this.managedInstanceChecker = managedInstanceChecker; } @Override @@ -125,6 +128,7 @@ public class ChangePasswordAction extends HttpFilter implements BaseUsersWsActio if (login.equals(userSession.getLogin())) { user = getUserOrThrow(dbSession, login); + managedInstanceChecker.throwIfInstanceIsManaged(); String previousPassword = getParamOrThrow(request, PARAM_PREVIOUS_PASSWORD); checkPreviousPassword(dbSession, user, previousPassword); checkNewPasswordSameAsOld(newPassword, previousPassword); @@ -132,12 +136,15 @@ public class ChangePasswordAction extends HttpFilter implements BaseUsersWsActio } else { userSession.checkIsSystemAdministrator(); user = getUserOrThrow(dbSession, login); + managedInstanceChecker.throwIfInstanceIsManaged(); dbClient.sessionTokensDao().deleteByUser(dbSession, user); } + updatePassword(dbSession, user, newPassword); setResponseStatus(response, HTTP_NO_CONTENT); } catch (BadRequestException badRequestException) { setResponseStatus(response, HTTP_BAD_REQUEST); + writeJsonResponse(badRequestException.getMessage(), response); LOG.debug(badRequestException.getMessage(), badRequestException); } catch (PasswordException passwordException) { LOG.debug(passwordException.getMessage(), passwordException); -- 2.39.5