From ef9cd7660741c4220c393898301cafac48c4a15a Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 30 Oct 2017 14:56:09 -0400 Subject: [PATCH] Polish oauth2 Fixes gh-4758 --- .../oauth2/client/ImplicitGrantConfigurer.java | 2 +- .../oauth2/client/OAuth2LoginConfigurer.java | 2 +- .../authentication/OAuth2LoginAuthenticationToken.java | 2 +- .../OidcAuthorizationCodeAuthenticationProvider.java | 7 +++++-- .../InMemoryClientRegistrationRepository.java | 1 - .../client/userinfo/DefaultOAuth2UserService.java | 10 ++++++++-- .../web/OAuth2AuthorizationRequestUriBuilder.java | 2 +- .../OAuth2AuthorizationRequestRedirectFilterTests.java | 1 - .../web/OAuth2AuthorizationRequestUriBuilderTests.java | 1 - .../security/oauth2/client/web/TestUtil.java | 3 --- .../security/oauth2/core/oidc/user/OidcUser.java | 8 +++----- 11 files changed, 20 insertions(+), 19 deletions(-) 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 70cea07f8a..163a13749c 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2002-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. 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 8e999553ff..c25971c760 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2002-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. diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationToken.java index 0fab46631d..f20e3c1856 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationToken.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2002-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. diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java index ebf19135b7..a9b4295328 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java @@ -139,8 +139,11 @@ public class OidcAuthorizationCodeAuthenticationProvider implements Authenticati ClientRegistration clientRegistration = authorizationCodeAuthentication.getClientRegistration(); if (!accessTokenResponse.getAdditionalParameters().containsKey(OidcParameterNames.ID_TOKEN)) { - throw new IllegalArgumentException( - "Missing (required) ID Token in Token Response for Client Registration: " + clientRegistration.getRegistrationId()); + OAuth2Error invalidIdTokenError = new OAuth2Error( + INVALID_ID_TOKEN_ERROR_CODE, + "Missing (required) ID Token in Token Response for Client Registration: " + clientRegistration.getRegistrationId(), + null); + throw new OAuth2AuthenticationException(invalidIdTokenError, invalidIdTokenError.toString()); } JwtDecoder jwtDecoder = this.getJwtDecoder(clientRegistration); 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 45968ee811..dfbae7c15b 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,7 +28,6 @@ import java.util.stream.Collector; import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.toConcurrentMap; -import static java.util.stream.Collectors.toMap; /** * A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory. diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java index 03e8f7f993..23eed15f17 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java @@ -19,6 +19,7 @@ import org.springframework.core.ParameterizedTypeReference; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.user.DefaultOAuth2User; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.security.oauth2.core.user.OAuth2UserAuthority; @@ -46,15 +47,20 @@ import java.util.Set; * @see DefaultOAuth2User */ public class DefaultOAuth2UserService implements OAuth2UserService { + private static final String MISSING_USER_NAME_ATTRIBUTE_ERROR_CODE = "missing_user_name_attribute"; private NimbusUserInfoResponseClient userInfoResponseClient = new NimbusUserInfoResponseClient(); @Override public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { String userNameAttributeName = userRequest.getClientRegistration().getProviderDetails().getUserInfoEndpoint().getUserNameAttributeName(); if (!StringUtils.hasText(userNameAttributeName)) { - throw new IllegalArgumentException( + OAuth2Error oauth2Error = new OAuth2Error( + MISSING_USER_NAME_ATTRIBUTE_ERROR_CODE, "Missing required \"user name\" attribute name in UserInfoEndpoint for Client Registration: " + - userRequest.getClientRegistration().getRegistrationId()); + userRequest.getClientRegistration().getRegistrationId(), + null + ); + throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); } ParameterizedTypeReference> typeReference = diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilder.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilder.java index b5840976ff..ae26b97054 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilder.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilder.java @@ -34,7 +34,7 @@ import java.util.Set; */ class OAuth2AuthorizationRequestUriBuilder { - public URI build(OAuth2AuthorizationRequest authorizationRequest) { + URI build(OAuth2AuthorizationRequest authorizationRequest) { Set scopes = authorizationRequest.getScopes(); UriComponentsBuilder uriBuilder = UriComponentsBuilder .fromUriString(authorizationRequest.getAuthorizationUri()) diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java index 1fb87a8d38..1f9341ecae 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java @@ -29,7 +29,6 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequ import javax.servlet.FilterChain; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.net.URI; /** * Tests {@link OAuth2AuthorizationRequestRedirectFilter}. diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilderTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilderTests.java index 98ce79f3ac..dfc41e7ea4 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilderTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestUriBuilderTests.java @@ -17,7 +17,6 @@ package org.springframework.security.oauth2.client.web; import org.junit.Test; -import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestUriBuilder; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import java.net.URI; diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java index c4c07d4bd8..2ca7fb258d 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java @@ -16,11 +16,8 @@ package org.springframework.security.oauth2.client.web; import org.springframework.security.oauth2.client.registration.ClientRegistration; -import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; -import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; import org.springframework.security.oauth2.core.AuthorizationGrantType; -import java.util.Arrays; /** * @author Joe Grandja diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/user/OidcUser.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/user/OidcUser.java index acf7cf375e..749f78fe7c 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/user/OidcUser.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/user/OidcUser.java @@ -17,14 +17,12 @@ package org.springframework.security.oauth2.core.oidc.user; import org.springframework.security.core.AuthenticatedPrincipal; import org.springframework.security.core.Authentication; -import org.springframework.security.oauth2.core.oidc.OidcUserInfo; -import org.springframework.security.oauth2.core.user.OAuth2User; -import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.IdTokenClaimAccessor; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.core.oidc.StandardClaimAccessor; -import org.springframework.util.Assert; +import org.springframework.security.oauth2.core.user.OAuth2User; -import java.util.HashMap; import java.util.Map; /**