From 5dbfa57c192d0ea283035ae5ba865c410b572814 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Wed, 26 Oct 2022 13:45:09 -0500 Subject: [PATCH 1/4] Resolve plugin dependencies --- build.gradle | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build.gradle b/build.gradle index 978deff2aa..b047649ee1 100644 --- a/build.gradle +++ b/build.gradle @@ -1,4 +1,10 @@ buildscript { + configurations.all { + resolutionStrategy.dependencySubstitution { + substitute module('org.apache.xerces:xercesImpl') with module('xerces:xercesImpl:2.9.1') + substitute module('org.apache.xerces:resolver') with module('xerces:resolver:2.9.1') + } + } dependencies { classpath 'io.spring.gradle:spring-build-conventions:0.0.23.4.RELEASE' classpath "org.springframework.boot:spring-boot-gradle-plugin:$springBootVersion" From c0b930e42173180a3faa385dd7595450a1168275 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Wed, 26 Oct 2022 23:25:52 -0500 Subject: [PATCH 2/4] Resolve plugin dependencies --- build.gradle | 6 ------ buildSrc/build.gradle | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 61500fb9da..bb04847c3f 100644 --- a/build.gradle +++ b/build.gradle @@ -1,10 +1,4 @@ buildscript { - configurations.all { - resolutionStrategy.dependencySubstitution { - substitute module('org.apache.xerces:xercesImpl') with module('xerces:xercesImpl:2.9.1') - substitute module('org.apache.xerces:resolver') with module('xerces:resolver:2.9.1') - } - } dependencies { classpath "io.spring.javaformat:spring-javaformat-gradle-plugin:$springJavaformatVersion" classpath 'io.spring.nohttp:nohttp-gradle:0.0.10' diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 9e22647737..c082a6c115 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -60,6 +60,14 @@ gradlePlugin { } configurations { + all { + resolutionStrategy.dependencySubstitution { + substitute(module("org.apache.xerces:xercesImpl")) + .using(module("xerces:xercesImpl:2.9.1")) + substitute(module("org.apache.xerces:resolver")) + .using(module("xerces:resolver:2.9.1")) + } + } implementation { exclude module: 'groovy-all' } From 75004587a419a96d18909030b20c6b16b226ecbe Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Wed, 26 Oct 2022 09:18:42 -0500 Subject: [PATCH 3/4] Fix scope mapping Issue gh-12101 --- ...tAuthorizationCodeTokenResponseClient.java | 23 +++----- ...tClientCredentialsTokenResponseClient.java | 23 +++----- .../DefaultPasswordTokenResponseClient.java | 23 +++----- ...eAuthorizationCodeTokenResponseClient.java | 12 +---- ...eClientCredentialsTokenResponseClient.java | 12 +---- ...ntReactivePasswordTokenResponseClient.java | 14 +---- ...orizationCodeTokenResponseClientTests.java | 6 +-- ...ntCredentialsTokenResponseClientTests.java | 6 +-- ...faultPasswordTokenResponseClientTests.java | 44 ++++++++++++---- ...tRefreshTokenTokenResponseClientTests.java | 35 ++++++++++--- ...orizationCodeTokenResponseClientTests.java | 6 +-- ...ntCredentialsTokenResponseClientTests.java | 8 +-- ...ctivePasswordTokenResponseClientTests.java | 52 ++++++++++++++++--- 13 files changed, 147 insertions(+), 117 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java index 174dc75e43..1644428a0c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,6 @@ import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; import org.springframework.web.client.ResponseErrorHandler; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestOperations; @@ -78,20 +77,12 @@ public final class DefaultAuthorizationCodeTokenResponseClient implements OAuth2 "An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null); throw new OAuth2AuthorizationException(oauth2Error, ex); } - - OAuth2AccessTokenResponse tokenResponse = response.getBody(); - - if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) { - // As per spec, in Section 5.1 Successful Access Token Response - // https://tools.ietf.org/html/rfc6749#section-5.1 - // If AccessTokenResponse.scope is empty, then default to the scope - // originally requested by the client in the Token Request - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(authorizationCodeGrantRequest.getClientRegistration().getScopes()) - .build(); - } - - return tokenResponse; + // As per spec, in Section 5.1 Successful Access Token Response + // https://tools.ietf.org/html/rfc6749#section-5.1 + // If AccessTokenResponse.scope is empty, then we assume all requested scopes were + // granted. + // However, we use the explicit scopes returned in the response (if any). + return response.getBody(); } /** diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java index fdd5eb1e75..36b0b8a1c1 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,6 @@ import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; import org.springframework.web.client.ResponseErrorHandler; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestOperations; @@ -78,20 +77,12 @@ public final class DefaultClientCredentialsTokenResponseClient implements OAuth2 "An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null); throw new OAuth2AuthorizationException(oauth2Error, ex); } - - OAuth2AccessTokenResponse tokenResponse = response.getBody(); - - if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) { - // As per spec, in Section 5.1 Successful Access Token Response - // https://tools.ietf.org/html/rfc6749#section-5.1 - // If AccessTokenResponse.scope is empty, then default to the scope - // originally requested by the client in the Token Request - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(clientCredentialsGrantRequest.getClientRegistration().getScopes()) - .build(); - } - - return tokenResponse; + // As per spec, in Section 5.1 Successful Access Token Response + // https://tools.ietf.org/html/rfc6749#section-5.1 + // If AccessTokenResponse.scope is empty, then we assume all requested scopes were + // granted. + // However, we use the explicit scopes returned in the response (if any). + return response.getBody(); } /** diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java index e2f7180d2e..55426285d1 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,6 @@ import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; import org.springframework.security.oauth2.core.http.converter.OAuth2AccessTokenResponseHttpMessageConverter; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; import org.springframework.web.client.ResponseErrorHandler; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestOperations; @@ -78,20 +77,12 @@ public final class DefaultPasswordTokenResponseClient implements OAuth2AccessTok "An error occurred while attempting to retrieve the OAuth 2.0 Access Token Response: " + ex.getMessage(), null); throw new OAuth2AuthorizationException(oauth2Error, ex); } - - OAuth2AccessTokenResponse tokenResponse = response.getBody(); - - if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) { - // As per spec, in Section 5.1 Successful Access Token Response - // https://tools.ietf.org/html/rfc6749#section-5.1 - // If AccessTokenResponse.scope is empty, then default to the scope - // originally requested by the client in the Token Request - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(passwordGrantRequest.getClientRegistration().getScopes()) - .build(); - } - - return tokenResponse; + // As per spec, in Section 5.1 Successful Access Token Response + // https://tools.ietf.org/html/rfc6749#section-5.1 + // If AccessTokenResponse.scope is empty, then we assume all requested scopes were + // granted. + // However, we use the explicit scopes returned in the response (if any). + return response.getBody(); } /** diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java index 76d49cc367..60617f4b43 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -79,15 +79,7 @@ public class WebClientReactiveAuthorizationCodeTokenResponseClient implements Re }) .body(body) .exchange() - .flatMap(response -> response.body(oauth2AccessTokenResponse())) - .map(response -> { - if (response.getAccessToken().getScopes().isEmpty()) { - response = OAuth2AccessTokenResponse.withResponse(response) - .scopes(authorizationExchange.getAuthorizationRequest().getScopes()) - .build(); - } - return response; - }); + .flatMap(response -> response.body(oauth2AccessTokenResponse())); }); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java index 6acfd38547..5d66f2e8c5 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -82,15 +82,7 @@ public class WebClientReactiveClientCredentialsTokenResponseClient implements Re null ))); } - return response.body(oauth2AccessTokenResponse()); }) - .map(response -> { - if (response.getAccessToken().getScopes().isEmpty()) { - response = OAuth2AccessTokenResponse.withResponse(response) - .scopes(authorizationGrantRequest.getClientRegistration().getScopes()) - .build(); - } - return response; - }); + return response.body(oauth2AccessTokenResponse()); }); }); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java index 41fe121694..236f125f85 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -79,18 +79,6 @@ public final class WebClientReactivePasswordTokenResponseClient implements React .then(Mono.error(new OAuth2AuthorizationException(oauth2Error))); } return response.body(oauth2AccessTokenResponse()); - }) - .map(tokenResponse -> { - if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) { - // As per spec, in Section 5.1 Successful Access Token Response - // https://tools.ietf.org/html/rfc6749#section-5.1 - // If AccessTokenResponse.scope is empty, then default to the scope - // originally requested by the client in the Token Request - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(passwordGrantRequest.getClientRegistration().getScopes()) - .build(); - } - return tokenResponse; }); }); } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java index 6751138cd3..34d913322f 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultAuthorizationCodeTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -218,7 +218,7 @@ public class DefaultAuthorizationCodeTokenResponseClientTests { } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" + @@ -230,7 +230,7 @@ public class DefaultAuthorizationCodeTokenResponseClientTests { OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(this.authorizationCodeGrantRequest()); - assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read", "write"); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); } @Test diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java index b24de2720b..fa40486141 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultClientCredentialsTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -222,7 +222,7 @@ public class DefaultClientCredentialsTokenResponseClientTests { } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" + @@ -235,7 +235,7 @@ public class DefaultClientCredentialsTokenResponseClientTests { OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(clientCredentialsGrantRequest); - assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read", "write"); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); } @Test diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java index 6390ae3959..95cc63ce67 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultPasswordTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -86,11 +86,14 @@ public class DefaultPasswordTokenResponseClientTests { @Test public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() throws Exception { - String accessTokenSuccessResponse = "{\n" + - " \"access_token\": \"access-token-1234\",\n" + - " \"token_type\": \"bearer\",\n" + - " \"expires_in\": \"3600\"\n" + - "}\n"; + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + + "}\n"; + // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); Instant expiresAtBefore = Instant.now().plusSeconds(3600); @@ -123,11 +126,14 @@ public class DefaultPasswordTokenResponseClientTests { @Test public void getTokenResponseWhenClientAuthenticationPostThenFormParametersAreSent() throws Exception { - String accessTokenSuccessResponse = "{\n" + - " \"access_token\": \"access-token-1234\",\n" + - " \"token_type\": \"bearer\",\n" + - " \"expires_in\": \"3600\"\n" + - "}\n"; + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read\"\n" + + "}\n"; + // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); ClientRegistration clientRegistration = this.clientRegistrationBuilder @@ -186,6 +192,22 @@ public class DefaultPasswordTokenResponseClientTests { assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("read"); } + @Test + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\"\n" + + "}\n"; + // @formatter:on + this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); + OAuth2PasswordGrantRequest passwordGrantRequest = new OAuth2PasswordGrantRequest( + this.clientRegistrationBuilder.build(), this.username, this.password); + OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(passwordGrantRequest); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); + } + @Test public void getTokenResponseWhenErrorResponseThenThrowOAuth2AuthorizationException() { String accessTokenErrorResponse = "{\n" + diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java index 44c455a944..be30ffd36c 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultRefreshTokenTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -88,11 +88,14 @@ public class DefaultRefreshTokenTokenResponseClientTests { @Test public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() throws Exception { - String accessTokenSuccessResponse = "{\n" + - " \"access_token\": \"access-token-1234\",\n" + - " \"token_type\": \"bearer\",\n" + - " \"expires_in\": \"3600\"\n" + - "}\n"; + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + + "}\n"; + // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); Instant expiresAtBefore = Instant.now().plusSeconds(3600); @@ -121,6 +124,26 @@ public class DefaultRefreshTokenTokenResponseClientTests { assertThat(accessTokenResponse.getRefreshToken().getTokenValue()).isEqualTo(this.refreshToken.getTokenValue()); } + @Test + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasOriginalScope() { + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\"\n" + + "}\n"; + // @formatter:on + this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); + ClientRegistration clientRegistration = this.clientRegistrationBuilder + .clientAuthenticationMethod(ClientAuthenticationMethod.POST).build(); + OAuth2RefreshTokenGrantRequest refreshTokenGrantRequest = new OAuth2RefreshTokenGrantRequest(clientRegistration, + this.accessToken, this.refreshToken); + OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient + .getTokenResponse(refreshTokenGrantRequest); + assertThat(accessTokenResponse.getAccessToken().getScopes()) + .containsExactly(this.accessToken.getScopes().toArray(new String[0])); + } + @Test public void getTokenResponseWhenClientAuthenticationPostThenFormParametersAreSent() throws Exception { String accessTokenSuccessResponse = "{\n" + diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java index d4104bae16..e6b57507c7 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -226,7 +226,7 @@ public class WebClientReactiveAuthorizationCodeTokenResponseClientTests { } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseUsingRequestedScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseWithNoScopes() { String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" + @@ -239,7 +239,7 @@ public class WebClientReactiveAuthorizationCodeTokenResponseClientTests { OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(authorizationCodeGrantRequest()).block(); - assertThat(accessTokenResponse.getAccessToken().getScopes()).containsExactly("openid", "profile", "email", "address"); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); } private OAuth2AuthorizationCodeGrantRequest authorizationCodeGrantRequest() { diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java index c4d92d629c..c2e2cb9275 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,6 +78,7 @@ public class WebClientReactiveClientCredentialsTokenResponseClientTests { String body = actualRequest.getUtf8Body(); assertThat(response.getAccessToken()).isNotNull(); + assertThat(response.getAccessToken().getScopes()).containsExactly("create"); assertThat(actualRequest.getHeader(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic Y2xpZW50LWlkOmNsaWVudC1zZWNyZXQ="); assertThat(body).isEqualTo("grant_type=client_credentials&scope=read%3Auser"); } @@ -102,12 +103,13 @@ public class WebClientReactiveClientCredentialsTokenResponseClientTests { String body = actualRequest.getUtf8Body(); assertThat(response.getAccessToken()).isNotNull(); + assertThat(response.getAccessToken().getScopes()).containsExactly("create"); assertThat(actualRequest.getHeader(HttpHeaders.AUTHORIZATION)).isNull(); assertThat(body).isEqualTo("grant_type=client_credentials&scope=read%3Auser&client_id=client-id&client_secret=client-secret"); } @Test - public void getTokenResponseWhenNoScopeThenClientRegistrationScopesDefaulted() { + public void getTokenResponseWhenNoScopeThenReturnAccessTokenResponseWithNoScopes() { ClientRegistration registration = this.clientRegistration.build(); enqueueJson("{\n" + " \"access_token\":\"MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3\",\n" @@ -119,7 +121,7 @@ public class WebClientReactiveClientCredentialsTokenResponseClientTests { OAuth2AccessTokenResponse response = this.client.getTokenResponse(request).block(); - assertThat(response.getAccessToken().getScopes()).isEqualTo(registration.getScopes()); + assertThat(response.getAccessToken().getScopes()).isEmpty(); } @Test(expected=IllegalArgumentException.class) diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClientTests.java index 00730543c7..c6d4133537 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -74,12 +74,50 @@ public class WebClientReactivePasswordTokenResponseClientTests { } @Test - public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() throws Exception { - String accessTokenSuccessResponse = "{\n" + - " \"access_token\": \"access-token-1234\",\n" + - " \"token_type\": \"bearer\",\n" + - " \"expires_in\": \"3600\"\n" + - "}\n"; + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseWithNoScope() + throws Exception { + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\"\n" + + "}\n"; + // @formatter:on + this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); + Instant expiresAtBefore = Instant.now().plusSeconds(3600); + ClientRegistration clientRegistration = this.clientRegistrationBuilder.build(); + OAuth2PasswordGrantRequest passwordGrantRequest = new OAuth2PasswordGrantRequest(clientRegistration, + this.username, this.password); + OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(passwordGrantRequest) + .block(); + Instant expiresAtAfter = Instant.now().plusSeconds(3600); + RecordedRequest recordedRequest = this.server.takeRequest(); + assertThat(recordedRequest.getMethod()).isEqualTo(HttpMethod.POST.toString()); + assertThat(recordedRequest.getHeader(HttpHeaders.ACCEPT)).isEqualTo(MediaType.APPLICATION_JSON_VALUE); + assertThat(recordedRequest.getHeader(HttpHeaders.CONTENT_TYPE)) + .isEqualTo(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"); + String formParameters = recordedRequest.getBody().readUtf8(); + assertThat(formParameters).contains("grant_type=password"); + assertThat(formParameters).contains("username=user1"); + assertThat(formParameters).contains("password=password"); + assertThat(formParameters).contains("scope=read+write"); + assertThat(accessTokenResponse.getAccessToken().getTokenValue()).isEqualTo("access-token-1234"); + assertThat(accessTokenResponse.getAccessToken().getTokenType()).isEqualTo(OAuth2AccessToken.TokenType.BEARER); + assertThat(accessTokenResponse.getAccessToken().getExpiresAt()).isBetween(expiresAtBefore, expiresAtAfter); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); + assertThat(accessTokenResponse.getRefreshToken()).isNull(); + } + + @Test + public void getTokenResponseWhenSuccessResponseIncludesScopeThenReturnAccessTokenResponse() throws Exception { + // @formatter:off + String accessTokenSuccessResponse = "{\n" + + " \"access_token\": \"access-token-1234\",\n" + + " \"token_type\": \"bearer\",\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + + "}\n"; + // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); Instant expiresAtBefore = Instant.now().plusSeconds(3600); From 1f481aafff14f324ffe2b43a973d3d5f54ae92d4 Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Wed, 26 Oct 2022 14:54:59 -0300 Subject: [PATCH 4/4] Fix AuthorizationFilter incorrectly extending OncePerRequestFilter Closes gh-12102 --- .../access/intercept/AuthorizationFilter.java | 97 +++++++++++++- .../intercept/AuthorizationFilterTests.java | 125 +++++++++++++++++- 2 files changed, 215 insertions(+), 7 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java index 19bcbfdc11..0dc292c2ff 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/AuthorizationFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,8 +18,11 @@ package org.springframework.security.web.access.intercept; import java.io.IOException; +import javax.servlet.DispatcherType; import javax.servlet.FilterChain; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -28,7 +31,7 @@ import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.util.Assert; -import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.filter.GenericFilterBean; /** * An authorization filter that restricts access to the URL using @@ -37,10 +40,16 @@ import org.springframework.web.filter.OncePerRequestFilter; * @author Evgeniy Cheban * @since 5.5 */ -public class AuthorizationFilter extends OncePerRequestFilter { +public class AuthorizationFilter extends GenericFilterBean { private final AuthorizationManager authorizationManager; + private boolean observeOncePerRequest = true; + + private boolean filterErrorDispatch = false; + + private boolean filterAsyncDispatch = false; + /** * Creates an instance. * @param authorizationManager the {@link AuthorizationManager} to use @@ -51,11 +60,53 @@ public class AuthorizationFilter extends OncePerRequestFilter { } @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain chain) throws ServletException, IOException { - this.authorizationManager.verify(this::getAuthentication, request); - filterChain.doFilter(request, response); + HttpServletRequest request = (HttpServletRequest) servletRequest; + HttpServletResponse response = (HttpServletResponse) servletResponse; + + if (this.observeOncePerRequest && isApplied(request)) { + chain.doFilter(request, response); + return; + } + + if (skipDispatch(request)) { + chain.doFilter(request, response); + return; + } + + String alreadyFilteredAttributeName = getAlreadyFilteredAttributeName(); + request.setAttribute(alreadyFilteredAttributeName, Boolean.TRUE); + try { + this.authorizationManager.verify(this::getAuthentication, request); + chain.doFilter(request, response); + } + finally { + request.removeAttribute(alreadyFilteredAttributeName); + } + } + + private boolean skipDispatch(HttpServletRequest request) { + if (DispatcherType.ERROR.equals(request.getDispatcherType()) && !this.filterErrorDispatch) { + return true; + } + if (DispatcherType.ASYNC.equals(request.getDispatcherType()) && !this.filterAsyncDispatch) { + return true; + } + return false; + } + + private boolean isApplied(HttpServletRequest request) { + return request.getAttribute(getAlreadyFilteredAttributeName()) != null; + } + + private String getAlreadyFilteredAttributeName() { + String name = getFilterName(); + if (name == null) { + name = getClass().getName(); + } + return name + ".APPLIED"; } private Authentication getAuthentication() { @@ -75,4 +126,38 @@ public class AuthorizationFilter extends OncePerRequestFilter { return this.authorizationManager; } + public boolean isObserveOncePerRequest() { + return this.observeOncePerRequest; + } + + /** + * Sets whether this filter apply only once per request. By default, this is + * true, meaning the filter will only execute once per request. Sometimes + * users may wish it to execute more than once per request, such as when JSP forwards + * are being used and filter security is desired on each included fragment of the HTTP + * request. + * @param observeOncePerRequest whether the filter should only be applied once per + * request + */ + public void setObserveOncePerRequest(boolean observeOncePerRequest) { + this.observeOncePerRequest = observeOncePerRequest; + } + + /** + * If set to true, the filter will be applied to error dispatcher. Defaults to false. + * @param filterErrorDispatch whether the filter should be applied to error dispatcher + */ + public void setFilterErrorDispatch(boolean filterErrorDispatch) { + this.filterErrorDispatch = filterErrorDispatch; + } + + /** + * If set to true, the filter will be applied to the async dispatcher. Defaults to + * false. + * @param filterAsyncDispatch whether the filter should be applied to async dispatch + */ + public void setFilterAsyncDispatch(boolean filterAsyncDispatch) { + this.filterAsyncDispatch = filterAsyncDispatch; + } + } diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java index 809cc9d8db..896b7e92e9 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/AuthorizationFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,15 +16,20 @@ package org.springframework.security.web.access.intercept; +import java.io.IOException; import java.util.function.Supplier; +import javax.servlet.DispatcherType; import javax.servlet.FilterChain; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.access.AccessDeniedException; @@ -36,6 +41,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -43,6 +49,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.willThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -53,6 +60,24 @@ import static org.mockito.Mockito.verifyNoInteractions; */ public class AuthorizationFilterTests { + private static final String ALREADY_FILTERED_ATTRIBUTE_NAME = "org.springframework.security.web.access.intercept.AuthorizationFilter.APPLIED"; + + private AuthorizationFilter filter; + + private AuthorizationManager authorizationManager; + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + private final MockHttpServletResponse response = new MockHttpServletResponse(); + + private final FilterChain chain = new MockFilterChain(); + + @Before + public void setup() { + this.authorizationManager = mock(AuthorizationManager.class); + this.filter = new AuthorizationFilter(this.authorizationManager); + } + @After public void tearDown() { SecurityContextHolder.clearContext(); @@ -132,4 +157,102 @@ public class AuthorizationFilterTests { assertThat(authorizationFilter.getAuthorizationManager()).isSameAs(authorizationManager); } + @Test + public void doFilterWhenObserveOncePerRequestTrueAndIsAppliedThenNotInvoked() throws ServletException, IOException { + setIsAppliedTrue(); + this.filter.setObserveOncePerRequest(true); + this.filter.doFilter(this.request, this.response, this.chain); + verifyNoInteractions(this.authorizationManager); + } + + @Test + public void doFilterWhenObserveOncePerRequestTrueAndNotAppliedThenInvoked() throws ServletException, IOException { + this.filter.setObserveOncePerRequest(true); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).verify(any(), any()); + } + + @Test + public void doFilterWhenObserveOncePerRequestFalseAndIsAppliedThenInvoked() throws ServletException, IOException { + setIsAppliedTrue(); + this.filter.setObserveOncePerRequest(false); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).verify(any(), any()); + } + + @Test + public void doFilterWhenObserveOncePerRequestFalseAndNotAppliedThenInvoked() throws ServletException, IOException { + this.filter.setObserveOncePerRequest(false); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).verify(any(), any()); + } + + @Test + public void doFilterWhenFilterErrorDispatchFalseAndIsErrorThenNotInvoked() throws ServletException, IOException { + this.request.setDispatcherType(DispatcherType.ERROR); + this.filter.setFilterErrorDispatch(false); + this.filter.doFilter(this.request, this.response, this.chain); + verifyNoInteractions(this.authorizationManager); + } + + @Test + public void doFilterWhenFilterErrorDispatchTrueAndIsErrorThenInvoked() throws ServletException, IOException { + this.request.setDispatcherType(DispatcherType.ERROR); + this.filter.setFilterErrorDispatch(true); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).verify(any(), any()); + } + + @Test + public void doFilterWhenFilterThenSetAlreadyFilteredAttribute() throws ServletException, IOException { + this.request = mock(MockHttpServletRequest.class); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.request).setAttribute(ALREADY_FILTERED_ATTRIBUTE_NAME, Boolean.TRUE); + } + + @Test + public void doFilterWhenFilterThenRemoveAlreadyFilteredAttribute() throws ServletException, IOException { + this.request = spy(MockHttpServletRequest.class); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.request).setAttribute(ALREADY_FILTERED_ATTRIBUTE_NAME, Boolean.TRUE); + assertThat(this.request.getAttribute(ALREADY_FILTERED_ATTRIBUTE_NAME)).isNull(); + } + + @Test + public void doFilterWhenFilterAsyncDispatchTrueAndIsAsyncThenInvoked() throws ServletException, IOException { + this.request.setDispatcherType(DispatcherType.ASYNC); + this.filter.setFilterAsyncDispatch(true); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).verify(any(), any()); + } + + @Test + public void doFilterWhenFilterAsyncDispatchFalseAndIsAsyncThenNotInvoked() throws ServletException, IOException { + this.request.setDispatcherType(DispatcherType.ASYNC); + this.filter.setFilterAsyncDispatch(false); + this.filter.doFilter(this.request, this.response, this.chain); + verifyNoInteractions(this.authorizationManager); + } + + @Test + public void filterWhenFilterErrorDispatchDefaultThenFalse() { + Boolean filterErrorDispatch = (Boolean) ReflectionTestUtils.getField(this.filter, "filterErrorDispatch"); + assertThat(filterErrorDispatch).isFalse(); + } + + @Test + public void filterWhenFilterAsyncDispatchDefaultThenFalse() { + Boolean filterAsyncDispatch = (Boolean) ReflectionTestUtils.getField(this.filter, "filterAsyncDispatch"); + assertThat(filterAsyncDispatch).isFalse(); + } + + @Test + public void filterWhenObserveOncePerRequestDefaultThenTrue() { + assertThat(this.filter.isObserveOncePerRequest()).isTrue(); + } + + private void setIsAppliedTrue() { + this.request.setAttribute(ALREADY_FILTERED_ATTRIBUTE_NAME, Boolean.TRUE); + } + }