Skip to content

Commit 11d67db

Browse files
fmichielssenfmichielssen
committed
Merge pull request 'Fix: correctly parse roles claim containing JSON string' (#47) from bugfix/25549 into develop
2 parents cb55cc8 + ee79783 commit 11d67db

3 files changed

Lines changed: 167 additions & 41 deletions

File tree

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

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -199,44 +199,61 @@ protected GrantedAuthoritiesMapper createAuthoritiesMapper() {
199199
}
200200

201201
Object claimValue = idToken.getClaims().get(rolesClaimName);
202-
if (claimValue == null) {
203-
log.debug("No matching claim found.");
204-
} else {
205-
log.debug(String.format("Matching claim found: %s -> %s (%s)", rolesClaimName, claimValue, claimValue.getClass()));
206-
}
207202

208-
// Workaround: in some cases, getClaimAsStringList fails to parse??
209-
List<String> roles = idToken.getClaimAsStringList(rolesClaimName);
210-
if (roles == null && claimValue instanceof String) {
211-
List<String> parsedRoles = new ArrayList<>();
212-
try {
213-
Object value = new JSONParser(JSONParser.MODE_PERMISSIVE).parse((String) claimValue);
214-
if (value instanceof List) {
215-
List<?> valueList = (List<?>) value;
216-
valueList.forEach(o -> parsedRoles.add(o.toString()));
217-
}
218-
} catch (ParseException e) {
219-
// Unable to parse JSON
220-
}
221-
roles = parsedRoles;
222-
}
223-
if (roles == null) {
224-
if (log.isDebugEnabled()) log.debug("Failed to parse claim value as an array: " + claimValue);
225-
continue;
226-
}
227-
228-
for (String role: roles) {
203+
for (String role: parseRolesClaim(log, rolesClaimName, claimValue)) {
229204
String mappedRole = role.toUpperCase().startsWith("ROLE_") ? role : "ROLE_" + role;
230205
mappedAuthorities.add(new SimpleGrantedAuthority(mappedRole.toUpperCase()));
231206
}
232-
if (log.isDebugEnabled()) log.debug("The following roles were successfully parsed: " + roles);
233207
}
234208
}
235209
return mappedAuthorities;
236210
};
237211
}
238212
}
239-
213+
214+
/**
215+
* Parses the claim containing the roles to a List of Strings.
216+
* See #25549 and TestOpenIdParseClaimRoles
217+
*/
218+
public static List<String> parseRolesClaim(Logger log, String rolesClaimName, Object claimValue) {
219+
if (claimValue == null) {
220+
log.debug(String.format("No roles claim with name %s found", rolesClaimName));
221+
return new ArrayList<>();
222+
} else {
223+
log.debug(String.format("Matching claim found: %s -> %s (%s)", rolesClaimName, claimValue, claimValue.getClass()));
224+
}
225+
226+
if (claimValue instanceof Collection) {
227+
List<String> result = new ArrayList<>();
228+
for (Object object : ((Collection<?>) claimValue)) {
229+
if (object != null) {
230+
result.add(object.toString());
231+
}
232+
}
233+
log.debug(String.format("Parsed roles claim as Java Collection: %s -> %s (%s)", rolesClaimName, result, result.getClass()));
234+
return result;
235+
}
236+
237+
if (claimValue instanceof String) {
238+
List<String> result = new ArrayList<>();
239+
try {
240+
Object value = new JSONParser(JSONParser.MODE_PERMISSIVE).parse((String) claimValue);
241+
if (value instanceof List) {
242+
List<?> valueList = (List<?>) value;
243+
valueList.forEach(o -> result.add(o.toString()));
244+
}
245+
} catch (ParseException e) {
246+
// Unable to parse JSON
247+
log.debug(String.format("Unable to parse claim as JSON: %s -> %s (%s)", rolesClaimName, claimValue, claimValue.getClass()));
248+
}
249+
log.debug(String.format("Parsed roles claim as JSON: %s -> %s (%s)", rolesClaimName, result, result.getClass()));
250+
return result;
251+
}
252+
253+
log.debug(String.format("No parser found for roles claim (unsupported type): %s -> %s (%s)", rolesClaimName, claimValue, claimValue.getClass()));
254+
return new ArrayList<>();
255+
}
256+
240257
protected OidcUserService createOidcUserService() {
241258
// Use a custom UserService that supports the 'emails' array attribute.
242259
return new OidcUserService() {
@@ -258,7 +275,8 @@ public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2Authenticatio
258275
}
259276
};
260277
}
261-
278+
279+
262280
public static class CustomNameOidcUser extends DefaultOidcUser {
263281

264282
private static final long serialVersionUID = 7563253562760236634L;

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,13 @@
2222

2323
import eu.openanalytics.containerproxy.ContainerProxyApplication;
2424
import eu.openanalytics.containerproxy.backend.AbstractContainerBackend;
25-
import eu.openanalytics.containerproxy.model.runtime.Proxy;
26-
import eu.openanalytics.containerproxy.model.spec.ProxySpec;
27-
import eu.openanalytics.containerproxy.service.ProxyService;
28-
import eu.openanalytics.containerproxy.service.UserService;
29-
import eu.openanalytics.containerproxy.test.proxy.MockedUserService;
3025
import eu.openanalytics.containerproxy.test.proxy.TestProxyService.TestConfiguration;
31-
import eu.openanalytics.containerproxy.util.ProxyMappingManager;
3226
import org.junit.Test;
3327
import org.junit.runner.RunWith;
3428
import org.springframework.boot.test.context.SpringBootTest;
35-
import org.springframework.context.annotation.Bean;
36-
import org.springframework.context.annotation.Primary;
37-
import org.springframework.core.env.Environment;
3829
import org.springframework.test.context.ActiveProfiles;
3930
import org.springframework.test.context.junit4.SpringRunner;
4031

41-
import javax.inject.Inject;
42-
import java.net.URI;
43-
4432
import static org.junit.Assert.assertEquals;
4533

4634
@SpringBootTest(classes= {TestConfiguration.class, ContainerProxyApplication.class})

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/**
2+
* ContainerProxy
3+
*
4+
* Copyright (C) 2016-2021 Open Analytics
5+
*
6+
* ===========================================================================
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the Apache License as published by
10+
* The Apache Software Foundation, either version 2 of the License, or
11+
* (at your option) any later version.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* Apache License for more details.
17+
*
18+
* You should have received a copy of the Apache License
19+
* along with this program. If not, see <http://www.apache.org/licenses/>
20+
*/
21+
package eu.openanalytics.containerproxy.test.unit;
22+
23+
import eu.openanalytics.containerproxy.ContainerProxyApplication;
24+
import eu.openanalytics.containerproxy.auth.impl.OpenIDAuthenticationBackend;
25+
import eu.openanalytics.containerproxy.test.proxy.TestProxyService;
26+
import net.minidev.json.JSONArray;
27+
import org.apache.logging.log4j.LogManager;
28+
import org.apache.logging.log4j.Logger;
29+
import org.junit.jupiter.api.Test;
30+
import org.junit.runner.RunWith;
31+
import org.springframework.boot.test.context.SpringBootTest;
32+
import org.springframework.test.context.ActiveProfiles;
33+
import org.springframework.test.context.junit4.SpringRunner;
34+
35+
import java.util.ArrayList;
36+
import java.util.Arrays;
37+
import java.util.List;
38+
39+
import static org.junit.jupiter.api.Assertions.assertEquals;
40+
41+
42+
@SpringBootTest(classes= {TestProxyService.TestConfiguration.class, ContainerProxyApplication.class})
43+
@RunWith(SpringRunner.class)
44+
@ActiveProfiles("test")
45+
public class TestOpenIdParseClaimRoles {
46+
47+
private final Logger logger = LogManager.getLogger(OpenIDAuthenticationBackend.class);
48+
49+
@Test
50+
public void testParseProperJsonArray() {
51+
JSONArray claimValue = new JSONArray();
52+
claimValue.add("operators");
53+
claimValue.add("default-roles-master");
54+
claimValue.add("uma_authorization");
55+
claimValue.add("offline_access");
56+
57+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
58+
59+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
60+
}
61+
62+
@Test
63+
public void testParseList() {
64+
List<String> claimValue = new ArrayList<>();
65+
claimValue.add("operators");
66+
claimValue.add("default-roles-master");
67+
claimValue.add("uma_authorization");
68+
claimValue.add("offline_access");
69+
70+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
71+
72+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
73+
}
74+
75+
@Test
76+
public void testParseProperJsonArrayAsString() {
77+
String claimValue = "[\"operators\",\"default-roles-master\",\"uma_authorization\",\"offline_access\"]";
78+
79+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
80+
81+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
82+
}
83+
84+
@Test
85+
public void testParseNonStandardJsonArrayAsString() {
86+
String claimValue = "[operators,default-roles-master,uma_authorization,offline_access]";
87+
88+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
89+
90+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
91+
}
92+
93+
@Test
94+
public void testParseNonStandardJsonArrayAsString2() {
95+
String claimValue = "[operators, default-roles-master,uma_authorization, offline_access ]";
96+
97+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
98+
99+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
100+
}
101+
102+
@Test
103+
public void testParseNonStandardJsonArrayAsString3() {
104+
String claimValue = "[operators, default-roles-master,uma_authorization, \"offline_access\"]";
105+
106+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
107+
108+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
109+
}
110+
111+
@Test
112+
public void testParseNonStandardJsonArrayAsString4() {
113+
String claimValue = "[operators, default-roles-master,uma_authorization, 'offline_access']";
114+
115+
List<String> result = OpenIDAuthenticationBackend.parseRolesClaim(logger, "realm_access_roles", claimValue);
116+
117+
assertEquals(Arrays.asList("operators","default-roles-master","uma_authorization","offline_access"), result);
118+
}
119+
120+
}

0 commit comments

Comments
 (0)