diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java index c502bc701ab..fa9120edafc 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.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. @@ -214,7 +214,7 @@ BodyInserters.FormInserter populateTokenRequestBody(T grantRequest, * no scopes. */ Set defaultScopes(T grantRequest) { - return scopes(grantRequest); + return Collections.emptySet(); } /** 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 db43cb47bfa..a6aafb7a13a 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. @@ -30,7 +30,6 @@ 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; @@ -76,19 +75,12 @@ public OAuth2AccessTokenResponse getTokenResponse( Assert.notNull(authorizationCodeGrantRequest, "authorizationCodeGrantRequest cannot be null"); RequestEntity request = this.requestEntityConverter.convert(authorizationCodeGrantRequest); ResponseEntity response = getResponse(request); - 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 - // @formatter:off - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(authorizationCodeGrantRequest.getClientRegistration().getScopes()) - .build(); - // @formatter:on - } - 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(); } private ResponseEntity getResponse(RequestEntity request) { 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 168a85a9da3..a1ee225a94d 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. @@ -30,7 +30,6 @@ 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; @@ -76,19 +75,12 @@ public OAuth2AccessTokenResponse getTokenResponse( Assert.notNull(clientCredentialsGrantRequest, "clientCredentialsGrantRequest cannot be null"); RequestEntity request = this.requestEntityConverter.convert(clientCredentialsGrantRequest); ResponseEntity response = getResponse(request); - 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 - // @formatter:off - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(clientCredentialsGrantRequest.getClientRegistration().getScopes()) - .build(); - // @formatter:on - } - 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(); } private ResponseEntity getResponse(RequestEntity request) { diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClient.java index 419b5f91d6b..feae305edae 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClient.java @@ -30,7 +30,6 @@ 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; @@ -73,19 +72,12 @@ public OAuth2AccessTokenResponse getTokenResponse(JwtBearerGrantRequest jwtBeare Assert.notNull(jwtBearerGrantRequest, "jwtBearerGrantRequest cannot be null"); RequestEntity request = this.requestEntityConverter.convert(jwtBearerGrantRequest); ResponseEntity response = getResponse(request); - 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 - // @formatter:off - tokenResponse = OAuth2AccessTokenResponse.withResponse(tokenResponse) - .scopes(jwtBearerGrantRequest.getClientRegistration().getScopes()) - .build(); - // @formatter:on - } - 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(); } private ResponseEntity getResponse(RequestEntity request) { 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 047e7878859..3f1171a9db3 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. @@ -30,7 +30,6 @@ 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; @@ -75,16 +74,12 @@ public OAuth2AccessTokenResponse getTokenResponse(OAuth2PasswordGrantRequest pas Assert.notNull(passwordGrantRequest, "passwordGrantRequest cannot be null"); RequestEntity request = this.requestEntityConverter.convert(passwordGrantRequest); ResponseEntity response = getResponse(request); - 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(); } private ResponseEntity getResponse(RequestEntity request) { 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 77926000d49..82d873619e4 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-2020 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. @@ -65,11 +65,6 @@ Set scopes(OAuth2AuthorizationCodeGrantRequest grantRequest) { return Collections.emptySet(); } - @Override - Set defaultScopes(OAuth2AuthorizationCodeGrantRequest grantRequest) { - return grantRequest.getAuthorizationExchange().getAuthorizationRequest().getScopes(); - } - @Override BodyInserters.FormInserter populateTokenRequestBody(OAuth2AuthorizationCodeGrantRequest grantRequest, BodyInserters.FormInserter body) { 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 7ad39faf0c5..f7df261b6b2 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-2020 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. 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 bac8801ea6b..f60e3567cc6 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-2020 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. 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 164d99b732c..d265828f249 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-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. @@ -295,7 +295,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { // @formatter:off String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" @@ -307,7 +307,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessToke this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient .getTokenResponse(authorizationCodeGrantRequest(this.clientRegistration.build())); - 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 50c51cc818d..38a2c62c61d 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-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. @@ -304,7 +304,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { // @formatter:off String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" @@ -317,7 +317,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessToke this.clientRegistration.build()); 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/DefaultJwtBearerTokenResponseClientTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClientTests.java index 4202a9ade73..3c749a8c69b 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClientTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultJwtBearerTokenResponseClientTests.java @@ -102,7 +102,8 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" - + " \"expires_in\": \"3600\"\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + "}\n"; // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); @@ -204,7 +205,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasDefaultScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessTokenHasNoScope() { // @formatter:off String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" @@ -217,7 +218,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenAccessToke this.jwtAssertion); OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient .getTokenResponse(jwtBearerGrantRequest); - 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 dd77eb4eff3..23cb9753422 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-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. @@ -102,7 +102,8 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" - + " \"expires_in\": \"3600\"\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + "}\n"; // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); @@ -136,7 +137,8 @@ public void getTokenResponseWhenAuthenticationClientSecretPostThenFormParameters String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" - + " \"expires_in\": \"3600\"\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read\"\n" + "}\n"; // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); @@ -268,6 +270,22 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenAccessTokenHasRe 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.clientRegistration.build(), this.username, this.password); + OAuth2AccessTokenResponse accessTokenResponse = this.tokenResponseClient.getTokenResponse(passwordGrantRequest); + assertThat(accessTokenResponse.getAccessToken().getScopes()).isEmpty(); + } + @Test public void getTokenResponseWhenErrorResponseThenThrowOAuth2AuthorizationException() { String accessTokenErrorResponse = "{\n" + " \"error\": \"unauthorized_client\"\n" + "}\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 1293f0d18ed..cad2bbcf043 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-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. @@ -104,7 +104,8 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" + " \"token_type\": \"bearer\",\n" - + " \"expires_in\": \"3600\"\n" + + " \"expires_in\": \"3600\",\n" + + " \"scope\": \"read write\"\n" + "}\n"; // @formatter:on this.server.enqueue(jsonResponse(accessTokenSuccessResponse)); @@ -131,6 +132,26 @@ public void getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse() t 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.clientRegistration + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_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 getTokenResponseWhenAuthenticationClientSecretPostThenFormParametersAreSent() throws Exception { // @formatter:off 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 444f83de60a..bdc682a2697 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 @@ -246,7 +246,7 @@ public void getTokenResponseWhenSuccessResponseIncludesScopeThenReturnAccessToke } @Test - public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseUsingRequestedScope() { + public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAccessTokenResponseWithNoScopes() { // @formatter:off String accessTokenSuccessResponse = "{\n" + " \"access_token\": \"access-token-1234\",\n" @@ -258,8 +258,7 @@ public void getTokenResponseWhenSuccessResponseDoesNotIncludeScopeThenReturnAcce this.clientRegistration.scope("openid", "profile", "email", "address"); 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 0b458ac7cea..16933ab2ef2 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-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. @@ -103,6 +103,7 @@ public void getTokenResponseWhenHeaderThenSuccess() throws Exception { RecordedRequest actualRequest = this.server.takeRequest(); 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"); @@ -128,6 +129,7 @@ public void getTokenResponseWhenSpecialCharactersThenSuccessWithEncodedClientCre RecordedRequest actualRequest = this.server.takeRequest(); String body = actualRequest.getBody().readUtf8(); assertThat(response.getAccessToken()).isNotNull(); + assertThat(response.getAccessToken().getScopes()).containsExactly("create"); String urlEncodedClientCredentialecret = URLEncoder.encode(clientCredentialWithAnsiKeyboardSpecialCharacters, StandardCharsets.UTF_8.toString()); String clientCredentials = Base64.getEncoder() @@ -155,6 +157,7 @@ public void getTokenResponseWhenPostThenSuccess() throws Exception { RecordedRequest actualRequest = this.server.takeRequest(); 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&client_id=client-id&client_secret=client-secret&scope=read%3Auser"); @@ -230,7 +233,7 @@ private void configureJwtClientAuthenticationConverter(Function authorizationManager; private AuthorizationEventPublisher eventPublisher = AuthorizationFilter::noPublish; - private boolean shouldFilterAllDispatcherTypes = false; + private boolean observeOncePerRequest = true; + + private boolean filterErrorDispatch = false; + + private boolean filterAsyncDispatch = false; /** * Creates an instance. @@ -62,40 +69,66 @@ public AuthorizationFilter(AuthorizationManager authorizatio } @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain chain) throws ServletException, IOException { - AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, request); - this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, request, decision); - if (decision != null && !decision.isGranted()) { - throw new AccessDeniedException("Access Denied"); + 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 { + AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, request); + this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, request, decision); + if (decision != null && !decision.isGranted()) { + throw new AccessDeniedException("Access Denied"); + } + chain.doFilter(request, response); + } + finally { + request.removeAttribute(alreadyFilteredAttributeName); } - filterChain.doFilter(request, response); } - private Authentication getAuthentication() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { - throw new AuthenticationCredentialsNotFoundException( - "An Authentication object was not found in the SecurityContext"); + private boolean skipDispatch(HttpServletRequest request) { + if (DispatcherType.ERROR.equals(request.getDispatcherType()) && !this.filterErrorDispatch) { + return true; } - return authentication; + if (DispatcherType.ASYNC.equals(request.getDispatcherType()) && !this.filterAsyncDispatch) { + return true; + } + return false; } - @Override - protected void doFilterNestedErrorDispatch(HttpServletRequest request, HttpServletResponse response, - FilterChain filterChain) throws ServletException, IOException { - doFilterInternal(request, response, filterChain); + private boolean isApplied(HttpServletRequest request) { + return request.getAttribute(getAlreadyFilteredAttributeName()) != null; } - @Override - protected boolean shouldNotFilterAsyncDispatch() { - return !this.shouldFilterAllDispatcherTypes; + private String getAlreadyFilteredAttributeName() { + String name = getFilterName(); + if (name == null) { + name = getClass().getName(); + } + return name + ".APPLIED"; } - @Override - protected boolean shouldNotFilterErrorDispatch() { - return !this.shouldFilterAllDispatcherTypes; + private Authentication getAuthentication() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null) { + throw new AuthenticationCredentialsNotFoundException( + "An Authentication object was not found in the SecurityContext"); + } + return authentication; } /** @@ -124,7 +157,9 @@ public AuthorizationManager getAuthorizationManager() { * @since 5.7 */ public void setShouldFilterAllDispatcherTypes(boolean shouldFilterAllDispatcherTypes) { - this.shouldFilterAllDispatcherTypes = shouldFilterAllDispatcherTypes; + this.observeOncePerRequest = !shouldFilterAllDispatcherTypes; + this.filterErrorDispatch = shouldFilterAllDispatcherTypes; + this.filterAsyncDispatch = shouldFilterAllDispatcherTypes; } private static void noPublish(Supplier authentication, T object, @@ -132,4 +167,38 @@ private static void noPublish(Supplier authentication, T obj } + 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 e0d885fee14..c483585f500 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 @@ -16,16 +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.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.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; @@ -39,6 +43,7 @@ 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 org.springframework.web.util.WebUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -49,6 +54,7 @@ import static org.mockito.BDDMockito.given; 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; @@ -59,6 +65,24 @@ */ 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(); + + @BeforeEach + public void setup() { + this.authorizationManager = mock(AuthorizationManager.class); + this.filter = new AuthorizationFilter(this.authorizationManager); + } + @AfterEach public void tearDown() { SecurityContextHolder.clearContext(); @@ -197,37 +221,101 @@ public void doFilterWhenErrorAndShouldFilterAllDispatcherTypesThenFilter() throw } @Test - public void doFilterNestedErrorDispatchWhenAuthorizationManagerThenUses() throws Exception { - AuthorizationManager authorizationManager = mock(AuthorizationManager.class); - AuthorizationFilter authorizationFilter = new AuthorizationFilter(authorizationManager); - authorizationFilter.setShouldFilterAllDispatcherTypes(true); - MockHttpServletRequest mockRequest = new MockHttpServletRequest(null, "/path"); - mockRequest.setDispatcherType(DispatcherType.ERROR); - mockRequest.setAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE, "/error"); - MockHttpServletResponse mockResponse = new MockHttpServletResponse(); - FilterChain mockFilterChain = mock(FilterChain.class); + public void doFilterWhenObserveOncePerRequestTrueAndIsAppliedThenNotInvoked() throws ServletException, IOException { + setIsAppliedTrue(); + this.filter.setObserveOncePerRequest(true); + this.filter.doFilter(this.request, this.response, this.chain); + verifyNoInteractions(this.authorizationManager); + } - authorizationFilter.doFilterNestedErrorDispatch(mockRequest, mockResponse, mockFilterChain); - verify(authorizationManager).check(any(Supplier.class), any(HttpServletRequest.class)); + @Test + public void doFilterWhenObserveOncePerRequestTrueAndNotAppliedThenInvoked() throws ServletException, IOException { + this.filter.setObserveOncePerRequest(true); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).check(any(), any()); } @Test - public void doFilterNestedErrorDispatchWhenAuthorizationEventPublisherThenUses() throws Exception { - AuthorizationFilter authorizationFilter = new AuthorizationFilter( - AuthenticatedAuthorizationManager.authenticated()); - MockHttpServletRequest mockRequest = new MockHttpServletRequest(null, "/path"); - MockHttpServletResponse mockResponse = new MockHttpServletResponse(); - FilterChain mockFilterChain = mock(FilterChain.class); + public void doFilterWhenObserveOncePerRequestFalseAndIsAppliedThenInvoked() throws ServletException, IOException { + setIsAppliedTrue(); + this.filter.setObserveOncePerRequest(false); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).check(any(), any()); + } - SecurityContext securityContext = new SecurityContextImpl(); - securityContext.setAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER")); - SecurityContextHolder.setContext(securityContext); + @Test + public void doFilterWhenObserveOncePerRequestFalseAndNotAppliedThenInvoked() throws ServletException, IOException { + this.filter.setObserveOncePerRequest(false); + this.filter.doFilter(this.request, this.response, this.chain); + verify(this.authorizationManager).check(any(), any()); + } - AuthorizationEventPublisher eventPublisher = mock(AuthorizationEventPublisher.class); - authorizationFilter.setAuthorizationEventPublisher(eventPublisher); - authorizationFilter.doFilterNestedErrorDispatch(mockRequest, mockResponse, mockFilterChain); - verify(eventPublisher).publishAuthorizationEvent(any(Supplier.class), any(HttpServletRequest.class), - any(AuthorizationDecision.class)); + @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).check(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).check(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); } }