Skip to content

Commit

Permalink
Add Compromised Password Checker
Browse files Browse the repository at this point in the history
Closes gh-7395
  • Loading branch information
marcusdacoregio committed Apr 1, 2024
1 parent 89175df commit 7d66525
Show file tree
Hide file tree
Showing 19 changed files with 1,100 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.springframework.core.annotation.Order;
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.core.password.CompromisedPasswordChecker;
import org.springframework.security.core.userdetails.UserDetailsPasswordService;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.crypto.password.PasswordEncoder;
Expand Down Expand Up @@ -65,6 +66,7 @@ public void configure(AuthenticationManagerBuilder auth) throws Exception {
}
PasswordEncoder passwordEncoder = getBeanOrNull(PasswordEncoder.class);
UserDetailsPasswordService passwordManager = getBeanOrNull(UserDetailsPasswordService.class);
CompromisedPasswordChecker passwordChecker = getBeanOrNull(CompromisedPasswordChecker.class);
DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
provider.setUserDetailsService(userDetailsService);
if (passwordEncoder != null) {
Expand All @@ -73,6 +75,9 @@ public void configure(AuthenticationManagerBuilder auth) throws Exception {
if (passwordManager != null) {
provider.setUserDetailsPasswordService(passwordManager);
}
if (passwordChecker != null) {
provider.setCompromisedPasswordChecker(passwordChecker);
}
provider.afterPropertiesSet();
auth.authenticationProvider(provider);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.springframework.security.authentication.ReactiveAuthenticationManager;
import org.springframework.security.authentication.UserDetailsRepositoryReactiveAuthenticationManager;
import org.springframework.security.config.web.server.ServerHttpSecurity;
import org.springframework.security.core.password.ReactiveCompromisedPasswordChecker;
import org.springframework.security.core.userdetails.ReactiveUserDetailsPasswordService;
import org.springframework.security.core.userdetails.ReactiveUserDetailsService;
import org.springframework.security.crypto.password.PasswordEncoder;
Expand Down Expand Up @@ -63,6 +64,8 @@ class ServerHttpSecurityConfiguration {

private ReactiveUserDetailsPasswordService userDetailsPasswordService;

private ReactiveCompromisedPasswordChecker compromisedPasswordChecker;

private ObservationRegistry observationRegistry = ObservationRegistry.NOOP;

@Autowired(required = false)
Expand Down Expand Up @@ -98,6 +101,11 @@ void setObservationRegistry(ObservationRegistry observationRegistry) {
this.observationRegistry = observationRegistry;
}

@Autowired(required = false)
void setCompromisedPasswordChecker(ReactiveCompromisedPasswordChecker compromisedPasswordChecker) {
this.compromisedPasswordChecker = compromisedPasswordChecker;
}

@Bean
static WebFluxConfigurer authenticationPrincipalArgumentResolverConfigurer(
ObjectProvider<AuthenticationPrincipalArgumentResolver> authenticationPrincipalArgumentResolver) {
Expand Down Expand Up @@ -153,6 +161,7 @@ private ReactiveAuthenticationManager authenticationManager() {
manager.setPasswordEncoder(this.passwordEncoder);
}
manager.setUserDetailsPasswordService(this.userDetailsPasswordService);
manager.setCompromisedPasswordChecker(this.compromisedPasswordChecker);
if (!this.observationRegistry.isNoop()) {
return new ObservationReactiveAuthenticationManager(this.observationRegistry, manager);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,13 @@
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.security.core.password.CompromisedPasswordCheckResult;
import org.springframework.security.core.password.CompromisedPasswordChecker;
import org.springframework.security.core.password.CompromisedPasswordException;
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.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.provisioning.UserDetailsManager;
import org.springframework.security.test.web.servlet.RequestCacheResultMatcher;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
Expand Down Expand Up @@ -395,6 +398,41 @@ public void configureWhenCustomDslAddedFromFactoriesAndDisablingUsingWithThenNot
this.mockMvc.perform(formLogin()).andExpectAll(status().isNotFound(), unauthenticated());
}

@Test
void loginWhenCompromisePasswordCheckerConfiguredAndPasswordCompromisedThenUnauthorized() throws Exception {
this.spring
.register(SecurityEnabledConfig.class, UserDetailsConfig.class, CompromisedPasswordCheckerConfig.class)
.autowire();
this.mockMvc.perform(formLogin().password("password"))
.andExpectAll(status().isFound(), redirectedUrl("/login?error"), unauthenticated());
}

@Test
void loginWhenCompromisedPasswordAndRedirectIfPasswordExceptionThenRedirectedToResetPassword() throws Exception {
this.spring
.register(SecurityEnabledRedirectIfPasswordExceptionConfig.class, UserDetailsConfig.class,
CompromisedPasswordCheckerConfig.class)
.autowire();
this.mockMvc.perform(formLogin().password("password"))
.andExpectAll(status().isFound(), redirectedUrl("/reset-password"), unauthenticated());
}

@Test
void loginWhenCompromisePasswordCheckerConfiguredAndPasswordNotCompromisedThenSuccess() throws Exception {
this.spring
.register(SecurityEnabledConfig.class, UserDetailsConfig.class, CompromisedPasswordCheckerConfig.class)
.autowire();
UserDetailsManager userDetailsManager = this.spring.getContext().getBean(UserDetailsManager.class);
UserDetails notCompromisedPwUser = User.withDefaultPasswordEncoder()
.username("user2")
.password("password2")
.roles("USER")
.build();
userDetailsManager.createUser(notCompromisedPwUser);
this.mockMvc.perform(formLogin().user("user2").password("password2"))
.andExpectAll(status().isFound(), redirectedUrl("/"), authenticated());
}

@RestController
static class NameController {

Expand Down Expand Up @@ -455,7 +493,7 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
static class UserDetailsConfig {

@Bean
UserDetailsService userDetailsService() {
InMemoryUserDetailsManager userDetailsService() {
// @formatter:off
UserDetails user = User.withDefaultPasswordEncoder()
.username("user")
Expand Down Expand Up @@ -732,4 +770,52 @@ public void init(HttpSecurity builder) throws Exception {

}

@Configuration(proxyBeanMethods = false)
static class CompromisedPasswordCheckerConfig {

@Bean
TestCompromisedPasswordChecker compromisedPasswordChecker() {
return new TestCompromisedPasswordChecker();
}

}

@Configuration(proxyBeanMethods = false)
@EnableWebSecurity
static class SecurityEnabledRedirectIfPasswordExceptionConfig {

@Bean
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// @formatter:off
return http
.authorizeHttpRequests((authorize) -> authorize
.anyRequest().authenticated()
)
.formLogin((form) -> form
.failureHandler((request, response, exception) -> {
if (exception instanceof CompromisedPasswordException) {
response.sendRedirect("/reset-password");
return;
}
response.sendRedirect("/login?error");
})
)
.build();
// @formatter:on
}

}

private static class TestCompromisedPasswordChecker implements CompromisedPasswordChecker {

@Override
public CompromisedPasswordCheckResult check(String password) {
if ("password".equals(password)) {
return new CompromisedPasswordCheckResult(true);
}
return new CompromisedPasswordCheckResult(false);
}

}

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

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

import java.net.URI;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import reactor.core.publisher.Mono;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.Customizer;
import org.springframework.security.config.test.SpringTestContext;
import org.springframework.security.config.test.SpringTestContextExtension;
import org.springframework.security.config.users.ReactiveAuthenticationTestConfiguration;
import org.springframework.security.config.web.server.ServerHttpSecurity;
import org.springframework.security.core.password.CompromisedPasswordCheckResult;
import org.springframework.security.core.password.CompromisedPasswordException;
import org.springframework.security.core.password.ReactiveCompromisedPasswordChecker;
import org.springframework.security.core.userdetails.MapReactiveUserDetailsService;
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.web.server.DefaultServerRedirectStrategy;
import org.springframework.security.web.server.SecurityWebFilterChain;
import org.springframework.test.web.reactive.server.WebTestClient;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.reactive.config.EnableWebFlux;
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.server.adapter.WebHttpHandlerBuilder;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers.csrf;

/**
* Tests for {@link ServerHttpSecurityConfiguration}.
Expand All @@ -37,6 +60,16 @@ public class ServerHttpSecurityConfigurationTests {

public final SpringTestContext spring = new SpringTestContext(this);

WebTestClient webClient;

@Autowired
void setup(ApplicationContext context) {
if (!context.containsBean(WebHttpHandlerBuilder.WEB_HANDLER_BEAN_NAME)) {
return;
}
this.webClient = WebTestClient.bindToApplicationContext(context).configureClient().build();
}

@Test
public void loadConfigWhenReactiveUserDetailsServiceConfiguredThenServerHttpSecurityExists() {
this.spring
Expand All @@ -57,9 +90,151 @@ public void loadConfigWhenProxyingEnabledAndSubclassThenServerHttpSecurityExists
assertThat(serverHttpSecurity).isNotNull();
}

@Test
void loginWhenCompromisePasswordCheckerConfiguredAndPasswordCompromisedThenUnauthorized() {
this.spring.register(FormLoginConfig.class, UserDetailsConfig.class, CompromisedPasswordCheckerConfig.class)
.autowire();
MultiValueMap<String, String> data = new LinkedMultiValueMap<>();
data.add("username", "user");
data.add("password", "password");
// @formatter:off
this.webClient.mutateWith(csrf())
.post()
.uri("/login")
.body(BodyInserters.fromFormData(data))
.exchange()
.expectStatus().is3xxRedirection()
.expectHeader().location("/login?error");
// @formatter:on
}

@Test
void loginWhenCompromisePasswordCheckerConfiguredAndPasswordNotCompromisedThenUnauthorized() {
this.spring.register(FormLoginConfig.class, UserDetailsConfig.class, CompromisedPasswordCheckerConfig.class)
.autowire();
MultiValueMap<String, String> data = new LinkedMultiValueMap<>();
data.add("username", "admin");
data.add("password", "password2");
// @formatter:off
this.webClient.mutateWith(csrf())
.post()
.uri("/login")
.body(BodyInserters.fromFormData(data))
.exchange()
.expectStatus().is3xxRedirection()
.expectHeader().location("/");
// @formatter:on
}

@Test
void loginWhenCompromisedPasswordAndRedirectIfPasswordExceptionThenRedirectedToResetPassword() {
this.spring
.register(FormLoginRedirectToResetPasswordConfig.class, UserDetailsConfig.class,
CompromisedPasswordCheckerConfig.class)
.autowire();
MultiValueMap<String, String> data = new LinkedMultiValueMap<>();
data.add("username", "user");
data.add("password", "password");
// @formatter:off
this.webClient.mutateWith(csrf())
.post()
.uri("/login")
.body(BodyInserters.fromFormData(data))
.exchange()
.expectStatus().is3xxRedirection()
.expectHeader().location("/reset-password");
// @formatter:on
}

@Configuration
static class SubclassConfig extends ServerHttpSecurityConfiguration {

}

@Configuration(proxyBeanMethods = false)
@EnableWebFlux
@EnableWebFluxSecurity
static class FormLoginConfig {

@Bean
SecurityWebFilterChain filterChain(ServerHttpSecurity http) {
// @formatter:off
http
.authorizeExchange((exchange) -> exchange
.anyExchange().authenticated()
)
.formLogin(Customizer.withDefaults());
// @formatter:on
return http.build();
}

}

@Configuration(proxyBeanMethods = false)
@EnableWebFlux
@EnableWebFluxSecurity
static class FormLoginRedirectToResetPasswordConfig {

@Bean
SecurityWebFilterChain filterChain(ServerHttpSecurity http) {
// @formatter:off
http
.authorizeExchange((exchange) -> exchange
.anyExchange().authenticated()
)
.formLogin((form) -> form
.authenticationFailureHandler((webFilterExchange, exception) -> {
String redirectUrl = "/login?error";
if (exception instanceof CompromisedPasswordException) {
redirectUrl = "/reset-password";
}
return new DefaultServerRedirectStrategy().sendRedirect(webFilterExchange.getExchange(), URI.create(redirectUrl));
})
);
// @formatter:on
return http.build();
}

}

@Configuration(proxyBeanMethods = false)
static class UserDetailsConfig {

@Bean
MapReactiveUserDetailsService userDetailsService() {
// @formatter:off
UserDetails user = PasswordEncodedUser.user();
UserDetails admin = User.withDefaultPasswordEncoder()
.username("admin")
.password("password2")
.roles("USER", "ADMIN")
.build();
// @formatter:on
return new MapReactiveUserDetailsService(user, admin);
}

}

@Configuration(proxyBeanMethods = false)
static class CompromisedPasswordCheckerConfig {

@Bean
TestReactivePasswordChecker compromisedPasswordChecker() {
return new TestReactivePasswordChecker();
}

}

static class TestReactivePasswordChecker implements ReactiveCompromisedPasswordChecker {

@Override
public Mono<CompromisedPasswordCheckResult> check(String password) {
if ("password".equals(password)) {
return Mono.just(new CompromisedPasswordCheckResult(true));
}
return Mono.just(new CompromisedPasswordCheckResult(false));
}

}

}

0 comments on commit 7d66525

Please sign in to comment.