SONAR-23594 Fix SSF-605

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 7f493a4d8413977d32b2023c653b89c8cb93612d..4439dc5ca8a7f67248506a568b412f90ba9ca1c9 100644 (file)
--- 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 b2862f86cfa1115e8a36458df684df20e6c866ff..2ab8ac9ccf2a238cc81dad7b50d30a2c6789890d 100644 (file)
--- 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);