Response to Additional Feedback

- Moved request attribute to WebAttributes
- Renamed ExceptionHandlingConfigurer methods
- Removed varargs from DelegatingMissingAuthorityAccessDeniedHandler

Issue gh-17901
Issue gh-17934
This commit is contained in:
Josh Cummings 2025-09-19 17:47:46 -06:00
parent 50ebd467c3
commit d757e6e44e
No known key found for this signature in database
GPG Key ID: 869B37A20E876129
13 changed files with 64 additions and 51 deletions

View File

@ -80,8 +80,7 @@ public final class ExceptionHandlingConfigurer<H extends HttpSecurityBuilder<H>>
private LinkedHashMap<RequestMatcher, AccessDeniedHandler> defaultDeniedHandlerMappings = new LinkedHashMap<>();
private final DelegatingMissingAuthorityAccessDeniedHandler.Builder missingAuthoritiesHandlerBuilder = DelegatingMissingAuthorityAccessDeniedHandler
.builder();
private DelegatingMissingAuthorityAccessDeniedHandler.@Nullable Builder missingAuthoritiesHandlerBuilder;
/**
* Creates a new instance
@ -142,8 +141,11 @@ public final class ExceptionHandlingConfigurer<H extends HttpSecurityBuilder<H>>
* @return the {@link ExceptionHandlingConfigurer} for further customizations
* @since 7.0
*/
public ExceptionHandlingConfigurer<H> defaultAuthenticationEntryPointFor(AuthenticationEntryPoint entryPoint,
public ExceptionHandlingConfigurer<H> defaultDeniedHandlerForMissingAuthority(AuthenticationEntryPoint entryPoint,
String authority) {
if (this.missingAuthoritiesHandlerBuilder == null) {
this.missingAuthoritiesHandlerBuilder = DelegatingMissingAuthorityAccessDeniedHandler.builder();
}
this.missingAuthoritiesHandlerBuilder.addEntryPointFor(entryPoint, authority);
return this;
}
@ -158,8 +160,11 @@ public final class ExceptionHandlingConfigurer<H extends HttpSecurityBuilder<H>>
* @return the {@link ExceptionHandlingConfigurer} for further customizations
* @since 7.0
*/
public ExceptionHandlingConfigurer<H> defaultAuthenticationEntryPointFor(
public ExceptionHandlingConfigurer<H> defaultDeniedHandlerForMissingAuthority(
Consumer<DelegatingAuthenticationEntryPoint.Builder> entryPoint, String authority) {
if (this.missingAuthoritiesHandlerBuilder == null) {
this.missingAuthoritiesHandlerBuilder = DelegatingMissingAuthorityAccessDeniedHandler.builder();
}
this.missingAuthoritiesHandlerBuilder.addEntryPointFor(entryPoint, authority);
return this;
}
@ -267,6 +272,9 @@ public final class ExceptionHandlingConfigurer<H extends HttpSecurityBuilder<H>>
private AccessDeniedHandler createDefaultDeniedHandler(H http) {
AccessDeniedHandler defaults = createDefaultAccessDeniedHandler(http);
if (this.missingAuthoritiesHandlerBuilder == null) {
return defaults;
}
DelegatingMissingAuthorityAccessDeniedHandler deniedHandler = this.missingAuthoritiesHandlerBuilder.build();
deniedHandler.setRequestCache(getRequestCache(http));
deniedHandler.setDefaultAccessDeniedHandler(defaults);

View File

@ -235,7 +235,7 @@ public final class FormLoginConfigurer<H extends HttpSecurityBuilder<H>> extends
if (exceptions != null) {
AuthenticationEntryPoint entryPoint = getAuthenticationEntryPoint();
RequestMatcher requestMatcher = getAuthenticationEntryPointMatcher(http);
exceptions.defaultAuthenticationEntryPointFor((ep) -> ep.addEntryPointFor(entryPoint, requestMatcher),
exceptions.defaultDeniedHandlerForMissingAuthority((ep) -> ep.addEntryPointFor(entryPoint, requestMatcher),
"FACTOR_PASSWORD");
}
}

View File

@ -194,8 +194,8 @@ public final class HttpBasicConfigurer<B extends HttpSecurityBuilder<B>>
}
AuthenticationEntryPoint entryPoint = postProcess(this.authenticationEntryPoint);
exceptionHandling.defaultAuthenticationEntryPointFor(entryPoint, preferredMatcher);
exceptionHandling.defaultAuthenticationEntryPointFor((ep) -> ep.addEntryPointFor(entryPoint, preferredMatcher),
"FACTOR_PASSWORD");
exceptionHandling.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(entryPoint, preferredMatcher), "FACTOR_PASSWORD");
}
private void registerDefaultLogoutSuccessHandler(B http, RequestMatcher preferredMatcher) {

View File

@ -158,7 +158,7 @@ public class WebAuthnConfigurer<H extends HttpSecurityBuilder<H>>
ExceptionHandlingConfigurer<H> exceptions = http.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptions != null) {
AuthenticationEntryPoint entryPoint = new LoginUrlAuthenticationEntryPoint("/login");
exceptions.defaultAuthenticationEntryPointFor(
exceptions.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(entryPoint, AnyRequestMatcher.INSTANCE), "FACTOR_WEBAUTHN");
}
}

View File

@ -185,7 +185,7 @@ public final class X509Configurer<H extends HttpSecurityBuilder<H>>
ExceptionHandlingConfigurer<H> exceptions = http.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptions != null) {
AuthenticationEntryPoint forbidden = new Http403ForbiddenEntryPoint();
exceptions.defaultAuthenticationEntryPointFor(
exceptions.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(forbidden, AnyRequestMatcher.INSTANCE), "FACTOR_X509");
}
}

View File

@ -565,8 +565,8 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>>
ExceptionHandlingConfigurer<B> exceptions = http.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptions != null) {
RequestMatcher requestMatcher = getAuthenticationEntryPointMatcher(http);
exceptions.defaultAuthenticationEntryPointFor((ep) -> ep.addEntryPointFor(loginEntryPoint, requestMatcher),
"FACTOR_AUTHORIZATION_CODE");
exceptions.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(loginEntryPoint, requestMatcher), "FACTOR_AUTHORIZATION_CODE");
}
return loginEntryPoint;
}

View File

@ -327,7 +327,7 @@ public final class OAuth2ResourceServerConfigurer<H extends HttpSecurityBuilder<
RequestMatcher preferredMatcher = new OrRequestMatcher(
Arrays.asList(this.requestMatcher, X_REQUESTED_WITH, restNotHtmlMatcher, allMatcher));
exceptionHandling.defaultAuthenticationEntryPointFor(this.authenticationEntryPoint, preferredMatcher);
exceptionHandling.defaultAuthenticationEntryPointFor(
exceptionHandling.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(this.authenticationEntryPoint, preferredMatcher), "FACTOR_BEARER");
}
}

View File

@ -140,7 +140,7 @@ public final class OneTimeTokenLoginConfigurer<H extends HttpSecurityBuilder<H>>
if (exceptions != null) {
AuthenticationEntryPoint entryPoint = getAuthenticationEntryPoint();
RequestMatcher requestMatcher = getAuthenticationEntryPointMatcher(http);
exceptions.defaultAuthenticationEntryPointFor((ep) -> ep.addEntryPointFor(entryPoint, requestMatcher),
exceptions.defaultDeniedHandlerForMissingAuthority((ep) -> ep.addEntryPointFor(entryPoint, requestMatcher),
"FACTOR_OTT");
}
}

View File

@ -352,8 +352,8 @@ public final class Saml2LoginConfigurer<B extends HttpSecurityBuilder<B>>
ExceptionHandlingConfigurer<B> exceptions = http.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptions != null) {
RequestMatcher requestMatcher = getAuthenticationEntryPointMatcher(http);
exceptions.defaultAuthenticationEntryPointFor((ep) -> ep.addEntryPointFor(loginEntryPoint, requestMatcher),
"FACTOR_SAML_RESPONSE");
exceptions.defaultDeniedHandlerForMissingAuthority(
(ep) -> ep.addEntryPointFor(loginEntryPoint, requestMatcher), "FACTOR_SAML_RESPONSE");
}
return loginEntryPoint;
}

View File

@ -31,8 +31,6 @@ import org.springframework.security.authorization.AuthorizationManager;
*/
public interface GrantedAuthority extends Serializable {
String MISSING_AUTHORITIES_ATTRIBUTE = GrantedAuthority.class + ".missingAuthorities";
/**
* If the <code>GrantedAuthority</code> can be represented as a <code>String</code>
* and that <code>String</code> is sufficient in precision to be relied upon for an

View File

@ -16,6 +16,9 @@
package org.springframework.security.web;
import jakarta.servlet.http.HttpServletRequest;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
/**
@ -52,6 +55,17 @@ public final class WebAttributes {
public static final String WEB_INVOCATION_PRIVILEGE_EVALUATOR_ATTRIBUTE = WebAttributes.class.getName()
+ ".WEB_INVOCATION_PRIVILEGE_EVALUATOR_ATTRIBUTE";
/**
* Used to set a {@code Collection} of {@link GrantedAuthority} instances into the
* {@link HttpServletRequest}.
* <p>
* Represents what authorities are missing to be authorized for the current request
*
* @since 7.0
* @see org.springframework.security.web.access.DelegatingMissingAuthorityAccessDeniedHandler
*/
public static final String MISSING_AUTHORITIES = WebAttributes.class + ".MISSING_AUTHORITIES";
private WebAttributes() {
}

View File

@ -35,11 +35,13 @@ import org.springframework.security.authorization.AuthorizationDeniedException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.authentication.DelegatingAuthenticationEntryPoint;
import org.springframework.security.web.savedrequest.NullRequestCache;
import org.springframework.security.web.savedrequest.RequestCache;
import org.springframework.security.web.util.ThrowableAnalyzer;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.util.Assert;
/**
* An {@link AccessDeniedHandler} that adapts {@link AuthenticationEntryPoint}s based on
@ -62,8 +64,8 @@ import org.springframework.security.web.util.matcher.AnyRequestMatcher;
*
* <code>
* AccessDeniedHandler handler = DelegatingMissingAuthorityAccessDeniedHandler.builder()
* .authorities("FACTOR_OTT").commence(new LoginUrlAuthenticationEntryPoint("/login"))
* .authorities("FACTOR_PASSWORD")...
* .addEntryPointFor(new LoginUrlAuthenticationEntryPoint("/login"), "FACTOR_OTT")
* .addEntryPointFor(new MyCustomEntryPoint(), "FACTOR_PASSWORD")
* .build();
* </code>
*
@ -84,6 +86,7 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
private AccessDeniedHandler defaultAccessDeniedHandler = new AccessDeniedHandlerImpl();
private DelegatingMissingAuthorityAccessDeniedHandler(Map<String, AuthenticationEntryPoint> entryPoints) {
Assert.notEmpty(entryPoints, "entryPoints cannot be empty");
this.entryPoints = entryPoints;
}
@ -97,7 +100,7 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
continue;
}
this.requestCache.saveRequest(request, response);
request.setAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE, List.of(needed));
request.setAttribute(WebAttributes.MISSING_AUTHORITIES, List.of(needed));
String message = String.format("Missing Authorities %s", List.of(needed));
AuthenticationException ex = new InsufficientAuthenticationException(message, denied);
entryPoint.commence(request, response, ex);
@ -112,6 +115,7 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
* @param defaultAccessDeniedHandler the default {@link AccessDeniedHandler} to use
*/
public void setDefaultAccessDeniedHandler(AccessDeniedHandler defaultAccessDeniedHandler) {
Assert.notNull(defaultAccessDeniedHandler, "defaultAccessDeniedHandler cannot be null");
this.defaultAccessDeniedHandler = defaultAccessDeniedHandler;
}
@ -123,6 +127,7 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
* @param requestCache the {@link RequestCache} to use
*/
public void setRequestCache(RequestCache requestCache) {
Assert.notNull(requestCache, "requestCachgrantedaue cannot be null");
this.requestCache = requestCache;
}
@ -158,57 +163,44 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce
*/
public static final class Builder {
private final Map<String, DelegatingAuthenticationEntryPoint.Builder> entryPointByRequestMatcherByAuthority = new LinkedHashMap<>();
private final Map<String, DelegatingAuthenticationEntryPoint.Builder> entryPointBuilderByAuthority = new LinkedHashMap<>();
private Builder() {
}
DelegatingAuthenticationEntryPoint.Builder entryPointBuilder(String authority) {
return this.entryPointByRequestMatcherByAuthority.computeIfAbsent(authority,
(k) -> DelegatingAuthenticationEntryPoint.builder());
}
void entryPoint(String authority, AuthenticationEntryPoint entryPoint) {
DelegatingAuthenticationEntryPoint.Builder builder = DelegatingAuthenticationEntryPoint.builder()
.addEntryPointFor(entryPoint, AnyRequestMatcher.INSTANCE);
this.entryPointByRequestMatcherByAuthority.put(authority, builder);
}
/**
* Bind these authorities to the given {@link AuthenticationEntryPoint}
* @param entryPoint the {@link AuthenticationEntryPoint} for the given
* authorities
* @param authorities the authorities
* Use this {@link AuthenticationEntryPoint} when the given
* {@code missingAuthority} is missing from the authenticated user
* @param entryPoint the {@link AuthenticationEntryPoint} for the given authority
* @param missingAuthority the authority
* @return the {@link Builder} for further configurations
*/
public Builder addEntryPointFor(AuthenticationEntryPoint entryPoint, String... authorities) {
for (String authority : authorities) {
Builder.this.entryPoint(authority, entryPoint);
}
public Builder addEntryPointFor(AuthenticationEntryPoint entryPoint, String missingAuthority) {
DelegatingAuthenticationEntryPoint.Builder builder = DelegatingAuthenticationEntryPoint.builder()
.addEntryPointFor(entryPoint, AnyRequestMatcher.INSTANCE);
this.entryPointBuilderByAuthority.put(missingAuthority, builder);
return this;
}
/**
* Bind these authorities to the given {@link AuthenticationEntryPoint}
* Use this {@link AuthenticationEntryPoint} when the given
* {@code missingAuthority} is missing from the authenticated user
* @param entryPoint a consumer to configure the underlying
* {@link DelegatingAuthenticationEntryPoint}
* @param authorities the authorities
* @param missingAuthority the authority
* @return the {@link Builder} for further configurations
*/
public Builder addEntryPointFor(Consumer<DelegatingAuthenticationEntryPoint.Builder> entryPoint,
String... authorities) {
for (String authority : authorities) {
entryPoint.accept(Builder.this.entryPointBuilder(authority));
}
String missingAuthority) {
entryPoint.accept(this.entryPointBuilderByAuthority.computeIfAbsent(missingAuthority,
(k) -> DelegatingAuthenticationEntryPoint.builder()));
return this;
}
public DelegatingMissingAuthorityAccessDeniedHandler build() {
Map<String, AuthenticationEntryPoint> entryPointByAuthority = new LinkedHashMap<>();
for (String authority : this.entryPointByRequestMatcherByAuthority.keySet()) {
entryPointByAuthority.put(authority, this.entryPointByRequestMatcherByAuthority.get(authority).build());
}
this.entryPointBuilderByAuthority.forEach((key, value) -> entryPointByAuthority.put(key, value.build()));
return new DelegatingMissingAuthorityAccessDeniedHandler(entryPointByAuthority);
}

View File

@ -38,6 +38,7 @@ import org.springframework.security.web.DefaultRedirectStrategy;
import org.springframework.security.web.PortMapper;
import org.springframework.security.web.PortMapperImpl;
import org.springframework.security.web.RedirectStrategy;
import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.access.ExceptionTranslationFilter;
import org.springframework.security.web.util.RedirectUrlBuilder;
import org.springframework.security.web.util.UrlUtils;
@ -117,7 +118,7 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin
@SuppressWarnings("unchecked")
protected String determineUrlToUseForThisRequest(HttpServletRequest request, HttpServletResponse response,
AuthenticationException exception) {
Collection<GrantedAuthority> authorities = getAttribute(request, GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE,
Collection<GrantedAuthority> authorities = getAttribute(request, WebAttributes.MISSING_AUTHORITIES,
Collection.class);
if (CollectionUtils.isEmpty(authorities)) {
return getLoginFormUrl();
@ -130,7 +131,7 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin
}
private static <T> @Nullable T getAttribute(HttpServletRequest request, String name, Class<T> clazz) {
Object value = request.getAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE);
Object value = request.getAttribute(name);
if (value == null) {
return null;
}