Skip to content

Commit c53e66a

Browse files
jgrandjajgrandja
committed
OAuth2AuthorizationEndpointFilter is applied after AuthorizationFilter
Closes gh-18251
1 parent 244b5a1 commit c53e66a

File tree

7 files changed

+231
-82
lines changed

7 files changed

+231
-82
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationEndpointConfigurer.java‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616

1717
package org.springframework.security.config.annotation.web.configurers.oauth2.server.authorization;
1818

19+
import java.lang.reflect.Method;
1920
import java.util.ArrayList;
2021
import java.util.List;
2122
import java.util.function.Consumer;
2223

24+
import jakarta.servlet.Filter;
2325
import jakarta.servlet.http.HttpServletRequest;
2426

2527
import org.springframework.http.HttpMethod;
@@ -36,10 +38,12 @@
3638
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationValidator;
3739
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationProvider;
3840
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationToken;
41+
import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository;
3942
import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings;
4043
import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter;
4144
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2AuthorizationCodeRequestAuthenticationConverter;
4245
import org.springframework.security.oauth2.server.authorization.web.authentication.OAuth2AuthorizationConsentAuthenticationConverter;
46+
import org.springframework.security.web.access.intercept.AuthorizationFilter;
4347
import org.springframework.security.web.authentication.AuthenticationConverter;
4448
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
4549
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
@@ -50,6 +54,7 @@
5054
import org.springframework.security.web.util.matcher.OrRequestMatcher;
5155
import org.springframework.security.web.util.matcher.RequestMatcher;
5256
import org.springframework.util.Assert;
57+
import org.springframework.util.ReflectionUtils;
5358
import org.springframework.util.StringUtils;
5459

5560
/**
@@ -83,6 +88,8 @@ public final class OAuth2AuthorizationEndpointConfigurer extends AbstractOAuth2C
8388

8489
private Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> authorizationCodeRequestAuthenticationValidator;
8590

91+
private Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> authorizationCodeRequestAuthenticationValidatorComposite;
92+
8693
private SessionAuthenticationStrategy sessionAuthenticationStrategy;
8794

8895
/**
@@ -248,8 +255,16 @@ void init(HttpSecurity httpSecurity) {
248255
authenticationProviders.addAll(0, this.authenticationProviders);
249256
}
250257
this.authenticationProvidersConsumer.accept(authenticationProviders);
251-
authenticationProviders.forEach(
252-
(authenticationProvider) -> httpSecurity.authenticationProvider(postProcess(authenticationProvider)));
258+
authenticationProviders.forEach((authenticationProvider) -> {
259+
httpSecurity.authenticationProvider(postProcess(authenticationProvider));
260+
if (authenticationProvider instanceof OAuth2AuthorizationCodeRequestAuthenticationProvider) {
261+
Method method = ReflectionUtils.findMethod(OAuth2AuthorizationCodeRequestAuthenticationProvider.class,
262+
"getAuthenticationValidatorComposite");
263+
ReflectionUtils.makeAccessible(method);
264+
this.authorizationCodeRequestAuthenticationValidatorComposite = (Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext>) ReflectionUtils
265+
.invokeMethod(method, authenticationProvider);
266+
}
267+
});
253268
}
254269

255270
@Override
@@ -282,7 +297,18 @@ void configure(HttpSecurity httpSecurity) {
282297
if (this.sessionAuthenticationStrategy != null) {
283298
authorizationEndpointFilter.setSessionAuthenticationStrategy(this.sessionAuthenticationStrategy);
284299
}
285-
httpSecurity.addFilterBefore(postProcess(authorizationEndpointFilter),
300+
httpSecurity.addFilterAfter(postProcess(authorizationEndpointFilter), AuthorizationFilter.class);
301+
// Create and add
302+
// OAuth2AuthorizationEndpointFilter.OAuth2AuthorizationCodeRequestValidatingFilter
303+
Method method = ReflectionUtils.findMethod(OAuth2AuthorizationEndpointFilter.class,
304+
"createAuthorizationCodeRequestValidatingFilter", RegisteredClientRepository.class, Consumer.class);
305+
ReflectionUtils.makeAccessible(method);
306+
RegisteredClientRepository registeredClientRepository = OAuth2ConfigurerUtils
307+
.getRegisteredClientRepository(httpSecurity);
308+
Filter authorizationCodeRequestValidatingFilter = (Filter) ReflectionUtils.invokeMethod(method,
309+
authorizationEndpointFilter, registeredClientRepository,
310+
this.authorizationCodeRequestAuthenticationValidatorComposite);
311+
httpSecurity.addFilterBefore(postProcess(authorizationCodeRequestValidatingFilter),
286312
AbstractPreAuthenticatedProcessingFilter.class);
287313
}
288314

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,8 @@ public void requestWhenRegisteredClientMissingThenBadRequest() throws Exception
307307
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
308308

309309
this.mvc
310-
.perform(
311-
get(DEFAULT_AUTHORIZATION_ENDPOINT_URI).params(getAuthorizationRequestParameters(registeredClient)))
310+
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
311+
.queryParams(getAuthorizationRequestParameters(registeredClient)))
312312
.andExpect(status().isBadRequest())
313313
.andReturn();
314314
}
@@ -851,21 +851,31 @@ public void requestWhenAuthorizationEndpointCustomizedThenUsed() throws Exceptio
851851
this.spring.register(AuthorizationServerConfigurationCustomAuthorizationEndpoint.class).autowire();
852852

853853
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
854+
this.registeredClientRepository.save(registeredClient);
855+
854856
TestingAuthenticationToken principal = new TestingAuthenticationToken("principalName", "password");
857+
Map<String, Object> additionalParameters = new HashMap<>();
858+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, S256_CODE_CHALLENGE);
859+
additionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
860+
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = new OAuth2AuthorizationCodeRequestAuthenticationToken(
861+
"https://provider.com/oauth2/authorize", registeredClient.getClientId(), principal,
862+
registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED, registeredClient.getScopes(),
863+
additionalParameters);
855864
OAuth2AuthorizationCode authorizationCode = new OAuth2AuthorizationCode("code", Instant.now(),
856865
Instant.now().plus(5, ChronoUnit.MINUTES));
857866
OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken(
858867
"https://provider.com/oauth2/authorize", registeredClient.getClientId(), principal, authorizationCode,
859868
registeredClient.getRedirectUris().iterator().next(), STATE_URL_UNENCODED,
860869
registeredClient.getScopes());
861-
given(authorizationRequestConverter.convert(any())).willReturn(authorizationCodeRequestAuthenticationResult);
870+
given(authorizationRequestConverter.convert(any())).willReturn(authorizationCodeRequestAuthentication);
862871
given(authorizationRequestAuthenticationProvider
863872
.supports(eq(OAuth2AuthorizationCodeRequestAuthenticationToken.class))).willReturn(true);
864873
given(authorizationRequestAuthenticationProvider.authenticate(any()))
865874
.willReturn(authorizationCodeRequestAuthenticationResult);
866875

867876
this.mvc
868-
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI).params(getAuthorizationRequestParameters(registeredClient))
877+
.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
878+
.queryParams(getAuthorizationRequestParameters(registeredClient))
869879
.with(user("user")))
870880
.andExpect(status().isOk());
871881

@@ -880,8 +890,7 @@ public void requestWhenAuthorizationEndpointCustomizedThenUsed() throws Exceptio
880890
|| converter instanceof OAuth2AuthorizationCodeRequestAuthenticationConverter
881891
|| converter instanceof OAuth2AuthorizationConsentAuthenticationConverter);
882892

883-
verify(authorizationRequestAuthenticationProvider)
884-
.authenticate(eq(authorizationCodeRequestAuthenticationResult));
893+
verify(authorizationRequestAuthenticationProvider).authenticate(eq(authorizationCodeRequestAuthentication));
885894

886895
@SuppressWarnings("unchecked")
887896
ArgumentCaptor<List<AuthenticationProvider>> authenticationProvidersCaptor = ArgumentCaptor

‎oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java‎

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -190,51 +190,55 @@ public Authentication authenticate(Authentication authentication) throws Authent
190190
OAuth2AuthorizationCodeRequestAuthenticationContext.Builder authenticationContextBuilder = OAuth2AuthorizationCodeRequestAuthenticationContext
191191
.with(authorizationCodeRequestAuthentication)
192192
.registeredClient(registeredClient);
193-
OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext = authenticationContextBuilder
194-
.build();
195193

196-
// grant_type
197-
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_AUTHORIZATION_GRANT_TYPE_VALIDATOR
198-
.accept(authenticationContext);
194+
if (!authorizationCodeRequestAuthentication.isValidated()) {
195+
OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext = authenticationContextBuilder
196+
.build();
199197

200-
// redirect_uri and scope
201-
this.authenticationValidator.accept(authenticationContext);
198+
// grant_type
199+
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_AUTHORIZATION_GRANT_TYPE_VALIDATOR
200+
.accept(authenticationContext);
202201

203-
// code_challenge (REQUIRED for public clients) - RFC 7636 (PKCE)
204-
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_CODE_CHALLENGE_VALIDATOR
205-
.accept(authenticationContext);
202+
// redirect_uri and scope
203+
this.authenticationValidator.accept(authenticationContext);
206204

207-
// prompt (OPTIONAL for OpenID Connect 1.0 Authentication Request)
208-
Set<String> promptValues = Collections.emptySet();
209-
if (authorizationCodeRequestAuthentication.getScopes().contains(OidcScopes.OPENID)) {
210-
String prompt = (String) authorizationCodeRequestAuthentication.getAdditionalParameters().get("prompt");
211-
if (StringUtils.hasText(prompt)) {
212-
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_PROMPT_VALIDATOR
213-
.accept(authenticationContext);
214-
promptValues = new HashSet<>(Arrays.asList(StringUtils.delimitedListToStringArray(prompt, " ")));
215-
}
216-
}
205+
// code_challenge (REQUIRED for public clients) - RFC 7636 (PKCE)
206+
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_CODE_CHALLENGE_VALIDATOR
207+
.accept(authenticationContext);
217208

218-
if (this.logger.isTraceEnabled()) {
219-
this.logger.trace("Validated authorization code request parameters");
209+
// prompt (OPTIONAL for OpenID Connect 1.0 Authentication Request)
210+
OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_PROMPT_VALIDATOR
211+
.accept(authenticationContext);
212+
213+
authorizationCodeRequestAuthentication.setValidated(true);
214+
215+
if (this.logger.isTraceEnabled()) {
216+
this.logger.trace("Validated authorization code request parameters");
217+
}
220218
}
221219

222220
// ---------------
223221
// The request is valid - ensure the resource owner is authenticated
224222
// ---------------
225223

226224
Authentication principal = (Authentication) authorizationCodeRequestAuthentication.getPrincipal();
225+
226+
Set<String> promptValues = Collections.emptySet();
227+
if (authorizationCodeRequestAuthentication.getScopes().contains(OidcScopes.OPENID)) {
228+
String prompt = (String) authorizationCodeRequestAuthentication.getAdditionalParameters().get("prompt");
229+
if (StringUtils.hasText(prompt)) {
230+
promptValues = new HashSet<>(Arrays.asList(StringUtils.delimitedListToStringArray(prompt, " ")));
231+
}
232+
}
233+
227234
if (!isPrincipalAuthenticated(principal)) {
228235
if (promptValues.contains(OidcPrompt.NONE)) {
229-
// Return an error instead of displaying the login page (via the
230-
// configured AuthenticationEntryPoint)
231236
throwError("login_required", "prompt", authorizationCodeRequestAuthentication, registeredClient);
232237
}
233-
if (this.logger.isTraceEnabled()) {
234-
this.logger.trace("Did not authenticate authorization code request since principal not authenticated");
238+
else {
239+
throwError(OAuth2ErrorCodes.INVALID_REQUEST, "principal", authorizationCodeRequestAuthentication,
240+
registeredClient);
235241
}
236-
// Return the authorization request as-is where isAuthenticated() is false
237-
return authorizationCodeRequestAuthentication;
238242
}
239243

240244
OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode()
@@ -400,6 +404,13 @@ public void setAuthorizationConsentRequired(
400404
this.authorizationConsentRequired = authorizationConsentRequired;
401405
}
402406

407+
Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> getAuthenticationValidatorComposite() {
408+
return OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_AUTHORIZATION_GRANT_TYPE_VALIDATOR
409+
.andThen(this.authenticationValidator)
410+
.andThen(OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_CODE_CHALLENGE_VALIDATOR)
411+
.andThen(OAuth2AuthorizationCodeRequestAuthenticationValidator.DEFAULT_PROMPT_VALIDATOR);
412+
}
413+
403414
private static boolean isAuthorizationConsentRequired(
404415
OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) {
405416
if (!authenticationContext.getRegisteredClient().getClientSettings().isRequireAuthorizationConsent()) {

‎oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationToken.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public class OAuth2AuthorizationCodeRequestAuthenticationToken
4242

4343
private final OAuth2AuthorizationCode authorizationCode;
4444

45+
private boolean validated;
46+
4547
/**
4648
* Constructs an {@code OAuth2AuthorizationCodeRequestAuthenticationToken} using the
4749
* provided parameters.
@@ -89,4 +91,12 @@ public OAuth2AuthorizationCode getAuthorizationCode() {
8991
return this.authorizationCode;
9092
}
9193

94+
final boolean isValidated() {
95+
return this.validated;
96+
}
97+
98+
final void setValidated(boolean validated) {
99+
this.validated = validated;
100+
}
101+
92102
}

0 commit comments

Comments
 (0)