Skip to content

Commit

Permalink
fix: User OIDC standard claims, if mapping is ambiguous
Browse files Browse the repository at this point in the history 
With #1925 there was a fix already, however using custom claims can lead to error situations.
With this fix, that error situations are prevented if standard claims in id_token, e.g. name is an array with different values,
but family_name and given_name are also in the token as string.
This fix is for OIDC IdP integration, therefore it makes sense to use as fallback also OIDC standard claims.
  • Loading branch information
@strehle
strehle committed Oct 23, 2023
1 parent 1bb8a2b commit 143c93d
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 16 deletions.
55 changes: 39 additions & 16 deletions ...java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,11 @@ protected UaaUser getUser(Authentication request, AuthenticationData authenticat
Map<String, Object> claims = authenticationData.getClaims();

String username = authenticationData.getUsername();
String givenName = getMappedClaim(givenNameClaim, "given_name", claims);
String familyName = getMappedClaim(familyNameClaim, "family_name", claims);
String phoneNumber = getMappedClaim(phoneClaim, "phone_number", claims);
String email = getMappedClaim(emailClaim, "email",claims);
Object verifiedObj = claims.get(emailVerifiedClaim == null ? "email_verified" : emailVerifiedClaim);
String givenName = getMappedClaim(givenNameClaim, ClaimConstants.GIVEN_NAME, claims);
String familyName = getMappedClaim(familyNameClaim, ClaimConstants.FAMILY_NAME, claims);
String phoneNumber = getMappedClaim(phoneClaim, ClaimConstants.PHONE_NUMBER, claims);
String email = getMappedClaim(emailClaim, ClaimConstants.EMAIL,claims);
Object verifiedObj = claims.get(emailVerifiedClaim == null ? ClaimConstants.EMAIL_VERIFIED : emailVerifiedClaim);
boolean verified = verifiedObj instanceof Boolean ? (Boolean)verifiedObj: false;

if (email == null) {
Expand Down Expand Up @@ -405,29 +405,52 @@ protected UaaUser getUser(Authentication request, AuthenticationData authenticat
return null;
}

private String getMappedClaim(String externalName, String internalName, Map<String, Object> claims) {
String claimName = isNull(externalName) ? internalName : externalName;
private String getMappedClaim(String externalName, String oidcName, Map<String, Object> claims) {
String claimValue = null;
BadCredentialsException exception = null;
String claimName = isNull(externalName) ? oidcName : externalName;
Object claimObject = claims.get(claimName);

if (isNull(claimObject)) {
return null;
Object oidcClaim = getOidcClaim(externalName, oidcName, claims, claimName);
if (isNull(oidcClaim)) {
return null;
}
claimObject = oidcClaim;
}
if (claimObject instanceof String) {
return (String) claimObject;
}
if (claimObject instanceof Collection) {
claimValue = (String) claimObject;
} else if (claimObject instanceof Collection) {
Set<String> entry = ((Collection<?>) claimObject).stream().map(String.class::cast).collect(Collectors.toSet());
if (entry.size() == 1 ) {
return entry.stream().collect(Collectors.toList()).get(0);
if (entry.size() == 1) {
claimValue = entry.stream().collect(Collectors.toList()).get(0);
} else if (entry.isEmpty()) {
return null;
} else {
logger.warn("Claim mapping for {} attribute is ambiguous. ({}) ", claimName, entry.size());
throw new BadCredentialsException("Claim mapping for " + internalName + " attribute is ambiguous");
exception = new BadCredentialsException("Claim mapping for " + oidcName + " attribute is ambiguous");
}
}
logger.warn("Claim attribute {} cannot be mapped because of invalid type {} ", claimName, claimObject.getClass().getSimpleName());
throw new BadCredentialsException("External token attribute " + claimName + " cannot be mapped to user attribute " + internalName);
if (isNull(claimValue)) {
Object oidcClaim = getOidcClaim(externalName, oidcName, claims, claimName);
if (oidcClaim instanceof String) {
claimValue = (String) oidcClaim;
} else {
logger.warn("Claim attribute {} cannot be mapped because of invalid type {} ", claimName,
ofNullable(claimObject).map(Object::getClass).map(Class::getSimpleName).orElse("unknown"));
throw exception != null ? exception :
new BadCredentialsException("External token attribute " + claimName + " cannot be mapped to user attribute " + oidcName);
}
}
return claimValue;
}

// OIDC standard claims, see https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
private static Object getOidcClaim(String externalName, String oidcName, Map<String, Object> claims, String claimName) {
if (claimName.equals(externalName)) {
return claims.get(oidcName);
}
return null;
}

private List<? extends GrantedAuthority> extractExternalOAuthUserAuthorities(Map<String, Object> attributeMappings, Map<String, Object> claims) {
Expand Down
37 changes: 37 additions & 0 deletions .../org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,4 +429,41 @@ public void getUser_doesThrowWhenIdTokenMappingIsWrongType() {
ExternalOAuthCodeToken oidcAuthentication = new ExternalOAuthCodeToken(null, origin, "http://google.com", idTokenJwt, "accesstoken", "signedrequest");
authManager.getUser(oidcAuthentication, authManager.getExternalAuthenticationDetails(oidcAuthentication));
}

@Test
public void getUser_doesNotThrowWhenIdTokenMappingIsArrayButAlsoOidcStandardClaims() {
Map<String, Object> header = map(
entry(HeaderParameterNames.ALGORITHM, JWSAlgorithm.HS256.getName()),
entry(HeaderParameterNames.KEY_ID, OIDC_PROVIDER_KEY)
);
Signer signer = new RsaSigner(oidcProviderTokenSigningKey);
Map<String, Object> claims = map(
entry("family_name", "Foo"),
entry("given_name", "Bar"),
entry("email", "bar.foo@domain.org"),
entry("external_family_name", Arrays.asList("foo", "Foo")),
entry("external_given_name", Arrays.asList("bar", "Bar")),
entry(ISS, oidcConfig.getIssuer()),
entry(AUD, "uaa-relying-party"),
entry(EXPIRY_IN_SECONDS, ((int) (System.currentTimeMillis()/1000L)) + 60),
entry(SUB, "abc-def-asdf")
);
Map<String, Object> externalGroupMapping = map(
entry(FAMILY_NAME_ATTRIBUTE_NAME, "external_family_name"),
entry(ExternalIdentityProviderDefinition.GIVEN_NAME_ATTRIBUTE_NAME, "external_given_name"),
entry(ExternalIdentityProviderDefinition.EMAIL_ATTRIBUTE_NAME, "external_email"),
entry(ExternalIdentityProviderDefinition.PHONE_NUMBER_ATTRIBUTE_NAME, "external_phone")
);
oidcConfig.setAttributeMappings(externalGroupMapping);
provider.setConfig(oidcConfig);
IdentityZoneHolder.get().getConfig().getTokenPolicy().setKeys(Collections.singletonMap("uaa-key", uaaIdentityZoneTokenSigningKey));
String idTokenJwt = UaaTokenUtils.constructToken(header, claims, signer);

ExternalOAuthCodeToken oidcAuthentication = new ExternalOAuthCodeToken(null, origin, "http://google.com", idTokenJwt, "accesstoken", "signedrequest");
UaaUser uaaUser = authManager.getUser(oidcAuthentication, authManager.getExternalAuthenticationDetails(oidcAuthentication));
assertNotNull(uaaUser);
assertEquals("Bar", uaaUser.getGivenName());
assertEquals("Foo", uaaUser.getFamilyName());
assertEquals("bar.foo@domain.org", uaaUser.getEmail());
}
}

0 comments on commit 143c93d

Please sign in to comment.