From 1a4fa58ca7353d3e52b8d62c6af1c9c014905e1b Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Wed, 14 May 2025 16:48:34 -0600 Subject: [PATCH 01/10] Add Password Advice Support --- .../annotation/web/builders/HttpSecurity.java | 4 +- .../WebMvcSecurityConfiguration.java | 11 ++ .../web/configurers/FormLoginConfigurer.java | 2 +- .../PasswordManagementConfigurer.java | 144 ++++++++++++++- .../PasswordManagementConfigurerTests.java | 164 ++++++++++++++++++ .../password/ChangeLengthPasswordAdvisor.java | 60 +++++++ .../password/ChangePasswordAdvice.java | 37 ++++ .../password/ChangePasswordAdvisor.java | 29 ++++ .../password/ChangePasswordReason.java | 23 +++ .../ChangePasswordServiceAdvisor.java | 39 +++++ .../ChangeRepeatedPasswordAdvisor.java | 61 +++++++ .../DelegatingChangePasswordAdvisor.java | 89 ++++++++++ .../password/SimpleChangePasswordAdvice.java | 48 +++++ .../password/UserDetailsPasswordManager.java | 40 +++++ .../InMemoryUserDetailsManager.java | 25 ++- .../ChangeCompromisedPasswordAdvisor.java | 77 ++++++++ .../password/ChangePasswordAdviceHandler.java | 39 +++++ ...ePasswordAdviceMethodArgumentResolver.java | 48 +++++ .../ChangePasswordAdviceRepository.java | 32 ++++ .../ChangePasswordAdvisingFilter.java | 51 ++++++ .../ChangePasswordProcessingFilter.java | 148 ++++++++++++++++ ...ultChangePasswordPageGeneratingFilter.java | 80 +++++++++ ...SessionChangePasswordAdviceRepository.java | 83 +++++++++ .../SimpleChangePasswordAdviceHandler.java | 76 ++++++++ 24 files changed, 1404 insertions(+), 6 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java create mode 100644 core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java index e59d438193a..125a64bdb3a 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java @@ -1778,7 +1778,9 @@ public HttpSecurity httpBasic(Customizer> http */ public HttpSecurity passwordManagement( Customizer> passwordManagementCustomizer) throws Exception { - passwordManagementCustomizer.customize(getOrApply(new PasswordManagementConfigurer<>())); + PasswordManagementConfigurer passwordManagement = new PasswordManagementConfigurer<>(); + passwordManagement.setApplicationContext(getContext()); + passwordManagementCustomizer.customize(getOrApply(passwordManagement)); return HttpSecurity.this; } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java index 37870e0aca2..c195c70f888 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java @@ -37,6 +37,9 @@ import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceMethodArgumentResolver; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.firewall.RequestRejectedHandler; @@ -72,6 +75,8 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex private AnnotationTemplateExpressionDefaults templateDefaults; + private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + @Override @SuppressWarnings("deprecation") public void addArgumentResolvers(List argumentResolvers) { @@ -88,6 +93,9 @@ public void addArgumentResolvers(List argumentRes currentSecurityContextArgumentResolver.setTemplateDefaults(this.templateDefaults); argumentResolvers.add(currentSecurityContextArgumentResolver); argumentResolvers.add(new CsrfTokenArgumentResolver()); + ChangePasswordAdviceMethodArgumentResolver resolver = new ChangePasswordAdviceMethodArgumentResolver(); + resolver.setChangePasswordAdviceRepository(this.changePasswordAdviceRepository); + argumentResolvers.add(resolver); } @Bean @@ -104,6 +112,9 @@ public void setApplicationContext(ApplicationContext applicationContext) throws if (applicationContext.getBeanNamesForType(AnnotationTemplateExpressionDefaults.class).length == 1) { this.templateDefaults = applicationContext.getBean(AnnotationTemplateExpressionDefaults.class); } + if (applicationContext.getBeanNamesForType(ChangePasswordAdviceRepository.class).length == 1) { + this.changePasswordAdviceRepository = applicationContext.getBean(ChangePasswordAdviceRepository.class); + } } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java index 03cf95b3901..bea56042c77 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java @@ -250,7 +250,7 @@ private String getUsernameParameter() { * Gets the HTTP parameter that is used to submit the password. * @return the HTTP parameter that is used to submit the password */ - private String getPasswordParameter() { + String getPasswordParameter() { return getAuthenticationFilter().getPasswordParameter(); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 0de4c03b513..01ee4ea9c17 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -16,9 +16,32 @@ package org.springframework.security.config.annotation.web.configurers; +import java.util.ArrayList; +import java.util.List; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.ChangePasswordServiceAdvisor; +import org.springframework.security.authentication.password.DelegatingChangePasswordAdvisor; +import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; +import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; +import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceHandler; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; +import org.springframework.security.web.authentication.password.ChangePasswordProcessingFilter; +import org.springframework.security.web.authentication.password.DefaultChangePasswordPageGeneratingFilter; +import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.SimpleChangePasswordAdviceHandler; +import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.util.Assert; /** @@ -28,14 +51,28 @@ * @since 5.6 */ public final class PasswordManagementConfigurer> - extends AbstractHttpConfigurer, B> { + extends AbstractHttpConfigurer, B> implements ApplicationContextAware { private static final String WELL_KNOWN_CHANGE_PASSWORD_PATTERN = "/.well-known/change-password"; - private static final String DEFAULT_CHANGE_PASSWORD_PAGE = "/change-password"; + private static final String DEFAULT_CHANGE_PASSWORD_PAGE = DefaultChangePasswordPageGeneratingFilter.DEFAULT_CHANGE_PASSWORD_URL; + + private ApplicationContext context; + + private boolean customChangePasswordPage = false; private String changePasswordPage = DEFAULT_CHANGE_PASSWORD_PAGE; + private String changePasswordProcessingUrl = ChangePasswordProcessingFilter.DEFAULT_PASSWORD_CHANGE_PROCESSING_URL; + + private ChangePasswordAdviceRepository changePasswordAdviceRepository; + + private ChangePasswordAdvisor changePasswordAdvisor; + + private ChangePasswordAdviceHandler changePasswordAdviceHandler; + + private UserDetailsPasswordManager userDetailsPasswordManager; + /** * Sets the change password page. Defaults to * {@link PasswordManagementConfigurer#DEFAULT_CHANGE_PASSWORD_PAGE}. @@ -45,9 +82,76 @@ public final class PasswordManagementConfigurer public PasswordManagementConfigurer changePasswordPage(String changePasswordPage) { Assert.hasText(changePasswordPage, "changePasswordPage cannot be empty"); this.changePasswordPage = changePasswordPage; + this.customChangePasswordPage = true; + return this; + } + + public PasswordManagementConfigurer changePasswordProcessingUrl(String changePasswordProcessingUrl) { + this.changePasswordProcessingUrl = changePasswordProcessingUrl; + return this; + } + + public PasswordManagementConfigurer changePasswordAdviceRepository( + ChangePasswordAdviceRepository changePasswordAdviceRepository) { + this.changePasswordAdviceRepository = changePasswordAdviceRepository; + return this; + } + + public PasswordManagementConfigurer changePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { + this.changePasswordAdvisor = changePasswordAdvisor; + return this; + } + + public PasswordManagementConfigurer changePasswordAdviceHandler( + ChangePasswordAdviceHandler changePasswordAdviceHandler) { + this.changePasswordAdviceHandler = changePasswordAdviceHandler; return this; } + public PasswordManagementConfigurer userDetailsPasswordManager( + UserDetailsPasswordManager userDetailsPasswordManager) { + this.userDetailsPasswordManager = userDetailsPasswordManager; + return this; + } + + @Override + public void init(B http) throws Exception { + UserDetailsPasswordManager passwordManager = (this.userDetailsPasswordManager == null) + ? this.context.getBeanProvider(UserDetailsPasswordManager.class).getIfUnique() + : this.userDetailsPasswordManager; + + if (passwordManager == null) { + return; + } + + ChangePasswordAdviceRepository changePasswordAdviceRepository = (this.changePasswordAdviceRepository != null) + ? this.changePasswordAdviceRepository + : this.context.getBeanProvider(ChangePasswordAdviceRepository.class) + .getIfUnique(HttpSessionChangePasswordAdviceRepository::new); + + ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor + : this.context.getBeanProvider(ChangePasswordAdvisor.class).getIfUnique(() -> { + List advisors = new ArrayList<>(); + advisors.add(new ChangeCompromisedPasswordAdvisor()); + advisors.add(new ChangePasswordServiceAdvisor(passwordManager)); + return new DelegatingChangePasswordAdvisor(advisors); + }); + + http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); + http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); + http.setSharedObject(ChangePasswordAdvisor.class, changePasswordAdvisor); + + FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); + String passwordParameter = (form != null) ? form.getPasswordParameter() : "password"; + http.getConfigurer(SessionManagementConfigurer.class) + .addSessionAuthenticationStrategy((authentication, request, response) -> { + UserDetails user = (UserDetails) authentication.getPrincipal(); + String password = request.getParameter(passwordParameter); + ChangePasswordAdvice advice = changePasswordAdvisor.advise(user, password); + changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); + }); + } + /** * {@inheritDoc} */ @@ -56,6 +160,42 @@ public void configure(B http) throws Exception { RequestMatcherRedirectFilter changePasswordFilter = new RequestMatcherRedirectFilter( getRequestMatcherBuilder().matcher(WELL_KNOWN_CHANGE_PASSWORD_PATTERN), this.changePasswordPage); http.addFilterBefore(postProcess(changePasswordFilter), UsernamePasswordAuthenticationFilter.class); + + if (http.getSharedObject(UserDetailsPasswordManager.class) == null) { + return; + } + + PasswordEncoder passwordEncoder = this.context.getBeanProvider(PasswordEncoder.class) + .getIfUnique(PasswordEncoderFactories::createDelegatingPasswordEncoder); + + ChangePasswordAdviceHandler changePasswordAdviceHandler = (this.changePasswordAdviceHandler != null) + ? this.changePasswordAdviceHandler : this.context.getBeanProvider(ChangePasswordAdviceHandler.class) + .getIfUnique(() -> new SimpleChangePasswordAdviceHandler(this.changePasswordPage)); + + if (!this.customChangePasswordPage) { + DefaultChangePasswordPageGeneratingFilter page = new DefaultChangePasswordPageGeneratingFilter(); + http.addFilterBefore(page, RequestCacheAwareFilter.class); + } + + ChangePasswordProcessingFilter processing = new ChangePasswordProcessingFilter( + http.getSharedObject(UserDetailsPasswordManager.class)); + processing + .setRequestMatcher(PathPatternRequestMatcher.withDefaults().matcher(this.changePasswordProcessingUrl)); + processing.setChangePasswordAdvisor(http.getSharedObject(ChangePasswordAdvisor.class)); + processing.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); + processing.setPasswordEncoder(passwordEncoder); + processing.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); + http.addFilterBefore(processing, RequestCacheAwareFilter.class); + + ChangePasswordAdvisingFilter advising = new ChangePasswordAdvisingFilter(); + advising.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); + advising.setChangePasswordAdviceHandler(changePasswordAdviceHandler); + http.addFilterBefore(advising, RequestCacheAwareFilter.class); + } + + @Override + public void setApplicationContext(ApplicationContext context) { + this.context = context; } } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index 0b65e42d4dd..bcb1c819893 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -16,22 +16,52 @@ package org.springframework.security.config.annotation.web.configurers; +import java.net.URI; +import java.util.UUID; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.ResponseEntity; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; +import org.springframework.security.authentication.password.UserDetailsPasswordManager; +import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.web.SecurityFilterChain; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.servlet.config.annotation.EnableWebMvc; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.hamcrest.Matchers.containsString; import static org.springframework.security.config.Customizer.withDefaults; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -87,6 +117,54 @@ public void whenSettingBlankChangePasswordPage() { .withMessage("changePasswordPage cannot be empty"); } + @Test + void whenAdminSetsExpiredAdviceThenUserLoginRedirectsToResetPassword() throws Exception { + this.spring.register(PasswordManagementConfig.class, AdminController.class, HomeController.class).autowire(); + UserDetailsService users = this.spring.getContext().getBean(UserDetailsService.class); + UserDetails admin = users.loadUserByUsername("admin"); + this.mvc.perform(get("/").with(user(admin))).andExpect(status().isOk()); + // change the password to a test value + String random = UUID.randomUUID().toString(); + this.mvc.perform(post("/change-password").with(csrf()).with(user(admin)).param("newPassword", random)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/")); + // admin "expires" their own password + this.mvc.perform(post("/admin/passwords/expire/admin").with(csrf()).with(user(admin))) + .andExpect(status().isCreated()); + // .andExpect(jsonPath("$.action").value(ChangePasswordAdvice.Action.MUST_CHANGE.toString())); + // requests redirect to /change-password + MvcResult result = this.mvc + .perform(post("/login").with(csrf()).param("username", "admin").param("password", random)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/")) + .andReturn(); + MockHttpSession session = (MockHttpSession) result.getRequest().getSession(); + this.mvc.perform(get("/").session(session)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/change-password")); + // reset the password to update + random = UUID.randomUUID().toString(); + this.mvc.perform(post("/change-password").with(csrf()).session(session).param("newPassword", random)) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/")); + // now we're good + this.mvc.perform(get("/").session(session)).andExpect(status().isOk()); + } + + @Test + void whenCompromisedThenUserLoginAllowed() throws Exception { + this.spring.register(PasswordManagementConfig.class, AdminController.class, HomeController.class).autowire(); + MvcResult result = this.mvc + .perform(post("/login").with(csrf()).param("username", "compromised").param("password", "password")) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("/")) + .andReturn(); + MockHttpSession session = (MockHttpSession) result.getRequest().getSession(); + this.mvc.perform(get("/").session(session)) + .andExpect(status().isOk()) + .andExpect(content().string(containsString("COMPROMISED"))); + } + @Configuration @EnableWebSecurity static class PasswordManagementWithDefaultChangePasswordPageConfig { @@ -119,4 +197,90 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class PasswordManagementConfig { + + @Bean + SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { + // @formatter:off + http + .authorizeHttpRequests((authz) -> authz + .requestMatchers("/admin/**").hasRole("ADMIN") + .anyRequest().authenticated() + ) + .formLogin(Customizer.withDefaults()) + .passwordManagement(Customizer.withDefaults()); + // @formatter:on + return http.build(); + } + + @Bean + UserDetailsService users() { + String adminPassword = UUID.randomUUID().toString(); + UserDetails compromised = User.withUsername("compromised").password("{noop}password").roles("USER").build(); + UserDetails admin = User.withUsername("admin").password("{noop}" + adminPassword).roles("ADMIN").build(); + return new InMemoryUserDetailsManager(compromised, admin); + } + + } + + @RequestMapping("/admin/passwords") + @RestController + static class AdminController { + + private final UserDetailsService users; + + private final UserDetailsPasswordManager passwords; + + private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + AdminController(UserDetailsService users) { + this.users = users; + this.passwords = (UserDetailsPasswordManager) users; + } + + @GetMapping("/advice/{username}") + ResponseEntity requireChangePassword(@PathVariable("username") String username) { + UserDetails user = this.users.loadUserByUsername(username); + if (user == null) { + return ResponseEntity.notFound().build(); + } + ChangePasswordAdvice advice = this.passwords.loadPasswordAdvice(user); + return ResponseEntity.ok(advice); + } + + @PostMapping("/expire/{username}") + ResponseEntity expirePassword(@PathVariable("username") String username) { + UserDetails user = this.users.loadUserByUsername(username); + if (user == null) { + return ResponseEntity.notFound().build(); + } + ChangePasswordAdvice advice = new SimpleChangePasswordAdvice(ChangePasswordAdvice.Action.MUST_CHANGE, + ChangePasswordReason.EXPIRED); + this.passwords.savePasswordAdvice(user, advice); + URI uri = URI.create("/admin/passwords/advice/" + username); + return ResponseEntity.created(uri).body(advice); + } + + @PostMapping("/change") + ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, + @RequestParam("password") String password) { + this.passwords.updatePassword(user, this.encoder.encode(password)); + return ResponseEntity.ok().build(); + } + + } + + @RestController + static class HomeController { + + @GetMapping + ChangePasswordAdvice index(ChangePasswordAdvice advice) { + return advice; + } + + } + } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java new file mode 100644 index 00000000000..caa19439bde --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -0,0 +1,60 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; +import org.springframework.security.core.userdetails.UserDetails; + +public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { + + private final int minLength; + + private final int maxLength; + + private Action tooShortAction = Action.MUST_CHANGE; + + private Action tooLongAction = Action.SHOULD_CHANGE; + + public ChangeLengthPasswordAdvisor(int minLength) { + this(minLength, Integer.MAX_VALUE); + } + + public ChangeLengthPasswordAdvisor(int minLength, int maxLength) { + this.minLength = minLength; + this.maxLength = maxLength; + } + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + if (password.length() < this.minLength) { + return new SimpleChangePasswordAdvice(this.tooShortAction, ChangePasswordReason.TOO_SHORT); + } + if (password.length() > this.maxLength) { + return new SimpleChangePasswordAdvice(this.tooLongAction, ChangePasswordReason.TOO_LONG); + } + return ChangePasswordAdvice.keep(); + } + + public void setTooShortAction(Action tooShortAction) { + this.tooShortAction = tooShortAction; + } + + public void setTooLongAction(Action tooLongAction) { + this.tooLongAction = tooLongAction; + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java new file mode 100644 index 00000000000..89c2ba392d9 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java @@ -0,0 +1,37 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import java.util.Collection; + +public interface ChangePasswordAdvice { + + Action getAction(); + + Collection getReasons(); + + static ChangePasswordAdvice keep() { + return new SimpleChangePasswordAdvice(Action.KEEP); + } + + enum Action { + + KEEP, SHOULD_CHANGE, MUST_CHANGE + + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java new file mode 100644 index 00000000000..8f6204c2f3a --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java @@ -0,0 +1,29 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.core.userdetails.UserDetails; + +public interface ChangePasswordAdvisor { + + ChangePasswordAdvice advise(UserDetails user, String password); + + default ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { + return advise(user, password); + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java new file mode 100644 index 00000000000..fd95c11e62b --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java @@ -0,0 +1,23 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +public enum ChangePasswordReason { + + COMPROMISED, EXPIRED, MISSING_CHARACTERS, REPEATED, TOO_SHORT, TOO_LONG, UNSUPPORTED_CHARACTERS + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java new file mode 100644 index 00000000000..ccdb8298225 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java @@ -0,0 +1,39 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.core.userdetails.UserDetails; + +public final class ChangePasswordServiceAdvisor implements ChangePasswordAdvisor { + + private final UserDetailsPasswordManager passwordManager; + + public ChangePasswordServiceAdvisor(UserDetailsPasswordManager passwordManager) { + this.passwordManager = passwordManager; + } + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + return this.passwordManager.loadPasswordAdvice(user); + } + + @Override + public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { + return null; + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java new file mode 100644 index 00000000000..71c71991d01 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java @@ -0,0 +1,61 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.util.Assert; + +public final class ChangeRepeatedPasswordAdvisor implements ChangePasswordAdvisor { + + private final UserDetailsService userDetailsService; + + private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + private Action action = Action.MUST_CHANGE; + + public ChangeRepeatedPasswordAdvisor(UserDetailsService userDetailsService) { + this.userDetailsService = userDetailsService; + } + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + return null; + } + + @Override + public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { + UserDetails withPassword = this.userDetailsService.loadUserByUsername(user.getUsername()); + if (this.passwordEncoder.matches(password, withPassword.getPassword())) { + return new SimpleChangePasswordAdvice(this.action, ChangePasswordReason.REPEATED); + } + return ChangePasswordAdvice.keep(); + } + + public void setPasswordEncoder(PasswordEncoder passwordEncoder) { + Assert.notNull(passwordEncoder, "passwordEncoder cannot be null"); + this.passwordEncoder = passwordEncoder; + } + + public void setAction(Action action) { + this.action = action; + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java new file mode 100644 index 00000000000..b28143cffcc --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java @@ -0,0 +1,89 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Objects; + +import org.springframework.security.core.userdetails.UserDetails; + +public final class DelegatingChangePasswordAdvisor implements ChangePasswordAdvisor { + + private final List advisors; + + public DelegatingChangePasswordAdvisor(List advisors) { + this.advisors = advisors; + } + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + Collection advice = this.advisors.stream() + .map((advisor) -> advisor.advise(user, password)) + .filter(Objects::nonNull) + .toList(); + return new CompositeChangePasswordAdvice(advice); + } + + @Override + public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { + Collection advice = this.advisors.stream() + .map((advisor) -> advisor.adviseForUpdate(user, password)) + .filter(Objects::nonNull) + .toList(); + return new CompositeChangePasswordAdvice(advice); + } + + private static final class CompositeChangePasswordAdvice implements ChangePasswordAdvice { + + private final Collection advice; + + private final Action action; + + private final Collection reasons; + + private CompositeChangePasswordAdvice(Collection advice) { + this.advice = advice; + Action action = Action.KEEP; + Collection reasons = new ArrayList<>(); + for (ChangePasswordAdvice a : advice) { + if (a.getAction() == Action.KEEP) { + continue; + } + if (action.ordinal() < a.getAction().ordinal()) { + action = a.getAction(); + } + reasons.addAll(a.getReasons()); + } + this.action = action; + this.reasons = reasons; + } + + @Override + public Action getAction() { + return this.action; + } + + @Override + public Collection getReasons() { + return this.reasons; + } + + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java new file mode 100644 index 00000000000..f25edce5e4d --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java @@ -0,0 +1,48 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +public final class SimpleChangePasswordAdvice implements ChangePasswordAdvice { + + private final Action action; + + private final Collection reasons; + + public SimpleChangePasswordAdvice(Action action, ChangePasswordReason... reasons) { + this(action, List.of(reasons)); + } + + public SimpleChangePasswordAdvice(Action action, Collection reasons) { + this.action = action; + this.reasons = new ArrayList<>(reasons); + } + + @Override + public Action getAction() { + return this.action; + } + + @Override + public Collection getReasons() { + return this.reasons; + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java new file mode 100644 index 00000000000..988736a86db --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java @@ -0,0 +1,40 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsPasswordService; + +public interface UserDetailsPasswordManager extends UserDetailsPasswordService { + + /** + * Update the password and remove any related password advice + * @param user the user to modify the password for + * @param newPassword the password to change to, encoded by the configured + * {@code PasswordEncoder} + * @return the updated {@link UserDetails} + */ + @Override + UserDetails updatePassword(UserDetails user, String newPassword); + + ChangePasswordAdvice loadPasswordAdvice(UserDetails user); + + void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice); + + void removePasswordAdvice(UserDetails user); + +} diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index 6380bdd7064..b5bfde4b943 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -31,13 +31,14 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.UserDetailsPasswordService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.core.userdetails.memory.UserAttribute; import org.springframework.security.core.userdetails.memory.UserAttributeEditor; @@ -53,12 +54,14 @@ * @author Luke Taylor * @since 3.1 */ -public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetailsPasswordService { +public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetailsPasswordManager { protected final Log logger = LogFactory.getLog(getClass()); private final Map users = new HashMap<>(); + private final Map advice = new HashMap<>(); + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder .getContextHolderStrategy(); @@ -162,6 +165,7 @@ public UserDetails updatePassword(UserDetails user, @Nullable String newPassword throw new RuntimeException("user '" + username + "' does not exist"); } mutableUser.setPassword(newPassword); + removePasswordAdvice(mutableUser); return mutableUser; } @@ -178,6 +182,23 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx user.isCredentialsNonExpired(), user.isAccountNonLocked(), user.getAuthorities()); } + @Override + public ChangePasswordAdvice loadPasswordAdvice(UserDetails user) { + return this.advice.get(user.getUsername()); + } + + @Override + public void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice) { + Assert.notNull(advice, + "advice must not be null; if you want to remove advice, please call removePasswordAdvice"); + this.advice.put(user.getUsername(), advice); + } + + @Override + public void removePasswordAdvice(UserDetails user) { + this.advice.remove(user.getUsername()); + } + /** * Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use * the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}. diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java new file mode 100644 index 00000000000..219da6e701b --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -0,0 +1,77 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.util.Collection; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.security.authentication.password.CompromisedPasswordChecker; +import org.springframework.security.authentication.password.CompromisedPasswordDecision; +import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; +import org.springframework.security.core.userdetails.UserDetails; + +public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { + + private final CompromisedPasswordChecker pwned = new HaveIBeenPwnedRestApiPasswordChecker(); + + private Action action = Action.SHOULD_CHANGE; + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + return new Advice(this.action, this.pwned.check(password)); + } + + public void setAction(Action action) { + this.action = action; + } + + public static final class Advice implements ChangePasswordAdvice { + + private final CompromisedPasswordDecision decision; + + private final ChangePasswordAdvice advice; + + public Advice(Action action, CompromisedPasswordDecision decision) { + this.decision = decision; + if (decision.isCompromised()) { + this.advice = new SimpleChangePasswordAdvice(action, ChangePasswordReason.COMPROMISED); + } + else { + this.advice = ChangePasswordAdvice.keep(); + } + } + + public CompromisedPasswordDecision getDecision() { + return this.decision; + } + + @Override + public Action getAction() { + return this.advice.getAction(); + } + + @Override + public Collection getReasons() { + return this.advice.getReasons(); + } + + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java new file mode 100644 index 00000000000..96a75d556cd --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java @@ -0,0 +1,39 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.io.IOException; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; + +public interface ChangePasswordAdviceHandler { + + void handle(HttpServletRequest request, HttpServletResponse response, FilterChain chain, + ChangePasswordAdvice advice) throws ServletException, IOException; + +} + +// authentication request process +// -------------- ------- ------- +// KEEP redirect to home continue filter redirect to home +// RESET redirect to home continue filter redirect to home +// REQUIRE_RESET redirect to home redirect to reset redirect to home diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java new file mode 100644 index 00000000000..3a5cd334767 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java @@ -0,0 +1,48 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import jakarta.servlet.http.HttpServletRequest; + +import org.springframework.core.MethodParameter; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.web.bind.support.WebDataBinderFactory; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.method.support.ModelAndViewContainer; + +public final class ChangePasswordAdviceMethodArgumentResolver implements HandlerMethodArgumentResolver { + + ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return ChangePasswordAdvice.class.isAssignableFrom(parameter.getParameterType()); + } + + @Override + public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, + NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { + return this.changePasswordAdviceRepository + .loadPasswordAdvice(webRequest.getNativeRequest(HttpServletRequest.class)); + } + + public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { + this.changePasswordAdviceRepository = changePasswordAdviceRepository; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java new file mode 100644 index 00000000000..7cee802964b --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java @@ -0,0 +1,32 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; + +public interface ChangePasswordAdviceRepository { + + ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request); + + void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, ChangePasswordAdvice advice); + + void removePasswordAdvice(HttpServletRequest request, HttpServletResponse response); + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java new file mode 100644 index 00000000000..aec9b12765b --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java @@ -0,0 +1,51 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.io.IOException; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.web.filter.OncePerRequestFilter; + +public class ChangePasswordAdvisingFilter extends OncePerRequestFilter { + + private ChangePasswordAdviceHandler changePasswordAdviceHandler = new SimpleChangePasswordAdviceHandler( + DefaultChangePasswordPageGeneratingFilter.DEFAULT_CHANGE_PASSWORD_URL); + + private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws ServletException, IOException { + ChangePasswordAdvice advice = this.changePasswordAdviceRepository.loadPasswordAdvice(request); + this.changePasswordAdviceHandler.handle(request, response, chain, advice); + } + + public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { + this.changePasswordAdviceRepository = changePasswordAdviceRepository; + } + + public void setChangePasswordAdviceHandler(ChangePasswordAdviceHandler changePasswordAdviceHandler) { + this.changePasswordAdviceHandler = changePasswordAdviceHandler; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java new file mode 100644 index 00000000000..3c17d62ed96 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java @@ -0,0 +1,148 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.io.IOException; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.security.authentication.InsufficientAuthenticationException; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.UserDetailsPasswordManager; +import org.springframework.security.authorization.AuthenticatedAuthorizationManager; +import org.springframework.security.authorization.AuthorizationDeniedException; +import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.security.web.access.AccessDeniedHandler; +import org.springframework.security.web.access.HttpStatusAccessDeniedHandler; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; +import org.springframework.security.web.authentication.AuthenticationEntryPointFailureHandler; +import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; +import org.springframework.security.web.authentication.HttpStatusEntryPoint; +import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.web.filter.OncePerRequestFilter; + +public class ChangePasswordProcessingFilter extends OncePerRequestFilter { + + public static final String DEFAULT_PASSWORD_CHANGE_PROCESSING_URL = "/change-password"; + + private final AuthenticationFailureHandler failureHandler = new AuthenticationEntryPointFailureHandler( + new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)); + + private final AuthorizationManager authorizationManager = AuthenticatedAuthorizationManager + .authenticated(); + + private final AccessDeniedHandler deniedHandler = new HttpStatusAccessDeniedHandler(HttpStatus.FORBIDDEN); + + private final AuthenticationSuccessHandler successHandler = new SavedRequestAwareAuthenticationSuccessHandler(); + + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + + private RequestMatcher requestMatcher = PathPatternRequestMatcher.withDefaults() + .matcher(HttpMethod.POST, DEFAULT_PASSWORD_CHANGE_PROCESSING_URL); + + private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + + private ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); + + private final UserDetailsPasswordManager passwordManager; + + public ChangePasswordProcessingFilter(UserDetailsPasswordManager passwordManager) { + this.passwordManager = passwordManager; + } + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws ServletException, IOException { + RequestMatcher.MatchResult match = this.requestMatcher.matcher(request); + if (!match.isMatch()) { + chain.doFilter(request, response); + return; + } + String password = request.getParameter("newPassword"); + if (password == null) { + chain.doFilter(request, response); + return; + } + Authentication authentication = this.securityContextHolderStrategy.getContext().getAuthentication(); + if (authentication == null) { + this.failureHandler.onAuthenticationFailure(request, response, + new InsufficientAuthenticationException("Authentication required to change password")); + return; + } + AuthorizationResult authorization = this.authorizationManager.authorize(() -> authentication, + new RequestAuthorizationContext(request, match.getVariables())); + if (authorization == null) { + this.deniedHandler.handle(request, response, new AuthorizationDeniedException("denied")); + return; + } + if (!authorization.isGranted()) { + this.deniedHandler.handle(request, response, + new AuthorizationDeniedException("access denied", authorization)); + return; + } + UserDetails user = (UserDetails) authentication.getPrincipal(); + ChangePasswordAdvice advice = this.changePasswordAdvisor.adviseForUpdate(user, password); + if (advice.getAction() == ChangePasswordAdvice.Action.KEEP) { + this.passwordManager.updatePassword(user, this.passwordEncoder.encode(password)); + this.changePasswordAdviceRepository.removePasswordAdvice(request, response); + } + else { + this.changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); + } + this.successHandler.onAuthenticationSuccess(request, response, authentication); + } + + public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository advice) { + this.changePasswordAdviceRepository = advice; + } + + public void setChangePasswordAdvisor(ChangePasswordAdvisor advisor) { + this.changePasswordAdvisor = advisor; + } + + public void setRequestMatcher(RequestMatcher requestMatcher) { + this.requestMatcher = requestMatcher; + } + + public void setPasswordEncoder(PasswordEncoder passwordEncoder) { + this.passwordEncoder = passwordEncoder; + } + + public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + this.securityContextHolderStrategy = securityContextHolderStrategy; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java new file mode 100644 index 00000000000..c6681b88214 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java @@ -0,0 +1,80 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.io.IOException; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.http.HttpMethod; +import org.springframework.security.web.csrf.CsrfToken; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.web.filter.OncePerRequestFilter; + +public class DefaultChangePasswordPageGeneratingFilter extends OncePerRequestFilter { + + public static final String DEFAULT_CHANGE_PASSWORD_URL = "/change-password"; + + private RequestMatcher requestMatcher = PathPatternRequestMatcher.withDefaults() + .matcher(HttpMethod.GET, DEFAULT_CHANGE_PASSWORD_URL); + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws ServletException, IOException { + if (!this.requestMatcher.matches(request)) { + chain.doFilter(request, response); + return; + } + String page = PASSWORD_RESET_TEMPLATE; + CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName()); + if (token != null) { + page = page.replace("{{parameter}}", token.getParameterName()).replace("{{value}}", token.getToken()); + } + response.setContentType("text/html;charset=UTF-8"); + response.getWriter().println(page); + } + + private static final String PASSWORD_RESET_TEMPLATE = """ + + + + + + Change Your Password + + + +
+ +
+ + + """; + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java new file mode 100644 index 00000000000..ca73e174481 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java @@ -0,0 +1,83 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.util.Collection; +import java.util.function.Supplier; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.lang.NonNull; +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.util.function.SingletonSupplier; + +public final class HttpSessionChangePasswordAdviceRepository implements ChangePasswordAdviceRepository { + + private static final String PASSWORD_ADVICE_ATTRIBUTE_NAME = HttpSessionChangePasswordAdviceRepository.class + .getName() + ".PASSWORD_ADVICE"; + + @Override + @NonNull + public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { + return new DeferredChangePasswordAdvice(() -> { + ChangePasswordAdvice advice = (ChangePasswordAdvice) request.getSession() + .getAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME); + if (advice != null) { + return advice; + } + return ChangePasswordAdvice.keep(); + }); + } + + @Override + public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, + ChangePasswordAdvice advice) { + if (advice.getAction() == ChangePasswordAdvice.Action.KEEP) { + removePasswordAdvice(request, response); + return; + } + request.getSession().setAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME, advice); + } + + @Override + public void removePasswordAdvice(HttpServletRequest request, HttpServletResponse response) { + request.getSession().removeAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME); + } + + private static final class DeferredChangePasswordAdvice implements ChangePasswordAdvice { + + private final Supplier advice; + + DeferredChangePasswordAdvice(Supplier advice) { + this.advice = SingletonSupplier.of(advice); + } + + @Override + public Action getAction() { + return this.advice.get().getAction(); + } + + @Override + public Collection getReasons() { + return this.advice.get().getReasons(); + } + + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java b/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java new file mode 100644 index 00000000000..a9f63da1284 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java @@ -0,0 +1,76 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import java.io.IOException; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.web.DefaultRedirectStrategy; +import org.springframework.security.web.RedirectStrategy; +import org.springframework.security.web.savedrequest.NullRequestCache; +import org.springframework.security.web.savedrequest.RequestCache; +import org.springframework.security.web.util.matcher.AnyRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.util.Assert; + +public final class SimpleChangePasswordAdviceHandler implements ChangePasswordAdviceHandler { + + private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); + + private final String changePasswordUrl; + + private RequestCache requestCache = new NullRequestCache(); + + private RequestMatcher requestMatcher = AnyRequestMatcher.INSTANCE; + + public SimpleChangePasswordAdviceHandler(String changePasswordUrl) { + Assert.hasText(changePasswordUrl, "changePasswordUrl cannot be empty"); + this.changePasswordUrl = changePasswordUrl; + } + + @Override + public void handle(HttpServletRequest request, HttpServletResponse response, FilterChain chain, + ChangePasswordAdvice advice) throws IOException, ServletException { + if (!this.requestMatcher.matches(request)) { + return; + } + if (request.getRequestURI().equals(this.changePasswordUrl)) { + chain.doFilter(request, response); + return; + } + if (advice.getAction() != ChangePasswordAdvice.Action.MUST_CHANGE) { + chain.doFilter(request, response); + return; + } + this.requestCache.saveRequest(request, response); + this.redirectStrategy.sendRedirect(request, response, this.changePasswordUrl); + } + + public void setRequestCache(RequestCache requestCache) { + this.requestCache = requestCache; + } + + public void setRequestMatcher(RequestMatcher requestMatcher) { + this.requestMatcher = requestMatcher; + } + +} From e0dbdee140c2d86af141ed75e64ae5530dce9bf3 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 20 May 2025 13:31:26 -0600 Subject: [PATCH 02/10] Split ChangePasswordAdvisor Methods There are now two interfaces, ChangeExistingPasswordAdvisor and ChangeUpdatingPasswordAdvisor. I have a sense that more information may be wanted down the road for ChangeUpdatingPasswordAdvisor; so this would allow them to evolve independently. --- .../PasswordManagementConfigurer.java | 42 +++++++++++-------- ...ava => ChangeExistingPasswordAdvisor.java} | 6 +-- .../password/ChangeLengthPasswordAdvisor.java | 2 +- .../password/ChangePasswordAdvice.java | 2 +- .../ChangePasswordServiceAdvisor.java | 7 +--- .../ChangeRepeatedPasswordAdvisor.java | 7 +--- .../ChangeUpdatingPasswordAdvisor.java | 25 +++++++++++ .../DelegatingChangePasswordAdvisor.java | 33 +++++++++------ .../password/SimpleChangePasswordAdvice.java | 16 +++---- .../ChangeCompromisedPasswordAdvisor.java | 6 ++- .../ChangePasswordProcessingFilter.java | 8 ++-- 11 files changed, 92 insertions(+), 62 deletions(-) rename core/src/main/java/org/springframework/security/authentication/password/{ChangePasswordAdvisor.java => ChangeExistingPasswordAdvisor.java} (83%) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 01ee4ea9c17..e22d7265c94 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -16,14 +16,12 @@ package org.springframework.security.config.annotation.web.configurers; -import java.util.ArrayList; -import java.util.List; - import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.security.authentication.password.ChangeExistingPasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordServiceAdvisor; +import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; import org.springframework.security.authentication.password.DelegatingChangePasswordAdvisor; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; @@ -67,7 +65,9 @@ public final class PasswordManagementConfigurer private ChangePasswordAdviceRepository changePasswordAdviceRepository; - private ChangePasswordAdvisor changePasswordAdvisor; + private ChangeExistingPasswordAdvisor changeExistingPasswordAdvisor; + + private ChangeUpdatingPasswordAdvisor changeUpdatingPasswordAdvisor; private ChangePasswordAdviceHandler changePasswordAdviceHandler; @@ -97,8 +97,15 @@ public PasswordManagementConfigurer changePasswordAdviceRepository( return this; } - public PasswordManagementConfigurer changePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { - this.changePasswordAdvisor = changePasswordAdvisor; + public PasswordManagementConfigurer changeExistingPasswordAdvisor( + ChangeExistingPasswordAdvisor changePasswordAdvisor) { + this.changeExistingPasswordAdvisor = changePasswordAdvisor; + return this; + } + + public PasswordManagementConfigurer changeUpdatingPasswordAdvisor( + ChangeUpdatingPasswordAdvisor changePasswordAdvisor) { + this.changeUpdatingPasswordAdvisor = changePasswordAdvisor; return this; } @@ -129,17 +136,18 @@ public void init(B http) throws Exception { : this.context.getBeanProvider(ChangePasswordAdviceRepository.class) .getIfUnique(HttpSessionChangePasswordAdviceRepository::new); - ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor - : this.context.getBeanProvider(ChangePasswordAdvisor.class).getIfUnique(() -> { - List advisors = new ArrayList<>(); - advisors.add(new ChangeCompromisedPasswordAdvisor()); - advisors.add(new ChangePasswordServiceAdvisor(passwordManager)); - return new DelegatingChangePasswordAdvisor(advisors); - }); + ChangeExistingPasswordAdvisor changeExistingPasswordAdvisor = (this.changeExistingPasswordAdvisor != null) + ? this.changeExistingPasswordAdvisor + : this.context.getBeanProvider(ChangeExistingPasswordAdvisor.class) + .getIfUnique(() -> DelegatingChangePasswordAdvisor.forExisting( + new ChangePasswordServiceAdvisor(passwordManager), new ChangeCompromisedPasswordAdvisor())); + ChangeUpdatingPasswordAdvisor changeUpdatingPasswordAdvisor = (this.changeExistingPasswordAdvisor != null) + ? this.changeUpdatingPasswordAdvisor : this.context.getBeanProvider(ChangeUpdatingPasswordAdvisor.class) + .getIfUnique(ChangeCompromisedPasswordAdvisor::new); http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); - http.setSharedObject(ChangePasswordAdvisor.class, changePasswordAdvisor); + http.setSharedObject(ChangeUpdatingPasswordAdvisor.class, changeUpdatingPasswordAdvisor); FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); String passwordParameter = (form != null) ? form.getPasswordParameter() : "password"; @@ -147,7 +155,7 @@ public void init(B http) throws Exception { .addSessionAuthenticationStrategy((authentication, request, response) -> { UserDetails user = (UserDetails) authentication.getPrincipal(); String password = request.getParameter(passwordParameter); - ChangePasswordAdvice advice = changePasswordAdvisor.advise(user, password); + ChangePasswordAdvice advice = changeExistingPasswordAdvisor.advise(user, password); changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); }); } @@ -181,7 +189,7 @@ public void configure(B http) throws Exception { http.getSharedObject(UserDetailsPasswordManager.class)); processing .setRequestMatcher(PathPatternRequestMatcher.withDefaults().matcher(this.changePasswordProcessingUrl)); - processing.setChangePasswordAdvisor(http.getSharedObject(ChangePasswordAdvisor.class)); + processing.setChangePasswordAdvisor(http.getSharedObject(ChangeUpdatingPasswordAdvisor.class)); processing.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); processing.setPasswordEncoder(passwordEncoder); processing.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java similarity index 83% rename from core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java index 8f6204c2f3a..60c0ddfef5c 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java @@ -18,12 +18,8 @@ import org.springframework.security.core.userdetails.UserDetails; -public interface ChangePasswordAdvisor { +public interface ChangeExistingPasswordAdvisor { ChangePasswordAdvice advise(UserDetails user, String password); - default ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { - return advise(user, password); - } - } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java index caa19439bde..100fe166a0e 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -19,7 +19,7 @@ import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.core.userdetails.UserDetails; -public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { +public class ChangeLengthPasswordAdvisor implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { private final int minLength; diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java index 89c2ba392d9..71d4b3d6cfe 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java @@ -25,7 +25,7 @@ public interface ChangePasswordAdvice { Collection getReasons(); static ChangePasswordAdvice keep() { - return new SimpleChangePasswordAdvice(Action.KEEP); + return SimpleChangePasswordAdvice.KEEP; } enum Action { diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java index ccdb8298225..c25bd1f9563 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java @@ -18,7 +18,7 @@ import org.springframework.security.core.userdetails.UserDetails; -public final class ChangePasswordServiceAdvisor implements ChangePasswordAdvisor { +public final class ChangePasswordServiceAdvisor implements ChangeExistingPasswordAdvisor { private final UserDetailsPasswordManager passwordManager; @@ -31,9 +31,4 @@ public ChangePasswordAdvice advise(UserDetails user, String password) { return this.passwordManager.loadPasswordAdvice(user); } - @Override - public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { - return null; - } - } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java index 71c71991d01..3d31a69885f 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java @@ -23,7 +23,7 @@ import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.util.Assert; -public final class ChangeRepeatedPasswordAdvisor implements ChangePasswordAdvisor { +public final class ChangeRepeatedPasswordAdvisor implements ChangeUpdatingPasswordAdvisor { private final UserDetailsService userDetailsService; @@ -37,11 +37,6 @@ public ChangeRepeatedPasswordAdvisor(UserDetailsService userDetailsService) { @Override public ChangePasswordAdvice advise(UserDetails user, String password) { - return null; - } - - @Override - public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { UserDetails withPassword = this.userDetailsService.loadUserByUsername(user.getUsername()); if (this.passwordEncoder.matches(password, withPassword.getPassword())) { return new SimpleChangePasswordAdvice(this.action, ChangePasswordReason.REPEATED); diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java new file mode 100644 index 00000000000..77a0a13a511 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java @@ -0,0 +1,25 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.springframework.security.core.userdetails.UserDetails; + +public interface ChangeUpdatingPasswordAdvisor { + + ChangePasswordAdvice advise(UserDetails user, String password); + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java index b28143cffcc..323c75e13fa 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java @@ -18,32 +18,39 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.function.BiFunction; +import java.util.stream.Stream; import org.springframework.security.core.userdetails.UserDetails; -public final class DelegatingChangePasswordAdvisor implements ChangePasswordAdvisor { +public final class DelegatingChangePasswordAdvisor + implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { - private final List advisors; + private final List> advisors; - public DelegatingChangePasswordAdvisor(List advisors) { - this.advisors = advisors; + private DelegatingChangePasswordAdvisor(List> advisors) { + this.advisors = Collections.unmodifiableList(advisors); } - @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { - Collection advice = this.advisors.stream() - .map((advisor) -> advisor.advise(user, password)) - .filter(Objects::nonNull) - .toList(); - return new CompositeChangePasswordAdvice(advice); + public static ChangeExistingPasswordAdvisor forExisting(ChangeExistingPasswordAdvisor... advisors) { + return new DelegatingChangePasswordAdvisor(Stream.of(advisors) + .map((advisor) -> (BiFunction) advisor::advise) + .toList()); + } + + public static ChangeUpdatingPasswordAdvisor forUpdating(ChangeUpdatingPasswordAdvisor... advisors) { + return new DelegatingChangePasswordAdvisor(Stream.of(advisors) + .map((advisor) -> (BiFunction) advisor::advise) + .toList()); } @Override - public ChangePasswordAdvice adviseForUpdate(UserDetails user, String password) { + public ChangePasswordAdvice advise(UserDetails user, String password) { Collection advice = this.advisors.stream() - .map((advisor) -> advisor.adviseForUpdate(user, password)) + .map((advisor) -> advisor.apply(user, password)) .filter(Objects::nonNull) .toList(); return new CompositeChangePasswordAdvice(advice); diff --git a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java index f25edce5e4d..712c227d546 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,23 +16,25 @@ package org.springframework.security.authentication.password; -import java.util.ArrayList; import java.util.Collection; import java.util.List; -public final class SimpleChangePasswordAdvice implements ChangePasswordAdvice { +public class SimpleChangePasswordAdvice implements ChangePasswordAdvice { + + static final SimpleChangePasswordAdvice KEEP = new SimpleChangePasswordAdvice(Action.KEEP); private final Action action; private final Collection reasons; - public SimpleChangePasswordAdvice(Action action, ChangePasswordReason... reasons) { - this(action, List.of(reasons)); + public SimpleChangePasswordAdvice(Action action, Collection reasons) { + this.action = action; + this.reasons = reasons; } - public SimpleChangePasswordAdvice(Action action, Collection reasons) { + public SimpleChangePasswordAdvice(Action action, ChangePasswordReason... reasons) { this.action = action; - this.reasons = new ArrayList<>(reasons); + this.reasons = List.of(reasons); } @Override diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java index 219da6e701b..3fe0126dd56 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -18,16 +18,18 @@ import java.util.Collection; +import org.springframework.security.authentication.password.ChangeExistingPasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; import org.springframework.security.core.userdetails.UserDetails; -public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { +public final class ChangeCompromisedPasswordAdvisor + implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { private final CompromisedPasswordChecker pwned = new HaveIBeenPwnedRestApiPasswordChecker(); diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java index 3c17d62ed96..ebc17fda3ed 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java @@ -27,7 +27,7 @@ import org.springframework.http.HttpStatus; import org.springframework.security.authentication.InsufficientAuthenticationException; import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.authorization.AuthenticatedAuthorizationManager; import org.springframework.security.authorization.AuthorizationDeniedException; @@ -75,7 +75,7 @@ public class ChangePasswordProcessingFilter extends OncePerRequestFilter { private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); - private ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); + private ChangeUpdatingPasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); private final UserDetailsPasswordManager passwordManager; @@ -114,7 +114,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse return; } UserDetails user = (UserDetails) authentication.getPrincipal(); - ChangePasswordAdvice advice = this.changePasswordAdvisor.adviseForUpdate(user, password); + ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); if (advice.getAction() == ChangePasswordAdvice.Action.KEEP) { this.passwordManager.updatePassword(user, this.passwordEncoder.encode(password)); this.changePasswordAdviceRepository.removePasswordAdvice(request, response); @@ -129,7 +129,7 @@ public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository adv this.changePasswordAdviceRepository = advice; } - public void setChangePasswordAdvisor(ChangePasswordAdvisor advisor) { + public void setChangePasswordAdvisor(ChangeUpdatingPasswordAdvisor advisor) { this.changePasswordAdvisor = advisor; } From f5214c0c4311588aa595a072c929d9a63967401f Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 23 May 2025 12:29:08 -0600 Subject: [PATCH 03/10] Removing Updating Password Support --- .../PasswordManagementConfigurer.java | 65 ++------ .../password/ChangeLengthPasswordAdvisor.java | 2 +- ...dvisor.java => ChangePasswordAdvisor.java} | 2 +- .../ChangePasswordServiceAdvisor.java | 2 +- .../ChangeRepeatedPasswordAdvisor.java | 56 ------- .../ChangeUpdatingPasswordAdvisor.java | 25 --- .../DelegatingChangePasswordAdvisor.java | 10 +- .../ChangeCompromisedPasswordAdvisor.java | 6 +- .../ChangePasswordProcessingFilter.java | 148 ------------------ ...ultChangePasswordPageGeneratingFilter.java | 80 ---------- 10 files changed, 18 insertions(+), 378 deletions(-) rename core/src/main/java/org/springframework/security/authentication/password/{ChangeExistingPasswordAdvisor.java => ChangePasswordAdvisor.java} (94%) delete mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java delete mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index e22d7265c94..c8d0b9ec13f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -18,28 +18,22 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.security.authentication.password.ChangeExistingPasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordServiceAdvisor; -import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; import org.springframework.security.authentication.password.DelegatingChangePasswordAdvisor; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.crypto.factory.PasswordEncoderFactories; -import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; import org.springframework.security.web.authentication.password.ChangePasswordAdviceHandler; import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; -import org.springframework.security.web.authentication.password.ChangePasswordProcessingFilter; -import org.springframework.security.web.authentication.password.DefaultChangePasswordPageGeneratingFilter; import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; import org.springframework.security.web.authentication.password.SimpleChangePasswordAdviceHandler; import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.util.Assert; /** @@ -53,7 +47,7 @@ public final class PasswordManagementConfigurer private static final String WELL_KNOWN_CHANGE_PASSWORD_PATTERN = "/.well-known/change-password"; - private static final String DEFAULT_CHANGE_PASSWORD_PAGE = DefaultChangePasswordPageGeneratingFilter.DEFAULT_CHANGE_PASSWORD_URL; + private static final String DEFAULT_CHANGE_PASSWORD_PAGE = "/change-password"; private ApplicationContext context; @@ -61,13 +55,9 @@ public final class PasswordManagementConfigurer private String changePasswordPage = DEFAULT_CHANGE_PASSWORD_PAGE; - private String changePasswordProcessingUrl = ChangePasswordProcessingFilter.DEFAULT_PASSWORD_CHANGE_PROCESSING_URL; - private ChangePasswordAdviceRepository changePasswordAdviceRepository; - private ChangeExistingPasswordAdvisor changeExistingPasswordAdvisor; - - private ChangeUpdatingPasswordAdvisor changeUpdatingPasswordAdvisor; + private ChangePasswordAdvisor changePasswordAdvisor; private ChangePasswordAdviceHandler changePasswordAdviceHandler; @@ -86,26 +76,15 @@ public PasswordManagementConfigurer changePasswordPage(String changePasswordP return this; } - public PasswordManagementConfigurer changePasswordProcessingUrl(String changePasswordProcessingUrl) { - this.changePasswordProcessingUrl = changePasswordProcessingUrl; - return this; - } - public PasswordManagementConfigurer changePasswordAdviceRepository( ChangePasswordAdviceRepository changePasswordAdviceRepository) { this.changePasswordAdviceRepository = changePasswordAdviceRepository; return this; } - public PasswordManagementConfigurer changeExistingPasswordAdvisor( - ChangeExistingPasswordAdvisor changePasswordAdvisor) { - this.changeExistingPasswordAdvisor = changePasswordAdvisor; - return this; - } - - public PasswordManagementConfigurer changeUpdatingPasswordAdvisor( - ChangeUpdatingPasswordAdvisor changePasswordAdvisor) { - this.changeUpdatingPasswordAdvisor = changePasswordAdvisor; + public PasswordManagementConfigurer changePasswordAdvisor( + ChangePasswordAdvisor changePasswordAdvisor) { + this.changePasswordAdvisor = changePasswordAdvisor; return this; } @@ -136,18 +115,14 @@ public void init(B http) throws Exception { : this.context.getBeanProvider(ChangePasswordAdviceRepository.class) .getIfUnique(HttpSessionChangePasswordAdviceRepository::new); - ChangeExistingPasswordAdvisor changeExistingPasswordAdvisor = (this.changeExistingPasswordAdvisor != null) - ? this.changeExistingPasswordAdvisor - : this.context.getBeanProvider(ChangeExistingPasswordAdvisor.class) - .getIfUnique(() -> DelegatingChangePasswordAdvisor.forExisting( + ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) + ? this.changePasswordAdvisor + : this.context.getBeanProvider(ChangePasswordAdvisor.class) + .getIfUnique(() -> DelegatingChangePasswordAdvisor.of( new ChangePasswordServiceAdvisor(passwordManager), new ChangeCompromisedPasswordAdvisor())); - ChangeUpdatingPasswordAdvisor changeUpdatingPasswordAdvisor = (this.changeExistingPasswordAdvisor != null) - ? this.changeUpdatingPasswordAdvisor : this.context.getBeanProvider(ChangeUpdatingPasswordAdvisor.class) - .getIfUnique(ChangeCompromisedPasswordAdvisor::new); http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); - http.setSharedObject(ChangeUpdatingPasswordAdvisor.class, changeUpdatingPasswordAdvisor); FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); String passwordParameter = (form != null) ? form.getPasswordParameter() : "password"; @@ -155,7 +130,7 @@ public void init(B http) throws Exception { .addSessionAuthenticationStrategy((authentication, request, response) -> { UserDetails user = (UserDetails) authentication.getPrincipal(); String password = request.getParameter(passwordParameter); - ChangePasswordAdvice advice = changeExistingPasswordAdvisor.advise(user, password); + ChangePasswordAdvice advice = changePasswordAdvisor.advise(user, password); changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); }); } @@ -173,28 +148,10 @@ public void configure(B http) throws Exception { return; } - PasswordEncoder passwordEncoder = this.context.getBeanProvider(PasswordEncoder.class) - .getIfUnique(PasswordEncoderFactories::createDelegatingPasswordEncoder); - ChangePasswordAdviceHandler changePasswordAdviceHandler = (this.changePasswordAdviceHandler != null) ? this.changePasswordAdviceHandler : this.context.getBeanProvider(ChangePasswordAdviceHandler.class) .getIfUnique(() -> new SimpleChangePasswordAdviceHandler(this.changePasswordPage)); - if (!this.customChangePasswordPage) { - DefaultChangePasswordPageGeneratingFilter page = new DefaultChangePasswordPageGeneratingFilter(); - http.addFilterBefore(page, RequestCacheAwareFilter.class); - } - - ChangePasswordProcessingFilter processing = new ChangePasswordProcessingFilter( - http.getSharedObject(UserDetailsPasswordManager.class)); - processing - .setRequestMatcher(PathPatternRequestMatcher.withDefaults().matcher(this.changePasswordProcessingUrl)); - processing.setChangePasswordAdvisor(http.getSharedObject(ChangeUpdatingPasswordAdvisor.class)); - processing.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); - processing.setPasswordEncoder(passwordEncoder); - processing.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); - http.addFilterBefore(processing, RequestCacheAwareFilter.class); - ChangePasswordAdvisingFilter advising = new ChangePasswordAdvisingFilter(); advising.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); advising.setChangePasswordAdviceHandler(changePasswordAdviceHandler); diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java index 100fe166a0e..caa19439bde 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -19,7 +19,7 @@ import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.core.userdetails.UserDetails; -public class ChangeLengthPasswordAdvisor implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { +public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { private final int minLength; diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java similarity index 94% rename from core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java index 60c0ddfef5c..2289714f055 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeExistingPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java @@ -18,7 +18,7 @@ import org.springframework.security.core.userdetails.UserDetails; -public interface ChangeExistingPasswordAdvisor { +public interface ChangePasswordAdvisor { ChangePasswordAdvice advise(UserDetails user, String password); diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java index c25bd1f9563..e89250491d1 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java @@ -18,7 +18,7 @@ import org.springframework.security.core.userdetails.UserDetails; -public final class ChangePasswordServiceAdvisor implements ChangeExistingPasswordAdvisor { +public final class ChangePasswordServiceAdvisor implements ChangePasswordAdvisor { private final UserDetailsPasswordManager passwordManager; diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java deleted file mode 100644 index 3d31a69885f..00000000000 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeRepeatedPasswordAdvisor.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authentication.password; - -import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.UserDetailsService; -import org.springframework.security.crypto.factory.PasswordEncoderFactories; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.util.Assert; - -public final class ChangeRepeatedPasswordAdvisor implements ChangeUpdatingPasswordAdvisor { - - private final UserDetailsService userDetailsService; - - private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); - - private Action action = Action.MUST_CHANGE; - - public ChangeRepeatedPasswordAdvisor(UserDetailsService userDetailsService) { - this.userDetailsService = userDetailsService; - } - - @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { - UserDetails withPassword = this.userDetailsService.loadUserByUsername(user.getUsername()); - if (this.passwordEncoder.matches(password, withPassword.getPassword())) { - return new SimpleChangePasswordAdvice(this.action, ChangePasswordReason.REPEATED); - } - return ChangePasswordAdvice.keep(); - } - - public void setPasswordEncoder(PasswordEncoder passwordEncoder) { - Assert.notNull(passwordEncoder, "passwordEncoder cannot be null"); - this.passwordEncoder = passwordEncoder; - } - - public void setAction(Action action) { - this.action = action; - } - -} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java deleted file mode 100644 index 77a0a13a511..00000000000 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeUpdatingPasswordAdvisor.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authentication.password; - -import org.springframework.security.core.userdetails.UserDetails; - -public interface ChangeUpdatingPasswordAdvisor { - - ChangePasswordAdvice advise(UserDetails user, String password); - -} diff --git a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java index 323c75e13fa..685d05b0f38 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java @@ -27,7 +27,7 @@ import org.springframework.security.core.userdetails.UserDetails; public final class DelegatingChangePasswordAdvisor - implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { + implements ChangePasswordAdvisor { private final List> advisors; @@ -35,13 +35,7 @@ private DelegatingChangePasswordAdvisor(List (BiFunction) advisor::advise) - .toList()); - } - - public static ChangeUpdatingPasswordAdvisor forUpdating(ChangeUpdatingPasswordAdvisor... advisors) { + public static ChangePasswordAdvisor of(ChangePasswordAdvisor... advisors) { return new DelegatingChangePasswordAdvisor(Stream.of(advisors) .map((advisor) -> (BiFunction) advisor::advise) .toList()); diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java index 3fe0126dd56..219da6e701b 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -18,18 +18,16 @@ import java.util.Collection; -import org.springframework.security.authentication.password.ChangeExistingPasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordReason; -import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; import org.springframework.security.core.userdetails.UserDetails; -public final class ChangeCompromisedPasswordAdvisor - implements ChangeExistingPasswordAdvisor, ChangeUpdatingPasswordAdvisor { +public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { private final CompromisedPasswordChecker pwned = new HaveIBeenPwnedRestApiPasswordChecker(); diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java deleted file mode 100644 index ebc17fda3ed..00000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordProcessingFilter.java +++ /dev/null @@ -1,148 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication.password; - -import java.io.IOException; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatus; -import org.springframework.security.authentication.InsufficientAuthenticationException; -import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangeUpdatingPasswordAdvisor; -import org.springframework.security.authentication.password.UserDetailsPasswordManager; -import org.springframework.security.authorization.AuthenticatedAuthorizationManager; -import org.springframework.security.authorization.AuthorizationDeniedException; -import org.springframework.security.authorization.AuthorizationManager; -import org.springframework.security.authorization.AuthorizationResult; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.core.context.SecurityContextHolderStrategy; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.crypto.factory.PasswordEncoderFactories; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.security.web.access.AccessDeniedHandler; -import org.springframework.security.web.access.HttpStatusAccessDeniedHandler; -import org.springframework.security.web.access.intercept.RequestAuthorizationContext; -import org.springframework.security.web.authentication.AuthenticationEntryPointFailureHandler; -import org.springframework.security.web.authentication.AuthenticationFailureHandler; -import org.springframework.security.web.authentication.AuthenticationSuccessHandler; -import org.springframework.security.web.authentication.HttpStatusEntryPoint; -import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.web.filter.OncePerRequestFilter; - -public class ChangePasswordProcessingFilter extends OncePerRequestFilter { - - public static final String DEFAULT_PASSWORD_CHANGE_PROCESSING_URL = "/change-password"; - - private final AuthenticationFailureHandler failureHandler = new AuthenticationEntryPointFailureHandler( - new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)); - - private final AuthorizationManager authorizationManager = AuthenticatedAuthorizationManager - .authenticated(); - - private final AccessDeniedHandler deniedHandler = new HttpStatusAccessDeniedHandler(HttpStatus.FORBIDDEN); - - private final AuthenticationSuccessHandler successHandler = new SavedRequestAwareAuthenticationSuccessHandler(); - - private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder - .getContextHolderStrategy(); - - private RequestMatcher requestMatcher = PathPatternRequestMatcher.withDefaults() - .matcher(HttpMethod.POST, DEFAULT_PASSWORD_CHANGE_PROCESSING_URL); - - private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); - - private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); - - private ChangeUpdatingPasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); - - private final UserDetailsPasswordManager passwordManager; - - public ChangePasswordProcessingFilter(UserDetailsPasswordManager passwordManager) { - this.passwordManager = passwordManager; - } - - @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) - throws ServletException, IOException { - RequestMatcher.MatchResult match = this.requestMatcher.matcher(request); - if (!match.isMatch()) { - chain.doFilter(request, response); - return; - } - String password = request.getParameter("newPassword"); - if (password == null) { - chain.doFilter(request, response); - return; - } - Authentication authentication = this.securityContextHolderStrategy.getContext().getAuthentication(); - if (authentication == null) { - this.failureHandler.onAuthenticationFailure(request, response, - new InsufficientAuthenticationException("Authentication required to change password")); - return; - } - AuthorizationResult authorization = this.authorizationManager.authorize(() -> authentication, - new RequestAuthorizationContext(request, match.getVariables())); - if (authorization == null) { - this.deniedHandler.handle(request, response, new AuthorizationDeniedException("denied")); - return; - } - if (!authorization.isGranted()) { - this.deniedHandler.handle(request, response, - new AuthorizationDeniedException("access denied", authorization)); - return; - } - UserDetails user = (UserDetails) authentication.getPrincipal(); - ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); - if (advice.getAction() == ChangePasswordAdvice.Action.KEEP) { - this.passwordManager.updatePassword(user, this.passwordEncoder.encode(password)); - this.changePasswordAdviceRepository.removePasswordAdvice(request, response); - } - else { - this.changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); - } - this.successHandler.onAuthenticationSuccess(request, response, authentication); - } - - public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository advice) { - this.changePasswordAdviceRepository = advice; - } - - public void setChangePasswordAdvisor(ChangeUpdatingPasswordAdvisor advisor) { - this.changePasswordAdvisor = advisor; - } - - public void setRequestMatcher(RequestMatcher requestMatcher) { - this.requestMatcher = requestMatcher; - } - - public void setPasswordEncoder(PasswordEncoder passwordEncoder) { - this.passwordEncoder = passwordEncoder; - } - - public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { - this.securityContextHolderStrategy = securityContextHolderStrategy; - } - -} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java deleted file mode 100644 index c6681b88214..00000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/password/DefaultChangePasswordPageGeneratingFilter.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication.password; - -import java.io.IOException; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.http.HttpMethod; -import org.springframework.security.web.csrf.CsrfToken; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.web.filter.OncePerRequestFilter; - -public class DefaultChangePasswordPageGeneratingFilter extends OncePerRequestFilter { - - public static final String DEFAULT_CHANGE_PASSWORD_URL = "/change-password"; - - private RequestMatcher requestMatcher = PathPatternRequestMatcher.withDefaults() - .matcher(HttpMethod.GET, DEFAULT_CHANGE_PASSWORD_URL); - - @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) - throws ServletException, IOException { - if (!this.requestMatcher.matches(request)) { - chain.doFilter(request, response); - return; - } - String page = PASSWORD_RESET_TEMPLATE; - CsrfToken token = (CsrfToken) request.getAttribute(CsrfToken.class.getName()); - if (token != null) { - page = page.replace("{{parameter}}", token.getParameterName()).replace("{{value}}", token.getToken()); - } - response.setContentType("text/html;charset=UTF-8"); - response.getWriter().println(page); - } - - private static final String PASSWORD_RESET_TEMPLATE = """ - - - - - - Change Your Password - - - -
- -
- - - """; - -} From 23a1541f5f00586b2cd78cb71ed9e5103a847a3e Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 29 Jul 2025 17:53:37 -0600 Subject: [PATCH 04/10] Improvements --- .../PasswordManagementConfigurer.java | 31 ++++---- .../PasswordManagementConfigurerTests.java | 74 ++++++++++++------- .../password/ChangeLengthPasswordAdvisor.java | 6 +- .../password/ChangePasswordAdvice.java | 8 +- .../password/ChangePasswordReasons.java | 18 +++++ .../ChangePasswordServiceAdvisor.java | 3 +- .../DelegatingChangePasswordAdvisor.java | 38 ++++------ .../password/SimpleChangePasswordAdvice.java | 10 +-- .../password/UserDetailsPasswordManager.java | 6 +- .../InMemoryUserDetailsManager.java | 4 +- .../ChangeCompromisedPasswordAdvisor.java | 8 +- .../password/ChangePasswordAdviceHandler.java | 2 +- ...rdAdviceSessionAuthenticationStrategy.java | 44 +++++++++++ .../ChangePasswordAdvisingFilter.java | 16 +++- ...SessionChangePasswordAdviceRepository.java | 7 +- 15 files changed, 180 insertions(+), 95 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index c8d0b9ec13f..6350fc634af 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -18,18 +18,17 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.ChangePasswordServiceAdvisor; import org.springframework.security.authentication.password.DelegatingChangePasswordAdvisor; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; -import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; import org.springframework.security.web.authentication.password.ChangePasswordAdviceHandler; import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceSessionAuthenticationStrategy; import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; import org.springframework.security.web.authentication.password.SimpleChangePasswordAdviceHandler; @@ -82,8 +81,7 @@ public PasswordManagementConfigurer changePasswordAdviceRepository( return this; } - public PasswordManagementConfigurer changePasswordAdvisor( - ChangePasswordAdvisor changePasswordAdvisor) { + public PasswordManagementConfigurer changePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { this.changePasswordAdvisor = changePasswordAdvisor; return this; } @@ -115,24 +113,25 @@ public void init(B http) throws Exception { : this.context.getBeanProvider(ChangePasswordAdviceRepository.class) .getIfUnique(HttpSessionChangePasswordAdviceRepository::new); - ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) - ? this.changePasswordAdvisor + ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor : this.context.getBeanProvider(ChangePasswordAdvisor.class) - .getIfUnique(() -> DelegatingChangePasswordAdvisor.of( - new ChangePasswordServiceAdvisor(passwordManager), new ChangeCompromisedPasswordAdvisor())); + .getIfUnique(() -> DelegatingChangePasswordAdvisor + .of(new ChangePasswordServiceAdvisor(passwordManager), new ChangeCompromisedPasswordAdvisor())); http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); - FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); - String passwordParameter = (form != null) ? form.getPasswordParameter() : "password"; + String passwordParameter = "password"; + FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); + if (form != null) { + passwordParameter = form.getPasswordParameter(); + } + ChangePasswordAdviceSessionAuthenticationStrategy sessionAuthenticationStrategy = new ChangePasswordAdviceSessionAuthenticationStrategy( + passwordParameter); + sessionAuthenticationStrategy.setChangePasswordAdviceRepository(changePasswordAdviceRepository); + sessionAuthenticationStrategy.setChangePasswordAdvisor(changePasswordAdvisor); http.getConfigurer(SessionManagementConfigurer.class) - .addSessionAuthenticationStrategy((authentication, request, response) -> { - UserDetails user = (UserDetails) authentication.getPrincipal(); - String password = request.getParameter(passwordParameter); - ChangePasswordAdvice advice = changePasswordAdvisor.advise(user, password); - changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); - }); + .addSessionAuthenticationStrategy(sessionAuthenticationStrategy); } /** diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index bcb1c819893..779d28008b0 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -19,6 +19,8 @@ import java.net.URI; import java.util.UUID; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -28,7 +30,8 @@ import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockHttpSession; import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.ChangePasswordReasons; import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.Customizer; @@ -37,13 +40,16 @@ import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.core.annotation.AuthenticationPrincipal; -import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.PasswordEncodedUser; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.factory.PasswordEncoderFactories; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; +import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; import org.springframework.web.bind.annotation.GetMapping; @@ -125,9 +131,8 @@ void whenAdminSetsExpiredAdviceThenUserLoginRedirectsToResetPassword() throws Ex this.mvc.perform(get("/").with(user(admin))).andExpect(status().isOk()); // change the password to a test value String random = UUID.randomUUID().toString(); - this.mvc.perform(post("/change-password").with(csrf()).with(user(admin)).param("newPassword", random)) - .andExpect(status().isFound()) - .andExpect(redirectedUrl("/")); + this.mvc.perform(post("/change-password").with(csrf()).with(user(admin)).param("password", random)) + .andExpect(status().isOk()); // admin "expires" their own password this.mvc.perform(post("/admin/passwords/expire/admin").with(csrf()).with(user(admin))) .andExpect(status().isCreated()); @@ -144,9 +149,8 @@ void whenAdminSetsExpiredAdviceThenUserLoginRedirectsToResetPassword() throws Ex .andExpect(redirectedUrl("/change-password")); // reset the password to update random = UUID.randomUUID().toString(); - this.mvc.perform(post("/change-password").with(csrf()).session(session).param("newPassword", random)) - .andExpect(status().isFound()) - .andExpect(redirectedUrl("/")); + this.mvc.perform(post("/change-password").with(csrf()).session(session).param("password", random)) + .andExpect(status().isOk()); // now we're good this.mvc.perform(get("/").session(session)).andExpect(status().isOk()); } @@ -155,14 +159,14 @@ void whenAdminSetsExpiredAdviceThenUserLoginRedirectsToResetPassword() throws Ex void whenCompromisedThenUserLoginAllowed() throws Exception { this.spring.register(PasswordManagementConfig.class, AdminController.class, HomeController.class).autowire(); MvcResult result = this.mvc - .perform(post("/login").with(csrf()).param("username", "compromised").param("password", "password")) + .perform(post("/login").with(csrf()).param("username", "user").param("password", "password")) .andExpect(status().isFound()) .andExpect(redirectedUrl("/")) .andReturn(); MockHttpSession session = (MockHttpSession) result.getRequest().getSession(); this.mvc.perform(get("/").session(session)) .andExpect(status().isOk()) - .andExpect(content().string(containsString("COMPROMISED"))); + .andExpect(content().string(containsString("compromised"))); } @Configuration @@ -207,8 +211,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { // @formatter:off http .authorizeHttpRequests((authz) -> authz - .requestMatchers("/admin/**").hasRole("ADMIN") - .anyRequest().authenticated() + .requestMatchers("/admin/**").hasRole("ADMIN") + .anyRequest().authenticated() ) .formLogin(Customizer.withDefaults()) .passwordManagement(Customizer.withDefaults()); @@ -219,8 +223,9 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { @Bean UserDetailsService users() { String adminPassword = UUID.randomUUID().toString(); - UserDetails compromised = User.withUsername("compromised").password("{noop}password").roles("USER").build(); - UserDetails admin = User.withUsername("admin").password("{noop}" + adminPassword).roles("ADMIN").build(); + UserDetails compromised = PasswordEncodedUser.user(); + UserDetails admin = PasswordEncodedUser.withUserDetails(PasswordEncodedUser.admin()) + .password(adminPassword).build(); return new InMemoryUserDetailsManager(compromised, admin); } @@ -234,11 +239,9 @@ static class AdminController { private final UserDetailsPasswordManager passwords; - private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); - - AdminController(UserDetailsService users) { + AdminController(UserDetailsService users, UserDetailsPasswordManager passwords) { this.users = users; - this.passwords = (UserDetailsPasswordManager) users; + this.passwords = passwords; } @GetMapping("/advice/{username}") @@ -258,29 +261,48 @@ ResponseEntity expirePassword(@PathVariable("username") St return ResponseEntity.notFound().build(); } ChangePasswordAdvice advice = new SimpleChangePasswordAdvice(ChangePasswordAdvice.Action.MUST_CHANGE, - ChangePasswordReason.EXPIRED); + ChangePasswordReasons.EXPIRED); this.passwords.savePasswordAdvice(user, advice); URI uri = URI.create("/admin/passwords/advice/" + username); return ResponseEntity.created(uri).body(advice); } - @PostMapping("/change") - ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, - @RequestParam("password") String password) { - this.passwords.updatePassword(user, this.encoder.encode(password)); - return ResponseEntity.ok().build(); - } - } @RestController static class HomeController { + private final UserDetailsPasswordManager passwords; + + private final ChangePasswordAdvisor changePasswordAdvisor = + new ChangeCompromisedPasswordAdvisor(); + + private final ChangePasswordAdviceRepository changePasswordAdviceRepository = + new HttpSessionChangePasswordAdviceRepository(); + + private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + HomeController(UserDetailsPasswordManager passwords) { + this.passwords = passwords; + } + @GetMapping ChangePasswordAdvice index(ChangePasswordAdvice advice) { return advice; } + @PostMapping("/change-password") + ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, + @RequestParam("password") String password, HttpServletRequest request, HttpServletResponse response) { + ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); + if (advice.getAction() != ChangePasswordAdvice.Action.ABSTAIN) { + return ResponseEntity.badRequest().body(advice); + } + this.passwords.updatePassword(user, this.encoder.encode(password)); + this.passwords.removePasswordAdvice(user); + this.changePasswordAdviceRepository.removePasswordAdvice(request, response); + return ResponseEntity.ok().build(); + } } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java index caa19439bde..9ab2a683006 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -41,12 +41,12 @@ public ChangeLengthPasswordAdvisor(int minLength, int maxLength) { @Override public ChangePasswordAdvice advise(UserDetails user, String password) { if (password.length() < this.minLength) { - return new SimpleChangePasswordAdvice(this.tooShortAction, ChangePasswordReason.TOO_SHORT); + return new SimpleChangePasswordAdvice(this.tooShortAction, ChangePasswordReasons.TOO_SHORT); } if (password.length() > this.maxLength) { - return new SimpleChangePasswordAdvice(this.tooLongAction, ChangePasswordReason.TOO_LONG); + return new SimpleChangePasswordAdvice(this.tooLongAction, ChangePasswordReasons.TOO_LONG); } - return ChangePasswordAdvice.keep(); + return ChangePasswordAdvice.abstain(); } public void setTooShortAction(Action tooShortAction) { diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java index 71d4b3d6cfe..af9d8ba5d93 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java @@ -22,15 +22,15 @@ public interface ChangePasswordAdvice { Action getAction(); - Collection getReasons(); + Collection getReasons(); - static ChangePasswordAdvice keep() { - return SimpleChangePasswordAdvice.KEEP; + static ChangePasswordAdvice abstain() { + return SimpleChangePasswordAdvice.ABSTAIN; } enum Action { - KEEP, SHOULD_CHANGE, MUST_CHANGE + ABSTAIN, SHOULD_CHANGE, MUST_CHANGE } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java new file mode 100644 index 00000000000..fc5ad745e3c --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java @@ -0,0 +1,18 @@ +package org.springframework.security.authentication.password; + +public interface ChangePasswordReasons { + + String COMPROMISED = "compromised"; + + String EXPIRED = "expired"; + + String MISSING_CHARACTERS = "missing_characters"; + + String REPEATED = "repeated"; + + String TOO_LONG = "too_long"; + + String TOO_SHORT = "too_short"; + + String UNSUPPORTED_CHARACTERS = "unsupported_characters"; +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java index e89250491d1..cfa92e8f851 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java @@ -28,7 +28,8 @@ public ChangePasswordServiceAdvisor(UserDetailsPasswordManager passwordManager) @Override public ChangePasswordAdvice advise(UserDetails user, String password) { - return this.passwordManager.loadPasswordAdvice(user); + ChangePasswordAdvice advice = this.passwordManager.loadPasswordAdvice(user); + return (advice != null) ? advice : ChangePasswordAdvice.abstain(); } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java index 685d05b0f38..33ec0a62cdb 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java @@ -20,58 +20,46 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Objects; -import java.util.function.BiFunction; -import java.util.stream.Stream; import org.springframework.security.core.userdetails.UserDetails; -public final class DelegatingChangePasswordAdvisor - implements ChangePasswordAdvisor { +public final class DelegatingChangePasswordAdvisor implements ChangePasswordAdvisor { - private final List> advisors; + private final List advisors; - private DelegatingChangePasswordAdvisor(List> advisors) { + private DelegatingChangePasswordAdvisor(List advisors) { this.advisors = Collections.unmodifiableList(advisors); } public static ChangePasswordAdvisor of(ChangePasswordAdvisor... advisors) { - return new DelegatingChangePasswordAdvisor(Stream.of(advisors) - .map((advisor) -> (BiFunction) advisor::advise) - .toList()); + return new DelegatingChangePasswordAdvisor(List.of(advisors)); } @Override public ChangePasswordAdvice advise(UserDetails user, String password) { Collection advice = this.advisors.stream() - .map((advisor) -> advisor.apply(user, password)) - .filter(Objects::nonNull) + .map((advisor) -> advisor.advise(user, password)) + .filter((a) -> a.getAction() != ChangePasswordAdvice.Action.ABSTAIN) .toList(); return new CompositeChangePasswordAdvice(advice); } private static final class CompositeChangePasswordAdvice implements ChangePasswordAdvice { - private final Collection advice; - private final Action action; - private final Collection reasons; + private final Collection reasons; private CompositeChangePasswordAdvice(Collection advice) { - this.advice = advice; - Action action = Action.KEEP; - Collection reasons = new ArrayList<>(); + Action mostUrgentAction = Action.ABSTAIN; + Collection reasons = new ArrayList<>(); for (ChangePasswordAdvice a : advice) { - if (a.getAction() == Action.KEEP) { - continue; - } - if (action.ordinal() < a.getAction().ordinal()) { - action = a.getAction(); + if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { + mostUrgentAction = a.getAction(); } reasons.addAll(a.getReasons()); } - this.action = action; + this.action = mostUrgentAction; this.reasons = reasons; } @@ -81,7 +69,7 @@ public Action getAction() { } @Override - public Collection getReasons() { + public Collection getReasons() { return this.reasons; } diff --git a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java index 712c227d546..27fafb25b0e 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java @@ -21,18 +21,18 @@ public class SimpleChangePasswordAdvice implements ChangePasswordAdvice { - static final SimpleChangePasswordAdvice KEEP = new SimpleChangePasswordAdvice(Action.KEEP); + static final SimpleChangePasswordAdvice ABSTAIN = new SimpleChangePasswordAdvice(Action.ABSTAIN); private final Action action; - private final Collection reasons; + private final Collection reasons; - public SimpleChangePasswordAdvice(Action action, Collection reasons) { + public SimpleChangePasswordAdvice(Action action, Collection reasons) { this.action = action; this.reasons = reasons; } - public SimpleChangePasswordAdvice(Action action, ChangePasswordReason... reasons) { + public SimpleChangePasswordAdvice(Action action, String... reasons) { this.action = action; this.reasons = List.of(reasons); } @@ -43,7 +43,7 @@ public Action getAction() { } @Override - public Collection getReasons() { + public Collection getReasons() { return this.reasons; } diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java index 988736a86db..063db140e70 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java @@ -16,6 +16,8 @@ package org.springframework.security.authentication.password; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsPasswordService; @@ -29,9 +31,9 @@ public interface UserDetailsPasswordManager extends UserDetailsPasswordService { * @return the updated {@link UserDetails} */ @Override - UserDetails updatePassword(UserDetails user, String newPassword); + UserDetails updatePassword(UserDetails user, @Nullable String newPassword); - ChangePasswordAdvice loadPasswordAdvice(UserDetails user); + @Nullable ChangePasswordAdvice loadPasswordAdvice(UserDetails user); void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice); diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index b5bfde4b943..8aa22a5264b 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -183,14 +183,12 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx } @Override - public ChangePasswordAdvice loadPasswordAdvice(UserDetails user) { + public @Nullable ChangePasswordAdvice loadPasswordAdvice(UserDetails user) { return this.advice.get(user.getUsername()); } @Override public void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice) { - Assert.notNull(advice, - "advice must not be null; if you want to remove advice, please call removePasswordAdvice"); this.advice.put(user.getUsername(), advice); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java index 219da6e701b..691a91268fb 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -21,7 +21,7 @@ import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.authentication.password.ChangePasswordReason; +import org.springframework.security.authentication.password.ChangePasswordReasons; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; @@ -51,10 +51,10 @@ public static final class Advice implements ChangePasswordAdvice { public Advice(Action action, CompromisedPasswordDecision decision) { this.decision = decision; if (decision.isCompromised()) { - this.advice = new SimpleChangePasswordAdvice(action, ChangePasswordReason.COMPROMISED); + this.advice = new SimpleChangePasswordAdvice(action, ChangePasswordReasons.COMPROMISED); } else { - this.advice = ChangePasswordAdvice.keep(); + this.advice = ChangePasswordAdvice.abstain(); } } @@ -68,7 +68,7 @@ public Action getAction() { } @Override - public Collection getReasons() { + public Collection getReasons() { return this.advice.getReasons(); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java index 96a75d556cd..3dc916069cf 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java @@ -34,6 +34,6 @@ void handle(HttpServletRequest request, HttpServletResponse response, FilterChai // authentication request process // -------------- ------- ------- -// KEEP redirect to home continue filter redirect to home +// KEEP redirect to home | continue filter | redirect to home // RESET redirect to home continue filter redirect to home // REQUIRE_RESET redirect to home redirect to reset redirect to home diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java new file mode 100644 index 00000000000..8f38b5380ee --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java @@ -0,0 +1,44 @@ +package org.springframework.security.web.authentication.password; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.web.authentication.session.SessionAuthenticationException; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; +import org.springframework.util.Assert; + +public final class ChangePasswordAdviceSessionAuthenticationStrategy implements SessionAuthenticationStrategy { + + private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + + private ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); + + private final String passwordParameter; + + public ChangePasswordAdviceSessionAuthenticationStrategy(String passwordParameter) { + this.passwordParameter = passwordParameter; + } + + @Override + public void onAuthentication(Authentication authentication, HttpServletRequest request, + HttpServletResponse response) throws SessionAuthenticationException { + UserDetails user = (UserDetails) authentication.getPrincipal(); + Assert.notNull(user, "cannot persist password advice since user principal is null"); + String password = request.getParameter(this.passwordParameter); + ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); + this.changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); + } + + public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { + this.changePasswordAdviceRepository = changePasswordAdviceRepository; + } + + public void setChangePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { + this.changePasswordAdvisor = changePasswordAdvisor; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java index aec9b12765b..eca38a2b527 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java @@ -24,22 +24,36 @@ import jakarta.servlet.http.HttpServletResponse; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.filter.OncePerRequestFilter; +import static org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher.pathPattern; + public class ChangePasswordAdvisingFilter extends OncePerRequestFilter { + private RequestMatcher shouldHandleAdvice = new NegatedRequestMatcher(pathPattern("/change-password")); + private ChangePasswordAdviceHandler changePasswordAdviceHandler = new SimpleChangePasswordAdviceHandler( - DefaultChangePasswordPageGeneratingFilter.DEFAULT_CHANGE_PASSWORD_URL); + "/.well-known/change-password"); private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException { + if (!this.shouldHandleAdvice.matches(request)) { + chain.doFilter(request, response); + return; + } ChangePasswordAdvice advice = this.changePasswordAdviceRepository.loadPasswordAdvice(request); this.changePasswordAdviceHandler.handle(request, response, chain, advice); } + public void setShouldHandleAdviceRequestMatcher(RequestMatcher shouldHandleAdvice) { + this.shouldHandleAdvice = shouldHandleAdvice; + } + public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { this.changePasswordAdviceRepository = changePasswordAdviceRepository; } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java index ca73e174481..02f654aa574 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java @@ -24,7 +24,6 @@ import org.springframework.lang.NonNull; import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordReason; import org.springframework.util.function.SingletonSupplier; public final class HttpSessionChangePasswordAdviceRepository implements ChangePasswordAdviceRepository { @@ -41,14 +40,14 @@ public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { if (advice != null) { return advice; } - return ChangePasswordAdvice.keep(); + return ChangePasswordAdvice.abstain(); }); } @Override public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, ChangePasswordAdvice advice) { - if (advice.getAction() == ChangePasswordAdvice.Action.KEEP) { + if (advice.getAction() == ChangePasswordAdvice.Action.ABSTAIN) { removePasswordAdvice(request, response); return; } @@ -74,7 +73,7 @@ public Action getAction() { } @Override - public Collection getReasons() { + public Collection getReasons() { return this.advice.get().getReasons(); } From a04035d5224d2dff9f99c108a4c7524d5d48e994 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Wed, 30 Jul 2025 13:11:31 -0600 Subject: [PATCH 05/10] Removed Handler - This allow the request matcher to be in the filter, preventing eager access to the session --- .../PasswordManagementConfigurer.java | 11 +-- .../password/ChangePasswordAdviceHandler.java | 39 ---------- .../ChangePasswordAdvisingFilter.java | 39 +++++++--- .../SimpleChangePasswordAdviceHandler.java | 76 ------------------- 4 files changed, 32 insertions(+), 133 deletions(-) delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 6350fc634af..59411842b45 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -31,7 +31,8 @@ import org.springframework.security.web.authentication.password.ChangePasswordAdviceSessionAuthenticationStrategy; import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; -import org.springframework.security.web.authentication.password.SimpleChangePasswordAdviceHandler; +import org.springframework.security.web.authentication.password.RedirectingChangePasswordAdviceHandler; +import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.util.Assert; @@ -147,13 +148,9 @@ public void configure(B http) throws Exception { return; } - ChangePasswordAdviceHandler changePasswordAdviceHandler = (this.changePasswordAdviceHandler != null) - ? this.changePasswordAdviceHandler : this.context.getBeanProvider(ChangePasswordAdviceHandler.class) - .getIfUnique(() -> new SimpleChangePasswordAdviceHandler(this.changePasswordPage)); - - ChangePasswordAdvisingFilter advising = new ChangePasswordAdvisingFilter(); + ChangePasswordAdvisingFilter advising = new ChangePasswordAdvisingFilter(this.changePasswordPage); advising.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); - advising.setChangePasswordAdviceHandler(changePasswordAdviceHandler); + advising.setRequestCache(http.getSharedObject(RequestCache.class)); http.addFilterBefore(advising, RequestCacheAwareFilter.class); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java deleted file mode 100644 index 3dc916069cf..00000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceHandler.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication.password; - -import java.io.IOException; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.security.authentication.password.ChangePasswordAdvice; - -public interface ChangePasswordAdviceHandler { - - void handle(HttpServletRequest request, HttpServletResponse response, FilterChain chain, - ChangePasswordAdvice advice) throws ServletException, IOException; - -} - -// authentication request process -// -------------- ------- ------- -// KEEP redirect to home | continue filter | redirect to home -// RESET redirect to home continue filter redirect to home -// REQUIRE_RESET redirect to home redirect to reset redirect to home diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java index eca38a2b527..1bc12c2f1aa 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java @@ -24,6 +24,10 @@ import jakarta.servlet.http.HttpServletResponse; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.web.DefaultRedirectStrategy; +import org.springframework.security.web.RedirectStrategy; +import org.springframework.security.web.savedrequest.NullRequestCache; +import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.filter.OncePerRequestFilter; @@ -32,34 +36,47 @@ public class ChangePasswordAdvisingFilter extends OncePerRequestFilter { - private RequestMatcher shouldHandleAdvice = new NegatedRequestMatcher(pathPattern("/change-password")); + private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); - private ChangePasswordAdviceHandler changePasswordAdviceHandler = new SimpleChangePasswordAdviceHandler( - "/.well-known/change-password"); + private final String changePasswordUrl; + + private RequestCache requestCache = new NullRequestCache(); private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + private RequestMatcher requestMatcher; + + public ChangePasswordAdvisingFilter(String changePasswordUrl) { + this.changePasswordUrl = changePasswordUrl; + this.requestMatcher = new NegatedRequestMatcher(pathPattern(changePasswordUrl)); + } + @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException { - if (!this.shouldHandleAdvice.matches(request)) { + if (!this.requestMatcher.matches(request)) { chain.doFilter(request, response); return; } ChangePasswordAdvice advice = this.changePasswordAdviceRepository.loadPasswordAdvice(request); - this.changePasswordAdviceHandler.handle(request, response, chain, advice); - } - - public void setShouldHandleAdviceRequestMatcher(RequestMatcher shouldHandleAdvice) { - this.shouldHandleAdvice = shouldHandleAdvice; + if (advice.getAction() != ChangePasswordAdvice.Action.MUST_CHANGE) { + chain.doFilter(request, response); + return; + } + this.requestCache.saveRequest(request, response); + this.redirectStrategy.sendRedirect(request, response, this.changePasswordUrl); } public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { this.changePasswordAdviceRepository = changePasswordAdviceRepository; } - public void setChangePasswordAdviceHandler(ChangePasswordAdviceHandler changePasswordAdviceHandler) { - this.changePasswordAdviceHandler = changePasswordAdviceHandler; + public void setRequestCache(RequestCache requestCache) { + this.requestCache = requestCache; + } + + public void setRequestMatcher(RequestMatcher requestMatcher) { + this.requestMatcher = requestMatcher; } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java b/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java deleted file mode 100644 index a9f63da1284..00000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/password/SimpleChangePasswordAdviceHandler.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.web.authentication.password; - -import java.io.IOException; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.web.DefaultRedirectStrategy; -import org.springframework.security.web.RedirectStrategy; -import org.springframework.security.web.savedrequest.NullRequestCache; -import org.springframework.security.web.savedrequest.RequestCache; -import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.util.Assert; - -public final class SimpleChangePasswordAdviceHandler implements ChangePasswordAdviceHandler { - - private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); - - private final String changePasswordUrl; - - private RequestCache requestCache = new NullRequestCache(); - - private RequestMatcher requestMatcher = AnyRequestMatcher.INSTANCE; - - public SimpleChangePasswordAdviceHandler(String changePasswordUrl) { - Assert.hasText(changePasswordUrl, "changePasswordUrl cannot be empty"); - this.changePasswordUrl = changePasswordUrl; - } - - @Override - public void handle(HttpServletRequest request, HttpServletResponse response, FilterChain chain, - ChangePasswordAdvice advice) throws IOException, ServletException { - if (!this.requestMatcher.matches(request)) { - return; - } - if (request.getRequestURI().equals(this.changePasswordUrl)) { - chain.doFilter(request, response); - return; - } - if (advice.getAction() != ChangePasswordAdvice.Action.MUST_CHANGE) { - chain.doFilter(request, response); - return; - } - this.requestCache.saveRequest(request, response); - this.redirectStrategy.sendRedirect(request, response, this.changePasswordUrl); - } - - public void setRequestCache(RequestCache requestCache) { - this.requestCache = requestCache; - } - - public void setRequestMatcher(RequestMatcher requestMatcher) { - this.requestMatcher = requestMatcher; - } - -} From cbc61bf9024e25d40f485f5b0bcf9d0fb71b6f90 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Wed, 30 Jul 2025 19:37:42 -0600 Subject: [PATCH 06/10] Remove Reasons - These aren't super helpful in the end. Applications can inspect the type and get a richer set of information. --- .../PasswordManagementConfigurer.java | 18 +---- .../PasswordManagementConfigurerTests.java | 9 +-- .../password/ChangeLengthPasswordAdvisor.java | 78 ++++++++++++++++++- .../password/ChangePasswordAdvice.java | 10 +-- .../password/ChangePasswordReasons.java | 18 ----- .../ChangePasswordServiceAdvisor.java | 35 --------- ...va => CompositeChangePasswordAdvisor.java} | 38 +++++---- .../password/SimpleChangePasswordAdvice.java | 50 ------------ ... => UserDetailsChangePasswordAdvisor.java} | 9 ++- .../password/UserDetailsPasswordManager.java | 16 ---- .../security/core/userdetails/User.java | 20 +++++ .../core/userdetails/UserDetails.java | 5 ++ .../InMemoryUserDetailsManager.java | 22 ++---- .../security/provisioning/MutableUser.java | 12 +++ .../provisioning/MutableUserDetails.java | 2 + .../InMemoryUserDetailsManagerTests.java | 6 ++ .../ChangeCompromisedPasswordAdvisor.java | 34 ++++---- ...SessionChangePasswordAdviceRepository.java | 14 ++-- 18 files changed, 188 insertions(+), 208 deletions(-) delete mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java delete mode 100644 core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java rename core/src/main/java/org/springframework/security/authentication/password/{DelegatingChangePasswordAdvisor.java => CompositeChangePasswordAdvisor.java} (65%) delete mode 100644 core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java rename core/src/main/java/org/springframework/security/authentication/password/{ChangePasswordReason.java => UserDetailsChangePasswordAdvisor.java} (70%) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 59411842b45..0b309c2850f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -19,19 +19,17 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.authentication.password.ChangePasswordServiceAdvisor; -import org.springframework.security.authentication.password.DelegatingChangePasswordAdvisor; +import org.springframework.security.authentication.password.CompositeChangePasswordAdvisor; +import org.springframework.security.authentication.password.UserDetailsChangePasswordAdvisor; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceHandler; import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; import org.springframework.security.web.authentication.password.ChangePasswordAdviceSessionAuthenticationStrategy; import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; -import org.springframework.security.web.authentication.password.RedirectingChangePasswordAdviceHandler; import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.util.Assert; @@ -59,8 +57,6 @@ public final class PasswordManagementConfigurer private ChangePasswordAdvisor changePasswordAdvisor; - private ChangePasswordAdviceHandler changePasswordAdviceHandler; - private UserDetailsPasswordManager userDetailsPasswordManager; /** @@ -87,12 +83,6 @@ public PasswordManagementConfigurer changePasswordAdvisor(ChangePasswordAdvis return this; } - public PasswordManagementConfigurer changePasswordAdviceHandler( - ChangePasswordAdviceHandler changePasswordAdviceHandler) { - this.changePasswordAdviceHandler = changePasswordAdviceHandler; - return this; - } - public PasswordManagementConfigurer userDetailsPasswordManager( UserDetailsPasswordManager userDetailsPasswordManager) { this.userDetailsPasswordManager = userDetailsPasswordManager; @@ -116,8 +106,8 @@ public void init(B http) throws Exception { ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor : this.context.getBeanProvider(ChangePasswordAdvisor.class) - .getIfUnique(() -> DelegatingChangePasswordAdvisor - .of(new ChangePasswordServiceAdvisor(passwordManager), new ChangeCompromisedPasswordAdvisor())); + .getIfUnique(() -> CompositeChangePasswordAdvisor + .of(new UserDetailsChangePasswordAdvisor(), new ChangeCompromisedPasswordAdvisor())); http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index 779d28008b0..58df715d189 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -31,8 +31,6 @@ import org.springframework.mock.web.MockHttpSession; import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.authentication.password.ChangePasswordReasons; -import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -250,8 +248,7 @@ ResponseEntity requireChangePassword(@PathVariable("userna if (user == null) { return ResponseEntity.notFound().build(); } - ChangePasswordAdvice advice = this.passwords.loadPasswordAdvice(user); - return ResponseEntity.ok(advice); + return ResponseEntity.ok(user.getChangePasswordAdvice()); } @PostMapping("/expire/{username}") @@ -260,8 +257,7 @@ ResponseEntity expirePassword(@PathVariable("username") St if (user == null) { return ResponseEntity.notFound().build(); } - ChangePasswordAdvice advice = new SimpleChangePasswordAdvice(ChangePasswordAdvice.Action.MUST_CHANGE, - ChangePasswordReasons.EXPIRED); + ChangePasswordAdvice advice = () -> ChangePasswordAdvice.Action.MUST_CHANGE; this.passwords.savePasswordAdvice(user, advice); URI uri = URI.create("/admin/passwords/advice/" + username); return ResponseEntity.created(uri).body(advice); @@ -299,7 +295,6 @@ ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, return ResponseEntity.badRequest().body(advice); } this.passwords.updatePassword(user, this.encoder.encode(password)); - this.passwords.removePasswordAdvice(user); this.changePasswordAdviceRepository.removePasswordAdvice(request, response); return ResponseEntity.ok().build(); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java index 9ab2a683006..edf7daede91 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -41,12 +41,12 @@ public ChangeLengthPasswordAdvisor(int minLength, int maxLength) { @Override public ChangePasswordAdvice advise(UserDetails user, String password) { if (password.length() < this.minLength) { - return new SimpleChangePasswordAdvice(this.tooShortAction, ChangePasswordReasons.TOO_SHORT); + return new TooShortAdvice(this.tooShortAction, this.minLength, password.length()); } if (password.length() > this.maxLength) { - return new SimpleChangePasswordAdvice(this.tooLongAction, ChangePasswordReasons.TOO_LONG); + return new TooLongAdvice(this.tooLongAction, this.maxLength, password.length()); } - return ChangePasswordAdvice.abstain(); + return ChangePasswordAdvice.ABSTAIN; } public void setTooShortAction(Action tooShortAction) { @@ -57,4 +57,76 @@ public void setTooLongAction(Action tooLongAction) { this.tooLongAction = tooLongAction; } + public static final class TooShortAdvice implements ChangePasswordAdvice { + private final Action action; + + private final int minLength; + + private final int actualLength; + + private TooShortAdvice(Action action, int minLength, int actualLength) { + this.action = action; + this.minLength = minLength; + this.actualLength = actualLength; + } + + @Override + public Action getAction() { + return this.action; + } + + public int getMinLength() { + return this.minLength; + } + + public int getActualLength() { + return this.actualLength; + } + + @Override + public String toString() { + return "TooShort [" + + "action=" + this.action + + ", minLength=" + this.minLength + + ", actualLength=" + this.actualLength + + "]"; + } + + } + + public static final class TooLongAdvice implements ChangePasswordAdvice { + private final Action action; + + private final int maxLength; + + private final int actualLength; + + private TooLongAdvice(Action action, int maxLength, int actualLength) { + this.action = action; + this.maxLength = maxLength; + this.actualLength = actualLength; + } + + @Override + public Action getAction() { + return this.action; + } + + public int getMaxLength() { + return this.maxLength; + } + + public int getActualLength() { + return this.actualLength; + } + + @Override + public String toString() { + return "TooLong [" + + "action=" + this.action + + ", maxLength=" + this.maxLength + + ", actualLength=" + this.actualLength + + "]"; + } + } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java index af9d8ba5d93..5ef8a2fb52f 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java @@ -16,17 +16,11 @@ package org.springframework.security.authentication.password; -import java.util.Collection; - public interface ChangePasswordAdvice { - Action getAction(); - - Collection getReasons(); + ChangePasswordAdvice ABSTAIN = () -> Action.ABSTAIN; - static ChangePasswordAdvice abstain() { - return SimpleChangePasswordAdvice.ABSTAIN; - } + Action getAction(); enum Action { diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java deleted file mode 100644 index fc5ad745e3c..00000000000 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReasons.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.springframework.security.authentication.password; - -public interface ChangePasswordReasons { - - String COMPROMISED = "compromised"; - - String EXPIRED = "expired"; - - String MISSING_CHARACTERS = "missing_characters"; - - String REPEATED = "repeated"; - - String TOO_LONG = "too_long"; - - String TOO_SHORT = "too_short"; - - String UNSUPPORTED_CHARACTERS = "unsupported_characters"; -} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java deleted file mode 100644 index cfa92e8f851..00000000000 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordServiceAdvisor.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authentication.password; - -import org.springframework.security.core.userdetails.UserDetails; - -public final class ChangePasswordServiceAdvisor implements ChangePasswordAdvisor { - - private final UserDetailsPasswordManager passwordManager; - - public ChangePasswordServiceAdvisor(UserDetailsPasswordManager passwordManager) { - this.passwordManager = passwordManager; - } - - @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { - ChangePasswordAdvice advice = this.passwordManager.loadPasswordAdvice(user); - return (advice != null) ? advice : ChangePasswordAdvice.abstain(); - } - -} diff --git a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java similarity index 65% rename from core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java index 33ec0a62cdb..f63b0138055 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/DelegatingChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java @@ -16,51 +16,51 @@ package org.springframework.security.authentication.password; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import org.springframework.security.core.userdetails.UserDetails; -public final class DelegatingChangePasswordAdvisor implements ChangePasswordAdvisor { +public final class CompositeChangePasswordAdvisor implements ChangePasswordAdvisor { private final List advisors; - private DelegatingChangePasswordAdvisor(List advisors) { + private CompositeChangePasswordAdvisor(List advisors) { this.advisors = Collections.unmodifiableList(advisors); } public static ChangePasswordAdvisor of(ChangePasswordAdvisor... advisors) { - return new DelegatingChangePasswordAdvisor(List.of(advisors)); + return new CompositeChangePasswordAdvisor(List.of(advisors)); } @Override public ChangePasswordAdvice advise(UserDetails user, String password) { Collection advice = this.advisors.stream() .map((advisor) -> advisor.advise(user, password)) - .filter((a) -> a.getAction() != ChangePasswordAdvice.Action.ABSTAIN) .toList(); - return new CompositeChangePasswordAdvice(advice); + return new Advice(advice); } - private static final class CompositeChangePasswordAdvice implements ChangePasswordAdvice { + public static final class Advice implements ChangePasswordAdvice { private final Action action; - private final Collection reasons; + private final Collection advice; - private CompositeChangePasswordAdvice(Collection advice) { + private Advice(Collection advice) { + this.action = findMostUrgentAction(advice); + this.advice = advice; + } + + private Action findMostUrgentAction(Collection advice) { Action mostUrgentAction = Action.ABSTAIN; - Collection reasons = new ArrayList<>(); for (ChangePasswordAdvice a : advice) { if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { mostUrgentAction = a.getAction(); } - reasons.addAll(a.getReasons()); } - this.action = mostUrgentAction; - this.reasons = reasons; + return mostUrgentAction; } @Override @@ -68,11 +68,17 @@ public Action getAction() { return this.action; } - @Override - public Collection getReasons() { - return this.reasons; + public Collection getAdvice() { + return this.advice; } + @Override + public String toString() { + return "Composite [" + + "action=" + this.action + + ", advice=" + this.advice + + "]"; + } } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java deleted file mode 100644 index 27fafb25b0e..00000000000 --- a/core/src/main/java/org/springframework/security/authentication/password/SimpleChangePasswordAdvice.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2002-2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.authentication.password; - -import java.util.Collection; -import java.util.List; - -public class SimpleChangePasswordAdvice implements ChangePasswordAdvice { - - static final SimpleChangePasswordAdvice ABSTAIN = new SimpleChangePasswordAdvice(Action.ABSTAIN); - - private final Action action; - - private final Collection reasons; - - public SimpleChangePasswordAdvice(Action action, Collection reasons) { - this.action = action; - this.reasons = reasons; - } - - public SimpleChangePasswordAdvice(Action action, String... reasons) { - this.action = action; - this.reasons = List.of(reasons); - } - - @Override - public Action getAction() { - return this.action; - } - - @Override - public Collection getReasons() { - return this.reasons; - } - -} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java similarity index 70% rename from core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java rename to core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java index fd95c11e62b..f0ce397dc5e 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordReason.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java @@ -16,8 +16,13 @@ package org.springframework.security.authentication.password; -public enum ChangePasswordReason { +import org.springframework.security.core.userdetails.UserDetails; - COMPROMISED, EXPIRED, MISSING_CHARACTERS, REPEATED, TOO_SHORT, TOO_LONG, UNSUPPORTED_CHARACTERS +public final class UserDetailsChangePasswordAdvisor implements ChangePasswordAdvisor { + + @Override + public ChangePasswordAdvice advise(UserDetails user, String password) { + return user.getChangePasswordAdvice(); + } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java index 063db140e70..9a55a0c99bf 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java @@ -16,27 +16,11 @@ package org.springframework.security.authentication.password; -import org.jspecify.annotations.Nullable; - import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsPasswordService; public interface UserDetailsPasswordManager extends UserDetailsPasswordService { - /** - * Update the password and remove any related password advice - * @param user the user to modify the password for - * @param newPassword the password to change to, encoded by the configured - * {@code PasswordEncoder} - * @return the updated {@link UserDetails} - */ - @Override - UserDetails updatePassword(UserDetails user, @Nullable String newPassword); - - @Nullable ChangePasswordAdvice loadPasswordAdvice(UserDetails user); - void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice); - void removePasswordAdvice(UserDetails user); - } diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index 3b9a871ba0f..e9e97a26a4a 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -32,6 +32,7 @@ import org.apache.commons.logging.LogFactory; import org.jspecify.annotations.Nullable; +import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; @@ -81,6 +82,8 @@ public class User implements UserDetails, CredentialsContainer { private final boolean enabled; + private final ChangePasswordAdvice changePasswordAdvice; + /** * Calls the more complex constructor with all boolean arguments set to {@code true}. */ @@ -115,9 +118,21 @@ public User(String username, @Nullable String password, boolean enabled, boolean this.accountNonExpired = accountNonExpired; this.credentialsNonExpired = credentialsNonExpired; this.accountNonLocked = accountNonLocked; + this.changePasswordAdvice = ChangePasswordAdvice.ABSTAIN; this.authorities = Collections.unmodifiableSet(sortAuthorities(authorities)); } + public User(UserDetails user) { + this.username = user.getUsername(); + this.password = user.getPassword(); + this.enabled = user.isEnabled(); + this.accountNonExpired = user.isAccountNonExpired(); + this.credentialsNonExpired = user.isCredentialsNonExpired(); + this.accountNonLocked = user.isAccountNonLocked(); + this.changePasswordAdvice = user.getChangePasswordAdvice(); + this.authorities = Collections.unmodifiableSet(sortAuthorities(user.getAuthorities())); + } + @Override public Collection getAuthorities() { return this.authorities; @@ -153,6 +168,11 @@ public boolean isCredentialsNonExpired() { return this.credentialsNonExpired; } + @Override + public ChangePasswordAdvice getChangePasswordAdvice() { + return this.changePasswordAdvice; + } + @Override public void eraseCredentials() { this.password = null; diff --git a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java index a80523f1570..de7be38eddc 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java @@ -21,6 +21,7 @@ import org.jspecify.annotations.Nullable; +import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; @@ -102,4 +103,8 @@ default boolean isEnabled() { return true; } + default ChangePasswordAdvice getChangePasswordAdvice() { + return ChangePasswordAdvice.ABSTAIN; + } + } diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index 8aa22a5264b..eb235723e33 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -165,7 +165,7 @@ public UserDetails updatePassword(UserDetails user, @Nullable String newPassword throw new RuntimeException("user '" + username + "' does not exist"); } mutableUser.setPassword(newPassword); - removePasswordAdvice(mutableUser); + savePasswordAdvice(mutableUser, ChangePasswordAdvice.ABSTAIN); return mutableUser; } @@ -178,23 +178,17 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx if (user instanceof CredentialsContainer) { return user; } - return new User(user.getUsername(), user.getPassword(), user.isEnabled(), user.isAccountNonExpired(), - user.isCredentialsNonExpired(), user.isAccountNonLocked(), user.getAuthorities()); - } - - @Override - public @Nullable ChangePasswordAdvice loadPasswordAdvice(UserDetails user) { - return this.advice.get(user.getUsername()); + return new User(user); } @Override public void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice) { - this.advice.put(user.getUsername(), advice); - } - - @Override - public void removePasswordAdvice(UserDetails user) { - this.advice.remove(user.getUsername()); + String username = user.getUsername(); + MutableUserDetails mutableUser = this.users.get(username.toLowerCase(Locale.ROOT)); + if (mutableUser == null) { + throw new RuntimeException("user '" + username + "' does not exist"); + } + mutableUser.setChangePasswordAdvice(advice); } /** diff --git a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java index b8a41836f38..1a3ce02425d 100644 --- a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java +++ b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java @@ -20,6 +20,7 @@ import org.jspecify.annotations.Nullable; +import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; import org.springframework.security.core.userdetails.UserDetails; @@ -34,6 +35,8 @@ class MutableUser implements MutableUserDetails { private @Nullable String password; + private ChangePasswordAdvice advice = ChangePasswordAdvice.ABSTAIN; + private final UserDetails delegate; MutableUser(UserDetails user) { @@ -51,6 +54,15 @@ public void setPassword(@Nullable String password) { this.password = password; } + @Override + public ChangePasswordAdvice getChangePasswordAdvice() { + return advice; + } + + public void setChangePasswordAdvice(ChangePasswordAdvice advice) { + this.advice = advice; + } + @Override public Collection getAuthorities() { return this.delegate.getAuthorities(); diff --git a/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java b/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java index cc3854f5ba2..124e814eb78 100644 --- a/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java +++ b/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java @@ -18,6 +18,7 @@ import org.jspecify.annotations.Nullable; +import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.core.userdetails.UserDetails; /** @@ -28,4 +29,5 @@ interface MutableUserDetails extends UserDetails { void setPassword(@Nullable String password); + void setChangePasswordAdvice(ChangePasswordAdvice advice); } diff --git a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java index 7d311f4c18f..d418ea69775 100644 --- a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java @@ -30,6 +30,7 @@ import org.springframework.security.authentication.TestAuthentication; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; +import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.core.Authentication; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.GrantedAuthority; @@ -227,6 +228,11 @@ public void setPassword(final String password) { this.password = password; } + @Override + public void setChangePasswordAdvice(ChangePasswordAdvice advice) { + throw new UnsupportedOperationException(); + } + @Override public String getUsername() { return this.delegate.getUsername(); diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java index 691a91268fb..78f400d77ca 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -16,15 +16,11 @@ package org.springframework.security.web.authentication.password; -import java.util.Collection; - import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.authentication.password.ChangePasswordReasons; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; -import org.springframework.security.authentication.password.SimpleChangePasswordAdvice; import org.springframework.security.core.userdetails.UserDetails; public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { @@ -35,7 +31,12 @@ public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdv @Override public ChangePasswordAdvice advise(UserDetails user, String password) { - return new Advice(this.action, this.pwned.check(password)); + CompromisedPasswordDecision decision = this.pwned.check(password); + if (decision.isCompromised()) { + return new Advice(this.action, decision); + } else { + return new Advice(Action.ABSTAIN, decision); + } } public void setAction(Action action) { @@ -44,34 +45,31 @@ public void setAction(Action action) { public static final class Advice implements ChangePasswordAdvice { - private final CompromisedPasswordDecision decision; + private final Action action; - private final ChangePasswordAdvice advice; + private final CompromisedPasswordDecision decision; public Advice(Action action, CompromisedPasswordDecision decision) { + this.action = action; this.decision = decision; - if (decision.isCompromised()) { - this.advice = new SimpleChangePasswordAdvice(action, ChangePasswordReasons.COMPROMISED); - } - else { - this.advice = ChangePasswordAdvice.abstain(); - } } - public CompromisedPasswordDecision getDecision() { + public CompromisedPasswordDecision getCompromisedPasswordDecision() { return this.decision; } @Override public Action getAction() { - return this.advice.getAction(); + return this.action; } @Override - public Collection getReasons() { - return this.advice.getReasons(); + public String toString() { + return "Compromised [" + + "action=" + this.action + + ", decision=" + this.decision + + "]"; } - } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java index 02f654aa574..2c9f1c1a2f1 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java @@ -16,13 +16,11 @@ package org.springframework.security.web.authentication.password; -import java.util.Collection; import java.util.function.Supplier; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.lang.NonNull; import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.util.function.SingletonSupplier; @@ -32,7 +30,6 @@ public final class HttpSessionChangePasswordAdviceRepository implements ChangePa .getName() + ".PASSWORD_ADVICE"; @Override - @NonNull public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { return new DeferredChangePasswordAdvice(() -> { ChangePasswordAdvice advice = (ChangePasswordAdvice) request.getSession() @@ -40,7 +37,7 @@ public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { if (advice != null) { return advice; } - return ChangePasswordAdvice.abstain(); + return ChangePasswordAdvice.ABSTAIN; }); } @@ -72,11 +69,14 @@ public Action getAction() { return this.advice.get().getAction(); } - @Override - public Collection getReasons() { - return this.advice.get().getReasons(); + public ChangePasswordAdvice getChangePasswordAdvice() { + return this.advice.get(); } + @Override + public String toString() { + return this.advice.get().toString(); + } } } From efdd412b14b3aa62d8e62897323873e8d23bc285 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Wed, 30 Jul 2025 20:01:09 -0600 Subject: [PATCH 07/10] Store Only Password Action In all likelihood, the only thing needed at the database level is the action that an admin requires be taken for a password. --- .../PasswordManagementConfigurer.java | 4 +- .../PasswordManagementConfigurerTests.java | 24 ++++++------ .../password/ChangeLengthPasswordAdvisor.java | 39 +++++++++---------- .../password/ChangePasswordAdvice.java | 10 +---- .../CompositeChangePasswordAdvisor.java | 14 +++---- .../password/PasswordAction.java | 7 ++++ .../UserDetailsChangePasswordAdvisor.java | 2 +- .../password/UserDetailsPasswordManager.java | 2 +- .../security/core/userdetails/User.java | 12 +++--- .../core/userdetails/UserDetails.java | 6 +-- .../InMemoryUserDetailsManager.java | 7 ++-- .../security/provisioning/MutableUser.java | 10 ++--- .../provisioning/MutableUserDetails.java | 5 ++- .../InMemoryUserDetailsManagerTests.java | 4 +- .../ChangeCompromisedPasswordAdvisor.java | 23 ++++++----- .../ChangePasswordAdvisingFilter.java | 3 +- ...SessionChangePasswordAdviceRepository.java | 6 ++- 17 files changed, 89 insertions(+), 89 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 0b309c2850f..8802fd505c7 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -106,8 +106,8 @@ public void init(B http) throws Exception { ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor : this.context.getBeanProvider(ChangePasswordAdvisor.class) - .getIfUnique(() -> CompositeChangePasswordAdvisor - .of(new UserDetailsChangePasswordAdvisor(), new ChangeCompromisedPasswordAdvisor())); + .getIfUnique(() -> CompositeChangePasswordAdvisor.of(new UserDetailsChangePasswordAdvisor(), + new ChangeCompromisedPasswordAdvisor())); http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index 58df715d189..79188ba83f7 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -31,6 +31,7 @@ import org.springframework.mock.web.MockHttpSession; import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -223,7 +224,8 @@ UserDetailsService users() { String adminPassword = UUID.randomUUID().toString(); UserDetails compromised = PasswordEncodedUser.user(); UserDetails admin = PasswordEncodedUser.withUserDetails(PasswordEncodedUser.admin()) - .password(adminPassword).build(); + .password(adminPassword) + .build(); return new InMemoryUserDetailsManager(compromised, admin); } @@ -243,24 +245,23 @@ static class AdminController { } @GetMapping("/advice/{username}") - ResponseEntity requireChangePassword(@PathVariable("username") String username) { + ResponseEntity requireChangePassword(@PathVariable("username") String username) { UserDetails user = this.users.loadUserByUsername(username); if (user == null) { return ResponseEntity.notFound().build(); } - return ResponseEntity.ok(user.getChangePasswordAdvice()); + return ResponseEntity.ok(user.getPasswordAction()); } @PostMapping("/expire/{username}") - ResponseEntity expirePassword(@PathVariable("username") String username) { + ResponseEntity expirePassword(@PathVariable("username") String username) { UserDetails user = this.users.loadUserByUsername(username); if (user == null) { return ResponseEntity.notFound().build(); } - ChangePasswordAdvice advice = () -> ChangePasswordAdvice.Action.MUST_CHANGE; - this.passwords.savePasswordAdvice(user, advice); + this.passwords.savePasswordAction(user, PasswordAction.MUST_CHANGE); URI uri = URI.create("/admin/passwords/advice/" + username); - return ResponseEntity.created(uri).body(advice); + return ResponseEntity.created(uri).body(PasswordAction.MUST_CHANGE); } } @@ -270,11 +271,9 @@ static class HomeController { private final UserDetailsPasswordManager passwords; - private final ChangePasswordAdvisor changePasswordAdvisor = - new ChangeCompromisedPasswordAdvisor(); + private final ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); - private final ChangePasswordAdviceRepository changePasswordAdviceRepository = - new HttpSessionChangePasswordAdviceRepository(); + private final ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); @@ -291,13 +290,14 @@ ChangePasswordAdvice index(ChangePasswordAdvice advice) { ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, @RequestParam("password") String password, HttpServletRequest request, HttpServletResponse response) { ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); - if (advice.getAction() != ChangePasswordAdvice.Action.ABSTAIN) { + if (advice.getAction() != PasswordAction.ABSTAIN) { return ResponseEntity.badRequest().body(advice); } this.passwords.updatePassword(user, this.encoder.encode(password)); this.changePasswordAdviceRepository.removePasswordAdvice(request, response); return ResponseEntity.ok().build(); } + } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java index edf7daede91..19f6de225c7 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java @@ -16,7 +16,6 @@ package org.springframework.security.authentication.password; -import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.core.userdetails.UserDetails; public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { @@ -25,9 +24,9 @@ public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { private final int maxLength; - private Action tooShortAction = Action.MUST_CHANGE; + private PasswordAction tooShortAction = PasswordAction.MUST_CHANGE; - private Action tooLongAction = Action.SHOULD_CHANGE; + private PasswordAction tooLongAction = PasswordAction.SHOULD_CHANGE; public ChangeLengthPasswordAdvisor(int minLength) { this(minLength, Integer.MAX_VALUE); @@ -49,29 +48,30 @@ public ChangePasswordAdvice advise(UserDetails user, String password) { return ChangePasswordAdvice.ABSTAIN; } - public void setTooShortAction(Action tooShortAction) { + public void setTooShortAction(PasswordAction tooShortAction) { this.tooShortAction = tooShortAction; } - public void setTooLongAction(Action tooLongAction) { + public void setTooLongAction(PasswordAction tooLongAction) { this.tooLongAction = tooLongAction; } public static final class TooShortAdvice implements ChangePasswordAdvice { - private final Action action; + + private final PasswordAction action; private final int minLength; private final int actualLength; - private TooShortAdvice(Action action, int minLength, int actualLength) { + private TooShortAdvice(PasswordAction action, int minLength, int actualLength) { this.action = action; this.minLength = minLength; this.actualLength = actualLength; } @Override - public Action getAction() { + public PasswordAction getAction() { return this.action; } @@ -85,30 +85,28 @@ public int getActualLength() { @Override public String toString() { - return "TooShort [" + - "action=" + this.action + - ", minLength=" + this.minLength + - ", actualLength=" + this.actualLength + - "]"; + return "TooShort [" + "action=" + this.action + ", minLength=" + this.minLength + ", actualLength=" + + this.actualLength + "]"; } } public static final class TooLongAdvice implements ChangePasswordAdvice { - private final Action action; + + private final PasswordAction action; private final int maxLength; private final int actualLength; - private TooLongAdvice(Action action, int maxLength, int actualLength) { + private TooLongAdvice(PasswordAction action, int maxLength, int actualLength) { this.action = action; this.maxLength = maxLength; this.actualLength = actualLength; } @Override - public Action getAction() { + public PasswordAction getAction() { return this.action; } @@ -122,11 +120,10 @@ public int getActualLength() { @Override public String toString() { - return "TooLong [" + - "action=" + this.action + - ", maxLength=" + this.maxLength + - ", actualLength=" + this.actualLength + - "]"; + return "TooLong [" + "action=" + this.action + ", maxLength=" + this.maxLength + ", actualLength=" + + this.actualLength + "]"; } + } + } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java index 5ef8a2fb52f..47183b4997d 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java @@ -18,14 +18,8 @@ public interface ChangePasswordAdvice { - ChangePasswordAdvice ABSTAIN = () -> Action.ABSTAIN; + ChangePasswordAdvice ABSTAIN = () -> PasswordAction.ABSTAIN; - Action getAction(); - - enum Action { - - ABSTAIN, SHOULD_CHANGE, MUST_CHANGE - - } + PasswordAction getAction(); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java index f63b0138055..aa4014fb96d 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java @@ -44,7 +44,7 @@ public ChangePasswordAdvice advise(UserDetails user, String password) { public static final class Advice implements ChangePasswordAdvice { - private final Action action; + private final PasswordAction action; private final Collection advice; @@ -53,8 +53,8 @@ private Advice(Collection advice) { this.advice = advice; } - private Action findMostUrgentAction(Collection advice) { - Action mostUrgentAction = Action.ABSTAIN; + private PasswordAction findMostUrgentAction(Collection advice) { + PasswordAction mostUrgentAction = PasswordAction.ABSTAIN; for (ChangePasswordAdvice a : advice) { if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { mostUrgentAction = a.getAction(); @@ -64,7 +64,7 @@ private Action findMostUrgentAction(Collection advice) { } @Override - public Action getAction() { + public PasswordAction getAction() { return this.action; } @@ -74,11 +74,9 @@ public Collection getAdvice() { @Override public String toString() { - return "Composite [" + - "action=" + this.action + - ", advice=" + this.advice + - "]"; + return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]"; } + } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java new file mode 100644 index 00000000000..9d4ca4cd543 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java @@ -0,0 +1,7 @@ +package org.springframework.security.authentication.password; + +public enum PasswordAction { + + ABSTAIN, SHOULD_CHANGE, MUST_CHANGE + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java index f0ce397dc5e..8cd04790d5b 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java @@ -22,7 +22,7 @@ public final class UserDetailsChangePasswordAdvisor implements ChangePasswordAdv @Override public ChangePasswordAdvice advise(UserDetails user, String password) { - return user.getChangePasswordAdvice(); + return user::getPasswordAction; } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java index 9a55a0c99bf..acb1700eb94 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java @@ -21,6 +21,6 @@ public interface UserDetailsPasswordManager extends UserDetailsPasswordService { - void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice); + void savePasswordAction(UserDetails user, PasswordAction action); } diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index e9e97a26a4a..3aadd2eb8a3 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -32,7 +32,7 @@ import org.apache.commons.logging.LogFactory; import org.jspecify.annotations.Nullable; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; @@ -82,7 +82,7 @@ public class User implements UserDetails, CredentialsContainer { private final boolean enabled; - private final ChangePasswordAdvice changePasswordAdvice; + private final PasswordAction passwordAction; /** * Calls the more complex constructor with all boolean arguments set to {@code true}. @@ -118,7 +118,7 @@ public User(String username, @Nullable String password, boolean enabled, boolean this.accountNonExpired = accountNonExpired; this.credentialsNonExpired = credentialsNonExpired; this.accountNonLocked = accountNonLocked; - this.changePasswordAdvice = ChangePasswordAdvice.ABSTAIN; + this.passwordAction = PasswordAction.ABSTAIN; this.authorities = Collections.unmodifiableSet(sortAuthorities(authorities)); } @@ -129,7 +129,7 @@ public User(UserDetails user) { this.accountNonExpired = user.isAccountNonExpired(); this.credentialsNonExpired = user.isCredentialsNonExpired(); this.accountNonLocked = user.isAccountNonLocked(); - this.changePasswordAdvice = user.getChangePasswordAdvice(); + this.passwordAction = user.getPasswordAction(); this.authorities = Collections.unmodifiableSet(sortAuthorities(user.getAuthorities())); } @@ -169,8 +169,8 @@ public boolean isCredentialsNonExpired() { } @Override - public ChangePasswordAdvice getChangePasswordAdvice() { - return this.changePasswordAdvice; + public PasswordAction getPasswordAction() { + return this.passwordAction; } @Override diff --git a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java index de7be38eddc..dec5a6cf91e 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java @@ -21,7 +21,7 @@ import org.jspecify.annotations.Nullable; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; @@ -103,8 +103,8 @@ default boolean isEnabled() { return true; } - default ChangePasswordAdvice getChangePasswordAdvice() { - return ChangePasswordAdvice.ABSTAIN; + default PasswordAction getPasswordAction() { + return PasswordAction.ABSTAIN; } } diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index eb235723e33..41c5fd62a1d 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -32,6 +32,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.CredentialsContainer; @@ -165,7 +166,7 @@ public UserDetails updatePassword(UserDetails user, @Nullable String newPassword throw new RuntimeException("user '" + username + "' does not exist"); } mutableUser.setPassword(newPassword); - savePasswordAdvice(mutableUser, ChangePasswordAdvice.ABSTAIN); + savePasswordAction(mutableUser, PasswordAction.ABSTAIN); return mutableUser; } @@ -182,13 +183,13 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx } @Override - public void savePasswordAdvice(UserDetails user, ChangePasswordAdvice advice) { + public void savePasswordAction(UserDetails user, PasswordAction action) { String username = user.getUsername(); MutableUserDetails mutableUser = this.users.get(username.toLowerCase(Locale.ROOT)); if (mutableUser == null) { throw new RuntimeException("user '" + username + "' does not exist"); } - mutableUser.setChangePasswordAdvice(advice); + mutableUser.setPasswordAction(action); } /** diff --git a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java index 1a3ce02425d..fa981bd2eac 100644 --- a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java +++ b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java @@ -20,7 +20,7 @@ import org.jspecify.annotations.Nullable; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; import org.springframework.security.core.userdetails.UserDetails; @@ -35,7 +35,7 @@ class MutableUser implements MutableUserDetails { private @Nullable String password; - private ChangePasswordAdvice advice = ChangePasswordAdvice.ABSTAIN; + private PasswordAction advice = PasswordAction.ABSTAIN; private final UserDetails delegate; @@ -55,11 +55,11 @@ public void setPassword(@Nullable String password) { } @Override - public ChangePasswordAdvice getChangePasswordAdvice() { - return advice; + public PasswordAction getPasswordAction() { + return this.advice; } - public void setChangePasswordAdvice(ChangePasswordAdvice advice) { + public void setPasswordAction(PasswordAction advice) { this.advice = advice; } diff --git a/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java b/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java index 124e814eb78..b403ce522ab 100644 --- a/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java +++ b/core/src/main/java/org/springframework/security/provisioning/MutableUserDetails.java @@ -18,7 +18,7 @@ import org.jspecify.annotations.Nullable; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.userdetails.UserDetails; /** @@ -29,5 +29,6 @@ interface MutableUserDetails extends UserDetails { void setPassword(@Nullable String password); - void setChangePasswordAdvice(ChangePasswordAdvice advice); + void setPasswordAction(PasswordAction action); + } diff --git a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java index d418ea69775..b9ef2c7bb2c 100644 --- a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java @@ -30,7 +30,7 @@ import org.springframework.security.authentication.TestAuthentication; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.Authentication; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.GrantedAuthority; @@ -229,7 +229,7 @@ public void setPassword(final String password) { } @Override - public void setChangePasswordAdvice(ChangePasswordAdvice advice) { + public void setPasswordAction(PasswordAction advice) { throw new UnsupportedOperationException(); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java index 78f400d77ca..c15e37e3c9b 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java @@ -17,39 +17,40 @@ package org.springframework.security.web.authentication.password; import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvice.Action; import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.core.userdetails.UserDetails; public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { private final CompromisedPasswordChecker pwned = new HaveIBeenPwnedRestApiPasswordChecker(); - private Action action = Action.SHOULD_CHANGE; + private PasswordAction action = PasswordAction.SHOULD_CHANGE; @Override public ChangePasswordAdvice advise(UserDetails user, String password) { CompromisedPasswordDecision decision = this.pwned.check(password); if (decision.isCompromised()) { return new Advice(this.action, decision); - } else { - return new Advice(Action.ABSTAIN, decision); + } + else { + return new Advice(PasswordAction.ABSTAIN, decision); } } - public void setAction(Action action) { + public void setAction(PasswordAction action) { this.action = action; } public static final class Advice implements ChangePasswordAdvice { - private final Action action; + private final PasswordAction action; private final CompromisedPasswordDecision decision; - public Advice(Action action, CompromisedPasswordDecision decision) { + public Advice(PasswordAction action, CompromisedPasswordDecision decision) { this.action = action; this.decision = decision; } @@ -59,17 +60,15 @@ public CompromisedPasswordDecision getCompromisedPasswordDecision() { } @Override - public Action getAction() { + public PasswordAction getAction() { return this.action; } @Override public String toString() { - return "Compromised [" + - "action=" + this.action + - ", decision=" + this.decision + - "]"; + return "Compromised [" + "action=" + this.action + ", decision=" + this.decision + "]"; } + } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java index 1bc12c2f1aa..6d6a630c296 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java @@ -24,6 +24,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.savedrequest.NullRequestCache; @@ -59,7 +60,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse return; } ChangePasswordAdvice advice = this.changePasswordAdviceRepository.loadPasswordAdvice(request); - if (advice.getAction() != ChangePasswordAdvice.Action.MUST_CHANGE) { + if (advice.getAction() != PasswordAction.MUST_CHANGE) { chain.doFilter(request, response); return; } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java index 2c9f1c1a2f1..d392b8c1a70 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java @@ -22,6 +22,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAction; import org.springframework.util.function.SingletonSupplier; public final class HttpSessionChangePasswordAdviceRepository implements ChangePasswordAdviceRepository { @@ -44,7 +45,7 @@ public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { @Override public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, ChangePasswordAdvice advice) { - if (advice.getAction() == ChangePasswordAdvice.Action.ABSTAIN) { + if (advice.getAction() == PasswordAction.ABSTAIN) { removePasswordAdvice(request, response); return; } @@ -65,7 +66,7 @@ private static final class DeferredChangePasswordAdvice implements ChangePasswor } @Override - public Action getAction() { + public PasswordAction getAction() { return this.advice.get().getAction(); } @@ -77,6 +78,7 @@ public ChangePasswordAdvice getChangePasswordAdvice() { public String toString() { return this.advice.get().toString(); } + } } From f9fc34a57271300e5d9b863324f76b295c7070c0 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 31 Jul 2025 16:02:58 -0600 Subject: [PATCH 08/10] Updates - Update Checkstyle - Simplify Naming - Add password action to User and UserDetails --- .../WebMvcSecurityConfiguration.java | 18 ++-- .../PasswordManagementConfigurer.java | 74 +++++----------- .../PasswordManagementConfigurerTests.java | 68 ++++++++------- ...sor.java => CompositePasswordAdvisor.java} | 30 +++---- .../CompositeUpdatePasswordAdvisor.java | 84 +++++++++++++++++++ ...dvisor.java => LengthPasswordAdvisor.java} | 32 +++++-- .../password/PasswordAction.java | 22 ++++- ...asswordAdvice.java => PasswordAdvice.java} | 9 +- ...swordAdvisor.java => PasswordAdvisor.java} | 10 ++- .../password/RepeatedPasswordAdvisor.java | 44 ++++++++++ ...anager.java => UpdatePasswordAdvisor.java} | 11 ++- ...r.java => UserDetailsPasswordAdvisor.java} | 8 +- .../security/core/userdetails/User.java | 33 +++++--- .../security/jackson2/UserMixin.java | 6 ++ .../InMemoryUserDetailsManager.java | 21 +---- .../security/provisioning/MutableUser.java | 8 +- .../InMemoryUserDetailsManagerTests.java | 12 ++- ...rdAdviceSessionAuthenticationStrategy.java | 44 ---------- ...r.java => CompromisedPasswordAdvisor.java} | 23 +++-- ... HttpSessionPasswordAdviceRepository.java} | 30 ++++--- ...PasswordAdviceMethodArgumentResolver.java} | 17 ++-- ...ory.java => PasswordAdviceRepository.java} | 10 +-- ...rdAdviceSessionAuthenticationStrategy.java | 60 +++++++++++++ ...ilter.java => PasswordAdvisingFilter.java} | 16 ++-- 24 files changed, 438 insertions(+), 252 deletions(-) rename core/src/main/java/org/springframework/security/authentication/password/{CompositeChangePasswordAdvisor.java => CompositePasswordAdvisor.java} (62%) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java rename core/src/main/java/org/springframework/security/authentication/password/{ChangeLengthPasswordAdvisor.java => LengthPasswordAdvisor.java} (74%) rename core/src/main/java/org/springframework/security/authentication/password/{ChangePasswordAdvice.java => PasswordAdvice.java} (76%) rename core/src/main/java/org/springframework/security/authentication/password/{ChangePasswordAdvisor.java => PasswordAdvisor.java} (73%) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java rename core/src/main/java/org/springframework/security/authentication/password/{UserDetailsPasswordManager.java => UpdatePasswordAdvisor.java} (70%) rename core/src/main/java/org/springframework/security/authentication/password/{UserDetailsChangePasswordAdvisor.java => UserDetailsPasswordAdvisor.java} (74%) delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java rename web/src/main/java/org/springframework/security/web/authentication/password/{ChangeCompromisedPasswordAdvisor.java => CompromisedPasswordAdvisor.java} (71%) rename web/src/main/java/org/springframework/security/web/authentication/password/{HttpSessionChangePasswordAdviceRepository.java => HttpSessionPasswordAdviceRepository.java} (65%) rename web/src/main/java/org/springframework/security/web/authentication/password/{ChangePasswordAdviceMethodArgumentResolver.java => PasswordAdviceMethodArgumentResolver.java} (63%) rename web/src/main/java/org/springframework/security/web/authentication/password/{ChangePasswordAdviceRepository.java => PasswordAdviceRepository.java} (74%) create mode 100644 web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceSessionAuthenticationStrategy.java rename web/src/main/java/org/springframework/security/web/authentication/password/{ChangePasswordAdvisingFilter.java => PasswordAdvisingFilter.java} (79%) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java index c195c70f888..f6543d5c6ca 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java @@ -37,9 +37,9 @@ import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceMethodArgumentResolver; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; -import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.HttpSessionPasswordAdviceRepository; +import org.springframework.security.web.authentication.password.PasswordAdviceMethodArgumentResolver; +import org.springframework.security.web.authentication.password.PasswordAdviceRepository; import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.firewall.RequestRejectedHandler; @@ -75,7 +75,7 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex private AnnotationTemplateExpressionDefaults templateDefaults; - private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + private PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository(); @Override @SuppressWarnings("deprecation") @@ -93,8 +93,8 @@ public void addArgumentResolvers(List argumentRes currentSecurityContextArgumentResolver.setTemplateDefaults(this.templateDefaults); argumentResolvers.add(currentSecurityContextArgumentResolver); argumentResolvers.add(new CsrfTokenArgumentResolver()); - ChangePasswordAdviceMethodArgumentResolver resolver = new ChangePasswordAdviceMethodArgumentResolver(); - resolver.setChangePasswordAdviceRepository(this.changePasswordAdviceRepository); + PasswordAdviceMethodArgumentResolver resolver = new PasswordAdviceMethodArgumentResolver(); + resolver.setPasswordAdviceRepository(this.passwordAdviceRepository); argumentResolvers.add(resolver); } @@ -112,8 +112,8 @@ public void setApplicationContext(ApplicationContext applicationContext) throws if (applicationContext.getBeanNamesForType(AnnotationTemplateExpressionDefaults.class).length == 1) { this.templateDefaults = applicationContext.getBean(AnnotationTemplateExpressionDefaults.class); } - if (applicationContext.getBeanNamesForType(ChangePasswordAdviceRepository.class).length == 1) { - this.changePasswordAdviceRepository = applicationContext.getBean(ChangePasswordAdviceRepository.class); + if (applicationContext.getBeanNamesForType(PasswordAdviceRepository.class).length == 1) { + this.passwordAdviceRepository = applicationContext.getBean(PasswordAdviceRepository.class); } } @@ -220,7 +220,7 @@ private static Filter createDoFilterDelegate(List filters) { /** * Find the FilterChainProxy in a List of Filter - * @param filters + * @param filterssetChangePass * @return non-null FilterChainProxy * @throws IllegalStateException if the FilterChainProxy cannot be found */ diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 8802fd505c7..e7bc9255731 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -18,18 +18,15 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.authentication.password.CompositeChangePasswordAdvisor; -import org.springframework.security.authentication.password.UserDetailsChangePasswordAdvisor; -import org.springframework.security.authentication.password.UserDetailsPasswordManager; +import org.springframework.security.authentication.password.PasswordAdvisor; +import org.springframework.security.authentication.password.UserDetailsPasswordAdvisor; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; -import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceSessionAuthenticationStrategy; -import org.springframework.security.web.authentication.password.ChangePasswordAdvisingFilter; -import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.HttpSessionPasswordAdviceRepository; +import org.springframework.security.web.authentication.password.PasswordAdviceRepository; +import org.springframework.security.web.authentication.password.PasswordAdviceSessionAuthenticationStrategy; +import org.springframework.security.web.authentication.password.PasswordAdvisingFilter; import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.util.Assert; @@ -53,11 +50,9 @@ public final class PasswordManagementConfigurer private String changePasswordPage = DEFAULT_CHANGE_PASSWORD_PAGE; - private ChangePasswordAdviceRepository changePasswordAdviceRepository; + private PasswordAdviceRepository passwordAdviceRepository; - private ChangePasswordAdvisor changePasswordAdvisor; - - private UserDetailsPasswordManager userDetailsPasswordManager; + private PasswordAdvisor passwordAdvisor; /** * Sets the change password page. Defaults to @@ -72,55 +67,36 @@ public PasswordManagementConfigurer changePasswordPage(String changePasswordP return this; } - public PasswordManagementConfigurer changePasswordAdviceRepository( - ChangePasswordAdviceRepository changePasswordAdviceRepository) { - this.changePasswordAdviceRepository = changePasswordAdviceRepository; + public PasswordManagementConfigurer passwordAdviceRepository(PasswordAdviceRepository passwordAdviceRepository) { + this.passwordAdviceRepository = passwordAdviceRepository; return this; } - public PasswordManagementConfigurer changePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { - this.changePasswordAdvisor = changePasswordAdvisor; - return this; - } - - public PasswordManagementConfigurer userDetailsPasswordManager( - UserDetailsPasswordManager userDetailsPasswordManager) { - this.userDetailsPasswordManager = userDetailsPasswordManager; + public PasswordManagementConfigurer passwordAdvisor(PasswordAdvisor passwordAdvisor) { + this.passwordAdvisor = passwordAdvisor; return this; } @Override public void init(B http) throws Exception { - UserDetailsPasswordManager passwordManager = (this.userDetailsPasswordManager == null) - ? this.context.getBeanProvider(UserDetailsPasswordManager.class).getIfUnique() - : this.userDetailsPasswordManager; + PasswordAdviceRepository passwordAdviceRepository = (this.passwordAdviceRepository != null) + ? this.passwordAdviceRepository : this.context.getBeanProvider(PasswordAdviceRepository.class) + .getIfUnique(HttpSessionPasswordAdviceRepository::new); - if (passwordManager == null) { - return; - } + PasswordAdvisor passwordAdvisor = (this.passwordAdvisor != null) ? this.passwordAdvisor + : this.context.getBeanProvider(PasswordAdvisor.class).getIfUnique(UserDetailsPasswordAdvisor::new); - ChangePasswordAdviceRepository changePasswordAdviceRepository = (this.changePasswordAdviceRepository != null) - ? this.changePasswordAdviceRepository - : this.context.getBeanProvider(ChangePasswordAdviceRepository.class) - .getIfUnique(HttpSessionChangePasswordAdviceRepository::new); - - ChangePasswordAdvisor changePasswordAdvisor = (this.changePasswordAdvisor != null) ? this.changePasswordAdvisor - : this.context.getBeanProvider(ChangePasswordAdvisor.class) - .getIfUnique(() -> CompositeChangePasswordAdvisor.of(new UserDetailsChangePasswordAdvisor(), - new ChangeCompromisedPasswordAdvisor())); - - http.setSharedObject(ChangePasswordAdviceRepository.class, changePasswordAdviceRepository); - http.setSharedObject(UserDetailsPasswordManager.class, passwordManager); + http.setSharedObject(PasswordAdviceRepository.class, passwordAdviceRepository); String passwordParameter = "password"; FormLoginConfigurer form = http.getConfigurer(FormLoginConfigurer.class); if (form != null) { passwordParameter = form.getPasswordParameter(); } - ChangePasswordAdviceSessionAuthenticationStrategy sessionAuthenticationStrategy = new ChangePasswordAdviceSessionAuthenticationStrategy( + PasswordAdviceSessionAuthenticationStrategy sessionAuthenticationStrategy = new PasswordAdviceSessionAuthenticationStrategy( passwordParameter); - sessionAuthenticationStrategy.setChangePasswordAdviceRepository(changePasswordAdviceRepository); - sessionAuthenticationStrategy.setChangePasswordAdvisor(changePasswordAdvisor); + sessionAuthenticationStrategy.setPasswordAdviceRepository(passwordAdviceRepository); + sessionAuthenticationStrategy.setPasswordAdvisor(passwordAdvisor); http.getConfigurer(SessionManagementConfigurer.class) .addSessionAuthenticationStrategy(sessionAuthenticationStrategy); } @@ -134,12 +110,8 @@ public void configure(B http) throws Exception { getRequestMatcherBuilder().matcher(WELL_KNOWN_CHANGE_PASSWORD_PATTERN), this.changePasswordPage); http.addFilterBefore(postProcess(changePasswordFilter), UsernamePasswordAuthenticationFilter.class); - if (http.getSharedObject(UserDetailsPasswordManager.class) == null) { - return; - } - - ChangePasswordAdvisingFilter advising = new ChangePasswordAdvisingFilter(this.changePasswordPage); - advising.setChangePasswordAdviceRepository(http.getSharedObject(ChangePasswordAdviceRepository.class)); + PasswordAdvisingFilter advising = new PasswordAdvisingFilter(this.changePasswordPage); + advising.setPasswordAdviceRepository(http.getSharedObject(PasswordAdviceRepository.class)); advising.setRequestCache(http.getSharedObject(RequestCache.class)); http.addFilterBefore(advising, RequestCacheAwareFilter.class); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index 79188ba83f7..03cddcc72a4 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -29,10 +29,9 @@ import org.springframework.context.annotation.Configuration; import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockHttpSession; -import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; import org.springframework.security.authentication.password.PasswordAction; -import org.springframework.security.authentication.password.UserDetailsPasswordManager; +import org.springframework.security.authentication.password.PasswordAdvice; +import org.springframework.security.authentication.password.UpdatePasswordAdvisor; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -40,15 +39,17 @@ import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.security.core.userdetails.PasswordEncodedUser; +import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.crypto.factory.PasswordEncoderFactories; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.provisioning.InMemoryUserDetailsManager; +import org.springframework.security.provisioning.UserDetailsManager; import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.authentication.password.ChangeCompromisedPasswordAdvisor; -import org.springframework.security.web.authentication.password.ChangePasswordAdviceRepository; -import org.springframework.security.web.authentication.password.HttpSessionChangePasswordAdviceRepository; +import org.springframework.security.web.authentication.password.CompromisedPasswordAdvisor; +import org.springframework.security.web.authentication.password.HttpSessionPasswordAdviceRepository; +import org.springframework.security.web.authentication.password.PasswordAdviceRepository; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; import org.springframework.web.bind.annotation.GetMapping; @@ -155,7 +156,7 @@ void whenAdminSetsExpiredAdviceThenUserLoginRedirectsToResetPassword() throws Ex } @Test - void whenCompromisedThenUserLoginAllowed() throws Exception { + void whenShouldChangeThenUserLoginAllowed() throws Exception { this.spring.register(PasswordManagementConfig.class, AdminController.class, HomeController.class).autowire(); MvcResult result = this.mvc .perform(post("/login").with(csrf()).param("username", "user").param("password", "password")) @@ -165,7 +166,7 @@ void whenCompromisedThenUserLoginAllowed() throws Exception { MockHttpSession session = (MockHttpSession) result.getRequest().getSession(); this.mvc.perform(get("/").session(session)) .andExpect(status().isOk()) - .andExpect(content().string(containsString("compromised"))); + .andExpect(content().string(containsString("SHOULD_CHANGE"))); } @Configuration @@ -206,27 +207,26 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception { static class PasswordManagementConfig { @Bean - SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { + SecurityFilterChain securityFilterChain(HttpSecurity http, UserDetailsService users) throws Exception { // @formatter:off http - .authorizeHttpRequests((authz) -> authz - .requestMatchers("/admin/**").hasRole("ADMIN") - .anyRequest().authenticated() - ) - .formLogin(Customizer.withDefaults()) - .passwordManagement(Customizer.withDefaults()); + .authorizeHttpRequests((authz) -> authz + .requestMatchers("/admin/**").hasRole("ADMIN") + .anyRequest().authenticated() + ) + .formLogin(Customizer.withDefaults()) + .passwordManagement(Customizer.withDefaults()); // @formatter:on return http.build(); } @Bean - UserDetailsService users() { - String adminPassword = UUID.randomUUID().toString(); - UserDetails compromised = PasswordEncodedUser.user(); - UserDetails admin = PasswordEncodedUser.withUserDetails(PasswordEncodedUser.admin()) - .password(adminPassword) + InMemoryUserDetailsManager users() { + UserDetails shouldChange = User.withUserDetails(PasswordEncodedUser.user()) + .passwordAction(PasswordAction.SHOULD_CHANGE) .build(); - return new InMemoryUserDetailsManager(compromised, admin); + UserDetails admin = PasswordEncodedUser.admin(); + return new InMemoryUserDetailsManager(shouldChange, admin); } } @@ -235,13 +235,10 @@ UserDetailsService users() { @RestController static class AdminController { - private final UserDetailsService users; - - private final UserDetailsPasswordManager passwords; + private final UserDetailsManager users; - AdminController(UserDetailsService users, UserDetailsPasswordManager passwords) { + AdminController(InMemoryUserDetailsManager users) { this.users = users; - this.passwords = passwords; } @GetMapping("/advice/{username}") @@ -259,7 +256,8 @@ ResponseEntity expirePassword(@PathVariable("username") String u if (user == null) { return ResponseEntity.notFound().build(); } - this.passwords.savePasswordAction(user, PasswordAction.MUST_CHANGE); + UserDetails mustChange = User.withUserDetails(user).passwordAction(PasswordAction.MUST_CHANGE).build(); + this.users.updateUser(mustChange); URI uri = URI.create("/admin/passwords/advice/" + username); return ResponseEntity.created(uri).body(PasswordAction.MUST_CHANGE); } @@ -269,32 +267,32 @@ ResponseEntity expirePassword(@PathVariable("username") String u @RestController static class HomeController { - private final UserDetailsPasswordManager passwords; + private final InMemoryUserDetailsManager passwords; - private final ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); + private final UpdatePasswordAdvisor passwordAdvisor = new CompromisedPasswordAdvisor(); - private final ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + private final PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository(); private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); - HomeController(UserDetailsPasswordManager passwords) { + HomeController(InMemoryUserDetailsManager passwords) { this.passwords = passwords; } @GetMapping - ChangePasswordAdvice index(ChangePasswordAdvice advice) { + PasswordAdvice index(PasswordAdvice advice) { return advice; } @PostMapping("/change-password") ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, @RequestParam("password") String password, HttpServletRequest request, HttpServletResponse response) { - ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); + PasswordAdvice advice = this.passwordAdvisor.advise(user, null, password); if (advice.getAction() != PasswordAction.ABSTAIN) { return ResponseEntity.badRequest().body(advice); } - this.passwords.updatePassword(user, this.encoder.encode(password)); - this.changePasswordAdviceRepository.removePasswordAdvice(request, response); + this.passwords.changePassword(null, this.encoder.encode(password)); + this.passwordAdviceRepository.removePasswordAdvice(request, response); return ResponseEntity.ok().build(); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java similarity index 62% rename from core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java index aa4014fb96d..21ed6f5d426 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/CompositeChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,42 +20,44 @@ import java.util.Collections; import java.util.List; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; -public final class CompositeChangePasswordAdvisor implements ChangePasswordAdvisor { +public final class CompositePasswordAdvisor implements PasswordAdvisor { - private final List advisors; + private final List advisors; - private CompositeChangePasswordAdvisor(List advisors) { + private CompositePasswordAdvisor(List advisors) { this.advisors = Collections.unmodifiableList(advisors); } - public static ChangePasswordAdvisor of(ChangePasswordAdvisor... advisors) { - return new CompositeChangePasswordAdvisor(List.of(advisors)); + public static PasswordAdvisor of(PasswordAdvisor... advisors) { + return new CompositePasswordAdvisor(List.of(advisors)); } @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { - Collection advice = this.advisors.stream() + public PasswordAdvice advise(UserDetails user, @Nullable String password) { + Collection advice = this.advisors.stream() .map((advisor) -> advisor.advise(user, password)) .toList(); return new Advice(advice); } - public static final class Advice implements ChangePasswordAdvice { + public static final class Advice implements PasswordAdvice { private final PasswordAction action; - private final Collection advice; + private final Collection advice; - private Advice(Collection advice) { + private Advice(Collection advice) { this.action = findMostUrgentAction(advice); this.advice = advice; } - private PasswordAction findMostUrgentAction(Collection advice) { + private PasswordAction findMostUrgentAction(Collection advice) { PasswordAction mostUrgentAction = PasswordAction.ABSTAIN; - for (ChangePasswordAdvice a : advice) { + for (PasswordAdvice a : advice) { if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { mostUrgentAction = a.getAction(); } @@ -68,7 +70,7 @@ public PasswordAction getAction() { return this.action; } - public Collection getAdvice() { + public Collection getAdvice() { return this.advice; } diff --git a/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java new file mode 100644 index 00000000000..be343184556 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java @@ -0,0 +1,84 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import org.jspecify.annotations.Nullable; + +import org.springframework.security.core.userdetails.UserDetails; + +public final class CompositeUpdatePasswordAdvisor implements UpdatePasswordAdvisor { + + private final List advisors; + + private CompositeUpdatePasswordAdvisor(List advisors) { + this.advisors = Collections.unmodifiableList(advisors); + } + + public static UpdatePasswordAdvisor of(UpdatePasswordAdvisor... advisors) { + return new CompositeUpdatePasswordAdvisor(List.of(advisors)); + } + + @Override + public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) { + Collection advice = this.advisors.stream() + .map((advisor) -> advisor.advise(user, oldPassword, newPassword)) + .toList(); + return new CompositeUpdatePasswordAdvisor.Advice(advice); + } + + public static final class Advice implements PasswordAdvice { + + private final PasswordAction action; + + private final Collection advice; + + private Advice(Collection advice) { + this.action = findMostUrgentAction(advice); + this.advice = advice; + } + + private PasswordAction findMostUrgentAction(Collection advice) { + PasswordAction mostUrgentAction = PasswordAction.ABSTAIN; + for (PasswordAdvice a : advice) { + if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { + mostUrgentAction = a.getAction(); + } + } + return mostUrgentAction; + } + + @Override + public PasswordAction getAction() { + return this.action; + } + + public Collection getAdvice() { + return this.advice; + } + + @Override + public String toString() { + return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]"; + } + + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java similarity index 74% rename from core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java index 19f6de225c7..1e2a1004a66 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangeLengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,12 @@ package org.springframework.security.authentication.password; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.util.Assert; -public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { +public final class LengthPasswordAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor { private final int minLength; @@ -28,24 +31,37 @@ public class ChangeLengthPasswordAdvisor implements ChangePasswordAdvisor { private PasswordAction tooLongAction = PasswordAction.SHOULD_CHANGE; - public ChangeLengthPasswordAdvisor(int minLength) { + public LengthPasswordAdvisor() { + this(12, 64); + } + + public LengthPasswordAdvisor(int minLength) { this(minLength, Integer.MAX_VALUE); } - public ChangeLengthPasswordAdvisor(int minLength, int maxLength) { + public LengthPasswordAdvisor(int minLength, int maxLength) { + Assert.isTrue(minLength > 0, "minLength must be greater than 0"); this.minLength = minLength; this.maxLength = maxLength; } @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { + public PasswordAdvice advise(UserDetails user, @Nullable String password) { + if (password == null) { + return new TooShortAdvice(this.tooShortAction, this.minLength, 0); + } if (password.length() < this.minLength) { return new TooShortAdvice(this.tooShortAction, this.minLength, password.length()); } if (password.length() > this.maxLength) { return new TooLongAdvice(this.tooLongAction, this.maxLength, password.length()); } - return ChangePasswordAdvice.ABSTAIN; + return PasswordAdvice.ABSTAIN; + } + + @Override + public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) { + return advise(user, newPassword); } public void setTooShortAction(PasswordAction tooShortAction) { @@ -56,7 +72,7 @@ public void setTooLongAction(PasswordAction tooLongAction) { this.tooLongAction = tooLongAction; } - public static final class TooShortAdvice implements ChangePasswordAdvice { + public static final class TooShortAdvice implements PasswordAdvice { private final PasswordAction action; @@ -91,7 +107,7 @@ public String toString() { } - public static final class TooLongAdvice implements ChangePasswordAdvice { + public static final class TooLongAdvice implements PasswordAdvice { private final PasswordAction action; diff --git a/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java index 9d4ca4cd543..04690a5c9d6 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java @@ -1,7 +1,27 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.springframework.security.authentication.password; public enum PasswordAction { - ABSTAIN, SHOULD_CHANGE, MUST_CHANGE + ABSTAIN, SHOULD_CHANGE, MUST_CHANGE; + + boolean advisedBy(PasswordAdvice advice) { + return advice.getAction().equals(this); + } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java similarity index 76% rename from core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java rename to core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java index 47183b4997d..d7b35d57c28 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,12 @@ package org.springframework.security.authentication.password; -public interface ChangePasswordAdvice { +import org.jspecify.annotations.NullMarked; - ChangePasswordAdvice ABSTAIN = () -> PasswordAction.ABSTAIN; +@NullMarked +public interface PasswordAdvice { + + PasswordAdvice ABSTAIN = () -> PasswordAction.ABSTAIN; PasswordAction getAction(); diff --git a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvisor.java similarity index 73% rename from core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/PasswordAdvisor.java index 2289714f055..01ddb2ec9aa 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/ChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,10 +16,14 @@ package org.springframework.security.authentication.password; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; -public interface ChangePasswordAdvisor { +@NullMarked +public interface PasswordAdvisor { - ChangePasswordAdvice advise(UserDetails user, String password); + PasswordAdvice advise(UserDetails user, @Nullable String password); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java new file mode 100644 index 00000000000..fa933194766 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java @@ -0,0 +1,44 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import java.util.Objects; + +import org.jspecify.annotations.Nullable; + +import org.springframework.security.core.userdetails.UserDetails; + +public final class RepeatedPasswordAdvisor implements UpdatePasswordAdvisor { + + @Override + public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) { + if (Objects.equals(oldPassword, newPassword)) { + return new Advice(); + } + return PasswordAdvice.ABSTAIN; + } + + public static final class Advice implements PasswordAdvice { + + @Override + public PasswordAction getAction() { + return PasswordAction.MUST_CHANGE; + } + + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java b/core/src/main/java/org/springframework/security/authentication/password/UpdatePasswordAdvisor.java similarity index 70% rename from core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java rename to core/src/main/java/org/springframework/security/authentication/password/UpdatePasswordAdvisor.java index acb1700eb94..f9a1f46bb85 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordManager.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UpdatePasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,14 @@ package org.springframework.security.authentication.password; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.UserDetailsPasswordService; -public interface UserDetailsPasswordManager extends UserDetailsPasswordService { +@NullMarked +public interface UpdatePasswordAdvisor { - void savePasswordAction(UserDetails user, PasswordAction action); + PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java similarity index 74% rename from core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java index 8cd04790d5b..ef9e8bc52d6 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsChangePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,12 +16,14 @@ package org.springframework.security.authentication.password; +import org.jspecify.annotations.Nullable; + import org.springframework.security.core.userdetails.UserDetails; -public final class UserDetailsChangePasswordAdvisor implements ChangePasswordAdvisor { +public final class UserDetailsPasswordAdvisor implements PasswordAdvisor { @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { + public PasswordAdvice advise(UserDetails user, @Nullable String password) { return user::getPasswordAction; } diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index 3aadd2eb8a3..8249a7d2a7a 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -114,23 +114,26 @@ public User(String username, @Nullable String password, boolean enabled, boolean Assert.isTrue(username != null && !"".equals(username), "Cannot pass null or empty values to constructor"); this.username = username; this.password = password; + this.passwordAction = PasswordAction.ABSTAIN; this.enabled = enabled; this.accountNonExpired = accountNonExpired; this.credentialsNonExpired = credentialsNonExpired; this.accountNonLocked = accountNonLocked; - this.passwordAction = PasswordAction.ABSTAIN; this.authorities = Collections.unmodifiableSet(sortAuthorities(authorities)); } - public User(UserDetails user) { - this.username = user.getUsername(); - this.password = user.getPassword(); - this.enabled = user.isEnabled(); - this.accountNonExpired = user.isAccountNonExpired(); - this.credentialsNonExpired = user.isCredentialsNonExpired(); - this.accountNonLocked = user.isAccountNonLocked(); - this.passwordAction = user.getPasswordAction(); - this.authorities = Collections.unmodifiableSet(sortAuthorities(user.getAuthorities())); + private User(String username, @Nullable String password, PasswordAction passwordAction, boolean enabled, + boolean accountNonExpired, boolean credentialsNonExpired, boolean accountNonLocked, + Collection authorities) { + Assert.isTrue(username != null && !"".equals(username), "Cannot pass null or empty values to constructor"); + this.username = username; + this.password = password; + this.passwordAction = passwordAction; + this.enabled = enabled; + this.accountNonExpired = accountNonExpired; + this.credentialsNonExpired = credentialsNonExpired; + this.accountNonLocked = accountNonLocked; + this.authorities = Collections.unmodifiableSet(sortAuthorities(authorities)); } @Override @@ -310,6 +313,7 @@ public static UserBuilder withDefaultPasswordEncoder() { public static UserBuilder withUserDetails(UserDetails userDetails) { // @formatter:off UserBuilder result = withUsername(userDetails.getUsername()) + .passwordAction(userDetails.getPasswordAction()) .accountExpired(!userDetails.isAccountNonExpired()) .accountLocked(!userDetails.isAccountNonLocked()) .authorities(userDetails.getAuthorities()) @@ -352,6 +356,8 @@ public static final class UserBuilder { private @Nullable String password; + private PasswordAction passwordAction = PasswordAction.ABSTAIN; + private List authorities = new ArrayList<>(); private boolean accountExpired; @@ -393,6 +399,11 @@ public UserBuilder password(@Nullable String password) { return this; } + public UserBuilder passwordAction(PasswordAction passwordAction) { + this.passwordAction = passwordAction; + return this; + } + /** * Encodes the current password (if non-null) and any future passwords supplied to * {@link #password(String)}. @@ -527,7 +538,7 @@ public UserBuilder disabled(boolean disabled) { public UserDetails build() { Assert.notNull(this.username, "username cannot be null"); String encodedPassword = (this.password != null) ? this.passwordEncoder.apply(this.password) : null; - return new User(this.username, encodedPassword, !this.disabled, !this.accountExpired, + return new User(this.username, encodedPassword, this.passwordAction, !this.disabled, !this.accountExpired, !this.credentialsExpired, !this.accountLocked, this.authorities); } diff --git a/core/src/main/java/org/springframework/security/jackson2/UserMixin.java b/core/src/main/java/org/springframework/security/jackson2/UserMixin.java index bef444fd112..c2c58b4f88d 100644 --- a/core/src/main/java/org/springframework/security/jackson2/UserMixin.java +++ b/core/src/main/java/org/springframework/security/jackson2/UserMixin.java @@ -17,10 +17,13 @@ package org.springframework.security.jackson2; import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import org.springframework.security.authentication.password.PasswordAction; + /** * This mixin class helps in serialize/deserialize * {@link org.springframework.security.core.userdetails.User}. This class also register a @@ -49,4 +52,7 @@ @JsonIgnoreProperties(ignoreUnknown = true) abstract class UserMixin { + @JsonIgnore + abstract PasswordAction getPasswordAction(); + } diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index 41c5fd62a1d..e729bdf523e 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -31,15 +31,14 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.PasswordAction; -import org.springframework.security.authentication.password.UserDetailsPasswordManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsPasswordService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.core.userdetails.memory.UserAttribute; import org.springframework.security.core.userdetails.memory.UserAttributeEditor; @@ -55,14 +54,12 @@ * @author Luke Taylor * @since 3.1 */ -public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetailsPasswordManager { +public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetailsPasswordService { protected final Log logger = LogFactory.getLog(getClass()); private final Map users = new HashMap<>(); - private final Map advice = new HashMap<>(); - private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder .getContextHolderStrategy(); @@ -156,6 +153,7 @@ public void changePassword(String oldPassword, String newPassword) { MutableUserDetails user = this.users.get(username); Assert.state(user != null, "Current user doesn't exist in database."); user.setPassword(newPassword); + user.setPasswordAction(PasswordAction.ABSTAIN); } @Override @@ -166,7 +164,6 @@ public UserDetails updatePassword(UserDetails user, @Nullable String newPassword throw new RuntimeException("user '" + username + "' does not exist"); } mutableUser.setPassword(newPassword); - savePasswordAction(mutableUser, PasswordAction.ABSTAIN); return mutableUser; } @@ -179,17 +176,7 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx if (user instanceof CredentialsContainer) { return user; } - return new User(user); - } - - @Override - public void savePasswordAction(UserDetails user, PasswordAction action) { - String username = user.getUsername(); - MutableUserDetails mutableUser = this.users.get(username.toLowerCase(Locale.ROOT)); - if (mutableUser == null) { - throw new RuntimeException("user '" + username + "' does not exist"); - } - mutableUser.setPasswordAction(action); + return User.withUserDetails(user).build(); } /** diff --git a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java index fa981bd2eac..fe6e2abf739 100644 --- a/core/src/main/java/org/springframework/security/provisioning/MutableUser.java +++ b/core/src/main/java/org/springframework/security/provisioning/MutableUser.java @@ -35,13 +35,14 @@ class MutableUser implements MutableUserDetails { private @Nullable String password; - private PasswordAction advice = PasswordAction.ABSTAIN; + private PasswordAction action; private final UserDetails delegate; MutableUser(UserDetails user) { this.delegate = user; this.password = user.getPassword(); + this.action = user.getPasswordAction(); } @Override @@ -56,11 +57,12 @@ public void setPassword(@Nullable String password) { @Override public PasswordAction getPasswordAction() { - return this.advice; + return this.action; } + @Override public void setPasswordAction(PasswordAction advice) { - this.advice = advice; + this.action = advice; } @Override diff --git a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java index b9ef2c7bb2c..e4ba2594892 100644 --- a/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java @@ -208,9 +208,12 @@ static class CustomUser implements MutableUserDetails, CredentialsContainer { private String password; + private PasswordAction action; + CustomUser(UserDetails user) { this.delegate = user; this.password = user.getPassword(); + this.action = user.getPasswordAction(); } @Override @@ -229,8 +232,13 @@ public void setPassword(final String password) { } @Override - public void setPasswordAction(PasswordAction advice) { - throw new UnsupportedOperationException(); + public PasswordAction getPasswordAction() { + return this.action; + } + + @Override + public void setPasswordAction(PasswordAction action) { + this.action = action; } @Override diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java deleted file mode 100644 index 8f38b5380ee..00000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceSessionAuthenticationStrategy.java +++ /dev/null @@ -1,44 +0,0 @@ -package org.springframework.security.web.authentication.password; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.web.authentication.session.SessionAuthenticationException; -import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; -import org.springframework.util.Assert; - -public final class ChangePasswordAdviceSessionAuthenticationStrategy implements SessionAuthenticationStrategy { - - private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); - - private ChangePasswordAdvisor changePasswordAdvisor = new ChangeCompromisedPasswordAdvisor(); - - private final String passwordParameter; - - public ChangePasswordAdviceSessionAuthenticationStrategy(String passwordParameter) { - this.passwordParameter = passwordParameter; - } - - @Override - public void onAuthentication(Authentication authentication, HttpServletRequest request, - HttpServletResponse response) throws SessionAuthenticationException { - UserDetails user = (UserDetails) authentication.getPrincipal(); - Assert.notNull(user, "cannot persist password advice since user principal is null"); - String password = request.getParameter(this.passwordParameter); - ChangePasswordAdvice advice = this.changePasswordAdvisor.advise(user, password); - this.changePasswordAdviceRepository.savePasswordAdvice(request, response, advice); - } - - public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { - this.changePasswordAdviceRepository = changePasswordAdviceRepository; - } - - public void setChangePasswordAdvisor(ChangePasswordAdvisor changePasswordAdvisor) { - this.changePasswordAdvisor = changePasswordAdvisor; - } - -} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java similarity index 71% rename from web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java rename to web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java index c15e37e3c9b..0113e8adff3 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangeCompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,21 +16,27 @@ package org.springframework.security.web.authentication.password; -import org.springframework.security.authentication.password.ChangePasswordAdvice; -import org.springframework.security.authentication.password.ChangePasswordAdvisor; +import org.jspecify.annotations.Nullable; + import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.authentication.password.CompromisedPasswordDecision; import org.springframework.security.authentication.password.PasswordAction; +import org.springframework.security.authentication.password.PasswordAdvice; +import org.springframework.security.authentication.password.PasswordAdvisor; +import org.springframework.security.authentication.password.UpdatePasswordAdvisor; import org.springframework.security.core.userdetails.UserDetails; -public final class ChangeCompromisedPasswordAdvisor implements ChangePasswordAdvisor { +public final class CompromisedPasswordAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor { private final CompromisedPasswordChecker pwned = new HaveIBeenPwnedRestApiPasswordChecker(); private PasswordAction action = PasswordAction.SHOULD_CHANGE; @Override - public ChangePasswordAdvice advise(UserDetails user, String password) { + public PasswordAdvice advise(UserDetails user, @Nullable String password) { + if (password == null) { + return PasswordAdvice.ABSTAIN; + } CompromisedPasswordDecision decision = this.pwned.check(password); if (decision.isCompromised()) { return new Advice(this.action, decision); @@ -40,11 +46,16 @@ public ChangePasswordAdvice advise(UserDetails user, String password) { } } + @Override + public PasswordAdvice advise(UserDetails user, String oldPassword, String newPassword) { + return advise(user, newPassword); + } + public void setAction(PasswordAction action) { this.action = action; } - public static final class Advice implements ChangePasswordAdvice { + public static final class Advice implements PasswordAdvice { private final PasswordAction action; diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java similarity index 65% rename from web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java rename to web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java index d392b8c1a70..1ed2a9bc463 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionChangePasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,30 +21,28 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.PasswordAction; +import org.springframework.security.authentication.password.PasswordAdvice; import org.springframework.util.function.SingletonSupplier; -public final class HttpSessionChangePasswordAdviceRepository implements ChangePasswordAdviceRepository { +public final class HttpSessionPasswordAdviceRepository implements PasswordAdviceRepository { - private static final String PASSWORD_ADVICE_ATTRIBUTE_NAME = HttpSessionChangePasswordAdviceRepository.class - .getName() + ".PASSWORD_ADVICE"; + private static final String PASSWORD_ADVICE_ATTRIBUTE_NAME = HttpSessionPasswordAdviceRepository.class.getName() + + ".PASSWORD_ADVICE"; @Override - public ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request) { - return new DeferredChangePasswordAdvice(() -> { - ChangePasswordAdvice advice = (ChangePasswordAdvice) request.getSession() - .getAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME); + public PasswordAdvice loadPasswordAdvice(HttpServletRequest request) { + return new DeferredPasswordAdvice(() -> { + PasswordAdvice advice = (PasswordAdvice) request.getSession().getAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME); if (advice != null) { return advice; } - return ChangePasswordAdvice.ABSTAIN; + return PasswordAdvice.ABSTAIN; }); } @Override - public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, - ChangePasswordAdvice advice) { + public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, PasswordAdvice advice) { if (advice.getAction() == PasswordAction.ABSTAIN) { removePasswordAdvice(request, response); return; @@ -57,11 +55,11 @@ public void removePasswordAdvice(HttpServletRequest request, HttpServletResponse request.getSession().removeAttribute(PASSWORD_ADVICE_ATTRIBUTE_NAME); } - private static final class DeferredChangePasswordAdvice implements ChangePasswordAdvice { + private static final class DeferredPasswordAdvice implements PasswordAdvice { - private final Supplier advice; + private final Supplier advice; - DeferredChangePasswordAdvice(Supplier advice) { + DeferredPasswordAdvice(Supplier advice) { this.advice = SingletonSupplier.of(advice); } @@ -70,7 +68,7 @@ public PasswordAction getAction() { return this.advice.get().getAction(); } - public ChangePasswordAdvice getChangePasswordAdvice() { + PasswordAdvice getChangePasswordAdvice() { return this.advice.get(); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceMethodArgumentResolver.java similarity index 63% rename from web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java rename to web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceMethodArgumentResolver.java index 3a5cd334767..e3f69b220b8 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceMethodArgumentResolver.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,30 +19,29 @@ import jakarta.servlet.http.HttpServletRequest; import org.springframework.core.MethodParameter; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAdvice; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.ModelAndViewContainer; -public final class ChangePasswordAdviceMethodArgumentResolver implements HandlerMethodArgumentResolver { +public final class PasswordAdviceMethodArgumentResolver implements HandlerMethodArgumentResolver { - ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository(); @Override public boolean supportsParameter(MethodParameter parameter) { - return ChangePasswordAdvice.class.isAssignableFrom(parameter.getParameterType()); + return PasswordAdvice.class.isAssignableFrom(parameter.getParameterType()); } @Override public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { - return this.changePasswordAdviceRepository - .loadPasswordAdvice(webRequest.getNativeRequest(HttpServletRequest.class)); + return this.passwordAdviceRepository.loadPasswordAdvice(webRequest.getNativeRequest(HttpServletRequest.class)); } - public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { - this.changePasswordAdviceRepository = changePasswordAdviceRepository; + public void setPasswordAdviceRepository(PasswordAdviceRepository passwordAdviceRepository) { + this.passwordAdviceRepository = passwordAdviceRepository; } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceRepository.java similarity index 74% rename from web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java rename to web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceRepository.java index 7cee802964b..4587d74b40d 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,13 +19,13 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.security.authentication.password.ChangePasswordAdvice; +import org.springframework.security.authentication.password.PasswordAdvice; -public interface ChangePasswordAdviceRepository { +public interface PasswordAdviceRepository { - ChangePasswordAdvice loadPasswordAdvice(HttpServletRequest request); + PasswordAdvice loadPasswordAdvice(HttpServletRequest request); - void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, ChangePasswordAdvice advice); + void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, PasswordAdvice advice); void removePasswordAdvice(HttpServletRequest request, HttpServletResponse response); diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceSessionAuthenticationStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceSessionAuthenticationStrategy.java new file mode 100644 index 00000000000..ed1187a8ea7 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdviceSessionAuthenticationStrategy.java @@ -0,0 +1,60 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.password; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.springframework.security.authentication.password.PasswordAdvice; +import org.springframework.security.authentication.password.PasswordAdvisor; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.web.authentication.session.SessionAuthenticationException; +import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; +import org.springframework.util.Assert; + +public final class PasswordAdviceSessionAuthenticationStrategy implements SessionAuthenticationStrategy { + + private PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository(); + + private PasswordAdvisor passwordAdvisor = new CompromisedPasswordAdvisor(); + + private final String passwordParameter; + + public PasswordAdviceSessionAuthenticationStrategy(String passwordParameter) { + this.passwordParameter = passwordParameter; + } + + @Override + public void onAuthentication(Authentication authentication, HttpServletRequest request, + HttpServletResponse response) throws SessionAuthenticationException { + UserDetails user = (UserDetails) authentication.getPrincipal(); + Assert.notNull(user, "cannot persist password advice since user principal is null"); + String password = request.getParameter(this.passwordParameter); + PasswordAdvice advice = this.passwordAdvisor.advise(user, password); + this.passwordAdviceRepository.savePasswordAdvice(request, response, advice); + } + + public void setPasswordAdviceRepository(PasswordAdviceRepository passwordAdviceRepository) { + this.passwordAdviceRepository = passwordAdviceRepository; + } + + public void setPasswordAdvisor(PasswordAdvisor passwordAdvisor) { + this.passwordAdvisor = passwordAdvisor; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java similarity index 79% rename from web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java rename to web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java index 6d6a630c296..10b315fd55b 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/ChangePasswordAdvisingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2004-present the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,8 +23,8 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.security.authentication.password.ChangePasswordAdvice; import org.springframework.security.authentication.password.PasswordAction; +import org.springframework.security.authentication.password.PasswordAdvice; import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.savedrequest.NullRequestCache; @@ -35,7 +35,7 @@ import static org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher.pathPattern; -public class ChangePasswordAdvisingFilter extends OncePerRequestFilter { +public class PasswordAdvisingFilter extends OncePerRequestFilter { private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); @@ -43,11 +43,11 @@ public class ChangePasswordAdvisingFilter extends OncePerRequestFilter { private RequestCache requestCache = new NullRequestCache(); - private ChangePasswordAdviceRepository changePasswordAdviceRepository = new HttpSessionChangePasswordAdviceRepository(); + private PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository(); private RequestMatcher requestMatcher; - public ChangePasswordAdvisingFilter(String changePasswordUrl) { + public PasswordAdvisingFilter(String changePasswordUrl) { this.changePasswordUrl = changePasswordUrl; this.requestMatcher = new NegatedRequestMatcher(pathPattern(changePasswordUrl)); } @@ -59,7 +59,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse chain.doFilter(request, response); return; } - ChangePasswordAdvice advice = this.changePasswordAdviceRepository.loadPasswordAdvice(request); + PasswordAdvice advice = this.passwordAdviceRepository.loadPasswordAdvice(request); if (advice.getAction() != PasswordAction.MUST_CHANGE) { chain.doFilter(request, response); return; @@ -68,8 +68,8 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse this.redirectStrategy.sendRedirect(request, response, this.changePasswordUrl); } - public void setChangePasswordAdviceRepository(ChangePasswordAdviceRepository changePasswordAdviceRepository) { - this.changePasswordAdviceRepository = changePasswordAdviceRepository; + public void setPasswordAdviceRepository(PasswordAdviceRepository passwordAdviceRepository) { + this.passwordAdviceRepository = passwordAdviceRepository; } public void setRequestCache(RequestCache requestCache) { From f7a03b84fa64390f8da91892f7d629bffc26a870 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 1 Aug 2025 17:18:14 -0600 Subject: [PATCH 09/10] Improvements - Improved class names - Introduced defaults to guide usage --- .../PasswordManagementConfigurerTests.java | 9 +- .../password/CompositePasswordAdvisor.java | 39 ++++---- .../CompositeUpdatePasswordAdvisor.java | 38 ++++---- .../password/PasswordAction.java | 4 +- .../password/PasswordAdvice.java | 2 - ...dvisor.java => PasswordLengthAdvisor.java} | 89 ++++++------------- .../password/RepeatedPasswordAdvisor.java | 13 ++- .../password/SimplePasswordAdvice.java | 42 +++++++++ .../password/UserDetailsPasswordAdvisor.java | 2 +- .../security/core/userdetails/User.java | 4 +- .../core/userdetails/UserDetails.java | 2 +- .../InMemoryUserDetailsManager.java | 2 +- .../password/CompromisedPasswordAdvisor.java | 22 ++--- .../HttpSessionPasswordAdviceRepository.java | 5 +- .../password/PasswordAdvisingFilter.java | 2 +- 15 files changed, 144 insertions(+), 131 deletions(-) rename core/src/main/java/org/springframework/security/authentication/password/{LengthPasswordAdvisor.java => PasswordLengthAdvisor.java} (50%) create mode 100644 core/src/main/java/org/springframework/security/authentication/password/SimplePasswordAdvice.java diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java index 03cddcc72a4..08cc6433ce6 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java @@ -288,10 +288,15 @@ PasswordAdvice index(PasswordAdvice advice) { ResponseEntity changePassword(@AuthenticationPrincipal UserDetails user, @RequestParam("password") String password, HttpServletRequest request, HttpServletResponse response) { PasswordAdvice advice = this.passwordAdvisor.advise(user, null, password); - if (advice.getAction() != PasswordAction.ABSTAIN) { + if (advice.getAction() != PasswordAction.NONE) { return ResponseEntity.badRequest().body(advice); } - this.passwords.changePassword(null, this.encoder.encode(password)); + UserDetails updated = User.withUserDetails(user) + .passwordEncoder(this.encoder::encode) + .password(password) + .passwordAction(PasswordAction.NONE) + .build(); + this.passwords.updateUser(updated); this.passwordAdviceRepository.removePasswordAdvice(request, response); return ResponseEntity.ok().build(); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java index 21ed6f5d426..13e0132a66f 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java @@ -18,7 +18,9 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.jspecify.annotations.Nullable; @@ -26,37 +28,45 @@ public final class CompositePasswordAdvisor implements PasswordAdvisor { - private final List advisors; + private final Collection advisors; - private CompositePasswordAdvisor(List advisors) { - this.advisors = Collections.unmodifiableList(advisors); + private CompositePasswordAdvisor(Collection advisors) { + this.advisors = Collections.unmodifiableCollection(advisors); } public static PasswordAdvisor of(PasswordAdvisor... advisors) { return new CompositePasswordAdvisor(List.of(advisors)); } + public static PasswordAdvisor withDefaults(PasswordAdvisor... advisors) { + Map, PasswordAdvisor> defaults = new HashMap<>(); + defaults.put(UserDetailsPasswordAdvisor.class, new UserDetailsPasswordAdvisor()); + defaults.put(PasswordLengthAdvisor.class, new PasswordLengthAdvisor()); + for (PasswordAdvisor advisor : advisors) { + defaults.put(advisor.getClass(), advisor); + } + return new CompositePasswordAdvisor(defaults.values()); + } + @Override public PasswordAdvice advise(UserDetails user, @Nullable String password) { Collection advice = this.advisors.stream() .map((advisor) -> advisor.advise(user, password)) .toList(); - return new Advice(advice); + return new CompositePasswordAdvice(advice); } - public static final class Advice implements PasswordAdvice { - - private final PasswordAction action; + public static final class CompositePasswordAdvice extends SimplePasswordAdvice { private final Collection advice; - private Advice(Collection advice) { - this.action = findMostUrgentAction(advice); + private CompositePasswordAdvice(Collection advice) { + super(findMostUrgentAction(advice)); this.advice = advice; } - private PasswordAction findMostUrgentAction(Collection advice) { - PasswordAction mostUrgentAction = PasswordAction.ABSTAIN; + private static PasswordAction findMostUrgentAction(Collection advice) { + PasswordAction mostUrgentAction = PasswordAction.NONE; for (PasswordAdvice a : advice) { if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { mostUrgentAction = a.getAction(); @@ -65,18 +75,13 @@ private PasswordAction findMostUrgentAction(Collection advice) { return mostUrgentAction; } - @Override - public PasswordAction getAction() { - return this.action; - } - public Collection getAdvice() { return this.advice; } @Override public String toString() { - return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]"; + return "Composite [" + "action=" + super.toString() + ", advice=" + this.advice + "]"; } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java index be343184556..681a9d70160 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java @@ -18,7 +18,9 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.jspecify.annotations.Nullable; @@ -26,37 +28,44 @@ public final class CompositeUpdatePasswordAdvisor implements UpdatePasswordAdvisor { - private final List advisors; + private final Collection advisors; - private CompositeUpdatePasswordAdvisor(List advisors) { - this.advisors = Collections.unmodifiableList(advisors); + private CompositeUpdatePasswordAdvisor(Collection advisors) { + this.advisors = Collections.unmodifiableCollection(advisors); } public static UpdatePasswordAdvisor of(UpdatePasswordAdvisor... advisors) { return new CompositeUpdatePasswordAdvisor(List.of(advisors)); } + public static UpdatePasswordAdvisor withDefaults(UpdatePasswordAdvisor... advisors) { + Map, UpdatePasswordAdvisor> defaults = new HashMap<>(); + defaults.put(RepeatedPasswordAdvisor.class, new RepeatedPasswordAdvisor()); + defaults.put(PasswordLengthAdvisor.class, new PasswordLengthAdvisor()); + for (UpdatePasswordAdvisor advisor : advisors) { + defaults.put(advisor.getClass(), advisor); + } + return new CompositeUpdatePasswordAdvisor(defaults.values()); + } @Override public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) { Collection advice = this.advisors.stream() .map((advisor) -> advisor.advise(user, oldPassword, newPassword)) .toList(); - return new CompositeUpdatePasswordAdvisor.Advice(advice); + return new CompositePasswordAdvice(advice); } - public static final class Advice implements PasswordAdvice { - - private final PasswordAction action; + public static final class CompositePasswordAdvice extends SimplePasswordAdvice { private final Collection advice; - private Advice(Collection advice) { - this.action = findMostUrgentAction(advice); + private CompositePasswordAdvice(Collection advice) { + super(findMostUrgentAction(advice)); this.advice = advice; } - private PasswordAction findMostUrgentAction(Collection advice) { - PasswordAction mostUrgentAction = PasswordAction.ABSTAIN; + private static PasswordAction findMostUrgentAction(Collection advice) { + PasswordAction mostUrgentAction = PasswordAction.NONE; for (PasswordAdvice a : advice) { if (mostUrgentAction.ordinal() < a.getAction().ordinal()) { mostUrgentAction = a.getAction(); @@ -65,18 +74,13 @@ private PasswordAction findMostUrgentAction(Collection advice) { return mostUrgentAction; } - @Override - public PasswordAction getAction() { - return this.action; - } - public Collection getAdvice() { return this.advice; } @Override public String toString() { - return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]"; + return "Composite [" + "action=" + super.toString() + ", advice=" + this.advice + "]"; } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java index 04690a5c9d6..c95ab63c504 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java @@ -18,9 +18,9 @@ public enum PasswordAction { - ABSTAIN, SHOULD_CHANGE, MUST_CHANGE; + NONE, SHOULD_CHANGE, MUST_CHANGE; - boolean advisedBy(PasswordAdvice advice) { + public boolean advisedBy(PasswordAdvice advice) { return advice.getAction().equals(this); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java index d7b35d57c28..30e89663962 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java @@ -21,8 +21,6 @@ @NullMarked public interface PasswordAdvice { - PasswordAdvice ABSTAIN = () -> PasswordAction.ABSTAIN; - PasswordAction getAction(); } diff --git a/core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/PasswordLengthAdvisor.java similarity index 50% rename from core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java rename to core/src/main/java/org/springframework/security/authentication/password/PasswordLengthAdvisor.java index 1e2a1004a66..fae525b1d58 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/PasswordLengthAdvisor.java @@ -21,25 +21,31 @@ import org.springframework.security.core.userdetails.UserDetails; import org.springframework.util.Assert; -public final class LengthPasswordAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor { +public final class PasswordLengthAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor { + + /** + * The ASVS v5.0 minimum password length + * @see ASVS + * 5.0 Password Security Standard + */ + private static final int ASVS_V5_MINIMUM_PASSWORD_LENGTH = 8; private final int minLength; private final int maxLength; - private PasswordAction tooShortAction = PasswordAction.MUST_CHANGE; - - private PasswordAction tooLongAction = PasswordAction.SHOULD_CHANGE; + private PasswordAction passwordAction = PasswordAction.SHOULD_CHANGE; - public LengthPasswordAdvisor() { - this(12, 64); + public PasswordLengthAdvisor() { + this(ASVS_V5_MINIMUM_PASSWORD_LENGTH); } - public LengthPasswordAdvisor(int minLength) { + public PasswordLengthAdvisor(int minLength) { this(minLength, Integer.MAX_VALUE); } - public LengthPasswordAdvisor(int minLength, int maxLength) { + public PasswordLengthAdvisor(int minLength, int maxLength) { Assert.isTrue(minLength > 0, "minLength must be greater than 0"); this.minLength = minLength; this.maxLength = maxLength; @@ -48,15 +54,15 @@ public LengthPasswordAdvisor(int minLength, int maxLength) { @Override public PasswordAdvice advise(UserDetails user, @Nullable String password) { if (password == null) { - return new TooShortAdvice(this.tooShortAction, this.minLength, 0); + return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, 0); } if (password.length() < this.minLength) { - return new TooShortAdvice(this.tooShortAction, this.minLength, password.length()); + return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, password.length()); } if (password.length() > this.maxLength) { - return new TooLongAdvice(this.tooLongAction, this.maxLength, password.length()); + return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, password.length()); } - return PasswordAdvice.ABSTAIN; + return SimplePasswordAdvice.NONE; } @Override @@ -64,66 +70,27 @@ public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nu return advise(user, newPassword); } - public void setTooShortAction(PasswordAction tooShortAction) { - this.tooShortAction = tooShortAction; + public void setPasswordAction(PasswordAction passwordAction) { + this.passwordAction = passwordAction; } - public void setTooLongAction(PasswordAction tooLongAction) { - this.tooLongAction = tooLongAction; - } - - public static final class TooShortAdvice implements PasswordAdvice { - - private final PasswordAction action; + public static final class PasswordLengthAdvice extends SimplePasswordAdvice { private final int minLength; - private final int actualLength; - - private TooShortAdvice(PasswordAction action, int minLength, int actualLength) { - this.action = action; - this.minLength = minLength; - this.actualLength = actualLength; - } - - @Override - public PasswordAction getAction() { - return this.action; - } - - public int getMinLength() { - return this.minLength; - } - - public int getActualLength() { - return this.actualLength; - } - - @Override - public String toString() { - return "TooShort [" + "action=" + this.action + ", minLength=" + this.minLength + ", actualLength=" - + this.actualLength + "]"; - } - - } - - public static final class TooLongAdvice implements PasswordAdvice { - - private final PasswordAction action; - private final int maxLength; private final int actualLength; - private TooLongAdvice(PasswordAction action, int maxLength, int actualLength) { - this.action = action; + private PasswordLengthAdvice(PasswordAction action, int minLength, int maxLength, int actualLength) { + super(action); + this.minLength = minLength; this.maxLength = maxLength; this.actualLength = actualLength; } - @Override - public PasswordAction getAction() { - return this.action; + public int getMinLength() { + return this.minLength; } public int getMaxLength() { @@ -136,8 +103,8 @@ public int getActualLength() { @Override public String toString() { - return "TooLong [" + "action=" + this.action + ", maxLength=" + this.maxLength + ", actualLength=" - + this.actualLength + "]"; + return "Length [action=" + super.toString() + ", minLength=" + this.minLength + ", maxLength=" + + this.maxLength + ", actualLength=" + this.actualLength + "]"; } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java index fa933194766..a3322ef0398 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java @@ -26,17 +26,14 @@ public final class RepeatedPasswordAdvisor implements UpdatePasswordAdvisor { @Override public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) { - if (Objects.equals(oldPassword, newPassword)) { - return new Advice(); - } - return PasswordAdvice.ABSTAIN; + boolean repeated = Objects.equals(oldPassword, newPassword); + return new RepeatedPasswordAdvice(repeated ? PasswordAction.MUST_CHANGE : PasswordAction.NONE); } - public static final class Advice implements PasswordAdvice { + public static final class RepeatedPasswordAdvice extends SimplePasswordAdvice { - @Override - public PasswordAction getAction() { - return PasswordAction.MUST_CHANGE; + RepeatedPasswordAdvice(PasswordAction action) { + super(action); } } diff --git a/core/src/main/java/org/springframework/security/authentication/password/SimplePasswordAdvice.java b/core/src/main/java/org/springframework/security/authentication/password/SimplePasswordAdvice.java new file mode 100644 index 00000000000..e32028ed17c --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/password/SimplePasswordAdvice.java @@ -0,0 +1,42 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.authentication.password; + +import org.jspecify.annotations.NullMarked; + +@NullMarked +public class SimplePasswordAdvice implements PasswordAdvice { + + public static final PasswordAdvice NONE = new SimplePasswordAdvice(PasswordAction.NONE); + + private final PasswordAction action; + + public SimplePasswordAdvice(PasswordAction action) { + this.action = action; + } + + @Override + public PasswordAction getAction() { + return this.action; + } + + @Override + public String toString() { + return "Simple [action=" + this.action + "]"; + } + +} diff --git a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java index ef9e8bc52d6..d417a510aef 100644 --- a/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java +++ b/core/src/main/java/org/springframework/security/authentication/password/UserDetailsPasswordAdvisor.java @@ -24,7 +24,7 @@ public final class UserDetailsPasswordAdvisor implements PasswordAdvisor { @Override public PasswordAdvice advise(UserDetails user, @Nullable String password) { - return user::getPasswordAction; + return new SimplePasswordAdvice(user.getPasswordAction()); } } diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index 8249a7d2a7a..cd7ee5f233c 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -114,7 +114,7 @@ public User(String username, @Nullable String password, boolean enabled, boolean Assert.isTrue(username != null && !"".equals(username), "Cannot pass null or empty values to constructor"); this.username = username; this.password = password; - this.passwordAction = PasswordAction.ABSTAIN; + this.passwordAction = PasswordAction.NONE; this.enabled = enabled; this.accountNonExpired = accountNonExpired; this.credentialsNonExpired = credentialsNonExpired; @@ -356,7 +356,7 @@ public static final class UserBuilder { private @Nullable String password; - private PasswordAction passwordAction = PasswordAction.ABSTAIN; + private PasswordAction passwordAction = PasswordAction.NONE; private List authorities = new ArrayList<>(); diff --git a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java index dec5a6cf91e..c9dd503c2ed 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/UserDetails.java @@ -104,7 +104,7 @@ default boolean isEnabled() { } default PasswordAction getPasswordAction() { - return PasswordAction.ABSTAIN; + return PasswordAction.NONE; } } diff --git a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java index e729bdf523e..f3a34062f1c 100644 --- a/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java @@ -153,7 +153,7 @@ public void changePassword(String oldPassword, String newPassword) { MutableUserDetails user = this.users.get(username); Assert.state(user != null, "Current user doesn't exist in database."); user.setPassword(newPassword); - user.setPasswordAction(PasswordAction.ABSTAIN); + user.setPasswordAction(PasswordAction.NONE); } @Override diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java b/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java index 0113e8adff3..41a24291e77 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/CompromisedPasswordAdvisor.java @@ -23,6 +23,7 @@ import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.authentication.password.PasswordAdvice; import org.springframework.security.authentication.password.PasswordAdvisor; +import org.springframework.security.authentication.password.SimplePasswordAdvice; import org.springframework.security.authentication.password.UpdatePasswordAdvisor; import org.springframework.security.core.userdetails.UserDetails; @@ -35,14 +36,14 @@ public final class CompromisedPasswordAdvisor implements PasswordAdvisor, Update @Override public PasswordAdvice advise(UserDetails user, @Nullable String password) { if (password == null) { - return PasswordAdvice.ABSTAIN; + return SimplePasswordAdvice.NONE; } CompromisedPasswordDecision decision = this.pwned.check(password); if (decision.isCompromised()) { - return new Advice(this.action, decision); + return new CompromisedPasswordAdvice(this.action, decision); } else { - return new Advice(PasswordAction.ABSTAIN, decision); + return new CompromisedPasswordAdvice(PasswordAction.NONE, decision); } } @@ -55,14 +56,12 @@ public void setAction(PasswordAction action) { this.action = action; } - public static final class Advice implements PasswordAdvice { - - private final PasswordAction action; + public static final class CompromisedPasswordAdvice extends SimplePasswordAdvice { private final CompromisedPasswordDecision decision; - public Advice(PasswordAction action, CompromisedPasswordDecision decision) { - this.action = action; + public CompromisedPasswordAdvice(PasswordAction action, CompromisedPasswordDecision decision) { + super(action); this.decision = decision; } @@ -70,14 +69,9 @@ public CompromisedPasswordDecision getCompromisedPasswordDecision() { return this.decision; } - @Override - public PasswordAction getAction() { - return this.action; - } - @Override public String toString() { - return "Compromised [" + "action=" + this.action + ", decision=" + this.decision + "]"; + return "Compromised [" + "action=" + super.toString() + ", decision=" + this.decision + "]"; } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java index 1ed2a9bc463..c964c1de553 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/HttpSessionPasswordAdviceRepository.java @@ -23,6 +23,7 @@ import org.springframework.security.authentication.password.PasswordAction; import org.springframework.security.authentication.password.PasswordAdvice; +import org.springframework.security.authentication.password.SimplePasswordAdvice; import org.springframework.util.function.SingletonSupplier; public final class HttpSessionPasswordAdviceRepository implements PasswordAdviceRepository { @@ -37,13 +38,13 @@ public PasswordAdvice loadPasswordAdvice(HttpServletRequest request) { if (advice != null) { return advice; } - return PasswordAdvice.ABSTAIN; + return SimplePasswordAdvice.NONE; }); } @Override public void savePasswordAdvice(HttpServletRequest request, HttpServletResponse response, PasswordAdvice advice) { - if (advice.getAction() == PasswordAction.ABSTAIN) { + if (PasswordAction.NONE.advisedBy(advice)) { removePasswordAdvice(request, response); return; } diff --git a/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java index 10b315fd55b..b2138add656 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/password/PasswordAdvisingFilter.java @@ -60,7 +60,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse return; } PasswordAdvice advice = this.passwordAdviceRepository.loadPasswordAdvice(request); - if (advice.getAction() != PasswordAction.MUST_CHANGE) { + if (!PasswordAction.MUST_CHANGE.advisedBy(advice)) { chain.doFilter(request, response); return; } From 3af8f3ff4d46333d0f9ea0972825e3d679f56d6c Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 1 Aug 2025 17:18:22 -0600 Subject: [PATCH 10/10] Added Documentation --- docs/modules/ROOT/nav.adoc | 1 + .../authentication/passwords/management.adoc | 213 ++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 docs/modules/ROOT/pages/servlet/authentication/passwords/management.adoc diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc index 65e88a720ea..aa8be7c6fc6 100644 --- a/docs/modules/ROOT/nav.adoc +++ b/docs/modules/ROOT/nav.adoc @@ -48,6 +48,7 @@ ***** xref:servlet/authentication/passwords/password-encoder.adoc[PasswordEncoder] ***** xref:servlet/authentication/passwords/dao-authentication-provider.adoc[DaoAuthenticationProvider] ***** xref:servlet/authentication/passwords/ldap.adoc[LDAP] +***** xref:servlet/authentication/passwords/management.adoc[Password Management] *** xref:servlet/authentication/persistence.adoc[Persistence] *** xref:servlet/authentication/passkeys.adoc[Passkeys] *** xref:servlet/authentication/onetimetoken.adoc[One-Time Token] diff --git a/docs/modules/ROOT/pages/servlet/authentication/passwords/management.adoc b/docs/modules/ROOT/pages/servlet/authentication/passwords/management.adoc new file mode 100644 index 00000000000..9b32cf6a26c --- /dev/null +++ b/docs/modules/ROOT/pages/servlet/authentication/passwords/management.adoc @@ -0,0 +1,213 @@ += Password Management + +Spring Security can offer advice about passwords through its `PasswordAdvisor` API. +It aims to help applications apply https://github.com/OWASP/ASVS/blob/v5.0.0/5.0/docs_en/OWASP_Application_Security_Verification_Standard_5.0.0_en.csv#L108[the ASVS 5.0 standard for Password Security]. + +Consider the following configuration: + +.Java +[source,java,role="primary"] +---- +http + .authorizeHttpRequests((authorize) -> authorize.anyRequest().authenticated()) + .formLogin(Customizer.withDefaults()) + .passwordManagement(Customizer.withDefaults()) +---- + +By adding the `passwordManagement` DSL, your application now has the ability to suggest or require a user to change their password if certain criteria are met. + +By default, `UserDetailsPasswordAdvisor` is consulted in a `SessionAuthenticationStrategy` after login is complete. +It calls `UserDetails#getPasswordAdvice` to look up any password advice stored on the user object. +If the advice on the user object is `PasswordAdvice.MUST_CHANGE`, then Spring Security will redirect the application to `/change-password` for every request in the application. + +== Configuring a Password Advisor + +There are wo kinds of advisors, `PasswordAdvisor` and `UpdatePasswordAdvisor`. +The first advisor type is for analyzing the password at authentication time. +This is useful should your website's password standards change or should the password become compromised and leaked. +It is also useful when your administrator has marked a certain user as needing to update their password, regardless of any other analysis. + +To take advantage of these advisors, you can publish a `PasswordAdvisor` as a bean: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +PasswordAdvisor passwordAdvisor() { + return CompositePasswordAdvisor.withDefaults( // <1> + new CompromisedPasswordAdvisor() // <2> + ); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun passwordAdvisor(): PasswordAdvisor { + return CompositePasswordAdvisor.withDefaults( // <1> + CompromisedPasswordAdvisor() // <2> + ) +} +---- +====== +<1> - `withDefaults` adds an advisor that checks the `UserDetails` object for any admin-set password action and an advisor that checks password length +<2> - An advisor that checks the HaveIBeenPwned breached password database + +== Requiring Password Changes + +By default password advisors mark a password as `SHOULD_CHANGE`. +This allows you to add this to your application passively. + +In the event you want to start requiring that users change their password, you can configure each password advisor with a policy. +For example, you can state that whenever a password is compromised, force the user to update their password at that time by configuring the `CompromisedPasswordAdvisor` policy as `MUST_CHANGE`: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +PasswordAdvisor passwordAdvisor() { + CompromisedPasswordAdvisor compromised = new CompromisedPasswordAdvisor(); + compromised.setAction(PasswordAction.MUST_CHANGE); + return CompositePasswordAdvisor.withDefaults(compromised); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun passwordAdvisor(): PasswordAdvisor { + val compromised = CompromisedPasswordAdvisor() + compromised.setAction(PasswordAction.MUST_CHANGE) + return CompositePasswordAdvisor.withDefaults(compromise) +} +---- +====== + +[TIP] +==== +While optional, it's helpful to include `UserDetailsPasswordAdvisor` in the set of password advisors as this allows admins to update the `passwordAction` value in `UserDetails` out-of-band and thus require password changes en masse. +This is included by default when calling `withDefaults`. +==== + +== Updating Passwords + +When a user updates their password, the following are recommended: + +1. You require the user provide their old password +2. You require the user provide and confirm their new password +3. You test the password against a set of `UpdatePasswordAdvisor` instances +4. You update both the password and any remaining password action +5. You log out the individual so they can re-login with their new password + +Here is a sample controller that does this: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Controller +class ChangePasswordController { + private final InMemoryUserDetailsManager users; + + private final PasswordEncoder passwordEncoder = + PasswordEncoderFactories.createDelegatingPasswordEncoder(); + + private final UpdatePasswordAdvisor passwordAdvisor = CompositeUpdatePasswordAdvisor.withDefaults( + new CompromisedPasswordAdvisor(), + new LengthPasswordAdvisor(12) // <1> + ); + + // constructor + + @PostMapping("/change-password") + String changePassword(Passwords passwords, @AuthenticationPrincipal UserDetails user, + HttpServletRequest request, HttpServletResponse response) { + + UserDetails latest = this.users.findUserByUsername(user.getUsername()); + if (!this.passwordEncoder.matches(latest.getPassword(), passwords.current())) { // <2> + request.setAttribute("error", "The provided current password doesn't match your password on file."); + return "change-password"; + } + if (!passwords.change().equals(passwords.confirm())) { // <3> + request.setAttribute("error", "The new password doesn't match its confirmation."); + return "change-password"; + } + PasswordAdvice advice = this.passwordAdvisor.advise(latest, latest.getPassword(), passwords.change()); // <4> + if (PasswordAction.NONE.advisedBy(advice)) { + UserDetails updated = User.withUserDetails(latest) + .passwordEncoder(this.passwordEncoder::encode) + .password(passwords.change()) + .passwordAction(PasswordAction.NONE).build(); // <5> + this.users.updateUser(updated); + return "forward:/logout"; // <6> + } + request.setAttribute(error, "Your password was rejected since " + advice); + return "change-password"; + } +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Controller +open class ChangePasswordController { + private val users: InMemoryUserDetailsManager + + private val passwordEncoder = + PasswordEncoderFactories.createDelegatingPasswordEncoder() + + private val passwordAdvisor = CompositeUpdatePasswordAdvisor.of( + CompromisedPasswordAdvisor(), + LengthPasswordAdvisor(12) // <1> + ) + + // constructor + + @PostMapping("/change-password") + fun changePassword(val passwords: Passwords, @AuthenticationPrincipal val user: UserDetails, + val request: HttpServletRequest): String { + + val latest = this.users.findUserByUsername(user.getUsername()) + if (!this.passwordEncoder.matches(latest.getPassword(), passwords.current())) { // <2> + request.setAttribute("error", "The provided current password doesn't match your password on file.") + return "change-password" + } + if (!passwords.change().equals(passwords.confirm())) { // <3> + request.setAttribute("error", "The new password doesn't match its confirmation.") + return "change-password" + } + val advice = this.passwordAdvisor.advise(latest, latest.getPassword(), passwords.change()) // <4> + if (PasswordAction.NONE.advisedBy(advice)) { + val updated = User.withUserDetails(latest) + .passwordEncoder(this.passwordEncoder::encode) + .password(passwords.change()) + .passwordAction(PasswordAction.NONE).build() // <5> + this.users.updateUser(updated) + return "forward:/logout" // <6> + } + request.setAttribute(error, "Your password was rejected since " + advice) + return "change-password" + } +} +---- +====== +<1> - Override the default `PasswordLengthAdvisor` to require a minimum length of 12 +<2> - Test that the user's current password matches the provided password; note that since credentials are often erased during login, you'll need to look up the user in order to check their password +<3> - Test that the new password and the confirmation fields match +<4> - Test the password against various criteria +<5> - If all the is met, then update the `UserDetails` object to have the new password and no more password advice +<6> - Forward to /logout to get the person logged out