From abda9cf968f84f4f5f41ecb1054963dcc6c34218 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 26 Nov 2020 12:41:10 +0100 Subject: [PATCH] SONAR-14175 Redirect when 'reset_password' is set --- server/sonar-web/public/WEB-INF/web.xml | 8 +- .../authentication/AuthenticationModule.java | 1 + .../authentication/ResetPasswordFilter.java | 81 ++++++++++ .../authentication/SafeModeUserSession.java | 5 + .../org/sonar/server/user/DoPrivileged.java | 5 + .../sonar/server/user/ServerUserSession.java | 5 + .../server/user/ThreadLocalUserSession.java | 5 + .../org/sonar/server/user/UserSession.java | 2 + .../org/sonar/server/user/UserUpdater.java | 1 + .../ResetPasswordFilterTest.java | 147 ++++++++++++++++++ .../SafeModeUserSessionTest.java | 2 + .../sonar/server/user/DoPrivilegedTest.java | 1 + .../server/user/ServerUserSessionTest.java | 48 +++--- .../user/ThreadLocalUserSessionTest.java | 13 +- .../server/user/UserUpdaterUpdateTest.java | 118 ++++++++------ .../tester/AbstractMockUserSession.java | 10 ++ .../tester/AnonymousMockUserSession.java | 3 +- .../sonar/server/tester/UserSessionRule.java | 5 + .../server/user/TestUserSessionFactory.java | 5 + .../platform/web/UserSessionFilter.java | 3 +- 20 files changed, 392 insertions(+), 76 deletions(-) create mode 100644 server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/ResetPasswordFilter.java create mode 100644 server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/ResetPasswordFilterTest.java diff --git a/server/sonar-web/public/WEB-INF/web.xml b/server/sonar-web/public/WEB-INF/web.xml index dd785a71287..d2ba0126c87 100644 --- a/server/sonar-web/public/WEB-INF/web.xml +++ b/server/sonar-web/public/WEB-INF/web.xml @@ -2,10 +2,10 @@ + xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd" + id="SonarQube" + version="3.0" + metadata-complete="true"> SonarQube diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java index 18681bee6a0..abe9238b597 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/AuthenticationModule.java @@ -45,6 +45,7 @@ public class AuthenticationModule extends Module { OAuth2ContextFactory.class, OAuthCsrfVerifier.class, RequestAuthenticatorImpl.class, + ResetPasswordFilter.class, ExpiredSessionsCleaner.class, ExpiredSessionsCleanerExecutorServiceImpl.class, UserLastConnectionDatesUpdaterImpl.class, diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/ResetPasswordFilter.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/ResetPasswordFilter.java new file mode 100644 index 00000000000..60f49d66645 --- /dev/null +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/ResetPasswordFilter.java @@ -0,0 +1,81 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.authentication; + +import com.google.common.collect.ImmutableSet; +import java.io.IOException; +import java.util.Set; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +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.web.ServletFilter; +import org.sonar.server.user.ThreadLocalUserSession; + +import static org.sonar.api.web.ServletFilter.UrlPattern.Builder.staticResourcePatterns; +import static org.sonar.server.authentication.AuthenticationRedirection.redirectTo; + +public class ResetPasswordFilter extends ServletFilter { + private static final String RESET_PASSWORD_PATH = "/account/reset_password"; + + private static final Set SKIPPED_URLS = ImmutableSet.of( + RESET_PASSWORD_PATH, + "/batch/*", "/api/*"); + + private final ThreadLocalUserSession userSession; + + public ResetPasswordFilter(ThreadLocalUserSession userSession) { + this.userSession = userSession; + } + + @Override + public UrlPattern doGetPattern() { + return UrlPattern.builder() + .includes("/*") + .excludes(staticResourcePatterns()) + .excludes(SKIPPED_URLS) + .build(); + } + + @Override + public void init(FilterConfig filterConfig) { + // nothing to do + } + + @Override + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain chain) throws IOException, ServletException { + HttpServletRequest request = (HttpServletRequest) servletRequest; + HttpServletResponse response = (HttpServletResponse) servletResponse; + + if (userSession.hasSession() && userSession.isLoggedIn() && userSession.shouldResetPassword()) { + redirectTo(response, request.getContextPath() + RESET_PASSWORD_PATH); + } + + chain.doFilter(request, response); + } + + @Override + public void destroy() { + // nothing to do + } +} diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SafeModeUserSession.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SafeModeUserSession.java index d1e01c20352..18a20f7868b 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SafeModeUserSession.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SafeModeUserSession.java @@ -69,6 +69,11 @@ public class SafeModeUserSession extends AbstractUserSession { return Collections.emptyList(); } + @Override + public boolean shouldResetPassword() { + return false; + } + @Override public Optional getIdentityProvider() { return Optional.empty(); diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/DoPrivileged.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/DoPrivileged.java index a1212802867..e1dd749f8f9 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/DoPrivileged.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/DoPrivileged.java @@ -86,6 +86,11 @@ public final class DoPrivileged { return Collections.emptyList(); } + @Override + public boolean shouldResetPassword() { + return false; + } + @Override public boolean isLoggedIn() { return false; diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ServerUserSession.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ServerUserSession.java index af7a154a72e..811abe66073 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ServerUserSession.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ServerUserSession.java @@ -95,6 +95,11 @@ public class ServerUserSession extends AbstractUserSession { return groups; } + @Override + public boolean shouldResetPassword() { + return userDto != null && userDto.isResetPassword(); + } + @Override public boolean isLoggedIn() { return userDto != null; diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java index cf9c3474428..e97ccac10b9 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java @@ -110,6 +110,11 @@ public class ThreadLocalUserSession implements UserSession { return this; } + @Override + public boolean shouldResetPassword() { + return get().shouldResetPassword(); + } + @Override public boolean hasPermission(GlobalPermission permission) { return get().hasPermission(permission); diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserSession.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserSession.java index fbe4a16ccbe..5ba9d59af76 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserSession.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserSession.java @@ -62,6 +62,8 @@ public interface UserSession { */ Collection getGroups(); + boolean shouldResetPassword(); + /** * This enum supports by name only the few providers for which specific code exists. */ diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java index 96807e15c03..ba996626e59 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/user/UserUpdater.java @@ -253,6 +253,7 @@ public class UserUpdater { String password = updateUser.password(); if (updateUser.isPasswordChanged() && validatePasswords(password, messages) && checkPasswordChangeAllowed(userDto, messages)) { localAuthentication.storeHashPassword(userDto, password); + userDto.setResetPassword(false); return true; } return false; diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/ResetPasswordFilterTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/ResetPasswordFilterTest.java new file mode 100644 index 00000000000..c0ce26d3417 --- /dev/null +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/ResetPasswordFilterTest.java @@ -0,0 +1,147 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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.authentication; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sonar.server.user.ThreadLocalUserSession; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@RunWith(DataProviderRunner.class) +public class ResetPasswordFilterTest { + + private final HttpServletRequest request = mock(HttpServletRequest.class); + private final HttpServletResponse response = mock(HttpServletResponse.class); + private final FilterChain chain = mock(FilterChain.class); + private final ThreadLocalUserSession session = mock(ThreadLocalUserSession.class); + + private final ResetPasswordFilter underTest = new ResetPasswordFilter(session); + + @Before + public void before() { + // set URI to valid for redirect + when(request.getRequestURI()).thenReturn("/"); + when(request.getContextPath()).thenReturn(""); + + // set reset password conditions + when(session.hasSession()).thenReturn(true); + when(session.isLoggedIn()).thenReturn(true); + when(session.shouldResetPassword()).thenReturn(true); + } + + @Test + public void verify_other_methods() { + underTest.init(mock(FilterConfig.class)); + underTest.destroy(); + + verifyNoInteractions(request, response, chain, session); + } + + @Test + public void redirect_if_reset_password_set() throws Exception { + underTest.doFilter(request, response, chain); + + verify(response).sendRedirect(eq("/account/reset_password")); + } + + @Test + public void redirect_if_reset_password_set_and_web_context_configured() throws Exception { + when(request.getContextPath()).thenReturn("/sonarqube"); + + underTest.doFilter(request, response, chain); + + verify(response).sendRedirect(eq("/sonarqube/account/reset_password")); + } + + @Test + public void redirect_if_request_uri_ends_with_slash() throws Exception { + when(request.getRequestURI()).thenReturn("/projects/"); + when(request.getContextPath()).thenReturn("/sonarqube"); + + underTest.doFilter(request, response, chain); + + verify(response).sendRedirect(eq("/sonarqube/account/reset_password")); + } + + @Test + public void do_not_redirect_if_no_session() throws Exception { + when(session.hasSession()).thenReturn(false); + + underTest.doFilter(request, response, chain); + + verify(response, never()).sendRedirect(any()); + } + + @Test + public void do_not_redirect_if_not_logged_in() throws Exception { + when(session.isLoggedIn()).thenReturn(false); + + underTest.doFilter(request, response, chain); + + verify(response, never()).sendRedirect(any()); + } + + @Test + public void do_not_redirect_if_reset_password_not_set() throws Exception { + when(session.shouldResetPassword()).thenReturn(false); + + underTest.doFilter(request, response, chain); + + verify(response, never()).sendRedirect(any()); + } + + @Test + @UseDataProvider("skipped_urls") + public void doGetPattern_verify(String urltoSkip) throws Exception { + when(request.getRequestURI()).thenReturn(urltoSkip); + when(request.getContextPath()).thenReturn(""); + underTest.doGetPattern().matches(urltoSkip); + + verify(response, never()).sendRedirect(any()); + } + + @DataProvider + public static Object[][] skipped_urls() { + return new Object[][] { + {"/batch/index"}, + {"/batch/file"}, + {"/api/issues"}, + {"/api/issues/"}, + {"/api/*"}, + {"/account/reset_password"}, + }; + } + +} diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SafeModeUserSessionTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SafeModeUserSessionTest.java index ffd301b1961..b75577598bb 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SafeModeUserSessionTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SafeModeUserSessionTest.java @@ -34,12 +34,14 @@ public class SafeModeUserSessionTest { assertThat(underTest.getLogin()).isNull(); assertThat(underTest.getUuid()).isNull(); assertThat(underTest.isLoggedIn()).isFalse(); + assertThat(underTest.shouldResetPassword()).isFalse(); assertThat(underTest.getName()).isNull(); assertThat(underTest.getGroups()).isEmpty(); } @Test public void session_has_no_permissions() { + assertThat(underTest.shouldResetPassword()).isFalse(); assertThat(underTest.isRoot()).isFalse(); assertThat(underTest.isSystemAdministrator()).isFalse(); assertThat(underTest.hasPermissionImpl(GlobalPermission.ADMINISTER)).isFalse(); diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/DoPrivilegedTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/DoPrivilegedTest.java index 197e2fb91c2..10f1e7f6eee 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/DoPrivilegedTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/DoPrivilegedTest.java @@ -49,6 +49,7 @@ public class DoPrivilegedTest { assertThat(catcher.userSession.isLoggedIn()).isFalse(); assertThat(catcher.userSession.hasComponentPermission("any permission", new ComponentDto())).isTrue(); assertThat(catcher.userSession.isSystemAdministrator()).isTrue(); + assertThat(catcher.userSession.shouldResetPassword()).isFalse(); // verify session in place after task is done assertThat(threadLocalUserSession.get()).isSameAs(session); diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ServerUserSessionTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ServerUserSessionTest.java index 6e2708f4d9e..c0c3ac23b98 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ServerUserSessionTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ServerUserSessionTest.java @@ -21,9 +21,9 @@ package org.sonar.server.user; import java.util.Arrays; import javax.annotation.Nullable; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.utils.System2; import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; @@ -36,6 +36,7 @@ import org.sonar.server.exceptions.ForbiddenException; import static com.google.common.base.Preconditions.checkState; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.sonar.core.permission.GlobalPermissions.PROVISIONING; import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN; import static org.sonar.db.component.ComponentTesting.newChildComponent; @@ -48,8 +49,6 @@ public class ServerUserSessionTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); - @Rule - public ExpectedException expectedException = ExpectedException.none(); private DbClient dbClient = db.getDbClient(); @Test @@ -61,6 +60,23 @@ public class ServerUserSessionTest { assertThat(session.isLoggedIn()).isFalse(); } + @Test + public void shouldResetPassword_is_false_on_anonymous() { + assertThat(newAnonymousSession().shouldResetPassword()).isFalse(); + } + + @Test + public void shouldResetPassword_is_false_if_set_on_UserDto() { + UserDto user = db.users().insertUser(userDto -> userDto.setResetPassword(false)); + assertThat(newUserSession(user).shouldResetPassword()).isFalse(); + } + + @Test + public void shouldResetPassword_is_true_if_set_on_UserDto() { + UserDto user = db.users().insertUser(userDto -> userDto.setResetPassword(true)); + assertThat(newUserSession(user).shouldResetPassword()).isTrue(); + } + @Test public void getGroups_is_empty_on_anonymous() { assertThat(newAnonymousSession().getGroups()).isEmpty(); @@ -113,9 +129,7 @@ public class ServerUserSessionTest { UserDto user = db.users().insertUser(); UserSession underTest = newUserSession(user); - expectInsufficientPrivilegesForbiddenException(); - - underTest.checkIsRoot(); + assertThatForbiddenExceptionIsThrown(underTest::checkIsRoot); } @Test @@ -163,18 +177,14 @@ public class ServerUserSessionTest { db.users().insertProjectPermissionOnUser(user, UserRole.USER, project); UserSession session = newUserSession(user); - expectInsufficientPrivilegesForbiddenException(); - - session.checkComponentUuidPermission(UserRole.USER, "another-uuid"); + assertThatForbiddenExceptionIsThrown(() -> session.checkComponentUuidPermission(UserRole.USER, "another-uuid")); } @Test public void checkPermission_throws_ForbiddenException_when_user_doesnt_have_the_specified_permission() { UserDto user = db.users().insertUser(); - expectInsufficientPrivilegesForbiddenException(); - - newUserSession(user).checkPermission(PROVISION_PROJECTS); + assertThatForbiddenExceptionIsThrown(() -> newUserSession(user).checkPermission(PROVISION_PROJECTS)); } @Test @@ -565,10 +575,9 @@ public class ServerUserSessionTest { UserSession session = newUserSession(user); - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); - - session.checkIsSystemAdministrator(); + assertThatThrownBy(session::checkIsSystemAdministrator) + .isInstanceOf(ForbiddenException.class) + .hasMessage("Insufficient privileges"); } @Test @@ -596,9 +605,10 @@ public class ServerUserSessionTest { return newUserSession(null); } - private void expectInsufficientPrivilegesForbiddenException() { - expectedException.expect(ForbiddenException.class); - expectedException.expectMessage("Insufficient privileges"); + private void assertThatForbiddenExceptionIsThrown(ThrowingCallable shouldRaiseThrowable) { + assertThatThrownBy(shouldRaiseThrowable) + .isInstanceOf(ForbiddenException.class) + .hasMessage("Insufficient privileges"); } } diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java index 9896a6f2d84..0375b6e7a4f 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java @@ -21,9 +21,7 @@ package org.sonar.server.user; import org.junit.After; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.db.user.GroupDto; import org.sonar.db.user.GroupTesting; import org.sonar.server.exceptions.UnauthorizedException; @@ -31,14 +29,12 @@ import org.sonar.server.tester.AnonymousMockUserSession; import org.sonar.server.tester.MockUserSession; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class ThreadLocalUserSessionTest { private ThreadLocalUserSession threadLocalUserSession = new ThreadLocalUserSession(); - @Rule - public ExpectedException thrown = ExpectedException.none(); - @Before public void setUp() { // for test isolation @@ -56,6 +52,7 @@ public class ThreadLocalUserSessionTest { GroupDto group = GroupTesting.newGroupDto(); MockUserSession expected = new MockUserSession("karadoc") .setUuid("karadoc-uuid") + .setResetPassword(true) .setGroups(group); threadLocalUserSession.set(expected); @@ -64,6 +61,7 @@ public class ThreadLocalUserSessionTest { assertThat(threadLocalUserSession.getLogin()).isEqualTo("karadoc"); assertThat(threadLocalUserSession.getUuid()).isEqualTo("karadoc-uuid"); assertThat(threadLocalUserSession.isLoggedIn()).isTrue(); + assertThat(threadLocalUserSession.shouldResetPassword()).isTrue(); assertThat(threadLocalUserSession.getGroups()).extracting(GroupDto::getUuid).containsOnly(group.getUuid()); } @@ -76,13 +74,14 @@ public class ThreadLocalUserSessionTest { assertThat(session).isSameAs(expected); assertThat(threadLocalUserSession.getLogin()).isNull(); assertThat(threadLocalUserSession.isLoggedIn()).isFalse(); + assertThat(threadLocalUserSession.shouldResetPassword()).isFalse(); assertThat(threadLocalUserSession.getGroups()).isEmpty(); } @Test public void throw_UnauthorizedException_when_no_session() { - thrown.expect(UnauthorizedException.class); - threadLocalUserSession.get(); + assertThatThrownBy(() -> threadLocalUserSession.get()) + .isInstanceOf(UnauthorizedException.class); } } diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java index b50f53ef7c6..63995f29ce7 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/user/UserUpdaterUpdateTest.java @@ -21,10 +21,10 @@ package org.sonar.server.user; import com.google.common.collect.Multimap; import java.util.List; +import java.util.function.Consumer; import org.elasticsearch.search.SearchHit; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.impl.utils.AlwaysIncreasingSystem2; import org.sonar.api.utils.System2; @@ -48,6 +48,7 @@ import org.sonar.server.usergroups.DefaultGroupFinder; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.tuple; import static org.assertj.core.data.MapEntry.entry; import static org.mockito.Mockito.mock; @@ -59,11 +60,11 @@ import static org.sonar.db.user.UserTesting.newUserDto; public class UserUpdaterUpdateTest { private static final String DEFAULT_LOGIN = "marius"; + private static final Consumer EMPTY_USER_CONSUMER = userDto -> { + }; private System2 system2 = new AlwaysIncreasingSystem2(); - @Rule - public ExpectedException expectedException = ExpectedException.none(); @Rule public EsTester es = EsTester.create(); @Rule @@ -371,6 +372,30 @@ public class UserUpdaterUpdateTest { assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); } + @Test + public void update_user_password_set_reset_password_flag_to_false() { + UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@lesbronzes.fr") + .setScmAccounts(asList("ma", "marius33")) + .setSalt("salt") + .setResetPassword(true) + .setCryptedPassword("crypted password")); + createDefaultGroup(); + + underTest.updateAndCommit(session, user, new UpdateUser() + .setPassword("password2"), u -> { + }); + + UserDto dto = dbClient.userDao().selectByLogin(session, DEFAULT_LOGIN); + assertThat(dto.getSalt()).isNotEqualTo("salt"); + assertThat(dto.getCryptedPassword()).isNotEqualTo("crypted password"); + assertThat(dto.isResetPassword()).isFalse(); + + // Following fields has not changed + assertThat(dto.getName()).isEqualTo("Marius"); + assertThat(dto.getScmAccountsAsList()).containsOnly("ma", "marius33"); + assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); + } + @Test public void update_only_external_id() { UserDto user = db.users().insertUser(newExternalUser(DEFAULT_LOGIN, "Marius", "marius@email.com") @@ -474,11 +499,11 @@ public class UserUpdaterUpdateTest { public void fail_to_set_null_password_when_local_user() { UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@email.com")); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Password can't be empty"); - underTest.updateAndCommit(session, user, new UpdateUser().setPassword(null), u -> { - }); + UpdateUser updateUser = new UpdateUser().setPassword(null); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Password can't be empty"); } @Test @@ -487,11 +512,11 @@ public class UserUpdaterUpdateTest { .setLogin(DEFAULT_LOGIN) .setLocal(false)); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Password cannot be changed when external authentication is used"); - underTest.updateAndCommit(session, user, new UpdateUser().setPassword("password2"), u -> { - }); + UpdateUser updateUser = new UpdateUser().setPassword("password2"); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Password cannot be changed when external authentication is used"); } @Test @@ -539,50 +564,52 @@ public class UserUpdaterUpdateTest { db.users().insertUser(newLocalUser("john", "John", "john@email.com").setScmAccounts(singletonList("jo"))); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("The scm account 'jo' is already used by user(s) : 'John (john)'"); - - underTest.updateAndCommit(session, user, new UpdateUser() + UpdateUser updateUser = new UpdateUser() .setName("Marius2") .setEmail("marius2@mail.com") .setPassword("password2") - .setScmAccounts(asList("jo")), u -> { - }); + .setScmAccounts(asList("jo")); + + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("The scm account 'jo' is already used by user(s) : 'John (john)'"); } @Test public void fail_to_update_user_when_scm_account_is_user_login() { UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@lesbronzes.fr")); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Login and email are automatically considered as SCM accounts"); - underTest.updateAndCommit(session, user, new UpdateUser().setScmAccounts(asList(DEFAULT_LOGIN)), u -> { - }); + UpdateUser updateUser = new UpdateUser().setScmAccounts(asList(DEFAULT_LOGIN)); + + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Login and email are automatically considered as SCM accounts"); } @Test public void fail_to_update_user_when_scm_account_is_existing_user_email() { UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@lesbronzes.fr")); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Login and email are automatically considered as SCM accounts"); - underTest.updateAndCommit(session, user, new UpdateUser().setScmAccounts(asList("marius@lesbronzes.fr")), u -> { - }); + UpdateUser updateUser = new UpdateUser().setScmAccounts(asList("marius@lesbronzes.fr")); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Login and email are automatically considered as SCM accounts"); } @Test public void fail_to_update_user_when_scm_account_is_new_user_email() { UserDto user = db.users().insertUser(newLocalUser(DEFAULT_LOGIN, "Marius", "marius@lesbronzes.fr")); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Login and email are automatically considered as SCM accounts"); - underTest.updateAndCommit(session, user, new UpdateUser() + UpdateUser updateUser = new UpdateUser() .setEmail("marius@newmail.com") - .setScmAccounts(asList("marius@newmail.com")), u -> { - }); + .setScmAccounts(singletonList("marius@newmail.com")); + + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Login and email are automatically considered as SCM accounts"); } @Test @@ -590,11 +617,11 @@ public class UserUpdaterUpdateTest { UserDto user = db.users().insertUser(); createDefaultGroup(); - expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Use only letters, numbers, and .-_@ please."); + UpdateUser updateUser = new UpdateUser().setLogin("With space"); - underTest.updateAndCommit(session, user, new UpdateUser().setLogin("With space"), u -> { - }); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Use only letters, numbers, and .-_@ please."); } @Test @@ -603,11 +630,10 @@ public class UserUpdaterUpdateTest { UserDto user = db.users().insertUser(u -> u.setActive(false)); UserDto existingUser = db.users().insertUser(u -> u.setLogin("existing_login")); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("A user with login 'existing_login' already exists"); - - underTest.updateAndCommit(session, user, new UpdateUser().setLogin(existingUser.getLogin()), u -> { - }); + UpdateUser updateUser = new UpdateUser().setLogin(existingUser.getLogin()); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("A user with login 'existing_login' already exists"); } @Test @@ -616,12 +642,16 @@ public class UserUpdaterUpdateTest { UserDto user = db.users().insertUser(u -> u.setActive(false)); UserDto existingUser = db.users().insertUser(u -> u.setExternalId("existing_external_id").setExternalIdentityProvider("existing_external_provider")); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("A user with provider id 'existing_external_id' and identity provider 'existing_external_provider' already exists"); + UpdateUser updateUser = new UpdateUser() + .setExternalIdentity( + new ExternalIdentity( + existingUser.getExternalIdentityProvider(), + existingUser.getExternalLogin(), + existingUser.getExternalId())); - underTest.updateAndCommit(session, user, new UpdateUser() - .setExternalIdentity(new ExternalIdentity(existingUser.getExternalIdentityProvider(), existingUser.getExternalLogin(), existingUser.getExternalId())), u -> { - }); + assertThatThrownBy(() -> underTest.updateAndCommit(session, user, updateUser, EMPTY_USER_CONSUMER)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("A user with provider id 'existing_external_id' and identity provider 'existing_external_provider' already exists"); } private GroupDto createDefaultGroup() { diff --git a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AbstractMockUserSession.java b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AbstractMockUserSession.java index dd2f1c208ec..2458d7079e4 100644 --- a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AbstractMockUserSession.java +++ b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AbstractMockUserSession.java @@ -44,6 +44,7 @@ public abstract class AbstractMockUserSession private Map projectUuidByComponentUuid = new HashMap<>(); private Set projectPermissions = new HashSet<>(); private boolean systemAdministrator = false; + private boolean resetPassword = false; protected AbstractMockUserSession(Class clazz) { this.clazz = clazz; @@ -137,4 +138,13 @@ public abstract class AbstractMockUserSession return isRoot() || systemAdministrator; } + public T setResetPassword(boolean b) { + this.resetPassword = b; + return clazz.cast(this); + } + + @Override + public boolean shouldResetPassword() { + return resetPassword; + } } diff --git a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AnonymousMockUserSession.java b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AnonymousMockUserSession.java index 37fb591cde1..3a01b2fa171 100644 --- a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AnonymousMockUserSession.java +++ b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/tester/AnonymousMockUserSession.java @@ -40,7 +40,8 @@ public class AnonymousMockUserSession extends AbstractMockUserSession getIdentityProvider() { return currentUserSession.getIdentityProvider(); diff --git a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/user/TestUserSessionFactory.java b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/user/TestUserSessionFactory.java index 89bd75a2462..14bd327a6d2 100644 --- a/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/user/TestUserSessionFactory.java +++ b/server/sonar-webserver-auth/src/testFixtures/java/org/sonar/server/user/TestUserSessionFactory.java @@ -81,6 +81,11 @@ public class TestUserSessionFactory implements UserSessionFactory { throw notImplemented(); } + @Override + public boolean shouldResetPassword() { + return user != null && user.isResetPassword(); + } + @Override public Optional getIdentityProvider() { throw notImplemented(); diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java index aa87a67963a..9a9bc56cdf4 100644 --- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java +++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java @@ -46,7 +46,8 @@ public class UserSessionFilter implements Filter { this.platform = PlatformImpl.getInstance(); } - @VisibleForTesting UserSessionFilter(Platform platform) { + @VisibleForTesting + UserSessionFilter(Platform platform) { this.platform = platform; } -- 2.39.5