-
Notifications
You must be signed in to change notification settings - Fork 166
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.
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
Fix sso oic OpenIdConnectAuthenticator #2845
Conversation
Hi @marevol and @hi-yamap, thanks for this MR, I've tested it out and it so far it looks like it's resolved my OIDC SSO issues. It looks like this has introduced a regression if groups are included in the JWT claim. I'm testing against the latest snapshot docker image.
A sanitised version of JWT claim looks like this (using okta)
You can see the groups claim is an array of groups as you'd expect, but it looks like the logic to build a string array was removed in this MR. Currently my workaround is to ensure the groups key is not present in the claim, which works but means I'm unable to map permissions. |
The implementation was modified to enable parsing of mixed-type arrays, which changed the type of private Object parseArray(JsonParser jsonParser) throws IOException {
List<Object> list = new ArrayList<>();
while (jsonParser.nextToken() != JsonToken.END_ARRAY) {
if (jsonParser.getCurrentToken() == JsonToken.START_OBJECT) {
list.add(parseObject(jsonParser));
} else if (jsonParser.getCurrentToken() == JsonToken.START_ARRAY) {
list.add(parseArray(jsonParser)); // Nested array
} else {
list.add(parsePrimitive(jsonParser));
}
}
if (list.stream().allMatch(String.class::isInstance)) {
return list.toArray(new String[list.size()]);
}
return list;
} |
Hey @hi-yamap, are you happy to put through a pull request for this? I can if not, would be good to fix this, as a list of groups seems pretty common. |
Fixed issues that occurred when using single sign-on with AWS Cognito.
https://discuss.codelibs.org/t/openid-sso/2467