Skip to content

Add Password Advice Support #17118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,9 @@ public HttpSecurity httpBasic(Customizer<HttpBasicConfigurer<HttpSecurity>> http
*/
public HttpSecurity passwordManagement(
Customizer<PasswordManagementConfigurer<HttpSecurity>> passwordManagementCustomizer) throws Exception {
passwordManagementCustomizer.customize(getOrApply(new PasswordManagementConfigurer<>()));
PasswordManagementConfigurer<HttpSecurity> passwordManagement = new PasswordManagementConfigurer<>();
passwordManagement.setApplicationContext(getContext());
passwordManagementCustomizer.customize(getOrApply(passwordManagement));
return HttpSecurity.this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.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;
Expand Down Expand Up @@ -72,6 +75,8 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex

private AnnotationTemplateExpressionDefaults templateDefaults;

private PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository();

@Override
@SuppressWarnings("deprecation")
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers) {
Expand All @@ -88,6 +93,9 @@ public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentRes
currentSecurityContextArgumentResolver.setTemplateDefaults(this.templateDefaults);
argumentResolvers.add(currentSecurityContextArgumentResolver);
argumentResolvers.add(new CsrfTokenArgumentResolver());
PasswordAdviceMethodArgumentResolver resolver = new PasswordAdviceMethodArgumentResolver();
resolver.setPasswordAdviceRepository(this.passwordAdviceRepository);
argumentResolvers.add(resolver);
}

@Bean
Expand All @@ -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(PasswordAdviceRepository.class).length == 1) {
this.passwordAdviceRepository = applicationContext.getBean(PasswordAdviceRepository.class);
}
}

/**
Expand Down Expand Up @@ -209,7 +220,7 @@ private static Filter createDoFilterDelegate(List<? extends Filter> 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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,19 @@

package org.springframework.security.config.annotation.web.configurers;

import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
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.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;

/**
Expand All @@ -28,14 +38,22 @@
* @since 5.6
*/
public final class PasswordManagementConfigurer<B extends HttpSecurityBuilder<B>>
extends AbstractHttpConfigurer<PasswordManagementConfigurer<B>, B> {
extends AbstractHttpConfigurer<PasswordManagementConfigurer<B>, 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 ApplicationContext context;

private boolean customChangePasswordPage = false;

private String changePasswordPage = DEFAULT_CHANGE_PASSWORD_PAGE;

private PasswordAdviceRepository passwordAdviceRepository;

private PasswordAdvisor passwordAdvisor;

/**
* Sets the change password page. Defaults to
* {@link PasswordManagementConfigurer#DEFAULT_CHANGE_PASSWORD_PAGE}.
Expand All @@ -45,9 +63,44 @@ public final class PasswordManagementConfigurer<B extends HttpSecurityBuilder<B>
public PasswordManagementConfigurer<B> changePasswordPage(String changePasswordPage) {
Assert.hasText(changePasswordPage, "changePasswordPage cannot be empty");
this.changePasswordPage = changePasswordPage;
this.customChangePasswordPage = true;
return this;
}

public PasswordManagementConfigurer<B> passwordAdviceRepository(PasswordAdviceRepository passwordAdviceRepository) {
this.passwordAdviceRepository = passwordAdviceRepository;
return this;
}

public PasswordManagementConfigurer<B> passwordAdvisor(PasswordAdvisor passwordAdvisor) {
this.passwordAdvisor = passwordAdvisor;
return this;
}

@Override
public void init(B http) throws Exception {
PasswordAdviceRepository passwordAdviceRepository = (this.passwordAdviceRepository != null)
? this.passwordAdviceRepository : this.context.getBeanProvider(PasswordAdviceRepository.class)
.getIfUnique(HttpSessionPasswordAdviceRepository::new);

PasswordAdvisor passwordAdvisor = (this.passwordAdvisor != null) ? this.passwordAdvisor
: this.context.getBeanProvider(PasswordAdvisor.class).getIfUnique(UserDetailsPasswordAdvisor::new);

http.setSharedObject(PasswordAdviceRepository.class, passwordAdviceRepository);

String passwordParameter = "password";
FormLoginConfigurer<B> form = http.getConfigurer(FormLoginConfigurer.class);
if (form != null) {
passwordParameter = form.getPasswordParameter();
}
PasswordAdviceSessionAuthenticationStrategy sessionAuthenticationStrategy = new PasswordAdviceSessionAuthenticationStrategy(
passwordParameter);
sessionAuthenticationStrategy.setPasswordAdviceRepository(passwordAdviceRepository);
sessionAuthenticationStrategy.setPasswordAdvisor(passwordAdvisor);
http.getConfigurer(SessionManagementConfigurer.class)
.addSessionAuthenticationStrategy(sessionAuthenticationStrategy);
}

/**
* {@inheritDoc}
*/
Expand All @@ -56,6 +109,16 @@ 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);

PasswordAdvisingFilter advising = new PasswordAdvisingFilter(this.changePasswordPage);
advising.setPasswordAdviceRepository(http.getSharedObject(PasswordAdviceRepository.class));
advising.setRequestCache(http.getSharedObject(RequestCache.class));
http.addFilterBefore(advising, RequestCacheAwareFilter.class);
}

@Override
public void setApplicationContext(ApplicationContext context) {
this.context = context;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,58 @@

package org.springframework.security.config.annotation.web.configurers;

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;

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.PasswordAction;
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;
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.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.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;
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;

Expand Down Expand Up @@ -87,6 +123,52 @@ 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("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());
// .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("password", random))
.andExpect(status().isOk());
// now we're good
this.mvc.perform(get("/").session(session)).andExpect(status().isOk());
}

@Test
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"))
.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("SHOULD_CHANGE")));
}

@Configuration
@EnableWebSecurity
static class PasswordManagementWithDefaultChangePasswordPageConfig {
Expand Down Expand Up @@ -119,4 +201,106 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception {

}

@Configuration
@EnableWebSecurity
@EnableWebMvc
static class PasswordManagementConfig {

@Bean
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());
// @formatter:on
return http.build();
}

@Bean
InMemoryUserDetailsManager users() {
UserDetails shouldChange = User.withUserDetails(PasswordEncodedUser.user())
.passwordAction(PasswordAction.SHOULD_CHANGE)
.build();
UserDetails admin = PasswordEncodedUser.admin();
return new InMemoryUserDetailsManager(shouldChange, admin);
}

}

@RequestMapping("/admin/passwords")
@RestController
static class AdminController {

private final UserDetailsManager users;

AdminController(InMemoryUserDetailsManager users) {
this.users = users;
}

@GetMapping("/advice/{username}")
ResponseEntity<PasswordAction> requireChangePassword(@PathVariable("username") String username) {
UserDetails user = this.users.loadUserByUsername(username);
if (user == null) {
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(user.getPasswordAction());
}

@PostMapping("/expire/{username}")
ResponseEntity<PasswordAction> expirePassword(@PathVariable("username") String username) {
UserDetails user = this.users.loadUserByUsername(username);
if (user == null) {
return ResponseEntity.notFound().build();
}
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);
}

}

@RestController
static class HomeController {

private final InMemoryUserDetailsManager passwords;

private final UpdatePasswordAdvisor passwordAdvisor = new CompromisedPasswordAdvisor();

private final PasswordAdviceRepository passwordAdviceRepository = new HttpSessionPasswordAdviceRepository();

private final PasswordEncoder encoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();

HomeController(InMemoryUserDetailsManager passwords) {
this.passwords = passwords;
}

@GetMapping
PasswordAdvice index(PasswordAdvice advice) {
return advice;
}

@PostMapping("/change-password")
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.NONE) {
return ResponseEntity.badRequest().body(advice);
}
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();
}

}

}
Loading
Loading