Propagate All Missing Factors

Closes gh-18000
This commit is contained in:
Josh Cummings 2025-10-15 15:46:30 -06:00
parent af1de950ae
commit cefc0cddec
3 changed files with 37 additions and 6 deletions

View File

@ -403,7 +403,8 @@ public class FormLoginConfigurerTests {
UserDetails user = PasswordEncodedUser.user(); UserDetails user = PasswordEncodedUser.user();
this.mockMvc.perform(get("/profile").with(user(user))) this.mockMvc.perform(get("/profile").with(user(user)))
.andExpect(status().is3xxRedirection()) .andExpect(status().is3xxRedirection())
.andExpect(redirectedUrl("http://localhost/login?factor.type=password&factor.reason=missing")); .andExpect(redirectedUrl(
"http://localhost/login?factor.type=password&factor.type=ott&factor.reason=missing&factor.reason=missing"));
this.mockMvc this.mockMvc
.perform(post("/ott/generate").param("username", "rod") .perform(post("/ott/generate").param("username", "rod")
.with(user(user)) .with(user(user))

View File

@ -20,6 +20,7 @@ import java.io.IOException;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -97,17 +98,20 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
@Override @Override
public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException denied) public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException denied)
throws IOException, ServletException { throws IOException, ServletException {
List<AuthorityRequiredFactorErrorEntry> authorityErrors = authorityErrors(denied); List<AuthorityRequiredFactorErrorEntry> errorEntries = authorityErrors(denied);
for (AuthorityRequiredFactorErrorEntry authorityError : authorityErrors) { List<RequiredFactorError> errors = errorEntries.stream()
.map(AuthorityRequiredFactorErrorEntry::getError)
.filter(Objects::nonNull)
.toList();
for (AuthorityRequiredFactorErrorEntry authorityError : errorEntries) {
String requiredAuthority = authorityError.getAuthority(); String requiredAuthority = authorityError.getAuthority();
AuthenticationEntryPoint entryPoint = this.entryPoints.get(requiredAuthority); AuthenticationEntryPoint entryPoint = this.entryPoints.get(requiredAuthority);
if (entryPoint == null) { if (entryPoint == null) {
continue; continue;
} }
this.requestCache.saveRequest(request, response); this.requestCache.saveRequest(request, response);
RequiredFactorError required = authorityError.getError(); if (!errors.isEmpty()) {
if (required != null) { request.setAttribute(WebAttributes.REQUIRED_FACTOR_ERRORS, errors);
request.setAttribute(WebAttributes.REQUIRED_FACTOR_ERRORS, List.of(required));
} }
String message = String.format("Missing Authorities %s", requiredAuthority); String message = String.format("Missing Authorities %s", requiredAuthority);
AuthenticationException ex = new InsufficientAuthenticationException(message, denied); AuthenticationException ex = new InsufficientAuthenticationException(message, denied);

View File

@ -17,6 +17,8 @@
package org.springframework.security.web.access; package org.springframework.security.web.access;
import java.util.Collection; import java.util.Collection;
import java.util.List;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -29,13 +31,18 @@ import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authorization.AuthorityAuthorizationDecision; import org.springframework.security.authorization.AuthorityAuthorizationDecision;
import org.springframework.security.authorization.AuthorizationDeniedException; import org.springframework.security.authorization.AuthorizationDeniedException;
import org.springframework.security.authorization.FactorAuthorizationDecision;
import org.springframework.security.authorization.RequiredFactor;
import org.springframework.security.authorization.RequiredFactorError;
import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.savedrequest.RequestCache;
import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@ -136,10 +143,29 @@ class DelegatingMissingAuthorityAccessDeniedHandlerTests {
verify(basicPasswordEntryPoint).commence(any(), any(), any()); verify(basicPasswordEntryPoint).commence(any(), any(), any());
} }
// gh-18000
@Test
void whenMultipleFactorErrorsThenPropagatesAll() throws Exception {
AccessDeniedHandler accessDeniedHandler = this.builder.build();
accessDeniedHandler.handle(this.request, this.response, missingFactors("FACTOR", "PASSWORD"));
verify(this.factorEntryPoint).commence(any(), any(), any());
Object attribute = this.request.getAttribute(WebAttributes.REQUIRED_FACTOR_ERRORS);
assertThat(attribute).isInstanceOf(Collection.class);
assertThat((Collection<?>) attribute).hasSize(2);
}
AuthorizationDeniedException missingAuthorities(String... authorities) { AuthorizationDeniedException missingAuthorities(String... authorities) {
Collection<GrantedAuthority> granted = AuthorityUtils.createAuthorityList(authorities); Collection<GrantedAuthority> granted = AuthorityUtils.createAuthorityList(authorities);
AuthorityAuthorizationDecision decision = new AuthorityAuthorizationDecision(false, granted); AuthorityAuthorizationDecision decision = new AuthorityAuthorizationDecision(false, granted);
return new AuthorizationDeniedException("access denied", decision); return new AuthorizationDeniedException("access denied", decision);
} }
AuthorizationDeniedException missingFactors(String... factors) {
List<RequiredFactorError> errors = Stream.of(factors)
.map((f) -> RequiredFactorError.createMissing(RequiredFactor.withAuthority(f).build()))
.toList();
FactorAuthorizationDecision decision = new FactorAuthorizationDecision(errors);
return new AuthorizationDeniedException("access denied", decision);
}
} }