Skip to content

Commit 4a6ac66

Browse files
LEDfanLEDfan
committed
Fix #32580: extract roles claim from user info
1 parent e9298c2 commit 4a6ac66

3 files changed

Lines changed: 48 additions & 37 deletions

File tree

‎src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
5555
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
5656
import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
57+
import org.springframework.security.oauth2.core.oidc.StandardClaimAccessor;
5758
import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
5859
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
5960
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
@@ -98,17 +99,16 @@ public class OpenIDAuthenticationBackend implements IAuthenticationBackend {
9899
@Inject
99100
private ContextPathHelper contextPathHelper;
100101

101-
102102
/**
103103
* Parses the claim containing the roles to a List of Strings.
104104
* See #25549 and TestOpenIdParseClaimRoles
105105
*/
106-
public static List<String> parseRolesClaim(Logger log, String rolesClaimName, Object claimValue) {
106+
public static List<String> parseRolesClaim(Logger log, String logInfo, String rolesClaimName, Object claimValue) {
107107
if (claimValue == null) {
108-
log.debug(String.format("No roles claim with name %s found", rolesClaimName));
108+
log.debug(String.format("No roles claim with name %s found in %s", rolesClaimName, logInfo));
109109
return new ArrayList<>();
110110
} else {
111-
log.debug(String.format("Matching claim found: %s -> %s (%s)", rolesClaimName, claimValue, claimValue.getClass()));
111+
log.debug(String.format("Matching claim found in %s: %s -> %s (%s)", logInfo, rolesClaimName, claimValue, claimValue.getClass()));
112112
}
113113

114114
if (claimValue instanceof Collection) {
@@ -141,6 +141,39 @@ public static List<String> parseRolesClaim(Logger log, String rolesClaimName, Ob
141141
return new ArrayList<>();
142142
}
143143

144+
public Set<GrantedAuthority> parseClaims(StandardClaimAccessor standardClaimAccessor, String rolesClaimName) {
145+
String logInfo;
146+
if (standardClaimAccessor instanceof OidcIdToken) {
147+
logInfo = "ID token";
148+
} else {
149+
logInfo = "user info";
150+
}
151+
if (log.isDebugEnabled()) {
152+
String lineSep = System.lineSeparator();
153+
String claims = standardClaimAccessor
154+
.getClaims()
155+
.entrySet()
156+
.stream()
157+
.map(e -> String.format("%s -> %s", e.getKey(), e.getValue()))
158+
.collect(Collectors.joining(lineSep));
159+
log.debug(String.format("Available claims in %s (%d):%s%s", logInfo, standardClaimAccessor.getClaims().size(), lineSep, claims));
160+
}
161+
162+
Object claimValue = standardClaimAccessor
163+
.getClaims()
164+
.get(rolesClaimName);
165+
166+
Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
167+
for (String role : parseRolesClaim(log, logInfo, rolesClaimName, claimValue)) {
168+
String mappedRole = role
169+
.toUpperCase()
170+
.startsWith("ROLE_") ? role : "ROLE_" + role;
171+
mappedAuthorities.add(new SimpleGrantedAuthority(mappedRole.toUpperCase()));
172+
}
173+
174+
return mappedAuthorities;
175+
}
176+
144177
private static OAuth2AuthorizedClient refreshClient(String principalName) {
145178
return oAuth2AuthorizedClientService.loadAuthorizedClient(REG_ID, principalName);
146179
}
@@ -260,31 +293,9 @@ protected GrantedAuthoritiesMapper createAuthoritiesMapper() {
260293
for (GrantedAuthority auth : authorities) {
261294
if (auth instanceof OidcUserAuthority) {
262295
OidcIdToken idToken = ((OidcUserAuthority) auth).getIdToken();
263-
264-
if (log.isDebugEnabled()) {
265-
String lineSep = System.getProperty("line.separator");
266-
String claims = idToken
267-
.getClaims()
268-
.entrySet()
269-
.stream()
270-
.map(e -> String.format("%s -> %s", e.getKey(), e.getValue()))
271-
.collect(Collectors.joining(lineSep));
272-
log.debug(String.format("Checking for roles in claim '%s'. Available claims in ID token (%d):%s%s",
273-
rolesClaimName, idToken
274-
.getClaims()
275-
.size(), lineSep, claims));
276-
}
277-
278-
Object claimValue = idToken
279-
.getClaims()
280-
.get(rolesClaimName);
281-
282-
for (String role : parseRolesClaim(log, rolesClaimName, claimValue)) {
283-
String mappedRole = role
284-
.toUpperCase()
285-
.startsWith("ROLE_") ? role : "ROLE_" + role;
286-
mappedAuthorities.add(new SimpleGrantedAuthority(mappedRole.toUpperCase()));
287-
}
296+
mappedAuthorities.addAll(parseClaims(idToken, rolesClaimName));
297+
OidcUserInfo userInfo = ((OidcUserAuthority) auth).getUserInfo();
298+
mappedAuthorities.addAll(parseClaims(userInfo, rolesClaimName));
288299
}
289300
}
290301
return mappedAuthorities;

‎src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ private Converter<Jwt, AbstractAuthenticationToken> jwtAuthenticationConverter()
320320
Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
321321
if (rolesClaim != null) {
322322
Object claimValue = source.getClaim(rolesClaim);
323-
for (String role : OpenIDAuthenticationBackend.parseRolesClaim(logger, rolesClaim, claimValue)) {
323+
for (String role : OpenIDAuthenticationBackend.parseRolesClaim(logger, "jwt", rolesClaim, claimValue)) {
324324
String mappedRole = role.toUpperCase().startsWith("ROLE_") ? role : "ROLE_" + role;
325325
mappedAuthorities.add(new SimpleGrantedAuthority(mappedRole.toUpperCase()));
326326
}

‎src/test/java/eu/openanalytics/containerproxy/test/unit/TestOpenIdParseClaimRoles.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public void testParseProperJsonArray() {
5555
claimValue.add("uma_authorization");
5656
claimValue.add("offline_access");
5757

58-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
58+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
5959

6060
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
6161
}
@@ -68,7 +68,7 @@ public void testParseList() {
6868
claimValue.add("uma_authorization");
6969
claimValue.add("offline_access");
7070

71-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
71+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
7272

7373
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
7474
}
@@ -77,7 +77,7 @@ public void testParseList() {
7777
public void testParseProperJsonArrayAsString() {
7878
String claimValue = "[\"operators\",\"default-roles-master\",\"uma_authorization\",\"offline_access\"]";
7979

80-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
80+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
8181

8282
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
8383
}
@@ -86,7 +86,7 @@ public void testParseProperJsonArrayAsString() {
8686
public void testParseNonStandardJsonArrayAsString() {
8787
String claimValue = "[operators,default-roles-master,uma_authorization,offline_access]";
8888

89-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
89+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
9090

9191
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
9292
}
@@ -95,7 +95,7 @@ public void testParseNonStandardJsonArrayAsString() {
9595
public void testParseNonStandardJsonArrayAsString2() {
9696
String claimValue = "[operators, default-roles-master,uma_authorization, offline_access ]";
9797

98-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
98+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
9999

100100
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
101101
}
@@ -104,7 +104,7 @@ public void testParseNonStandardJsonArrayAsString2() {
104104
public void testParseNonStandardJsonArrayAsString3() {
105105
String claimValue = "[operators, default-roles-master,uma_authorization, \"offline_access\"]";
106106

107-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
107+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
108108

109109
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
110110
}
@@ -113,7 +113,7 @@ public void testParseNonStandardJsonArrayAsString3() {
113113
public void testParseNonStandardJsonArrayAsString4() {
114114
String claimValue = "[operators, default-roles-master,uma_authorization, 'offline_access']";
115115

116-
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
116+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "", "realm_access_roles", claimValue);
117117

118118
Assertions.assertEquals(Arrays.asList("operators", "default-roles-master", "uma_authorization", "offline_access"), result);
119119
}

0 commit comments

Comments
 (0)