From c441f99567f02eed1eec3f7ae1a2d93cf4410c8e Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Thu, 12 Oct 2017 10:38:24 -0400 Subject: [PATCH] Polish oauth2-client --- .../annotation/web/builders/HttpSecurity.java | 4 +- .../AuthorizationCodeGrantConfigurer.java | 12 +-- .../client/ImplicitGrantConfigurer.java | 12 +-- .../oauth2/client/OAuth2LoginConfigurer.java | 6 +- ...thorizationCodeAuthenticationProvider.java | 8 +- .../AuthorizationCodeAuthenticator.java | 10 ++- .../OAuth2AuthenticationException.java | 24 +++--- .../OAuth2ClientAuthenticationToken.java | 14 ++-- .../OAuth2UserAuthenticationToken.java | 4 +- .../jwt/JwtDecoderRegistry.java | 2 +- .../jwt/nimbus/NimbusJwtDecoderRegistry.java | 4 +- .../registration/ClientRegistration.java | 30 ++++---- .../InMemoryClientRegistrationRepository.java | 10 +-- .../RegistrationIdIdentifierStrategy.java | 35 --------- .../client/token/SecurityTokenRepository.java | 3 +- .../CustomUserTypesOAuth2UserService.java | 10 +-- .../client/user/DefaultOAuth2UserService.java | 9 +-- .../user/DelegatingOAuth2UserService.java | 12 +-- .../user/nimbus/NimbusClientHttpResponse.java | 74 ------------------- .../user/nimbus/NimbusUserInfoRetriever.java | 44 +++++++++++ ...AuthorizationCodeAuthenticationFilter.java | 15 ++-- .../AuthorizationRequestRedirectFilter.java | 20 ++--- .../web/AuthorizationRequestRepository.java | 6 +- ...DefaultAuthorizationRequestUriBuilder.java | 2 +- .../client/web/DefaultStateGenerator.java | 18 ++--- ...SessionAuthorizationRequestRepository.java | 15 ++-- ...NimbusAuthorizationCodeTokenExchanger.java | 27 +++---- .../OidcAuthorizationCodeAuthenticator.java | 17 +++-- .../OidcClientAuthenticationToken.java | 2 +- .../OidcUserAuthenticationToken.java | 4 +- .../oidc/client/user/OidcUserService.java | 6 +- ...rizationCodeAuthenticationFilterTests.java | 4 +- ...thorizationRequestRedirectFilterTests.java | 2 +- .../core/user/DefaultOAuth2UserTests.java | 2 +- 34 files changed, 191 insertions(+), 276 deletions(-) delete mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/RegistrationIdIdentifierStrategy.java delete mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusClientHttpResponse.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java index a51b206992..f4947beab7 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java @@ -991,7 +991,7 @@ public final class HttpSecurity extends * .and() * .oauth2Login() * .clients(this.clientRegistrationRepository()) - * .authorizationRequestBuilder(this.authorizationRequestBuilder()) + * .authorizationRequestUriBuilder(this.authorizationRequestUriBuilder()) * .authorizationCodeTokenExchanger(this.authorizationCodeTokenExchanger()) * .userInfoEndpoint() * .userInfoService(this.userInfoService()) @@ -1008,7 +1008,7 @@ public final class HttpSecurity extends * } * * @Bean - * public AuthorizationRequestUriBuilder authorizationRequestBuilder() { + * public AuthorizationRequestUriBuilder authorizationRequestUriBuilder() { * // Custom URI builder for the "Authorization Request" * return new AuthorizationRequestUriBuilderImpl(); * } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java index 8a0417f6ac..5580579e1d 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java @@ -54,7 +54,7 @@ public class AuthorizationCodeGrantConfigurer> // ***** Authorization Request members private AuthorizationRequestRedirectFilter authorizationRequestFilter; private String authorizationRequestBaseUri; - private AuthorizationRequestUriBuilder authorizationRequestBuilder; + private AuthorizationRequestUriBuilder authorizationRequestUriBuilder; private AuthorizationRequestRepository authorizationRequestRepository; // ***** Authorization Response members @@ -71,9 +71,9 @@ public class AuthorizationCodeGrantConfigurer> return this; } - public AuthorizationCodeGrantConfigurer authorizationRequestBuilder(AuthorizationRequestUriBuilder authorizationRequestBuilder) { - Assert.notNull(authorizationRequestBuilder, "authorizationRequestBuilder cannot be null"); - this.authorizationRequestBuilder = authorizationRequestBuilder; + public AuthorizationCodeGrantConfigurer authorizationRequestUriBuilder(AuthorizationRequestUriBuilder authorizationRequestUriBuilder) { + Assert.notNull(authorizationRequestUriBuilder, "authorizationRequestUriBuilder cannot be null"); + this.authorizationRequestUriBuilder = authorizationRequestUriBuilder; return this; } @@ -134,8 +134,8 @@ public class AuthorizationCodeGrantConfigurer> this.authorizationRequestFilter = new AuthorizationRequestRedirectFilter( this.getAuthorizationRequestBaseUri(), this.getClientRegistrationRepository()); - if (this.authorizationRequestBuilder != null) { - this.authorizationRequestFilter.setAuthorizationUriBuilder(this.authorizationRequestBuilder); + if (this.authorizationRequestUriBuilder != null) { + this.authorizationRequestFilter.setAuthorizationRequestUriBuilder(this.authorizationRequestUriBuilder); } if (this.authorizationRequestRepository != null) { this.authorizationRequestFilter.setAuthorizationRequestRepository(this.authorizationRequestRepository); diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/ImplicitGrantConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/ImplicitGrantConfigurer.java index 6f5352efb6..32c53bbaad 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/ImplicitGrantConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/ImplicitGrantConfigurer.java @@ -33,7 +33,7 @@ public final class ImplicitGrantConfigurer> ext AbstractHttpConfigurer, B> { private String authorizationRequestBaseUri; - private AuthorizationRequestUriBuilder authorizationRequestBuilder; + private AuthorizationRequestUriBuilder authorizationRequestUriBuilder; public ImplicitGrantConfigurer authorizationRequestBaseUri(String authorizationRequestBaseUri) { Assert.hasText(authorizationRequestBaseUri, "authorizationRequestBaseUri cannot be empty"); @@ -41,9 +41,9 @@ public final class ImplicitGrantConfigurer> ext return this; } - public ImplicitGrantConfigurer authorizationRequestBuilder(AuthorizationRequestUriBuilder authorizationRequestBuilder) { - Assert.notNull(authorizationRequestBuilder, "authorizationRequestBuilder cannot be null"); - this.authorizationRequestBuilder = authorizationRequestBuilder; + public ImplicitGrantConfigurer authorizationRequestUriBuilder(AuthorizationRequestUriBuilder authorizationRequestUriBuilder) { + Assert.notNull(authorizationRequestUriBuilder, "authorizationRequestUriBuilder cannot be null"); + this.authorizationRequestUriBuilder = authorizationRequestUriBuilder; return this; } @@ -57,8 +57,8 @@ public final class ImplicitGrantConfigurer> ext public void configure(B http) throws Exception { AuthorizationRequestRedirectFilter authorizationRequestFilter = new AuthorizationRequestRedirectFilter( this.getAuthorizationRequestBaseUri(), this.getClientRegistrationRepository()); - if (this.authorizationRequestBuilder != null) { - authorizationRequestFilter.setAuthorizationUriBuilder(this.authorizationRequestBuilder); + if (this.authorizationRequestUriBuilder != null) { + authorizationRequestFilter.setAuthorizationRequestUriBuilder(this.authorizationRequestUriBuilder); } http.addFilter(this.postProcess(authorizationRequestFilter)); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 41f739ae4c..a3d40c0209 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -100,9 +100,9 @@ public final class OAuth2LoginConfigurer> exten return this; } - public AuthorizationEndpointConfig authorizationRequestBuilder(AuthorizationRequestUriBuilder authorizationRequestBuilder) { - Assert.notNull(authorizationRequestBuilder, "authorizationRequestBuilder cannot be null"); - authorizationCodeGrantConfigurer.authorizationRequestBuilder(authorizationRequestBuilder); + public AuthorizationEndpointConfig authorizationRequestUriBuilder(AuthorizationRequestUriBuilder authorizationRequestUriBuilder) { + Assert.notNull(authorizationRequestUriBuilder, "authorizationRequestUriBuilder cannot be null"); + authorizationCodeGrantConfigurer.authorizationRequestUriBuilder(authorizationRequestUriBuilder); return this; } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java index 6e7b8c91b7..ac2521beb6 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java @@ -87,14 +87,14 @@ public class AuthorizationCodeAuthenticationProvider implements AuthenticationPr throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); } - OAuth2ClientAuthenticationToken oauth2ClientAuthentication = + OAuth2ClientAuthenticationToken clientAuthentication = this.authorizationCodeAuthenticator.authenticate(authorizationCodeAuthentication); this.accessTokenRepository.saveSecurityToken( - oauth2ClientAuthentication.getAccessToken(), - oauth2ClientAuthentication.getClientRegistration()); + clientAuthentication.getAccessToken(), + clientAuthentication.getClientRegistration()); - return oauth2ClientAuthentication; + return clientAuthentication; } public final void setAccessTokenRepository(SecurityTokenRepository accessTokenRepository) { diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticator.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticator.java index 9ddb260100..825f6021bb 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticator.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticator.java @@ -27,6 +27,8 @@ import org.springframework.util.Assert; * * @author Joe Grandja * @since 5.0 + * @see AuthorizationCodeAuthenticationToken + * @see AuthorizationGrantTokenExchanger */ public class AuthorizationCodeAuthenticator implements AuthorizationGrantAuthenticator { private final AuthorizationGrantTokenExchanger authorizationCodeTokenExchanger; @@ -46,7 +48,7 @@ public class AuthorizationCodeAuthenticator implements AuthorizationGrantAuthent // If the openid scope value is not present, the behavior is entirely unspecified. if (authorizationCodeAuthentication.getAuthorizationRequest().getScope().contains("openid")) { // The OpenID Connect implementation of AuthorizationGrantAuthenticator - // must handle OpenID Connect Authentication Requests + // should handle OpenID Connect Authentication Requests return null; } @@ -57,10 +59,10 @@ public class AuthorizationCodeAuthenticator implements AuthorizationGrantAuthent tokenResponse.getTokenValue(), tokenResponse.getIssuedAt(), tokenResponse.getExpiresAt(), tokenResponse.getScope()); - OAuth2ClientAuthenticationToken oauth2ClientAuthentication = + OAuth2ClientAuthenticationToken clientAuthentication = new OAuth2ClientAuthenticationToken(authorizationCodeAuthentication.getClientRegistration(), accessToken); - oauth2ClientAuthentication.setDetails(authorizationCodeAuthentication.getDetails()); + clientAuthentication.setDetails(authorizationCodeAuthentication.getDetails()); - return oauth2ClientAuthentication; + return clientAuthentication; } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationException.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationException.java index 43465923d5..3fa592d107 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationException.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationException.java @@ -39,28 +39,28 @@ import org.springframework.util.Assert; * @since 5.0 */ public class OAuth2AuthenticationException extends AuthenticationException { - private OAuth2Error errorObject; + private OAuth2Error error; - public OAuth2AuthenticationException(OAuth2Error errorObject, Throwable cause) { - this(errorObject, cause.getMessage(), cause); + public OAuth2AuthenticationException(OAuth2Error error, Throwable cause) { + this(error, cause.getMessage(), cause); } - public OAuth2AuthenticationException(OAuth2Error errorObject, String message) { + public OAuth2AuthenticationException(OAuth2Error error, String message) { super(message); - this.setErrorObject(errorObject); + this.setError(error); } - public OAuth2AuthenticationException(OAuth2Error errorObject, String message, Throwable cause) { + public OAuth2AuthenticationException(OAuth2Error error, String message, Throwable cause) { super(message, cause); - this.setErrorObject(errorObject); + this.setError(error); } - public OAuth2Error getErrorObject() { - return errorObject; + public OAuth2Error getError() { + return this.error; } - private void setErrorObject(OAuth2Error errorObject) { - Assert.notNull(errorObject, "OAuth2 Error object cannot be null"); - this.errorObject = errorObject; + private void setError(OAuth2Error error) { + Assert.notNull(error, "error cannot be null"); + this.error = error; } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2ClientAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2ClientAuthenticationToken.java index 96215d064a..7b79204d38 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2ClientAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2ClientAuthenticationToken.java @@ -18,12 +18,12 @@ package org.springframework.security.oauth2.client.authentication; import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.SpringSecurityCoreVersion; -import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.core.AccessToken; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; +import java.util.Collections; import java.util.Set; /** @@ -48,7 +48,7 @@ public class OAuth2ClientAuthenticationToken extends AbstractAuthenticationToken private final AccessToken accessToken; public OAuth2ClientAuthenticationToken(ClientRegistration clientRegistration, AccessToken accessToken) { - super(AuthorityUtils.NO_AUTHORITIES); + super(Collections.emptyList()); Assert.notNull(clientRegistration, "clientRegistration cannot be null"); Assert.notNull(accessToken, "accessToken cannot be null"); this.clientRegistration = clientRegistration; @@ -63,7 +63,7 @@ public class OAuth2ClientAuthenticationToken extends AbstractAuthenticationToken @Override public Object getCredentials() { - return this.getAccessToken(); + return ""; // No need to expose this.getClientRegistration().getClientSecret() } public ClientRegistration getClientRegistration() { @@ -74,13 +74,13 @@ public class OAuth2ClientAuthenticationToken extends AbstractAuthenticationToken return this.accessToken; } - public Set getAuthorizedScope() { + public final Set getAuthorizedScope() { // As per spec, in section 5.1 Successful Access Token Response // https://tools.ietf.org/html/rfc6749#section-5.1 // If AccessToken.scope is empty, then default to the scope // originally requested by the client in the Authorization Request - return (!CollectionUtils.isEmpty(this.getAccessToken().getScope()) ? - this.getAccessToken().getScope() : - this.getClientRegistration().getScope()); + return (CollectionUtils.isEmpty(this.getAccessToken().getScope()) ? + this.getClientRegistration().getScope() : + this.getAccessToken().getScope()); } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2UserAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2UserAuthenticationToken.java index 0671d99fc0..6a18633078 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2UserAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2UserAuthenticationToken.java @@ -19,11 +19,11 @@ import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityCoreVersion; -import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.util.Assert; import java.util.Collection; +import java.util.Collections; /** * An implementation of an {@link AbstractAuthenticationToken} @@ -44,7 +44,7 @@ public class OAuth2UserAuthenticationToken extends AbstractAuthenticationToken { private final OAuth2ClientAuthenticationToken clientAuthentication; public OAuth2UserAuthenticationToken(OAuth2ClientAuthenticationToken clientAuthentication) { - this(null, AuthorityUtils.NO_AUTHORITIES, clientAuthentication); + this(null, Collections.emptyList(), clientAuthentication); } public OAuth2UserAuthenticationToken(OAuth2User principal, Collection authorities, diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/JwtDecoderRegistry.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/JwtDecoderRegistry.java index 9a9985a9e1..90c47a7395 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/JwtDecoderRegistry.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/JwtDecoderRegistry.java @@ -19,7 +19,7 @@ import org.springframework.security.jwt.JwtDecoder; import org.springframework.security.oauth2.client.registration.ClientRegistration; /** - * A registry for {@link JwtDecoder}'s that are associated to a {@link ClientRegistration}. + * A registry of {@link JwtDecoder}'s that are associated to a {@link ClientRegistration}. * * @author Joe Grandja * @since 5.0 diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/nimbus/NimbusJwtDecoderRegistry.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/nimbus/NimbusJwtDecoderRegistry.java index 5dc897cc89..00f27e352c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/nimbus/NimbusJwtDecoderRegistry.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/jwt/nimbus/NimbusJwtDecoderRegistry.java @@ -26,8 +26,8 @@ import java.util.HashMap; import java.util.Map; /** - * A {@link JwtDecoderRegistry} that uses the Nimbus JOSE + JWT SDK - * to create/manage instances of {@link NimbusJwtDecoderJwkSupport} internally. + * A {@link JwtDecoderRegistry} that creates/manages instances of + * {@link NimbusJwtDecoderJwkSupport}, which uses the Nimbus JOSE + JWT SDK internally. * * @author Joe Grandja * @since 5.0 diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index 6758f04e44..40254bc5a8 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -186,19 +186,19 @@ public class ClientRegistration { } public static class Builder { - protected String registrationId; - protected String clientId; - protected String clientSecret; - protected ClientAuthenticationMethod clientAuthenticationMethod = ClientAuthenticationMethod.BASIC; - protected AuthorizationGrantType authorizationGrantType; - protected String redirectUri; - protected Set scope; - protected String authorizationUri; - protected String tokenUri; - protected String userInfoUri; - protected String userNameAttributeName; - protected String jwkSetUri; - protected String clientName; + private String registrationId; + private String clientId; + private String clientSecret; + private ClientAuthenticationMethod clientAuthenticationMethod = ClientAuthenticationMethod.BASIC; + private AuthorizationGrantType authorizationGrantType; + private String redirectUri; + private Set scope; + private String authorizationUri; + private String tokenUri; + private String userInfoUri; + private String userNameAttributeName; + private String jwkSetUri; + private String clientName; public Builder(String registrationId) { this.registrationId = registrationId; @@ -212,7 +212,7 @@ public class ClientRegistration { this.authorizationGrantType(clientRegistrationProperties.getAuthorizationGrantType()); this.redirectUri(clientRegistrationProperties.getRedirectUri()); if (!CollectionUtils.isEmpty(clientRegistrationProperties.getScope())) { - this.scope(clientRegistrationProperties.getScope().stream().toArray(String[]::new)); + this.scope(clientRegistrationProperties.getScope().toArray(new String[0])); } this.authorizationUri(clientRegistrationProperties.getAuthorizationUri()); this.tokenUri(clientRegistrationProperties.getTokenUri()); @@ -230,7 +230,7 @@ public class ClientRegistration { this.authorizationGrantType(clientRegistration.getAuthorizationGrantType()); this.redirectUri(clientRegistration.getRedirectUri()); if (!CollectionUtils.isEmpty(clientRegistration.getScope())) { - this.scope(clientRegistration.getScope().stream().toArray(String[]::new)); + this.scope(clientRegistration.getScope().toArray(new String[0])); } this.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri()); this.tokenUri(clientRegistration.getProviderDetails().getTokenUri()); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java index a7f319b5a5..5707813cfd 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java @@ -28,21 +28,21 @@ import java.util.Map; * * @author Joe Grandja * @since 5.0 + * @see ClientRegistrationRepository * @see ClientRegistration */ public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository, Iterable { - private final ClientRegistrationIdentifierStrategy identifierStrategy = new RegistrationIdIdentifierStrategy(); private final Map registrations; public InMemoryClientRegistrationRepository(List registrations) { Assert.notEmpty(registrations, "registrations cannot be empty"); Map registrationsMap = new HashMap<>(); registrations.forEach(registration -> { - String identifier = this.identifierStrategy.getIdentifier(registration); - if (registrationsMap.containsKey(identifier)) { - throw new IllegalArgumentException("ClientRegistration must be unique. Found duplicate identifier: " + identifier); + if (registrationsMap.containsKey(registration.getRegistrationId())) { + throw new IllegalArgumentException("ClientRegistration must be unique. Found duplicate registrationId: " + + registration.getRegistrationId()); } - registrationsMap.put(identifier, registration); + registrationsMap.put(registration.getRegistrationId(), registration); }); this.registrations = Collections.unmodifiableMap(registrationsMap); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/RegistrationIdIdentifierStrategy.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/RegistrationIdIdentifierStrategy.java deleted file mode 100644 index be56eb986f..0000000000 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/RegistrationIdIdentifierStrategy.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2012-2017 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.oauth2.client.registration; - -import org.springframework.util.Assert; - -/** - * A {@link ClientRegistrationIdentifierStrategy} that identifies a {@link ClientRegistration} - * using the {@link ClientRegistration#getRegistrationId()}. - * - * @author Joe Grandja - * @since 5.0 - * @see ClientRegistration - */ -public class RegistrationIdIdentifierStrategy implements ClientRegistrationIdentifierStrategy { - - @Override - public String getIdentifier(ClientRegistration clientRegistration) { - Assert.notNull(clientRegistration, "clientRegistration cannot be null"); - return clientRegistration.getRegistrationId(); - } -} diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/token/SecurityTokenRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/token/SecurityTokenRepository.java index ded51d04b2..0adc155e1c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/token/SecurityTokenRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/token/SecurityTokenRepository.java @@ -20,8 +20,7 @@ import org.springframework.security.oauth2.core.SecurityToken; /** * Implementations of this interface are responsible for the persistence - * and association of an OAuth 2.0 / OpenID Connect 1.0 - * {@link SecurityToken} to a {@link ClientRegistration Client}. + * and association of a {@link SecurityToken} to a {@link ClientRegistration Client}. * * @author Joe Grandja * @since 5.0 diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/CustomUserTypesOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/CustomUserTypesOAuth2UserService.java index cb5ef52f96..cfd65d9f5b 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/CustomUserTypesOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/CustomUserTypesOAuth2UserService.java @@ -58,7 +58,7 @@ public class CustomUserTypesOAuth2UserService implements OAuth2UserService { public OAuth2User loadUser(OAuth2ClientAuthenticationToken clientAuthentication) throws OAuth2AuthenticationException { URI userInfoUri = URI.create(clientAuthentication.getClientRegistration().getProviderDetails().getUserInfoEndpoint().getUri()); Class customUserType; - if ((customUserType = this.getCustomUserTypes().get(userInfoUri)) == null) { + if ((customUserType = this.customUserTypes.get(userInfoUri)) == null) { return null; } @@ -82,14 +82,6 @@ public class CustomUserTypesOAuth2UserService implements OAuth2UserService { return customUser; } - protected Map> getCustomUserTypes() { - return this.customUserTypes; - } - - protected UserInfoRetriever getUserInfoRetriever() { - return this.userInfoRetriever; - } - public final void setUserInfoRetriever(UserInfoRetriever userInfoRetriever) { Assert.notNull(userInfoRetriever, "userInfoRetriever cannot be null"); this.userInfoRetriever = userInfoRetriever; diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DefaultOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DefaultOAuth2UserService.java index a8df30ca62..5eb6d76294 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DefaultOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DefaultOAuth2UserService.java @@ -53,9 +53,6 @@ import java.util.Set; public class DefaultOAuth2UserService implements OAuth2UserService { private UserInfoRetriever userInfoRetriever = new NimbusUserInfoRetriever(); - public DefaultOAuth2UserService() { - } - @Override public OAuth2User loadUser(OAuth2ClientAuthenticationToken clientAuthentication) throws OAuth2AuthenticationException { if (OidcClientAuthenticationToken.class.isAssignableFrom(clientAuthentication.getClass())) { @@ -69,7 +66,7 @@ public class DefaultOAuth2UserService implements OAuth2UserService { clientAuthentication.getClientRegistration().getRegistrationId()); } - Map userAttributes = this.getUserInfoRetriever().retrieve(clientAuthentication); + Map userAttributes = this.userInfoRetriever.retrieve(clientAuthentication); GrantedAuthority authority = new OAuth2UserAuthority(userAttributes); Set authorities = new HashSet<>(); authorities.add(authority); @@ -77,10 +74,6 @@ public class DefaultOAuth2UserService implements OAuth2UserService { return new DefaultOAuth2User(authorities, userAttributes, userNameAttributeName); } - protected UserInfoRetriever getUserInfoRetriever() { - return this.userInfoRetriever; - } - public final void setUserInfoRetriever(UserInfoRetriever userInfoRetriever) { Assert.notNull(userInfoRetriever, "userInfoRetriever cannot be null"); this.userInfoRetriever = userInfoRetriever; diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DelegatingOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DelegatingOAuth2UserService.java index e6024113c4..e366c050bc 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DelegatingOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/DelegatingOAuth2UserService.java @@ -37,17 +37,17 @@ import java.util.Objects; * @see OAuth2User */ public class DelegatingOAuth2UserService implements OAuth2UserService { - private final List oauth2UserServices; + private final List userServices; - public DelegatingOAuth2UserService(List oauth2UserServices) { - Assert.notEmpty(oauth2UserServices, "oauth2UserServices cannot be empty"); - this.oauth2UserServices = oauth2UserServices; + public DelegatingOAuth2UserService(List userServices) { + Assert.notEmpty(userServices, "userServices cannot be empty"); + this.userServices = userServices; } @Override public OAuth2User loadUser(OAuth2ClientAuthenticationToken clientAuthentication) throws OAuth2AuthenticationException { - OAuth2User oauth2User = this.oauth2UserServices.stream() - .map(oauth2UserService -> oauth2UserService.loadUser(clientAuthentication)) + OAuth2User oauth2User = this.userServices.stream() + .map(userService -> userService.loadUser(clientAuthentication)) .filter(Objects::nonNull) .findFirst() .orElse(null); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusClientHttpResponse.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusClientHttpResponse.java deleted file mode 100644 index 2bf866bc05..0000000000 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusClientHttpResponse.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2012-2017 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.oauth2.client.user.nimbus; - -import com.nimbusds.oauth2.sdk.http.HTTPResponse; -import org.springframework.http.HttpHeaders; -import org.springframework.http.client.AbstractClientHttpResponse; -import org.springframework.http.client.ClientHttpResponse; -import org.springframework.util.Assert; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.Charset; - -/** - * An implementation of a {@link ClientHttpResponse} which is used by {@link NimbusUserInfoRetriever}. - * - *

- * NOTE: This class is intended for internal use only. - * - * @author Joe Grandja - * @since 5.0 - */ -final class NimbusClientHttpResponse extends AbstractClientHttpResponse { - private final HTTPResponse httpResponse; - private final HttpHeaders headers; - - NimbusClientHttpResponse(HTTPResponse httpResponse) { - Assert.notNull(httpResponse, "httpResponse cannot be null"); - this.httpResponse = httpResponse; - this.headers = new HttpHeaders(); - this.headers.setAll(httpResponse.getHeaders()); - } - - @Override - public int getRawStatusCode() throws IOException { - return this.httpResponse.getStatusCode(); - } - - @Override - public String getStatusText() throws IOException { - return String.valueOf(this.getRawStatusCode()); - } - - @Override - public void close() { - } - - @Override - public InputStream getBody() throws IOException { - InputStream inputStream = new ByteArrayInputStream( - this.httpResponse.getContent().getBytes(Charset.forName("UTF-8"))); - return inputStream; - } - - @Override - public HttpHeaders getHeaders() { - return this.headers; - } -} diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusUserInfoRetriever.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusUserInfoRetriever.java index ebc55423d3..8b2aec4e7c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusUserInfoRetriever.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/user/nimbus/NimbusUserInfoRetriever.java @@ -22,6 +22,8 @@ import com.nimbusds.oauth2.sdk.http.HTTPResponse; import com.nimbusds.oauth2.sdk.token.BearerAccessToken; import com.nimbusds.openid.connect.sdk.UserInfoErrorResponse; import com.nimbusds.openid.connect.sdk.UserInfoRequest; +import org.springframework.http.HttpHeaders; +import org.springframework.http.client.AbstractClientHttpResponse; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.security.authentication.AuthenticationServiceException; @@ -29,9 +31,13 @@ import org.springframework.security.oauth2.client.authentication.OAuth2Authentic import org.springframework.security.oauth2.client.authentication.OAuth2ClientAuthenticationToken; import org.springframework.security.oauth2.client.user.UserInfoRetriever; import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.util.Assert; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URI; +import java.nio.charset.Charset; import java.util.Map; /** @@ -100,4 +106,42 @@ public class NimbusUserInfoRetriever implements UserInfoRetriever { throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString(), ex); } } + + private static class NimbusClientHttpResponse extends AbstractClientHttpResponse { + private final HTTPResponse httpResponse; + private final HttpHeaders headers; + + private NimbusClientHttpResponse(HTTPResponse httpResponse) { + Assert.notNull(httpResponse, "httpResponse cannot be null"); + this.httpResponse = httpResponse; + this.headers = new HttpHeaders(); + this.headers.setAll(httpResponse.getHeaders()); + } + + @Override + public int getRawStatusCode() throws IOException { + return this.httpResponse.getStatusCode(); + } + + @Override + public String getStatusText() throws IOException { + return String.valueOf(this.getRawStatusCode()); + } + + @Override + public void close() { + } + + @Override + public InputStream getBody() throws IOException { + InputStream inputStream = new ByteArrayInputStream( + this.httpResponse.getContent().getBytes(Charset.forName("UTF-8"))); + return inputStream; + } + + @Override + public HttpHeaders getHeaders() { + return this.headers; + } + } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java index b3aff79014..5bbc1ea2ce 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java @@ -65,9 +65,10 @@ import java.io.IOException; * @see AbstractAuthenticationProcessingFilter * @see AuthorizationCodeAuthenticationToken * @see AuthorizationCodeAuthenticationProvider - * @see AuthorizationRequestRedirectFilter + * @see AuthorizationResponse * @see AuthorizationRequest * @see AuthorizationRequestRepository + * @see AuthorizationRequestRedirectFilter * @see ClientRegistrationRepository * @see Section 4.1 Authorization Code Grant * @see Section 4.1.2 Authorization Response @@ -125,11 +126,11 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio clientRegistration, authorizationRequest, authorizationResponse); authorizationCodeAuthentication.setDetails(this.authenticationDetailsSource.buildDetails(request)); - OAuth2ClientAuthenticationToken oauth2ClientAuthentication = + OAuth2ClientAuthenticationToken clientAuthentication = (OAuth2ClientAuthenticationToken)this.getAuthenticationManager().authenticate(authorizationCodeAuthentication); return this.getAuthenticationManager().authenticate( - new OAuth2UserAuthenticationToken(oauth2ClientAuthentication)); + new OAuth2UserAuthenticationToken(clientAuthentication)); } public final RequestMatcher getAuthorizationResponseMatcher() { @@ -192,12 +193,12 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio .state(state) .build(); } else { - String description = request.getParameter(OAuth2Parameter.ERROR_DESCRIPTION); - String uri = request.getParameter(OAuth2Parameter.ERROR_URI); + String errorDescription = request.getParameter(OAuth2Parameter.ERROR_DESCRIPTION); + String errorUri = request.getParameter(OAuth2Parameter.ERROR_URI); return AuthorizationResponse.error(errorCode) .redirectUri(redirectUri) - .errorDescription(description) - .errorUri(uri) + .errorDescription(errorDescription) + .errorUri(errorUri) .state(state) .build(); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilter.java index cbc76fda36..d62a7b244c 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilter.java @@ -64,10 +64,10 @@ import java.util.Map; */ public class AuthorizationRequestRedirectFilter extends OncePerRequestFilter { public static final String DEFAULT_AUTHORIZATION_REQUEST_BASE_URI = "/oauth2/authorization"; - public static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"; + private static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"; private final AntPathRequestMatcher authorizationRequestMatcher; private final ClientRegistrationRepository clientRegistrationRepository; - private AuthorizationRequestUriBuilder authorizationUriBuilder = new DefaultAuthorizationRequestUriBuilder(); + private AuthorizationRequestUriBuilder authorizationRequestUriBuilder = new DefaultAuthorizationRequestUriBuilder(); private final RedirectStrategy authorizationRedirectStrategy = new DefaultRedirectStrategy(); private final StringKeyGenerator stateGenerator = new DefaultStateGenerator(); private AuthorizationRequestRepository authorizationRequestRepository = new HttpSessionAuthorizationRequestRepository(); @@ -86,9 +86,9 @@ public class AuthorizationRequestRedirectFilter extends OncePerRequestFilter { this.clientRegistrationRepository = clientRegistrationRepository; } - public final void setAuthorizationUriBuilder(AuthorizationRequestUriBuilder authorizationUriBuilder) { - Assert.notNull(authorizationUriBuilder, "authorizationUriBuilder cannot be null"); - this.authorizationUriBuilder = authorizationUriBuilder; + public final void setAuthorizationRequestUriBuilder(AuthorizationRequestUriBuilder authorizationRequestUriBuilder) { + Assert.notNull(authorizationRequestUriBuilder, "authorizationRequestUriBuilder cannot be null"); + this.authorizationRequestUriBuilder = authorizationRequestUriBuilder; } public final void setAuthorizationRequestRepository(AuthorizationRequestRepository authorizationRequestRepository) { @@ -112,18 +112,18 @@ public class AuthorizationRequestRedirectFilter extends OncePerRequestFilter { filterChain.doFilter(request, response); } - protected boolean shouldRequestAuthorization(HttpServletRequest request, HttpServletResponse response) { + private boolean shouldRequestAuthorization(HttpServletRequest request, HttpServletResponse response) { return this.authorizationRequestMatcher.matches(request); } - protected void sendRedirectForAuthorization(HttpServletRequest request, HttpServletResponse response) + private void sendRedirectForAuthorization(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { String registrationId = this.authorizationRequestMatcher .extractUriTemplateVariables(request).get(REGISTRATION_ID_URI_VARIABLE_NAME); ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(registrationId); if (clientRegistration == null) { - throw new IllegalArgumentException("Invalid Client Identifier (Registration Id): " + registrationId); + throw new IllegalArgumentException("Invalid Client Registration with Id: " + registrationId); } String redirectUriStr = this.expandRedirectUri(request, clientRegistration); @@ -153,11 +153,11 @@ public class AuthorizationRequestRedirectFilter extends OncePerRequestFilter { this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest, request, response); } - URI redirectUri = this.authorizationUriBuilder.build(authorizationRequest); + URI redirectUri = this.authorizationRequestUriBuilder.build(authorizationRequest); this.authorizationRedirectStrategy.sendRedirect(request, response, redirectUri.toString()); } - protected void unsuccessfulRedirectForAuthorization(HttpServletRequest request, HttpServletResponse response, + private void unsuccessfulRedirectForAuthorization(HttpServletRequest request, HttpServletResponse response, Exception failed) throws IOException, ServletException { if (logger.isDebugEnabled()) { diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRepository.java index 9d4f35cdf9..0bf5467af4 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRepository.java @@ -27,8 +27,8 @@ import javax.servlet.http.HttpServletResponse; *

* Used by the {@link AuthorizationRequestRedirectFilter} for persisting the Authorization Request * before it initiates the authorization code grant flow. - * As well, used by the {@link AuthorizationCodeAuthenticationFilter} when resolving - * the associated Authorization Request during the handling of the Authorization Response. + * As well, used by the {@link AuthorizationCodeAuthenticationFilter} for resolving + * the associated Authorization Request when handling the Authorization Response. * * @author Joe Grandja * @since 5.0 @@ -42,6 +42,6 @@ public interface AuthorizationRequestRepository { void saveAuthorizationRequest(AuthorizationRequest authorizationRequest, HttpServletRequest request, HttpServletResponse response); - AuthorizationRequest removeAuthorizationRequest(HttpServletRequest request); + void removeAuthorizationRequest(HttpServletRequest request); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultAuthorizationRequestUriBuilder.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultAuthorizationRequestUriBuilder.java index 0aa3883e3f..6234509317 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultAuthorizationRequestUriBuilder.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultAuthorizationRequestUriBuilder.java @@ -24,7 +24,7 @@ import java.util.stream.Collectors; /** * The default implementation of an {@link AuthorizationRequestUriBuilder}, - * which internally uses an {@link UriComponentsBuilder} to construct the OAuth 2.0 Authorization Request. + * which internally uses a {@link UriComponentsBuilder} to construct the OAuth 2.0 Authorization Request. * * @author Joe Grandja * @since 5.0 diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultStateGenerator.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultStateGenerator.java index 40aa829c80..0a5e3141b1 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultStateGenerator.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultStateGenerator.java @@ -15,16 +15,16 @@ */ package org.springframework.security.oauth2.client.web; -import java.util.Base64; - import org.springframework.security.crypto.keygen.BytesKeyGenerator; import org.springframework.security.crypto.keygen.KeyGenerators; import org.springframework.security.crypto.keygen.StringKeyGenerator; +import org.springframework.security.oauth2.core.endpoint.OAuth2Parameter; import org.springframework.util.Assert; +import java.util.Base64; + /** - * The default implementation for generating the - * {@link org.springframework.security.oauth2.core.endpoint.OAuth2Parameter#STATE} parameter + * The default implementation for generating the {@link OAuth2Parameter#STATE} parameter * used in the Authorization Request and correlated in the Authorization Response (or Error Response). * *

@@ -36,16 +36,16 @@ import org.springframework.util.Assert; * @since 5.0 */ public class DefaultStateGenerator implements StringKeyGenerator { - private static final int DEFAULT_BYTE_LENGTH = 32; + private static final int DEFAULT_KEY_LENGTH = 32; private final BytesKeyGenerator keyGenerator; public DefaultStateGenerator() { - this(DEFAULT_BYTE_LENGTH); + this(DEFAULT_KEY_LENGTH); } - public DefaultStateGenerator(int byteLength) { - Assert.isTrue(byteLength > 0, "byteLength must be greater than 0"); - this.keyGenerator = KeyGenerators.secureRandom(byteLength); + public DefaultStateGenerator(int keyLength) { + Assert.isTrue(keyLength >= DEFAULT_KEY_LENGTH, "keyLength must be greater than " + DEFAULT_KEY_LENGTH); + this.keyGenerator = KeyGenerators.secureRandom(keyLength); } @Override diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/HttpSessionAuthorizationRequestRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/HttpSessionAuthorizationRequestRepository.java index b05ce8600c..e5d22f6238 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/HttpSessionAuthorizationRequestRepository.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/HttpSessionAuthorizationRequestRepository.java @@ -32,16 +32,15 @@ import javax.servlet.http.HttpSession; public final class HttpSessionAuthorizationRequestRepository implements AuthorizationRequestRepository { private static final String DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME = HttpSessionAuthorizationRequestRepository.class.getName() + ".AUTHORIZATION_REQUEST"; - private String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME; + private final String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME; @Override public AuthorizationRequest loadAuthorizationRequest(HttpServletRequest request) { - AuthorizationRequest authorizationRequest = null; HttpSession session = request.getSession(false); if (session != null) { - authorizationRequest = (AuthorizationRequest) session.getAttribute(this.sessionAttributeName); + return (AuthorizationRequest) session.getAttribute(this.sessionAttributeName); } - return authorizationRequest; + return null; } @Override @@ -55,11 +54,7 @@ public final class HttpSessionAuthorizationRequestRepository implements Authoriz } @Override - public AuthorizationRequest removeAuthorizationRequest(HttpServletRequest request) { - AuthorizationRequest authorizationRequest = this.loadAuthorizationRequest(request); - if (authorizationRequest != null) { - request.getSession().removeAttribute(this.sessionAttributeName); - } - return authorizationRequest; + public void removeAuthorizationRequest(HttpServletRequest request) { + request.getSession().removeAttribute(this.sessionAttributeName); } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/nimbus/NimbusAuthorizationCodeTokenExchanger.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/nimbus/NimbusAuthorizationCodeTokenExchanger.java index fe20fa1c4a..ee8c061dd3 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/nimbus/NimbusAuthorizationCodeTokenExchanger.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/nimbus/NimbusAuthorizationCodeTokenExchanger.java @@ -33,9 +33,9 @@ import com.nimbusds.oauth2.sdk.id.ClientID; import org.springframework.http.MediaType; import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.oauth2.client.authentication.AuthorizationCodeAuthenticationToken; -import org.springframework.security.oauth2.client.web.AuthorizationGrantTokenExchanger; import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationException; import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.web.AuthorizationGrantTokenExchanger; import org.springframework.security.oauth2.core.AccessToken; import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.OAuth2Error; @@ -45,10 +45,10 @@ import org.springframework.util.CollectionUtils; import java.io.IOException; import java.net.URI; import java.util.Collections; -import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; /** * An implementation of an {@link AuthorizationGrantTokenExchanger} that "exchanges" @@ -60,6 +60,7 @@ import java.util.stream.Collectors; * * @author Joe Grandja * @since 5.0 + * @see AuthorizationGrantTokenExchanger * @see AuthorizationCodeAuthenticationToken * @see TokenResponse * @see Nimbus OAuth 2.0 SDK @@ -70,17 +71,17 @@ public class NimbusAuthorizationCodeTokenExchanger implements AuthorizationGrant private static final String INVALID_TOKEN_RESPONSE_ERROR_CODE = "invalid_token_response"; @Override - public TokenResponse exchange(AuthorizationCodeAuthenticationToken authorizationCodeAuthenticationToken) + public TokenResponse exchange(AuthorizationCodeAuthenticationToken authorizationCodeAuthentication) throws OAuth2AuthenticationException { - ClientRegistration clientRegistration = authorizationCodeAuthenticationToken.getClientRegistration(); + ClientRegistration clientRegistration = authorizationCodeAuthentication.getClientRegistration(); // Build the authorization code grant request for the token endpoint AuthorizationCode authorizationCode = new AuthorizationCode( - authorizationCodeAuthenticationToken.getAuthorizationResponse().getCode()); - URI redirectUri = this.toURI(clientRegistration.getRedirectUri()); + authorizationCodeAuthentication.getAuthorizationResponse().getCode()); + URI redirectUri = toURI(clientRegistration.getRedirectUri()); AuthorizationGrant authorizationCodeGrant = new AuthorizationCodeGrant(authorizationCode, redirectUri); - URI tokenUri = this.toURI(clientRegistration.getProviderDetails().getTokenUri()); + URI tokenUri = toURI(clientRegistration.getProviderDetails().getTokenUri()); // Set the credentials to authenticate the client at the token endpoint ClientID clientId = new ClientID(clientRegistration.getClientId()); @@ -102,11 +103,8 @@ public class NimbusAuthorizationCodeTokenExchanger implements AuthorizationGrant httpRequest.setReadTimeout(30000); tokenResponse = com.nimbusds.oauth2.sdk.TokenResponse.parse(httpRequest.send()); } catch (ParseException pe) { - // This error occurs if the Access Token Response is not well-formed, - // for example, a required attribute is missing throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_TOKEN_RESPONSE_ERROR_CODE), pe); } catch (IOException ioe) { - // This error occurs when there is a network-related issue throw new AuthenticationServiceException("An error occurred while sending the Access Token Request: " + ioe.getMessage(), ioe); } @@ -129,10 +127,9 @@ public class NimbusAuthorizationCodeTokenExchanger implements AuthorizationGrant long expiresIn = accessTokenResponse.getTokens().getAccessToken().getLifetime(); Set scope = Collections.emptySet(); if (!CollectionUtils.isEmpty(accessTokenResponse.getTokens().getAccessToken().getScope())) { - scope = new HashSet<>(accessTokenResponse.getTokens().getAccessToken().getScope().toStringList()); + scope = new LinkedHashSet<>(accessTokenResponse.getTokens().getAccessToken().getScope().toStringList()); } - Map additionalParameters = accessTokenResponse.getCustomParameters().entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + Map additionalParameters = new LinkedHashMap<>(accessTokenResponse.getCustomParameters()); return TokenResponse.withToken(accessToken) .tokenType(accessTokenType) @@ -142,7 +139,7 @@ public class NimbusAuthorizationCodeTokenExchanger implements AuthorizationGrant .build(); } - private URI toURI(String uriStr) { + private static URI toURI(String uriStr) { try { return new URI(uriStr); } catch (Exception ex) { diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcAuthorizationCodeAuthenticator.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcAuthorizationCodeAuthenticator.java index bb926d75a2..ff0852c95b 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcAuthorizationCodeAuthenticator.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcAuthorizationCodeAuthenticator.java @@ -27,6 +27,7 @@ import org.springframework.security.oauth2.client.web.AuthorizationGrantTokenExc import org.springframework.security.oauth2.core.AccessToken; import org.springframework.security.oauth2.core.endpoint.TokenResponse; import org.springframework.security.oauth2.oidc.core.IdToken; +import org.springframework.security.oauth2.oidc.core.OidcScope; import org.springframework.security.oauth2.oidc.core.endpoint.OidcParameter; import org.springframework.util.Assert; @@ -37,6 +38,10 @@ import org.springframework.util.Assert; * * @author Joe Grandja * @since 5.0 + * @see AuthorizationGrantAuthenticator + * @see AuthorizationCodeAuthenticationToken + * @see AuthorizationGrantTokenExchanger + * @see JwtDecoderRegistry */ public class OidcAuthorizationCodeAuthenticator implements AuthorizationGrantAuthenticator { private final AuthorizationGrantTokenExchanger authorizationCodeTokenExchanger; @@ -60,7 +65,7 @@ public class OidcAuthorizationCodeAuthenticator implements AuthorizationGrantAut // scope // REQUIRED. OpenID Connect requests MUST contain the "openid" scope value. // If the openid scope value is not present, the behavior is entirely unspecified. - if (!authorizationCodeAuthentication.getAuthorizationRequest().getScope().contains("openid")) { + if (!authorizationCodeAuthentication.getAuthorizationRequest().getScope().contains(OidcScope.OPENID)) { return null; } @@ -75,21 +80,21 @@ public class OidcAuthorizationCodeAuthenticator implements AuthorizationGrantAut if (!tokenResponse.getAdditionalParameters().containsKey(OidcParameter.ID_TOKEN)) { throw new IllegalArgumentException( - "Missing (required) ID Token in Token Response for Client Registration: '" + clientRegistration.getRegistrationId() + "'"); + "Missing (required) ID Token in Token Response for Client Registration: " + clientRegistration.getRegistrationId()); } JwtDecoder jwtDecoder = this.jwtDecoderRegistry.getJwtDecoder(clientRegistration); if (jwtDecoder == null) { - throw new IllegalArgumentException("Unable to find a registered JwtDecoder for Client Registration: '" + clientRegistration.getRegistrationId() + + throw new IllegalArgumentException("Failed to find a registered JwtDecoder for Client Registration: '" + clientRegistration.getRegistrationId() + "'. Check to ensure you have configured the JwkSet URI."); } Jwt jwt = jwtDecoder.decode((String)tokenResponse.getAdditionalParameters().get(OidcParameter.ID_TOKEN)); IdToken idToken = new IdToken(jwt.getTokenValue(), jwt.getIssuedAt(), jwt.getExpiresAt(), jwt.getClaims()); - OidcClientAuthenticationToken oidcClientAuthentication = + OidcClientAuthenticationToken clientAuthentication = new OidcClientAuthenticationToken(clientRegistration, accessToken, idToken); - oidcClientAuthentication.setDetails(authorizationCodeAuthentication.getDetails()); + clientAuthentication.setDetails(authorizationCodeAuthentication.getDetails()); - return oidcClientAuthentication; + return clientAuthentication; } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcClientAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcClientAuthenticationToken.java index 57c704dec6..23b01b19f4 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcClientAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcClientAuthenticationToken.java @@ -24,7 +24,7 @@ import org.springframework.util.Assert; /** * A {@link OAuth2ClientAuthenticationToken} that represents an - * OpenID Connect 1.0 Client {@link Authentication} (also known as Relying Party). + * OpenID Connect 1.0 Client {@link Authentication}. * *

* A client is considered "authenticated", diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcUserAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcUserAuthenticationToken.java index 78c2979b47..d6c66650dd 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcUserAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/authentication/OidcUserAuthenticationToken.java @@ -17,12 +17,12 @@ package org.springframework.security.oauth2.oidc.client.authentication; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.client.authentication.OAuth2ClientAuthenticationToken; import org.springframework.security.oauth2.client.authentication.OAuth2UserAuthenticationToken; import org.springframework.security.oauth2.oidc.core.user.OidcUser; import java.util.Collection; +import java.util.Collections; /** * A {@link OAuth2UserAuthenticationToken} that represents an @@ -41,7 +41,7 @@ import java.util.Collection; public class OidcUserAuthenticationToken extends OAuth2UserAuthenticationToken { public OidcUserAuthenticationToken(OidcClientAuthenticationToken clientAuthentication) { - this(null, AuthorityUtils.NO_AUTHORITIES, clientAuthentication); + this(null, Collections.emptyList(), clientAuthentication); } public OidcUserAuthenticationToken(OidcUser principal, Collection authorities, diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/user/OidcUserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/user/OidcUserService.java index c6c8ef1e81..5e234a6abf 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/user/OidcUserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/oidc/client/user/OidcUserService.java @@ -65,7 +65,7 @@ public class OidcUserService implements OAuth2UserService { UserInfo userInfo = null; if (this.shouldRetrieveUserInfo(oidcClientAuthentication)) { - Map userAttributes = this.getUserInfoRetriever().retrieve(oidcClientAuthentication); + Map userAttributes = this.userInfoRetriever.retrieve(oidcClientAuthentication); userInfo = new UserInfo(userAttributes); } @@ -76,10 +76,6 @@ public class OidcUserService implements OAuth2UserService { return new DefaultOidcUser(authorities, oidcClientAuthentication.getIdToken(), userInfo); } - protected UserInfoRetriever getUserInfoRetriever() { - return this.userInfoRetriever; - } - public final void setUserInfoRetriever(UserInfoRetriever userInfoRetriever) { Assert.notNull(userInfoRetriever, "userInfoRetriever cannot be null"); this.userInfoRetriever = userInfoRetriever; diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilterTests.java index fab9a25266..48b47836d4 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilterTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilterTests.java @@ -165,8 +165,8 @@ public class AuthorizationCodeAuthenticationFilterTests { Assertions.assertThat(authenticationExceptionArgCaptor.getValue()).isInstanceOf(OAuth2AuthenticationException.class); OAuth2AuthenticationException oauth2AuthenticationException = (OAuth2AuthenticationException)authenticationExceptionArgCaptor.getValue(); - Assertions.assertThat(oauth2AuthenticationException.getErrorObject()).isNotNull(); - Assertions.assertThat(oauth2AuthenticationException.getErrorObject().getErrorCode()).isEqualTo(errorCode); + Assertions.assertThat(oauth2AuthenticationException.getError()).isNotNull(); + Assertions.assertThat(oauth2AuthenticationException.getError().getErrorCode()).isEqualTo(errorCode); } private AuthorizationCodeAuthenticationFilter setupFilter(ClientRegistration... clientRegistrations) throws Exception { diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilterTests.java index c01c1f973c..a7ae22c23c 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilterTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/AuthorizationRequestRedirectFilterTests.java @@ -128,7 +128,7 @@ public class AuthorizationRequestRedirectFilterTests { ClientRegistrationRepository clientRegistrationRepository = TestUtil.clientRegistrationRepository(clientRegistrations); AuthorizationRequestRedirectFilter filter = new AuthorizationRequestRedirectFilter(clientRegistrationRepository); - filter.setAuthorizationUriBuilder(authorizationUriBuilder); + filter.setAuthorizationRequestUriBuilder(authorizationUriBuilder); return filter; } diff --git a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/user/DefaultOAuth2UserTests.java b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/user/DefaultOAuth2UserTests.java index e211ffc4c0..fff939a7d1 100644 --- a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/user/DefaultOAuth2UserTests.java +++ b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/user/DefaultOAuth2UserTests.java @@ -102,7 +102,7 @@ public class DefaultOAuth2UserTests { @Test public void constructorWhenNameAttributeKeyIsInvalidThenThrowsException() { this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("Invalid nameAttributeKey: invalid"); + this.thrown.expectMessage("Missing attribute 'invalid' in attributes"); new DefaultOAuth2User(TEST_AUTHORITIES, TEST_ATTRIBUTES, "invalid"); }