-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: role extraction from access token in keycloak oidc #1916
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
Conversation
Have you looked at the history of this provider? Is it possible the behaviour has changed in keycloak and your change makes it supported for newer versions but breaks older versions? If you can get a keycloak maintainer to comment that would help as well |
One of the devs from redhat already confirmed this: Furthermore, access_token is from Oauth2 spec (and used in keycloak provider), and id_token is from oidc spec. |
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
IMHO, this is still relevant. |
This sounds like something we should fix, but I'm wondering if there's a way to avoid a breaking change here, any thoughts? |
I'm not quite sure what it could break as the later adds aud claims defacto among other things if I remember correctly. |
Hello, I’ve asked Keycloak developers for some clarification: |
As stated by @Elektordi ,
If you want to use access_token you can use the |
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
Still relevant |
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
Yes, I know what Oauth2 and OIDC are (technically OIDC is based on Oauth2, so the access token is also and OIDC thing, but I won’t nitpick on this).
Well, if you want pure OIDC, I guess you can just use the OpenID Connect provider. I’m sure you’re aware that the Keycloak provider is already making use of the access token to get the roles and put them in the group list 😉 oauth2-proxy/providers/keycloak_oidc.go Lines 109 to 121 in b78c391
oauth2-proxy/providers/keycloak_oidc.go Lines 142 to 159 in b78c391
|
@yann-soubeyrand yes, I know, but in this case the I'd also argue: why having 2 code bases to talk to keycloak using the exact same token while the very purpose of |
The problem is that, by default in Keycloak, no ID token claim contains the roles. So it seems there’s no way to avoid user configuration and to exclusively use the ID token. That’s why I asked on Keycloak discussions an advice on the way to go, but I got no answer so far. |
I agree, but roles is a more specific feature, and not a needed one (you can use OIDC without roles).
That is what I was thinking, the And, btw, multiple people have opened issues because of this bug. |
OK, but then what’s the added value of the Keycloak OIDC provider compared to the generic OIDC provider? |
good question, initially probably not that much, now there is another pull request for backend logout for example |
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
Not stalled! |
I just ran into the same issue. For some reason, Keycloak 25 seems to add some kind of GUID into the aud scope instead of the actual client id. |
@Elektordi @JoelSpeed @kvanzuijlen little up ? |
Could someone provide a how-to for the workaround? |
I "fixed" it using oidc_extra_audiences with a fixed value. Maybe
|
@PaddyKe solution worked for me, thanks! I'm using Keycloak 25.0.1 and oauth2-proxy 7.6.0 |
It is a known issue with Keycloak: oauth2-proxy/oauth2-proxy#1913 oauth2-proxy/oauth2-proxy#1916 Signed-off-by: Jakub Sokołowski <jakub@status.im>
Hey guys, as this has been open for years now and I fully agree with @babs on the following:
I investigated this whole mess in-depth! First of all, I want to get some confusion out of the way.
TLDR; The Keycloak provider is deprecated and the KeycloakOIDC provider extends the generic OIDC provider and therefore offers all generic OIDC options and on top the extraction of roles from Keycloaks access token. A couple years ago, Nick and Joel started working on generalising the code base. One of the goals was and still is, to try to have at little as possible specific provider code as possible, for this reason the generic OIDC provider was introduced and in the long run all OIDC compliant providers should only extend its functionality. The Keycloak provider was kept to stay backwards compatible and not break existing users setup. We now highlight in the docs and in the provider code that the normal Keycloak provider is therefore deprecated and people should use the KeycloakOIDC provider. Main point which this PR was supposed to address:Standard Compliance Breaking change oauth2-proxy/providers/keycloak_oidc.go Lines 100 to 107 in 367183d
oauth2-proxy/providers/keycloak_oidc.go Lines 83 to 98 in 367183d
Those aren't included in the ID Token returned by Keycloak: ID Token:
Access Token:
Drill downAs oauth2-proxy/providers/oidc.go Lines 222 to 250 in 367183d
oauth2-proxy/providers/oidc.go Lines 109 to 115 in 367183d
And those locations are already properly using the So what is the problem then? What really needs to be fixed is how we extract the claims from the access token of Keycloak. Which at the point of usage in Therefore I propose we remove the unnecessary oauth2-proxy/providers/keycloak_oidc.go Lines 109 to 114 in 367183d
and instead fix the actual problem like so: func (p *KeycloakOIDCProvider) getAccessClaims(ctx context.Context, s *sessions.SessionState) (*accessClaims, error) {
// extract payload from jwt
// pseudo code
// payload := decode(jwtParts[1])
var claims *accessClaims
if err := json.Unmarshal(payload, claims); err != nil {
return nil, err
}
return claims, nil
} @Elektordi What you correctly identified, is the fact that the tests did indeed only use the Access Token instead of the ID Token, which was an an oversight and should stay as proposed by your PR. In hindsight, this might be why the assumption was made that the |
c729c46
to
05ab7d2
Compare
|
5d6835a
to
20eeae5
Compare
20eeae5
to
1fe0ac8
Compare
The keycloak_oidc provider uses the access_token instead of the id_token.
Description
I had 500 errors when using keycloak_oidc provider out of the box, and logs where indicating a problem with the audience:
[oauthproxy.go:830] Error creating session during OAuth2 callback: audience claims [aud] do not exist in claims: [...]
Motivation and Context
oauth2-proxy relies on the audience "aud" field, and the officiel keycloak documentation states that the access_token is not design to hold the audience of the client requesting the token:
https://www.keycloak.org/docs/latest/server_admin/#_audience_resolve
It is the id_token which is designed for this, and this token is for the OIDC protocol. (where the access_token is for OAuth2 protocol)
It was also confirmed by Keycloak devs on their mailing list some time ago: https://lists.jboss.org/pipermail/keycloak-user/2019-August/018924.html
How Has This Been Tested?
It has been tested with a brand now keycloak and oauth2-proxy, both deployed with docker, and using traefik/whoami as a backend.
Command line:
docker run --rm --name oauth2proxy -p 4180:4180 oauth2-proxy --provider=keycloak-oidc --oidc-issuer-url=http://172.17.0.1:8080/realms/test --email-domain=* --upstream=http://172.17.0.1:49156/ --reverse-proxy --http-address=0.0.0.0:4180 --client-id=dev-localhost --client-secret=xxxxxxxxxxxx --cookie-secret=xxxxxxxxxxxxxxxx --redirect-url=http://localhost:4180/oauth2/callback --skip-jwt-bearer-tokens --insecure-oidc-allow-unverified-email
It was tested with both web access, and bearer api access.
Before change: HTTP 500 error on auth2-proxy
After change : No problem.
Checklist:
(I'm not sure if I need to update "Breaking Changes" in CHANGELOG, because it may break installations where people have customized their KC to add specific stuff to their access_token, even is the specs says you should not)