]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-16177 fix SSF-206
authorAurelien Poscia <aurelien.poscia@sonarsource.com>
Tue, 24 May 2022 09:36:23 +0000 (11:36 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 30 May 2022 20:02:56 +0000 (20:02 +0000)
server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/BaseUsersWsAction.java [new file with mode: 0644]
server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWs.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWsAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UsersWsTest.java [new file with mode: 0644]
server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/ServletRequest.java
sonar-ws/src/main/java/org/sonarqube/ws/client/user/UsersWsParameters.java

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/BaseUsersWsAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/BaseUsersWsAction.java
new file mode 100644 (file)
index 0000000..8f7d198
--- /dev/null
@@ -0,0 +1,27 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.user.ws;
+
+import org.sonar.api.server.ws.Definable;
+import org.sonar.api.server.ws.WebService;
+
+public interface BaseUsersWsAction extends Definable<WebService.NewController> {
+  // Marker interface for UsersWs actions
+}
index 2d1852fa4c4becf88402ae67ac27909732cafa68..b28a6d53d11e0137e924753776ec214feb27a386 100644 (file)
  */
 package org.sonar.server.user.ws;
 
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import org.sonar.api.server.ws.Change;
-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.web.ServletFilter;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.user.UserDto;
 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.exceptions.BadRequestException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.UpdateUser;
 import org.sonar.server.user.UserSession;
 import org.sonar.server.user.UserUpdater;
+import org.sonar.server.ws.ServletFilterHandler;
 
 import static java.lang.String.format;
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
 import static org.sonarqube.ws.WsUtils.checkArgument;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_LOGIN;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_PASSWORD;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_PREVIOUS_PASSWORD;
 
-public class ChangePasswordAction implements UsersWsAction {
+public class ChangePasswordAction extends ServletFilter implements BaseUsersWsAction {
 
-  private static final String PARAM_LOGIN = "login";
-  private static final String PARAM_PASSWORD = "password";
-  private static final String PARAM_PREVIOUS_PASSWORD = "previousPassword";
+  private static final String CHANGE_PASSWORD = "change_password";
+  private static final String CHANGE_PASSWORD_URL = "/" + UsersWs.API_USERS + "/" + CHANGE_PASSWORD;
+  private static final String MSG_PARAMETER_MISSING = "The '%s' parameter is missing";
 
   private final DbClient dbClient;
   private final UserUpdater userUpdater;
   private final UserSession userSession;
   private final CredentialsLocalAuthentication localAuthentication;
+  private final JwtHttpHandler jwtHttpHandler;
 
-  public ChangePasswordAction(DbClient dbClient, UserUpdater userUpdater, UserSession userSession, CredentialsLocalAuthentication localAuthentication) {
+  public ChangePasswordAction(DbClient dbClient, UserUpdater userUpdater, UserSession userSession, CredentialsLocalAuthentication localAuthentication,
+    JwtHttpHandler jwtHttpHandler) {
     this.dbClient = dbClient;
     this.userUpdater = userUpdater;
     this.userSession = userSession;
     this.localAuthentication = localAuthentication;
+    this.jwtHttpHandler = jwtHttpHandler;
+  }
+
+  @Override
+  public UrlPattern doGetPattern() {
+    return UrlPattern.create(CHANGE_PASSWORD_URL);
   }
 
   @Override
   public void define(WebService.NewController controller) {
-    WebService.NewAction action = controller.createAction("change_password")
+    WebService.NewAction action = controller.createAction(CHANGE_PASSWORD)
       .setDescription("Update a user's password. Authenticated users can change their own password, " +
         "provided that the account is not linked to an external authentication system. " +
         "Administer System permission is required to change another user's password.")
       .setSince("5.2")
       .setPost(true)
-      .setHandler(this)
+      .setHandler(ServletFilterHandler.INSTANCE)
       .setChangelog(new Change("8.6", "It's no more possible for the password to be the same as the previous one"));
 
     action.createParam(PARAM_LOGIN)
@@ -83,33 +105,34 @@ public class ChangePasswordAction implements UsersWsAction {
   }
 
   @Override
-  public void handle(Request request, Response response) throws Exception {
+  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
     userSession.checkLoggedIn();
-
     try (DbSession dbSession = dbClient.openSession(false)) {
-      String login = request.mandatoryParam(PARAM_LOGIN);
-      String password = request.mandatoryParam(PARAM_PASSWORD);
-      UserDto user = getUser(dbSession, login);
+      String login = getParamOrThrow(request, PARAM_LOGIN);
+      String newPassword = getParamOrThrow(request, PARAM_PASSWORD);
+      UserDto user = getUserOrThrow(dbSession, login);
       if (login.equals(userSession.getLogin())) {
-        String previousPassword = request.mandatoryParam(PARAM_PREVIOUS_PASSWORD);
+        String previousPassword = getParamOrThrow(request, PARAM_PREVIOUS_PASSWORD);
         checkPreviousPassword(dbSession, user, previousPassword);
-        checkArgument(!previousPassword.equals(password), "Password must be different from old password");
+        checkArgument(!previousPassword.equals(newPassword), "Password must be different from old password");
+        deleteTokensAndRefreshSession(request, response, dbSession, user);
       } else {
         userSession.checkIsSystemAdministrator();
+        dbClient.sessionTokensDao().deleteByUser(dbSession, user);
       }
-      UpdateUser updateUser = new UpdateUser().setPassword(password);
-      userUpdater.updateAndCommit(dbSession, user, updateUser, u -> {
-      });
+      UpdateUser updateUser = new UpdateUser().setPassword(newPassword);
+      userUpdater.updateAndCommit(dbSession, user, updateUser, u -> {});
+      setResponseStatus(response, HTTP_NO_CONTENT);
+    } catch (BadRequestException badRequestException) {
+      setResponseStatus(response, HTTP_BAD_REQUEST);
     }
-    response.noContent();
   }
 
-  private UserDto getUser(DbSession dbSession, String login) {
-    UserDto user = dbClient.userDao().selectByLogin(dbSession, login);
-    if (user == null || !user.isActive()) {
-      throw new NotFoundException(format("User with login '%s' has not been found", login));
-    }
-    return user;
+
+  private static String getParamOrThrow(ServletRequest request, String key) {
+    String value = request.getParameter(key);
+    checkArgument(value != null && !value.isEmpty(), MSG_PARAMETER_MISSING, key);
+    return value;
   }
 
   private void checkPreviousPassword(DbSession dbSession, UserDto user, String password) {
@@ -119,4 +142,28 @@ public class ChangePasswordAction implements UsersWsAction {
       throw new IllegalArgumentException("Incorrect password");
     }
   }
+
+  private UserDto getUserOrThrow(DbSession dbSession, String login) {
+    UserDto user = dbClient.userDao().selectByLogin(dbSession, login);
+    if (user == null || !user.isActive()) {
+      throw new NotFoundException(format("User with login '%s' has not been found", login));
+    }
+    return user;
+  }
+
+  private void deleteTokensAndRefreshSession(ServletRequest request, ServletResponse response, DbSession dbSession, UserDto user) {
+    dbClient.sessionTokensDao().deleteByUser(dbSession, user);
+    refreshJwtToken(request, response, user);
+  }
+
+  private void refreshJwtToken(ServletRequest request, ServletResponse response, UserDto user) {
+    HttpServletRequest httpRequest = (HttpServletRequest) request;
+    HttpServletResponse httpResponse = (HttpServletResponse) response;
+    jwtHttpHandler.removeToken(httpRequest, httpResponse);
+    jwtHttpHandler.generateToken(user, httpRequest, httpResponse);
+  }
+
+  private static void setResponseStatus(ServletResponse response, int newStatusCode) {
+    ((HttpServletResponse) response).setStatus(newStatusCode);
+  }
 }
index 568629f02fd6d61f57b0e0b5739c7ca9e50d51ee..4235ff0fed082b7c105775416e102764fc7e5498 100644 (file)
  */
 package org.sonar.server.user.ws;
 
+import java.util.Collection;
 import org.sonar.api.server.ws.WebService;
 
 public class UsersWs implements WebService {
 
-  private final UsersWsAction[] actions;
+  static final String API_USERS = "api/users";
+  static final String DESCRIPTION = "Manage users.";
+  static final String SINCE_VERSION = "3.6";
 
-  public UsersWs(UsersWsAction... actions) {
-    this.actions = actions;
+  private final Collection<BaseUsersWsAction> usersWsActions;
+
+  public UsersWs(Collection<BaseUsersWsAction> usersWsActions) {
+    this.usersWsActions = usersWsActions;
   }
 
   @Override
   public void define(Context context) {
-    NewController controller = context.createController("api/users")
-      .setSince("3.6")
-      .setDescription("Manage users.");
-
-    for (UsersWsAction action : actions) {
-      action.define(controller);
-    }
+    NewController controller = context.createController(API_USERS)
+      .setSince(SINCE_VERSION)
+      .setDescription(DESCRIPTION);
 
+    usersWsActions.forEach(action -> action.define(controller));
     controller.done();
   }
 }
index ea57f9fecfe08c4602d2facd5a44a2dd8cf55f97..cd3e44d137eb444288f22c49629c92efec4c3ffa 100644 (file)
@@ -21,6 +21,6 @@ package org.sonar.server.user.ws;
 
 import org.sonar.server.ws.WsAction;
 
-public interface UsersWsAction extends WsAction {
+public interface UsersWsAction extends WsAction, BaseUsersWsAction {
   // Marker interface for UsersWs actions
 }
index e9d71f23928407f1e864a93f54d0650bd9df6e63..7d755135e7783006e9207844a9477c4e6ecef39d 100644 (file)
  */
 package org.sonar.server.user.ws;
 
+import java.io.IOException;
+import java.util.Optional;
+import javax.annotation.Nullable;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.server.ws.WebService;
+import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.audit.NoOpAuditPersister;
+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.es.EsTester;
-import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
@@ -38,23 +48,40 @@ import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.usergroups.DefaultGroupFinder;
-import org.sonar.server.ws.TestRequest;
-import org.sonar.server.ws.TestResponse;
-import org.sonar.server.ws.WsActionTester;
+import org.sonar.server.ws.ServletFilterHandler;
 
 import static java.lang.String.format;
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
 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.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_LOGIN;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_PASSWORD;
+import static org.sonarqube.ws.client.user.UsersWsParameters.PARAM_PREVIOUS_PASSWORD;
 
 public class ChangePasswordActionTest {
 
+  private static final String OLD_PASSWORD = "1234";
+  private static final String NEW_PASSWORD = "12345";
+
   @Rule
   public DbTester db = DbTester.create();
   @Rule
   public EsTester es = EsTester.createCustom(UserIndexDefinition.createForTest());
   @Rule
   public UserSessionRule userSessionRule = UserSessionRule.standalone().logIn();
+
+  private final ArgumentCaptor<UserDto> userDtoCaptor = ArgumentCaptor.forClass(UserDto.class);
+
+  private final HttpServletRequest request = mock(HttpServletRequest.class);
+  private final HttpServletResponse response = mock(HttpServletResponse.class);
+  private final FilterChain chain = mock(FilterChain.class);
+
   private final MapSettings settings = new MapSettings().setProperty("sonar.internal.pbkdf2.iterations", "1");
   private final CredentialsLocalAuthentication localAuthentication = new CredentialsLocalAuthentication(db.getDbClient(), settings.asConfig());
 
@@ -62,7 +89,9 @@ public class ChangePasswordActionTest {
     new UserIndexer(db.getDbClient(), es.client()), new DefaultGroupFinder(db.getDbClient()),
     new MapSettings().asConfig(), new NoOpAuditPersister(), localAuthentication);
 
-  private final WsActionTester tester = new WsActionTester(new ChangePasswordAction(db.getDbClient(), userUpdater, userSessionRule, localAuthentication));
+  private final JwtHttpHandler jwtHttpHandler = mock(JwtHttpHandler.class);
+
+  private final ChangePasswordAction changePasswordAction = new ChangePasswordAction(db.getDbClient(), userUpdater, userSessionRule, localAuthentication, jwtHttpHandler);
 
   @Before
   public void setUp() {
@@ -70,66 +99,71 @@ public class ChangePasswordActionTest {
   }
 
   @Test
-  public void a_user_can_update_his_password() {
-    String oldPassword = "Valar Dohaeris";
-    UserDto user = createLocalUser(oldPassword);
-    String oldCryptedPassword = db.getDbClient().userDao().selectByLogin(db.getSession(), user.getLogin()).getCryptedPassword();
-    userSessionRule.logIn(user);
+  public void a_user_can_update_his_password() throws ServletException, IOException {
+    UserTestData user = createLocalUser(OLD_PASSWORD);
+    String oldCryptedPassword = findEncryptedPassword(user.getLogin());
+    userSessionRule.logIn(user.getUserDto());
 
-    TestResponse response = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("previousPassword", "Valar Dohaeris")
-      .setParam("password", "Valar Morghulis")
-      .execute();
+    executeTest(user.getLogin(), OLD_PASSWORD, NEW_PASSWORD);
 
-    assertThat(response.getStatus()).isEqualTo(204);
-    String newCryptedPassword = db.getDbClient().userDao().selectByLogin(db.getSession(), user.getLogin()).getCryptedPassword();
+    String newCryptedPassword = findEncryptedPassword(user.getLogin());
     assertThat(newCryptedPassword).isNotEqualTo(oldCryptedPassword);
+    verify(jwtHttpHandler).removeToken(request, response);
+    verify(jwtHttpHandler).generateToken(userDtoCaptor.capture(), eq(request), eq(response));
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isEmpty();
+    assertThat(userDtoCaptor.getValue().getLogin()).isEqualTo(user.getLogin());
+    verify(response).setStatus(HTTP_NO_CONTENT);
   }
 
   @Test
-  public void system_administrator_can_update_password_of_user() {
-    UserDto admin = createLocalUser();
-    userSessionRule.logIn(admin).setSystemAdministrator();
-    UserDto user = createLocalUser();
-    String originalPassword = db.getDbClient().userDao().selectByLogin(db.getSession(), user.getLogin()).getCryptedPassword();
+  public void system_administrator_can_update_password_of_user() throws ServletException, IOException {
+    UserTestData admin = createLocalUser();
+    userSessionRule.logIn(admin.getUserDto()).setSystemAdministrator();
+    UserTestData user = createLocalUser();
+    String originalPassword = findEncryptedPassword(user.getLogin());
+    db.commit();
 
-    tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("password", "Valar Morghulis")
-      .execute();
+    executeTest(user.getLogin(), null, NEW_PASSWORD);
 
-    String newPassword = db.getDbClient().userDao().selectByLogin(db.getSession(), user.getLogin()).getCryptedPassword();
+    String newPassword = findEncryptedPassword(user.getLogin());
     assertThat(newPassword).isNotEqualTo(originalPassword);
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isEmpty();
+    assertThat(findSessionTokenDto(db.getSession(), admin.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
+    verify(response).setStatus(HTTP_NO_CONTENT);
   }
 
-  @Test
-  public void fail_to_update_someone_else_password_if_not_admin() {
-    UserDto user = createLocalUser();
-    userSessionRule.logIn(user);
-    UserDto anotherUser = createLocalUser();
+  private String findEncryptedPassword(String login) {
+    return db.getDbClient().userDao().selectByLogin(db.getSession(), login).getCryptedPassword();
+  }
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", anotherUser.getLogin())
-      .setParam("previousPassword", "I dunno")
-      .setParam("password", "Valar Morghulis");
+  private Optional<SessionTokenDto> findSessionTokenDto(DbSession dbSession, String tokenUuid) {
+    return db.getDbClient().sessionTokensDao().selectByUuid(dbSession, tokenUuid);
+  }
 
-    assertThatThrownBy(request::execute)
+  @Test
+  public void fail_to_update_someone_else_password_if_not_admin() throws ServletException, IOException {
+    UserTestData user = createLocalUser();
+    userSessionRule.logIn(user.getLogin());
+    UserTestData anotherLocalUser = createLocalUser();
+
+    assertThatThrownBy(() -> executeTest(anotherLocalUser.getLogin(), "I dunno", NEW_PASSWORD))
       .isInstanceOf(ForbiddenException.class);
+    verifyNoInteractions(jwtHttpHandler);
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
   public void fail_to_update_unknown_user() {
-    UserDto admin = createLocalUser();
-    userSessionRule.logIn(admin).setSystemAdministrator();
+    UserTestData admin = createLocalUser();
+    userSessionRule.logIn(admin.getUserDto()).setSystemAdministrator();
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", "polop")
-      .setParam("password", "polop");
-
-    assertThatThrownBy(request::execute)
+    assertThatThrownBy(() -> executeTest("polop", null, "polop"))
       .isInstanceOf(NotFoundException.class)
       .hasMessage("User with login 'polop' has not been found");
+    assertThat(findSessionTokenDto(db.getSession(), admin.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
@@ -137,94 +171,146 @@ public class ChangePasswordActionTest {
     UserDto user = db.users().insertUser(u -> u.setActive(false));
     userSessionRule.logIn(user);
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("password", "polop");
-
-    assertThatThrownBy(request::execute)
+    assertThatThrownBy(() -> executeTest(user.getLogin(), null, "polop"))
       .isInstanceOf(NotFoundException.class)
       .hasMessage(format("User with login '%s' has not been found", user.getLogin()));
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
-  public void fail_to_update_password_on_self_without_old_password() {
-    UserDto user = createLocalUser();
-    userSessionRule.logIn(user);
+  public void fail_to_update_password_on_self_without_login() {
+    when(request.getParameter(PARAM_PASSWORD)).thenReturn("new password");
+    when(request.getParameter(PARAM_PREVIOUS_PASSWORD)).thenReturn(NEW_PASSWORD);
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("password", "Valar Morghulis");
+    assertThatThrownBy(() -> executeTest(null, OLD_PASSWORD, NEW_PASSWORD))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("The 'login' parameter is missing");
+    verifyNoInteractions(jwtHttpHandler);
+  }
 
-    assertThatThrownBy(request::execute)
+  @Test
+  public void fail_to_update_password_on_self_without_old_password() {
+    UserTestData user = createLocalUser();
+    userSessionRule.logIn(user.getUserDto());
+
+    assertThatThrownBy(() -> executeTest(user.getLogin(), null, NEW_PASSWORD))
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("The 'previousPassword' parameter is missing");
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
-  public void fail_to_update_password_on_self_with_bad_old_password() {
-    UserDto user = createLocalUser();
-    userSessionRule.logIn(user);
+  public void fail_to_update_password_on_self_without_new_password() {
+    UserTestData user = createLocalUser();
+    userSessionRule.logIn(user.getUserDto());
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("previousPassword", "I dunno")
-      .setParam("password", "Valar Morghulis");
+    assertThatThrownBy(() -> executeTest(user.getLogin(), OLD_PASSWORD, null))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("The 'password' parameter is missing");
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
+  }
 
-    assertThatThrownBy(request::execute)
+  @Test
+  public void fail_to_update_password_on_self_with_bad_old_password() {
+    UserTestData user = createLocalUser();
+    userSessionRule.logIn(user.getUserDto());
+
+    assertThatThrownBy(() -> executeTest(user.getLogin(), "I dunno", NEW_PASSWORD))
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("Incorrect password");
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
-  public void fail_to_update_password_on_external_auth() {
+  public void fail_to_update_password_on_external_auth() throws ServletException, IOException {
     UserDto admin = db.users().insertUser();
     userSessionRule.logIn(admin).setSystemAdministrator();
     UserDto user = db.users().insertUser(u -> u.setLocal(false));
 
-    TestRequest request = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("previousPassword", "I dunno")
-      .setParam("password", "Valar Morghulis");
-
-    assertThatThrownBy(request::execute)
-      .isInstanceOf(BadRequestException.class)
-      .hasMessage("Password cannot be changed when external authentication is used");
+    executeTest(user.getLogin(), "I dunno", NEW_PASSWORD);
+    verify(response).setStatus(HTTP_BAD_REQUEST);
   }
 
   @Test
   public void fail_to_update_to_same_password() {
-    String oldPassword = "Valar Dohaeris";
-    UserDto user = createLocalUser(oldPassword);
-    userSessionRule.logIn(user);
-
-    TestRequest request = tester.newRequest()
-      .setParam("login", user.getLogin())
-      .setParam("previousPassword", oldPassword)
-      .setParam("password", oldPassword);
+    UserTestData user = createLocalUser(OLD_PASSWORD);
+    userSessionRule.logIn(user.getUserDto());
 
-    assertThatThrownBy(request::execute)
+    assertThatThrownBy(() -> executeTest(user.getLogin(), OLD_PASSWORD, OLD_PASSWORD))
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("Password must be different from old password");
+    assertThat(findSessionTokenDto(db.getSession(), user.getSessionTokenUuid())).isPresent();
+    verifyNoInteractions(jwtHttpHandler);
   }
 
   @Test
-  public void test_definition() {
-    WebService.Action action = tester.getDef();
-    assertThat(action).isNotNull();
-    assertThat(action.isPost()).isTrue();
-    assertThat(action.params()).hasSize(3);
+  public void verify_definition() {
+    String controllerKey = "foo";
+    WebService.Context context = new WebService.Context();
+    WebService.NewController newController = context.createController(controllerKey);
+
+    changePasswordAction.define(newController);
+    newController.done();
+
+    WebService.Action changePassword = context.controller(controllerKey).action("change_password");
+    assertThat(changePassword).isNotNull();
+    assertThat(changePassword.handler()).isInstanceOf(ServletFilterHandler.class);
+    assertThat(changePassword.isPost()).isTrue();
+    assertThat(changePassword.params())
+      .extracting(WebService.Param::key)
+      .containsExactlyInAnyOrder(PARAM_LOGIN, PARAM_PASSWORD, PARAM_PREVIOUS_PASSWORD);
   }
 
-  private UserDto createLocalUser() {
-    return db.users().insertUser(u -> u.setLocal(true));
+  private void executeTest(@Nullable String login, @Nullable String oldPassword, @Nullable String newPassword) throws IOException, ServletException {
+    when(request.getParameter(PARAM_LOGIN)).thenReturn(login);
+    when(request.getParameter(PARAM_PREVIOUS_PASSWORD)).thenReturn(oldPassword);
+    when(request.getParameter(PARAM_PASSWORD)).thenReturn(newPassword);
+    changePasswordAction.doFilter(request, response, chain);
   }
 
-  private UserDto createLocalUser(String password) {
-    UserDto user = createLocalUser();
-    localAuthentication.storeHashPassword(user, password);
-    db.getDbClient().userDao().update(db.getSession(), user);
+  private UserTestData createLocalUser(String password) {
+    UserTestData userTestData = createLocalUser();
+    localAuthentication.storeHashPassword(userTestData.getUserDto(), password);
+    db.getDbClient().userDao().update(db.getSession(), userTestData.getUserDto());
     db.commit();
-    return user;
+    return userTestData;
+  }
+
+  private UserTestData createLocalUser() {
+    UserDto userDto = db.users().insertUser(u -> u.setLocal(true));
+    SessionTokenDto sessionTokenForUser = createSessionTokenForUser(userDto);
+    db.commit();
+    return new UserTestData(userDto, sessionTokenForUser);
+  }
+
+  private SessionTokenDto createSessionTokenForUser(UserDto user) {
+    SessionTokenDto userTokenDto = new SessionTokenDto().setUserUuid(user.getUuid()).setExpirationDate(1000L);
+    return db.getDbClient().sessionTokensDao().insert(db.getSession(), userTokenDto);
+  }
+
+  private static class UserTestData {
+    private final UserDto userDto;
+    private final SessionTokenDto sessionTokenDto;
+
+    private UserTestData(UserDto userDto, SessionTokenDto sessionTokenDto) {
+      this.userDto = userDto;
+      this.sessionTokenDto = sessionTokenDto;
+    }
+
+    UserDto getUserDto() {
+      return userDto;
+    }
+
+    String getLogin() {
+      return userDto.getLogin();
+    }
+
+    String getSessionTokenUuid() {
+      return sessionTokenDto.getUuid();
+    }
   }
 
 }
diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UsersWsTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UsersWsTest.java
new file mode 100644 (file)
index 0000000..da4ecb5
--- /dev/null
@@ -0,0 +1,56 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.user.ws;
+
+import java.util.List;
+import org.junit.Test;
+import org.sonar.api.server.ws.WebService;
+import org.sonar.server.ws.ServletFilterHandler;
+
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class UsersWsTest {
+  private static final TestAction TEST_ACTION_1 = new TestAction();
+  private static final TestAction TEST_ACTION_2 = new TestAction();
+
+  private final UsersWs underTest = new UsersWs(List.of(TEST_ACTION_1, TEST_ACTION_2));
+
+  @Test
+  public void define_ws() {
+    WebService.Context context = new WebService.Context();
+
+    underTest.define(context);
+
+    WebService.Controller controller = context.controller(UsersWs.API_USERS);
+    assertThat(controller).isNotNull();
+    assertThat(controller.since()).isEqualTo(UsersWs.SINCE_VERSION);
+    assertThat(controller.description()).isEqualTo(UsersWs.DESCRIPTION);
+  }
+
+  private static class TestAction implements BaseUsersWsAction {
+
+    @Override
+    public void define(WebService.NewController context) {
+      context.createAction(randomAlphanumeric(10)).setHandler(ServletFilterHandler.INSTANCE);
+    }
+
+  }
+}
index 432f32b605c4c291c63302fc3f4cc3db198ef441..cfc18bb29698d283d015559d8164f30085573d60 100644 (file)
@@ -69,10 +69,6 @@ public class ServletRequest extends ValidatingRequest {
         MediaTypes.DEFAULT));
   }
 
-  public HttpServletRequest getHttpRequest() {
-    return source;
-  }
-
   @Override
   public BufferedReader getReader() {
     try {
index 63172d054d205ab4db47091efa9dc2fbc1cbdb89..feacc9b2de98410c53f540c61ae3b61ce1a41240 100644 (file)
@@ -34,6 +34,7 @@ public class UsersWsParameters {
 
   public static final String PARAM_LOGIN = "login";
   public static final String PARAM_PASSWORD = "password";
+  public static final String PARAM_PREVIOUS_PASSWORD = "previousPassword";
   public static final String PARAM_NAME = "name";
   public static final String PARAM_EMAIL = "email";