Skip to content

Externalize coercion in ClaimAccessor #6799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Externalize coercion in ClaimAccessor #6799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions ...springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@
*/
package org.springframework.security.oauth2.client.oidc.authentication;

import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.Converter;
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.OAuth2TokenValidator;
import org.springframework.security.oauth2.core.converter.ClaimConversionService;
import org.springframework.security.oauth2.core.converter.ClaimTypeConverter;
import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
import org.springframework.security.oauth2.core.oidc.StandardClaimNames;
import org.springframework.security.oauth2.jose.jws.JwsAlgorithm;
import org.springframework.security.oauth2.jose.jws.MacAlgorithm;
import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm;
Expand All @@ -31,7 +37,10 @@
import org.springframework.util.StringUtils;

import javax.crypto.spec.SecretKeySpec;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -61,17 +70,55 @@ public final class OidcIdTokenDecoderFactory implements JwtDecoderFactory<Client
put(MacAlgorithm.HS512, "HmacSHA512");
}
};
private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER =
new ClaimTypeConverter(createDefaultClaimTypeConverters());
private final Map<String, JwtDecoder> jwtDecoders = new ConcurrentHashMap<>();
private Function<ClientRegistration, OAuth2TokenValidator<Jwt>> jwtValidatorFactory = OidcIdTokenValidator::new;
private Function<ClientRegistration, JwsAlgorithm> jwsAlgorithmResolver = clientRegistration -> SignatureAlgorithm.RS256;
private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory =
clientRegistration -> DEFAULT_CLAIM_TYPE_CONVERTER;

/**
* Returns the default {@link Converter}'s used for type conversion of claim values for an {@link OidcIdToken}.
*
* @return a {@link Map} of {@link Converter}'s keyed by {@link IdTokenClaimNames claim name}
*/
public static Map<String, Converter<Object, ?>> createDefaultClaimTypeConverters() {
Converter<Object, ?> booleanConverter = getConverter(TypeDescriptor.valueOf(Boolean.class));
Converter<Object, ?> instantConverter = getConverter(TypeDescriptor.valueOf(Instant.class));
Converter<Object, ?> urlConverter = getConverter(TypeDescriptor.valueOf(URL.class));
Converter<Object, ?> collectionStringConverter = getConverter(
TypeDescriptor.collection(Collection.class, TypeDescriptor.valueOf(String.class)));

Map<String, Converter<Object, ?>> claimTypeConverters = new HashMap<>();
claimTypeConverters.put(IdTokenClaimNames.ISS, urlConverter);
claimTypeConverters.put(IdTokenClaimNames.AUD, collectionStringConverter);
claimTypeConverters.put(IdTokenClaimNames.EXP, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.IAT, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.AUTH_TIME, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.AMR, collectionStringConverter);
claimTypeConverters.put(StandardClaimNames.EMAIL_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.PHONE_NUMBER_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.UPDATED_AT, instantConverter);
return claimTypeConverters;
}

private static Converter<Object, ?> getConverter(TypeDescriptor targetDescriptor) {
final TypeDescriptor sourceDescriptor = TypeDescriptor.valueOf(Object.class);
return source -> ClaimConversionService.getSharedInstance().convert(source, sourceDescriptor, targetDescriptor);
}

@Override
public JwtDecoder createDecoder(ClientRegistration clientRegistration) {
Assert.notNull(clientRegistration, "clientRegistration cannot be null");
return this.jwtDecoders.computeIfAbsent(clientRegistration.getRegistrationId(), key -> {
NimbusJwtDecoder jwtDecoder = buildDecoder(clientRegistration);
OAuth2TokenValidator<Jwt> jwtValidator = this.jwtValidatorFactory.apply(clientRegistration);
jwtDecoder.setJwtValidator(jwtValidator);
jwtDecoder.setJwtValidator(this.jwtValidatorFactory.apply(clientRegistration));
Converter<Map<String, Object>, Map<String, Object>> claimTypeConverter =
this.claimTypeConverterFactory.apply(clientRegistration);
if (claimTypeConverter != null) {
jwtDecoder.setClaimSetConverter(claimTypeConverter);
}
return jwtDecoder;
});
}
Expand Down Expand Up @@ -163,4 +210,16 @@ public final void setJwsAlgorithmResolver(Function<ClientRegistration, JwsAlgori
Assert.notNull(jwsAlgorithmResolver, "jwsAlgorithmResolver cannot be null");
this.jwsAlgorithmResolver = jwsAlgorithmResolver;
}

/**
* Sets the factory that provides a {@link Converter} used for type conversion of claim values for an {@link OidcIdToken}.
* The default is {@link ClaimTypeConverter} for all {@link ClientRegistration clients}.
*
* @param claimTypeConverterFactory the factory that provides a {@link Converter} used for type conversion
* of claim values for a specific {@link ClientRegistration client}
*/
public final void setClaimTypeConverterFactory(Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory) {
Assert.notNull(claimTypeConverterFactory, "claimTypeConverterFactory cannot be null");
this.claimTypeConverterFactory = claimTypeConverterFactory;
}
}
63 changes: 61 additions & 2 deletions ...amework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@
*/
package org.springframework.security.oauth2.client.oidc.authentication;

import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.Converter;
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.OAuth2TokenValidator;
import org.springframework.security.oauth2.core.converter.ClaimConversionService;
import org.springframework.security.oauth2.core.converter.ClaimTypeConverter;
import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
import org.springframework.security.oauth2.core.oidc.StandardClaimNames;
import org.springframework.security.oauth2.jose.jws.JwsAlgorithm;
import org.springframework.security.oauth2.jose.jws.MacAlgorithm;
import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm;
Expand All @@ -31,7 +37,10 @@
import org.springframework.util.StringUtils;

import javax.crypto.spec.SecretKeySpec;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -61,17 +70,55 @@ public final class ReactiveOidcIdTokenDecoderFactory implements ReactiveJwtDecod
put(MacAlgorithm.HS512, "HmacSHA512");
}
};
private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud a bit, but this API doesn't account for the fact that conversion may be a blocking operation. For example, if we have an InputStream that needs converted to an Object (i.e. reading in JSON) that is blocking and the API does not return a reactive type.

I'm wondering if in practice this is a concern in this situation since we have likely already done all the IO (it probably is converting from a String not an InputStream). What do we think the likelihood is that we will need to do a blocking operation during conversion? Are there other reasons we might need to block during conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwinch

I'm wondering if in practice this is a concern in this situation since we have likely already done all the IO

The source of claims passed into the Converter should have already been parsed from it's JSON representation. For the ID Token use case, Nimbus's JSON parser would have read/parsed all claims in the raw ID Token (including custom claims) into an instance of JWTClaimsSet. Even for cases where there is a custom claim and it's a JSON object, Nimbus would parse it into a Map.

Are there other reasons we might need to block during conversion?

At this point, I do not see a use case where a blocking operation could happen with the default NimbusReactiveJwtDecoder.

new ClaimTypeConverter(createDefaultClaimTypeConverters());
private final Map<String, ReactiveJwtDecoder> jwtDecoders = new ConcurrentHashMap<>();
private Function<ClientRegistration, OAuth2TokenValidator<Jwt>> jwtValidatorFactory = OidcIdTokenValidator::new;
private Function<ClientRegistration, JwsAlgorithm> jwsAlgorithmResolver = clientRegistration -> SignatureAlgorithm.RS256;
private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory =
clientRegistration -> DEFAULT_CLAIM_TYPE_CONVERTER;

/**
* Returns the default {@link Converter}'s used for type conversion of claim values for an {@link OidcIdToken}.
*
* @return a {@link Map} of {@link Converter}'s keyed by {@link IdTokenClaimNames claim name}
*/
public static Map<String, Converter<Object, ?>> createDefaultClaimTypeConverters() {
Converter<Object, ?> booleanConverter = getConverter(TypeDescriptor.valueOf(Boolean.class));
Converter<Object, ?> instantConverter = getConverter(TypeDescriptor.valueOf(Instant.class));
Converter<Object, ?> urlConverter = getConverter(TypeDescriptor.valueOf(URL.class));
Converter<Object, ?> collectionStringConverter = getConverter(
TypeDescriptor.collection(Collection.class, TypeDescriptor.valueOf(String.class)));

Map<String, Converter<Object, ?>> claimTypeConverters = new HashMap<>();
claimTypeConverters.put(IdTokenClaimNames.ISS, urlConverter);
claimTypeConverters.put(IdTokenClaimNames.AUD, collectionStringConverter);
claimTypeConverters.put(IdTokenClaimNames.EXP, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.IAT, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.AUTH_TIME, instantConverter);
claimTypeConverters.put(IdTokenClaimNames.AMR, collectionStringConverter);
claimTypeConverters.put(StandardClaimNames.EMAIL_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.PHONE_NUMBER_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.UPDATED_AT, instantConverter);
return claimTypeConverters;
}

private static Converter<Object, ?> getConverter(TypeDescriptor targetDescriptor) {
final TypeDescriptor sourceDescriptor = TypeDescriptor.valueOf(Object.class);
return source -> ClaimConversionService.getSharedInstance().convert(source, sourceDescriptor, targetDescriptor);
}

@Override
public ReactiveJwtDecoder createDecoder(ClientRegistration clientRegistration) {
Assert.notNull(clientRegistration, "clientRegistration cannot be null");
return this.jwtDecoders.computeIfAbsent(clientRegistration.getRegistrationId(), key -> {
NimbusReactiveJwtDecoder jwtDecoder = buildDecoder(clientRegistration);
OAuth2TokenValidator<Jwt> jwtValidator = this.jwtValidatorFactory.apply(clientRegistration);
jwtDecoder.setJwtValidator(jwtValidator);
jwtDecoder.setJwtValidator(this.jwtValidatorFactory.apply(clientRegistration));
Converter<Map<String, Object>, Map<String, Object>> claimTypeConverter =
this.claimTypeConverterFactory.apply(clientRegistration);
if (claimTypeConverter != null) {
jwtDecoder.setClaimSetConverter(claimTypeConverter);
}
return jwtDecoder;
});
}
Expand Down Expand Up @@ -163,4 +210,16 @@ public final void setJwsAlgorithmResolver(Function<ClientRegistration, JwsAlgori
Assert.notNull(jwsAlgorithmResolver, "jwsAlgorithmResolver cannot be null");
this.jwsAlgorithmResolver = jwsAlgorithmResolver;
}

/**
* Sets the factory that provides a {@link Converter} used for type conversion of claim values for an {@link OidcIdToken}.
* The default is {@link ClaimTypeConverter} for all {@link ClientRegistration clients}.
*
* @param claimTypeConverterFactory the factory that provides a {@link Converter} used for type conversion
* of claim values for a specific {@link ClientRegistration client}
*/
public final void setClaimTypeConverterFactory(Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory) {
Assert.notNull(claimTypeConverterFactory, "claimTypeConverterFactory cannot be null");
this.claimTypeConverterFactory = claimTypeConverterFactory;
}
}
70 changes: 65 additions & 5 deletions ...g/springframework/security/oauth2/client/oidc/userinfo/OidcReactiveOAuth2UserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -15,25 +15,34 @@
*/
package org.springframework.security.oauth2.client.oidc.userinfo;

import java.util.HashSet;
import java.util.Set;

import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.Converter;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.client.userinfo.DefaultReactiveOAuth2UserService;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
import org.springframework.security.oauth2.client.userinfo.ReactiveOAuth2UserService;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
import org.springframework.security.oauth2.core.converter.ClaimConversionService;
import org.springframework.security.oauth2.core.converter.ClaimTypeConverter;
import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
import org.springframework.security.oauth2.core.oidc.StandardClaimNames;
import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;

import reactor.core.publisher.Mono;

import java.time.Instant;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

/**
* An implementation of an {@link ReactiveOAuth2UserService} that supports OpenID Connect 1.0 Provider's.
*
Expand All @@ -50,8 +59,36 @@ public class OidcReactiveOAuth2UserService implements

private static final String INVALID_USER_INFO_RESPONSE_ERROR_CODE = "invalid_user_info_response";

private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER =
new ClaimTypeConverter(createDefaultClaimTypeConverters());

private ReactiveOAuth2UserService<OAuth2UserRequest, OAuth2User> oauth2UserService = new DefaultReactiveOAuth2UserService();

private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory =
clientRegistration -> DEFAULT_CLAIM_TYPE_CONVERTER;

/**
* Returns the default {@link Converter}'s used for type conversion of claim values for an {@link OidcUserInfo}.

* @since 5.2
* @return a {@link Map} of {@link Converter}'s keyed by {@link StandardClaimNames claim name}
*/
public static Map<String, Converter<Object, ?>> createDefaultClaimTypeConverters() {
Converter<Object, ?> booleanConverter = getConverter(TypeDescriptor.valueOf(Boolean.class));
Converter<Object, ?> instantConverter = getConverter(TypeDescriptor.valueOf(Instant.class));

Map<String, Converter<Object, ?>> claimTypeConverters = new HashMap<>();
claimTypeConverters.put(StandardClaimNames.EMAIL_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.PHONE_NUMBER_VERIFIED, booleanConverter);
claimTypeConverters.put(StandardClaimNames.UPDATED_AT, instantConverter);
return claimTypeConverters;
}

private static Converter<Object, ?> getConverter(TypeDescriptor targetDescriptor) {
final TypeDescriptor sourceDescriptor = TypeDescriptor.valueOf(Object.class);
return source -> ClaimConversionService.getSharedInstance().convert(source, sourceDescriptor, targetDescriptor);
}

@Override
public Mono<OidcUser> loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
Assert.notNull(userRequest, "userRequest cannot be null");
Expand All @@ -76,8 +113,10 @@ private Mono<OidcUserInfo> getUserInfo(OidcUserRequest userRequest) {
if (!OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest)) {
return Mono.empty();
}

return this.oauth2UserService.loadUser(userRequest)
.map(OAuth2User::getAttributes)
.map(claims -> convertClaims(claims, userRequest.getClientRegistration()))
.map(OidcUserInfo::new)
.doOnNext(userInfo -> {
String subject = userInfo.getSubject();
Expand All @@ -88,8 +127,29 @@ private Mono<OidcUserInfo> getUserInfo(OidcUserRequest userRequest) {
});
}

private Map<String, Object> convertClaims(Map<String, Object> claims, ClientRegistration clientRegistration) {
Converter<Map<String, Object>, Map<String, Object>> claimTypeConverter =
this.claimTypeConverterFactory.apply(clientRegistration);
return claimTypeConverter != null ?
claimTypeConverter.convert(claims) :
DEFAULT_CLAIM_TYPE_CONVERTER.convert(claims);
}

public void setOauth2UserService(ReactiveOAuth2UserService<OAuth2UserRequest, OAuth2User> oauth2UserService) {
Assert.notNull(oauth2UserService, "oauth2UserService cannot be null");
this.oauth2UserService = oauth2UserService;
}

/**
* Sets the factory that provides a {@link Converter} used for type conversion of claim values for an {@link OidcUserInfo}.
* The default is {@link ClaimTypeConverter} for all {@link ClientRegistration clients}.
*
* @since 5.2
* @param claimTypeConverterFactory the factory that provides a {@link Converter} used for type conversion
* of claim values for a specific {@link ClientRegistration client}
*/
public final void setClaimTypeConverterFactory(Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory) {
Assert.notNull(claimTypeConverterFactory, "claimTypeConverterFactory cannot be null");
this.claimTypeConverterFactory = claimTypeConverterFactory;
}
}
Loading