Skip to content

Commit eaf1b64

Browse files
committed
The jwt-bearer grant confuses the internal (mapped) authorities(scopes) from the incoming(external) groups
The way the flow should work is 1. Extract external groups via the external_groups attribute mapping 2. Filter the external groups via the external group white list (Note #1) 3. Store the filtered external groups (external authorities) 4. Map the filtered external groups to scope, and store the scopes (authorities) Note #1 For LDAP, and empty whitelist means no external groups are allowed For SAML and OAUTH/OIDC an empty whitelist means all external groups are allowed
1 parent ba4b3f1 commit eaf1b64

2 files changed

Lines changed: 15 additions & 4 deletions

File tree

server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ public AuthenticationData getExternalAuthenticationDetails(Authentication authen
271271
authenticationData.setUsername(username);
272272

273273
List<? extends GrantedAuthority> oidcAuthorities = extractExternalOAuthUserAuthorities(attributeMappings, claims);
274+
oidcAuthorities = filterOidcAuthorities(config, oidcAuthorities);
274275
List<? extends GrantedAuthority> authorities;
275276
AbstractExternalOAuthIdentityProviderDefinition.OAuthGroupMappingMode groupMappingMode = config.getGroupMappingMode() != null ?
276277
config.getGroupMappingMode() : AbstractExternalOAuthIdentityProviderDefinition.OAuthGroupMappingMode.EXPLICITLY_MAPPED;
@@ -283,7 +284,8 @@ public AuthenticationData getExternalAuthenticationDetails(Authentication authen
283284
authorities = mapAuthorities(codeToken.getOrigin(), oidcAuthorities);
284285
break;
285286
}
286-
authenticationData.setAuthorities(filterOidcAuthorities(config, authorities));
287+
authenticationData.setAuthorities(authorities); //the filter should apply to external authorities - not internal
288+
authenticationData.setExternalAuthorities(oidcAuthorities);
287289
ofNullable(attributeMappings).ifPresent(map -> authenticationData.setAttributeMappings(new HashMap<>(map)));
288290
return authenticationData;
289291
}
@@ -358,7 +360,7 @@ protected void populateAuthenticationAttributes(UaaAuthentication authentication
358360
authentication.setUserAttributes(userAttributes);
359361
authentication.setExternalGroups(
360362
ofNullable(
361-
authenticationData.getAuthorities()
363+
authenticationData.getExternalAuthorities()
362364
)
363365
.orElse(emptyList())
364366
.stream()
@@ -975,6 +977,7 @@ protected static class AuthenticationData {
975977
private Map<String, Object> claims;
976978
private String username;
977979
private List<? extends GrantedAuthority> authorities;
980+
private List<? extends GrantedAuthority> externalAuthorities;
978981
private Map<String, Object> attributeMappings;
979982

980983
public Map<String, Object> getAttributeMappings() {
@@ -1008,5 +1011,13 @@ public List<? extends GrantedAuthority> getAuthorities() {
10081011
public void setAuthorities(List<? extends GrantedAuthority> authorities) {
10091012
this.authorities = authorities;
10101013
}
1014+
1015+
public List<? extends GrantedAuthority> getExternalAuthorities() {
1016+
return externalAuthorities;
1017+
}
1018+
1019+
public void setExternalAuthorities(List<? extends GrantedAuthority> externalAuthorities) {
1020+
this.externalAuthorities = externalAuthorities;
1021+
}
10111022
}
10121023
}

server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ void getExternalAuthenticationDetails_whenUaaToken_internalAndExternalRolesAreDi
419419
//external authorities
420420
assertThat(authenticationData.getExternalAuthorities()).hasSize(2);
421421
Set<String> externalAuthorities = AuthorityUtils.authorityListToSet(authenticationData.getExternalAuthorities());
422-
assertThat(roles.toArray()).contains(externalAuthorities.toArray());
422+
assertThat(externalAuthorities).containsAll(roles);
423423
//internal (mapped) authorities
424424
assertThat(authenticationData.getAuthorities()).hasSize(3);
425425
Set<String> internalAuthorities = AuthorityUtils.authorityListToSet(authenticationData.getAuthorities());
@@ -466,7 +466,7 @@ void getExternalAuthenticationDetails_whenUaaToken_ExternalRolesAreFiltered() {
466466
//external authorities
467467
assertThat(authenticationData.getExternalAuthorities()).hasSize(1);
468468
Set<String> externalAuthorities = AuthorityUtils.authorityListToSet(authenticationData.getExternalAuthorities());
469-
assertThat(roles.toArray()).contains(externalAuthorities.toArray());
469+
assertThat(externalAuthorities).containsExactly("manager.us");
470470
//internal (mapped) authorities
471471
assertThat(authenticationData.getAuthorities()).hasSize(1);
472472
Set<String> internalAuthorities = AuthorityUtils.authorityListToSet(authenticationData.getAuthorities());

0 commit comments

Comments
 (0)