Skip to content
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

OIDC: Userinfo not consulted when looking for roles claim #85

Closed
akkornel opened this issue Mar 5, 2024 · 2 comments
Closed

OIDC: Userinfo not consulted when looking for roles claim #85

akkornel opened this issue Mar 5, 2024 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@akkornel
Copy link

akkornel commented Mar 5, 2024

Hello! I'd like to report an issue related to OpenID Connect (OIDC) and the roles-claim: Shinyproxy does not support parsing a roles claim from OIDC userinfo.

I am configuring a new ShinyProxy installation, using OpenID Connect. The IdP is running Shibboleth. (Shibboleth has been known for a long time as a SAML IdP; since version 3.x, Shibboleth has supported OIDC as an IdP.) ShinyProxy is configured with Auth/Token/Userinfo/JWKS URls, and PKCE is enabled.

In my case, the roles information is contained in an OIDC claim named eduPersonEntitlement, and to get this claim, I need to request the scope eduperson_entitlement. I have confirmed that ShinyProxy is properly requesting this scope from the IdP (I can see it when I look at the HTTP requests using Firefox dev tools). I have talked to the Shibboleth admins, and they confirmed I am making the request properly, and that the eduPersonEntitlement claim is being provided.

I turned on auth debugging, so I could see the claims that were being examined, but I still did not see my eduPersonEntitlement claim. I also noticed that some other claims were not being provided. I started to wonder if the userinfo endpoint was not being checked.

Eventually, I found createAuthoritiesMapper in eu.openanalytics.containerproxy.auth.OpenIDAuthenticationBackend:

protected GrantedAuthoritiesMapper createAuthoritiesMapper() {
String rolesClaimName = environment.getProperty("proxy.openid.roles-claim");
if (rolesClaimName == null || rolesClaimName.isEmpty()) {
return authorities -> authorities;
} else {
return authorities -> {
Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
for (GrantedAuthority auth: authorities) {
if (auth instanceof OidcUserAuthority) {
OidcIdToken idToken = ((OidcUserAuthority) auth).getIdToken();
if (log.isDebugEnabled()) {
String lineSep = System.getProperty("line.separator");
String claims = idToken.getClaims().entrySet().stream()
.map(e -> String.format("%s -> %s", e.getKey(), e.getValue()))
.collect(Collectors.joining(lineSep));
log.debug(String.format("Checking for roles in claim '%s'. Available claims in ID token (%d):%s%s",
rolesClaimName, idToken.getClaims().size(), lineSep, claims));
}
Object claimValue = idToken.getClaims().get(rolesClaimName);
for (String role: parseRolesClaim(log, rolesClaimName, claimValue)) {
String mappedRole = role.toUpperCase().startsWith("ROLE_") ? role : "ROLE_" + role;
mappedAuthorities.add(new SimpleGrantedAuthority(mappedRole.toUpperCase()));
}
}
}
return mappedAuthorities;
};
}
}

I see the code is looking at the ID Token (on line 218), but it doesn't look like the userinfo is being checked. So, I think that is the problem: When looking for the roles, only the ID Token is being checked, not the userinfo.

Although I was able to find (what I think is) the issue, I do not have the Java skills necessary to patch, build, or test.

Please let me know if you have any questions, or if I put this report in the wrong place. Thanks very much!

@LEDfan LEDfan added the enhancement New feature or request label Mar 11, 2024
@LEDfan LEDfan added this to the Next milestone Mar 11, 2024
@LEDfan
Copy link
Member

LEDfan commented Mar 11, 2024

Hi, thank you for your detailed information. I just implemented this and it will be included in the next release, which will be available soon.

@LEDfan
Copy link
Member

LEDfan commented May 7, 2024

Hi, this is now part of the ShinyProxy 3.1.0 we released today. Thanks again for reporting this bug!

@LEDfan LEDfan closed this as completed May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants