From d757e6e44e84a5926ebcbdb9804d7a3945a0c208 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 19 Sep 2025 17:47:46 -0600 Subject: [PATCH] Response to Additional Feedback - Moved request attribute to WebAttributes - Renamed ExceptionHandlingConfigurer methods - Removed varargs from DelegatingMissingAuthorityAccessDeniedHandler Issue gh-17901 Issue gh-17934 --- .../ExceptionHandlingConfigurer.java | 16 ++++-- .../web/configurers/FormLoginConfigurer.java | 2 +- .../web/configurers/HttpBasicConfigurer.java | 4 +- .../web/configurers/WebAuthnConfigurer.java | 2 +- .../web/configurers/X509Configurer.java | 2 +- .../oauth2/client/OAuth2LoginConfigurer.java | 4 +- .../OAuth2ResourceServerConfigurer.java | 2 +- .../ott/OneTimeTokenLoginConfigurer.java | 2 +- .../saml2/Saml2LoginConfigurer.java | 4 +- .../security/core/GrantedAuthority.java | 2 - .../security/web/WebAttributes.java | 14 +++++ ...ngMissingAuthorityAccessDeniedHandler.java | 56 ++++++++----------- .../LoginUrlAuthenticationEntryPoint.java | 5 +- 13 files changed, 64 insertions(+), 51 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurer.java index 295a33e2e2..dd093e8141 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ExceptionHandlingConfigurer.java @@ -80,8 +80,7 @@ public final class ExceptionHandlingConfigurer> private LinkedHashMap 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> * @return the {@link ExceptionHandlingConfigurer} for further customizations * @since 7.0 */ - public ExceptionHandlingConfigurer defaultAuthenticationEntryPointFor(AuthenticationEntryPoint entryPoint, + public ExceptionHandlingConfigurer 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> * @return the {@link ExceptionHandlingConfigurer} for further customizations * @since 7.0 */ - public ExceptionHandlingConfigurer defaultAuthenticationEntryPointFor( + public ExceptionHandlingConfigurer defaultDeniedHandlerForMissingAuthority( Consumer 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> 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); 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 7ef16749b9..c78152454a 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 @@ -235,7 +235,7 @@ public final class FormLoginConfigurer> 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"); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java index 8f32dc9a7e..701fc415a6 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/HttpBasicConfigurer.java @@ -194,8 +194,8 @@ public final class HttpBasicConfigurer> } 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) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurer.java index e980242a0c..39f2768d86 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/WebAuthnConfigurer.java @@ -158,7 +158,7 @@ public class WebAuthnConfigurer> ExceptionHandlingConfigurer 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"); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java index d3746d73f4..868fef50ae 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java @@ -185,7 +185,7 @@ public final class X509Configurer> ExceptionHandlingConfigurer exceptions = http.getConfigurer(ExceptionHandlingConfigurer.class); if (exceptions != null) { AuthenticationEntryPoint forbidden = new Http403ForbiddenEntryPoint(); - exceptions.defaultAuthenticationEntryPointFor( + exceptions.defaultDeniedHandlerForMissingAuthority( (ep) -> ep.addEntryPointFor(forbidden, AnyRequestMatcher.INSTANCE), "FACTOR_X509"); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 800ea9f7c7..ecf695bcc0 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -565,8 +565,8 @@ public final class OAuth2LoginConfigurer> ExceptionHandlingConfigurer 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; } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurer.java index 8448be4648..5c9b56198d 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurer.java @@ -327,7 +327,7 @@ public final class OAuth2ResourceServerConfigurer ep.addEntryPointFor(this.authenticationEntryPoint, preferredMatcher), "FACTOR_BEARER"); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java index e9ee9b7d7f..c193caf971 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java @@ -140,7 +140,7 @@ public final class OneTimeTokenLoginConfigurer> 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"); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java index 19dda33cd5..be783ef0f6 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java @@ -352,8 +352,8 @@ public final class Saml2LoginConfigurer> ExceptionHandlingConfigurer 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; } diff --git a/core/src/main/java/org/springframework/security/core/GrantedAuthority.java b/core/src/main/java/org/springframework/security/core/GrantedAuthority.java index 8e7ff7ddc5..143b254b85 100644 --- a/core/src/main/java/org/springframework/security/core/GrantedAuthority.java +++ b/core/src/main/java/org/springframework/security/core/GrantedAuthority.java @@ -31,8 +31,6 @@ import org.springframework.security.authorization.AuthorizationManager; */ public interface GrantedAuthority extends Serializable { - String MISSING_AUTHORITIES_ATTRIBUTE = GrantedAuthority.class + ".missingAuthorities"; - /** * If the GrantedAuthority can be represented as a String * and that String is sufficient in precision to be relied upon for an diff --git a/web/src/main/java/org/springframework/security/web/WebAttributes.java b/web/src/main/java/org/springframework/security/web/WebAttributes.java index b9effcd807..53ec1c698c 100644 --- a/web/src/main/java/org/springframework/security/web/WebAttributes.java +++ b/web/src/main/java/org/springframework/security/web/WebAttributes.java @@ -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}. + *

+ * 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() { } diff --git a/web/src/main/java/org/springframework/security/web/access/DelegatingMissingAuthorityAccessDeniedHandler.java b/web/src/main/java/org/springframework/security/web/access/DelegatingMissingAuthorityAccessDeniedHandler.java index 1e8773be52..fbf2ff753c 100644 --- a/web/src/main/java/org/springframework/security/web/access/DelegatingMissingAuthorityAccessDeniedHandler.java +++ b/web/src/main/java/org/springframework/security/web/access/DelegatingMissingAuthorityAccessDeniedHandler.java @@ -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; * * * 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(); * * @@ -84,6 +86,7 @@ public final class DelegatingMissingAuthorityAccessDeniedHandler implements Acce private AccessDeniedHandler defaultAccessDeniedHandler = new AccessDeniedHandlerImpl(); private DelegatingMissingAuthorityAccessDeniedHandler(Map 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 entryPointByRequestMatcherByAuthority = new LinkedHashMap<>(); + private final Map 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 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 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); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java index f5f58e7607..dbbba096da 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java +++ b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java @@ -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 authorities = getAttribute(request, GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE, + Collection 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 @Nullable T getAttribute(HttpServletRequest request, String name, Class clazz) { - Object value = request.getAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE); + Object value = request.getAttribute(name); if (value == null) { return null; }