Polish gh-441

This commit is contained in:
Joe Grandja
2021-10-22 14:41:05 -04:00
parent 8e8e6d1b17
commit 72c5e24ab8
11 changed files with 70 additions and 83 deletions

View File

@@ -57,7 +57,7 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur
/**
* Sets the {@link Function} used to extract claims from an {@link OAuth2AuthenticationContext}
* to an instance of {@link OidcUserInfo}.
* to an instance of {@link OidcUserInfo} for the UserInfo response.
*
* <p>
* The {@link OAuth2AuthenticationContext} gives the mapper access to the {@link OidcUserInfoAuthenticationToken}.
@@ -109,4 +109,5 @@ public final class OidcUserInfoEndpointConfigurer extends AbstractOAuth2Configur
RequestMatcher getRequestMatcher() {
return this.requestMatcher;
}
}

View File

@@ -37,7 +37,7 @@ import org.springframework.security.oauth2.core.oidc.StandardClaimNames;
import org.springframework.util.Assert;
/**
* A {@link HttpMessageConverter} for an {@link OidcUserInfo OpenID Connect UserInfo Request and Response}.
* A {@link HttpMessageConverter} for an {@link OidcUserInfo OpenID Connect UserInfo Response}.
*
* @author Ido Salomon
* @author Steve Riesenberg
@@ -75,7 +75,7 @@ public class OidcUserInfoHttpMessageConverter extends AbstractHttpMessageConvert
return this.userInfoConverter.convert(userInfoParameters);
} catch (Exception ex) {
throw new HttpMessageNotReadableException(
"An error occurred reading the UserInfo: " + ex.getMessage(), ex, inputMessage);
"An error occurred reading the UserInfo response: " + ex.getMessage(), ex, inputMessage);
}
}
@@ -101,8 +101,7 @@ public class OidcUserInfoHttpMessageConverter extends AbstractHttpMessageConvert
* Sets the {@link Converter} used for converting the UserInfo parameters
* to an {@link OidcUserInfo}.
*
* @param userInfoConverter the {@link Converter} used for converting to an
* {@link OidcUserInfo}
* @param userInfoConverter the {@link Converter} used for converting to an {@link OidcUserInfo}
*/
public final void setUserInfoConverter(Converter<Map<String, Object>, OidcUserInfo> userInfoConverter) {
Assert.notNull(userInfoConverter, "userInfoConverter cannot be null");
@@ -123,7 +122,6 @@ public class OidcUserInfoHttpMessageConverter extends AbstractHttpMessageConvert
}
private static final class MapOidcUserInfoConverter implements Converter<Map<String, Object>, OidcUserInfo> {
private static final ClaimConversionService CLAIM_CONVERSION_SERVICE = ClaimConversionService.getSharedInstance();
private static final TypeDescriptor OBJECT_TYPE_DESCRIPTOR = TypeDescriptor.valueOf(Object.class);
private static final TypeDescriptor BOOLEAN_TYPE_DESCRIPTOR = TypeDescriptor.valueOf(Boolean.class);

View File

@@ -100,7 +100,7 @@ public final class ProviderSettings extends AbstractSettings {
/**
* Returns the Provider's OpenID Connect 1.0 UserInfo endpoint. The default is {@code /userinfo}.
*
* @return the OpenID Connect 1.0 User Info endpoint
* @return the OpenID Connect 1.0 UserInfo endpoint
*/
public String getOidcUserInfoEndpoint() {
return getSetting(ConfigurationSettingNames.Provider.OIDC_USER_INFO_ENDPOINT);
@@ -215,7 +215,7 @@ public final class ProviderSettings extends AbstractSettings {
/**
* Sets the Provider's OpenID Connect 1.0 UserInfo endpoint.
*
* @param oidcUserInfoEndpoint the OpenID Connect 1.0 User Info endpoint
* @param oidcUserInfoEndpoint the OpenID Connect 1.0 UserInfo endpoint
* @return the {@link Builder} for further configuration
*/
public Builder oidcUserInfoEndpoint(String oidcUserInfoEndpoint) {

View File

@@ -50,9 +50,7 @@ import org.springframework.util.Assert;
* @see <a href="https://openid.net/specs/openid-connect-core-1_0.html#UserInfo">5.3. UserInfo Endpoint</a>
*/
public final class OidcUserInfoAuthenticationProvider implements AuthenticationProvider {
private final OAuth2AuthorizationService authorizationService;
private Function<OAuth2AuthenticationContext, OidcUserInfo> userInfoMapper = new DefaultOidcUserInfoMapper();
/**
@@ -107,6 +105,7 @@ public final class OidcUserInfoAuthenticationProvider implements AuthenticationP
userInfoAuthentication, context);
OidcUserInfo userInfo = this.userInfoMapper.apply(authenticationContext);
return new OidcUserInfoAuthenticationToken(accessTokenAuthentication, userInfo);
}
@@ -116,7 +115,7 @@ public final class OidcUserInfoAuthenticationProvider implements AuthenticationP
}
/**
* Sets the {@link Function} used when mapping from an {@link OAuth2AuthenticationContext}
* Sets the {@link Function} used to extract claims from an {@link OAuth2AuthenticationContext}
* to an instance of {@link OidcUserInfo} for the UserInfo response.
*
* <p>
@@ -128,7 +127,7 @@ public final class OidcUserInfoAuthenticationProvider implements AuthenticationP
* {@link OAuth2AccessToken} associated with the bearer token used to make the request.</li>
* </ul>
*
* @param userInfoMapper the {@link Function} used when mapping from an {@link OAuth2AuthenticationContext}
* @param userInfoMapper the {@link Function} used to extract claims from an {@link OAuth2AuthenticationContext} to an instance of {@link OidcUserInfo}
*/
public void setUserInfoMapper(Function<OAuth2AuthenticationContext, OidcUserInfo> userInfoMapper) {
Assert.notNull(userInfoMapper, "userInfoMapper cannot be null");
@@ -173,7 +172,7 @@ public final class OidcUserInfoAuthenticationProvider implements AuthenticationP
return new OidcUserInfo(scopeRequestedClaims);
}
private Map<String, Object> getClaimsRequestedByScope(Map<String, Object> claims, Set<String> requestedScopes) {
private static Map<String, Object> getClaimsRequestedByScope(Map<String, Object> claims, Set<String> requestedScopes) {
Set<String> scopeRequestedClaimNames = new HashSet<>(32);
scopeRequestedClaimNames.add(StandardClaimNames.SUB);
@@ -195,5 +194,7 @@ public final class OidcUserInfoAuthenticationProvider implements AuthenticationP
return requestedClaims;
}
}
}

View File

@@ -33,16 +33,14 @@ import org.springframework.util.Assert;
* @see OidcUserInfoAuthenticationProvider
*/
public class OidcUserInfoAuthenticationToken extends AbstractAuthenticationToken {
private static final long serialVersionUID = Version.SERIAL_VERSION_UID;
private final Authentication principal;
private final OidcUserInfo userInfo;
/**
* Constructs an {@code OidcUserInfoAuthenticationToken} using the provided parameters.
*
* @param principal the authenticated principal
* @param principal the principal
*/
public OidcUserInfoAuthenticationToken(Authentication principal) {
super(Collections.emptyList());
@@ -64,7 +62,7 @@ public class OidcUserInfoAuthenticationToken extends AbstractAuthenticationToken
Assert.notNull(userInfo, "userInfo cannot be null");
this.principal = principal;
this.userInfo = userInfo;
setAuthenticated(principal.isAuthenticated());
setAuthenticated(true);
}
@Override
@@ -85,4 +83,5 @@ public class OidcUserInfoAuthenticationToken extends AbstractAuthenticationToken
public OidcUserInfo getUserInfo() {
return this.userInfo;
}
}

View File

@@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletResponse;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.server.ServletServerHttpResponse;
import org.springframework.security.authentication.AuthenticationManager;
@@ -125,7 +124,7 @@ public final class OidcUserInfoEndpointFilter extends OncePerRequestFilter {
private void sendUserInfoResponse(HttpServletResponse response, OidcUserInfo userInfo) throws IOException {
ServletServerHttpResponse httpResponse = new ServletServerHttpResponse(response);
this.userInfoHttpMessageConverter.write(userInfo, MediaType.APPLICATION_JSON, httpResponse);
this.userInfoHttpMessageConverter.write(userInfo, null, httpResponse);
}
private void sendErrorResponse(HttpServletResponse response, OAuth2Error error) throws IOException {

View File

@@ -17,6 +17,7 @@ package org.springframework.security.config.annotation.web.configurers.oauth2.se
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;
@@ -123,7 +124,7 @@ public class OidcUserInfoTests {
}
@Test
public void requestWhenSignedJwtAndCustomUserInfoMapperThenUserInfoResponse() throws Exception {
public void requestWhenSignedJwtAndCustomUserInfoMapperThenMapJwtClaimsToUserInfoResponse() throws Exception {
this.spring.register(CustomUserInfoConfiguration.class).autowire();
OAuth2Authorization authorization = createAuthorization();
@@ -159,7 +160,7 @@ public class OidcUserInfoTests {
jsonPath("locale").value("en-US"),
jsonPath("phone_number").value("+1 (604) 555-1234;ext=5678"),
jsonPath("phone_number_verified").value("false"),
jsonPath("address").value("Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance"),
jsonPath("address.formatted").value("Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance"),
jsonPath("updated_at").value("1970-01-01T00:00:00Z")
);
// @formatter:on
@@ -210,7 +211,7 @@ public class OidcUserInfoTests {
.locale("en-US")
.phoneNumber("+1 (604) 555-1234;ext=5678")
.phoneNumberVerified("false")
.address("Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance")
.claim("address", Collections.singletonMap("formatted", "Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance"))
.updatedAt("1970-01-01T00:00:00Z")
.build();
// @formatter:on
@@ -304,5 +305,7 @@ public class OidcUserInfoTests {
JwtEncoder jwtEncoder(JWKSource<SecurityContext> jwkSource) {
return new NimbusJwsEncoder(jwkSource);
}
}
}

View File

@@ -130,18 +130,18 @@ public class OidcUserInfoHttpMessageConverterTests {
assertThatExceptionOfType(HttpMessageNotReadableException.class)
.isThrownBy(() -> this.messageConverter.readInternal(OidcUserInfo.class, response))
.withMessageContaining("An error occurred reading the UserInfo")
.withMessageContaining("An error occurred reading the UserInfo response")
.withMessageContaining(errorMessage);
}
@Test
public void readInternalWhenInvalidResponseThenThrowException() {
String providerConfigurationResponse = "{}";
MockClientHttpResponse response = new MockClientHttpResponse(providerConfigurationResponse.getBytes(), HttpStatus.OK);
String userInfoResponse = "{}";
MockClientHttpResponse response = new MockClientHttpResponse(userInfoResponse.getBytes(), HttpStatus.OK);
assertThatExceptionOfType(HttpMessageNotReadableException.class)
.isThrownBy(() -> this.messageConverter.readInternal(OidcUserInfo.class, response))
.withMessageContaining("An error occurred reading the UserInfo")
.withMessageContaining("An error occurred reading the UserInfo response")
.withMessageContaining("claims cannot be empty");
}

View File

@@ -60,9 +60,9 @@ public class OidcUserInfoAuthenticationProviderTests {
private OidcUserInfoAuthenticationProvider authenticationProvider;
@Before
public void setUp() throws Exception {
public void setUp() {
this.authorizationService = mock(OAuth2AuthorizationService.class);
this.authenticationProvider = new OidcUserInfoAuthenticationProvider(authorizationService);
this.authenticationProvider = new OidcUserInfoAuthenticationProvider(this.authorizationService);
}
@Test
@@ -224,7 +224,7 @@ public class OidcUserInfoAuthenticationProviderTests {
assertThat(userInfo.getLocale()).isEqualTo("en-US");
assertThat(userInfo.getPhoneNumber()).isEqualTo("+1 (604) 555-1234;ext=5678");
assertThat(userInfo.getPhoneNumberVerified()).isEqualTo(false);
assertThat(userInfo.getClaimAsString(StandardClaimNames.ADDRESS))
assertThat(userInfo.getAddress().getFormatted())
.isEqualTo("Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance");
assertThat(userInfo.getUpdatedAt()).isEqualTo(Instant.parse("1970-01-01T00:00:00Z"));
@@ -259,6 +259,7 @@ public class OidcUserInfoAuthenticationProviderTests {
}
private static OidcUserInfo createUserInfo() {
// @formatter:off
return OidcUserInfo.builder()
.subject("user1")
.name("First Last")
@@ -278,8 +279,10 @@ public class OidcUserInfoAuthenticationProviderTests {
.locale("en-US")
.phoneNumber("+1 (604) 555-1234;ext=5678")
.phoneNumberVerified("false")
.address("Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance")
.claim("address", Collections.singletonMap("formatted", "Champ de Mars\n5 Av. Anatole France\n75007 Paris\nFrance"))
.updatedAt("1970-01-01T00:00:00Z")
.build();
// @formatter:on
}
}

View File

@@ -32,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
* @author Steve Riesenberg
*/
public class OidcUserInfoAuthenticationTokenTests {
@Test
public void constructorWhenPrincipalNullThenThrowIllegalArgumentException() {
assertThatIllegalArgumentException()
@@ -55,6 +56,6 @@ public class OidcUserInfoAuthenticationTokenTests {
OidcUserInfoAuthenticationToken authentication = new OidcUserInfoAuthenticationToken(principal, userInfo);
assertThat(authentication.getPrincipal()).isEqualTo(principal);
assertThat(authentication.getUserInfo()).isEqualTo(userInfo);
assertThat(authentication.isAuthenticated()).isFalse();
assertThat(authentication.isAuthenticated()).isTrue();
}
}

View File

@@ -20,9 +20,9 @@ import java.util.Collections;
import javax.servlet.FilterChain;
import org.junit.Before;
import org.junit.Test;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.converter.HttpMessageConverter;
@@ -30,9 +30,10 @@ import org.springframework.mock.http.client.MockClientHttpResponse;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
import org.springframework.security.oauth2.core.http.converter.OAuth2ErrorHttpMessageConverter;
@@ -59,8 +60,16 @@ import static org.mockito.Mockito.when;
*/
public class OidcUserInfoEndpointFilterTests {
private static final String DEFAULT_OIDC_USER_INFO_ENDPOINT_URI = "/userinfo";
private AuthenticationManager authenticationManager;
private OidcUserInfoEndpointFilter filter;
private final HttpMessageConverter<OAuth2Error> errorHttpResponseConverter = new OAuth2ErrorHttpMessageConverter();
@Before
public void setup() {
this.authenticationManager = mock(AuthenticationManager.class);
this.filter = new OidcUserInfoEndpointFilter(this.authenticationManager, DEFAULT_OIDC_USER_INFO_ENDPOINT_URI);
}
@Test
public void constructorWhenAuthenticationManagerNullThenThrowIllegalArgumentException() {
assertThatIllegalArgumentException()
@@ -70,91 +79,64 @@ public class OidcUserInfoEndpointFilterTests {
@Test
public void constructorWhenUserInfoEndpointUriIsEmptyThenThrowIllegalArgumentException() {
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
assertThatIllegalArgumentException()
.isThrownBy(() -> new OidcUserInfoEndpointFilter(authenticationManager, ""))
.isThrownBy(() -> new OidcUserInfoEndpointFilter(this.authenticationManager, ""))
.withMessage("userInfoEndpointUri cannot be empty");
}
@Test
public void doFilterWhenNotUserInfoRequestThenNotProcessed() throws Exception {
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
OidcUserInfoEndpointFilter userInfoEndpointFilter =
new OidcUserInfoEndpointFilter(authenticationManager, DEFAULT_OIDC_USER_INFO_ENDPOINT_URI);
String requestUri = "/path";
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
request.setServletPath(requestUri);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
userInfoEndpointFilter.doFilter(request, response, filterChain);
this.filter.doFilter(request, response, filterChain);
verify(filterChain).doFilter(request, response);
}
@Test
public void doFilterWhenUserInfoRequestPutThenNotProcessed() throws Exception {
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
OidcUserInfoEndpointFilter userInfoEndpointFilter =
new OidcUserInfoEndpointFilter(authenticationManager, DEFAULT_OIDC_USER_INFO_ENDPOINT_URI);
String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI;
MockHttpServletRequest request = new MockHttpServletRequest("PUT", requestUri);
request.setServletPath(requestUri);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
userInfoEndpointFilter.doFilter(request, response, filterChain);
this.filter.doFilter(request, response, filterChain);
verifyNoInteractions(authenticationManager);
verifyNoInteractions(this.authenticationManager);
verify(filterChain).doFilter(request, response);
}
@Test
public void doFilterWhenUserInfoRequestGetThenSuccess() throws Exception {
JwtAuthenticationToken principal = createJwtAuthenticationToken();
SecurityContextHolder.getContext().setAuthentication(principal);
OidcUserInfoAuthenticationToken authenticationResult = new OidcUserInfoAuthenticationToken(principal, createUserInfo());
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
when(authenticationManager.authenticate(any())).thenReturn(authenticationResult);
OidcUserInfoEndpointFilter userInfoEndpointFilter = new OidcUserInfoEndpointFilter(authenticationManager);
String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI;
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
request.setServletPath(requestUri);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
userInfoEndpointFilter.doFilter(request, response, filterChain);
verify(authenticationManager).authenticate(any());
verifyNoInteractions(filterChain);
assertThat(response.getContentType()).isEqualTo(MediaType.APPLICATION_JSON_VALUE);
assertUserInfoResponse(response.getContentAsString());
doFilterWhenUserInfoRequestThenSuccess("GET");
}
@Test
public void doFilterWhenUserInfoRequestPostThenSuccess() throws Exception {
doFilterWhenUserInfoRequestThenSuccess("POST");
}
private void doFilterWhenUserInfoRequestThenSuccess(String httpMethod) throws Exception {
JwtAuthenticationToken principal = createJwtAuthenticationToken();
SecurityContextHolder.getContext().setAuthentication(principal);
OidcUserInfoAuthenticationToken authentication = new OidcUserInfoAuthenticationToken(principal, createUserInfo());
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
when(authenticationManager.authenticate(any())).thenReturn(authentication);
OidcUserInfoEndpointFilter userInfoEndpointFilter = new OidcUserInfoEndpointFilter(authenticationManager);
when(this.authenticationManager.authenticate(any())).thenReturn(authentication);
String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI;
MockHttpServletRequest request = new MockHttpServletRequest("POST", requestUri);
MockHttpServletRequest request = new MockHttpServletRequest(httpMethod, requestUri);
request.setServletPath(requestUri);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
userInfoEndpointFilter.doFilter(request, response, filterChain);
this.filter.doFilter(request, response, filterChain);
verify(authenticationManager).authenticate(any());
verify(this.authenticationManager).authenticate(any());
verifyNoInteractions(filterChain);
assertThat(response.getContentType()).isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -162,27 +144,26 @@ public class OidcUserInfoEndpointFilterTests {
}
@Test
public void doFilterWhenAuthenticationNullThenInvalidRequestError() throws Exception {
AuthenticationManager authenticationManager = mock(AuthenticationManager.class);
when(authenticationManager.authenticate(any(Authentication.class)))
.thenReturn(new UsernamePasswordAuthenticationToken("user", "password"));
OidcUserInfoEndpointFilter userInfoEndpointFilter = new OidcUserInfoEndpointFilter(authenticationManager);
public void doFilterWhenUserInfoRequestInvalidTokenThenUnauthorizedError() throws Exception {
Authentication principal = new TestingAuthenticationToken("principal", "credentials");
SecurityContextHolder.getContext().setAuthentication(principal);
when(this.authenticationManager.authenticate(any()))
.thenThrow(new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_TOKEN));
String requestUri = DEFAULT_OIDC_USER_INFO_ENDPOINT_URI;
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
request.setServletPath(requestUri);
request.addHeader(HttpHeaders.AUTHORIZATION, "Bearer token");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
userInfoEndpointFilter.doFilter(request, response, filterChain);
this.filter.doFilter(request, response, filterChain);
verifyNoInteractions(filterChain);
assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
assertThat(response.getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value());
OAuth2Error error = readError(response);
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST);
assertThat(error.getDescription()).isEqualTo("OpenID Connect 1.0 UserInfo Error: principal cannot be null");
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_TOKEN);
}
private OAuth2Error readError(MockHttpServletResponse response) throws Exception {
@@ -251,4 +232,5 @@ public class OidcUserInfoEndpointFilterTests {
assertThat(userInfoResponse).contains("\"address\":\"Champ de Mars\\n5 Av. Anatole France\\n75007 Paris\\nFrance\"");
assertThat(userInfoResponse).contains("\"updated_at\":\"1970-01-01T00:00:00Z\"");
}
}