diff options
8 files changed, 350 insertions, 135 deletions
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 index 00000000000..8f7d1987098 --- /dev/null +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/BaseUsersWsAction.java @@ -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 +} 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 2d1852fa4c4..b28a6d53d11 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 @@ -19,51 +19,73 @@ */ 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); + } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWs.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWs.java index 568629f02fd..4235ff0fed0 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWs.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWs.java @@ -19,26 +19,28 @@ */ 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(); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWsAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWsAction.java index ea57f9fecfe..cd3e44d137e 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWsAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/UsersWsAction.java @@ -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 } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java index e9d71f23928..7d755135e77 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java @@ -19,17 +19,27 @@ */ 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 index 00000000000..da4ecb5d132 --- /dev/null +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/UsersWsTest.java @@ -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); + } + + } +} diff --git a/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/ServletRequest.java b/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/ServletRequest.java index 432f32b605c..cfc18bb2969 100644 --- a/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/ServletRequest.java +++ b/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/ServletRequest.java @@ -69,10 +69,6 @@ public class ServletRequest extends ValidatingRequest { MediaTypes.DEFAULT)); } - public HttpServletRequest getHttpRequest() { - return source; - } - @Override public BufferedReader getReader() { try { diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/user/UsersWsParameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/user/UsersWsParameters.java index 63172d054d2..feacc9b2de9 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/user/UsersWsParameters.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/user/UsersWsParameters.java @@ -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"; |