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

Optionally query OIDC UserInfo to gather group claims #8272

Closed
cdobbyn opened this issue Jan 25, 2022 · 7 comments · Fixed by #12062
Closed

Optionally query OIDC UserInfo to gather group claims #8272

cdobbyn opened this issue Jan 25, 2022 · 7 comments · Fixed by #12062
Assignees
Labels
component:sso Issues related to Argo CD configurations enhancement New feature or request type:usability Enhancement of an existing feature

Comments

@cdobbyn
Copy link

cdobbyn commented Jan 25, 2022

Summary

Today the OIDC implementation assumes that every OIDC provider provides "fat tokens" (containing the id token, profile attributes, and groups). It should be optionally available to query /userinfo to gather the necessary profile and group information.

Motivation

This will increase the usability of OIDC with ArgoCD and enable more compatibility with IDPs.

Today ArgoCD with OIDC is incompatible with Okta unless you pay for an extra feature. It is absolutely possible to use SAML + Dex to accomplish this functionality, however that is simply adding another IDP into the mix.

Proposal

I see that you're using the coreos go-oidc package. As part of OIDC settings add a queryUserDataForClaims boolean that when true will use the UserInfo function to gather this data. Instead of parsing the token for claims, the returned object is used instead.

@cdobbyn cdobbyn added the enhancement New feature or request label Jan 25, 2022
@jannfis jannfis added component:sso Issues related to Argo CD configurations type:usability Enhancement of an existing feature labels Jan 28, 2022
@chetan-rns chetan-rns self-assigned this Jan 31, 2022
@longshine
Copy link

+1 with same issue

@mkilchhofer
Copy link
Member

mkilchhofer commented Jul 4, 2022

We at @swisspost would also love to see that feature.

@crenshaw-dev
Copy link
Collaborator

If anyone has time to write the PR, I'd be more than happy to review!

@mkilchhofer
Copy link
Member

I'd happy to contribute but I am really not familiar with the internals/codebase of Argo CD.
But I found that workflows supports this since August 2021: argoproj/argo-workflows#6572

Maybe we can implement it in a similar way?

@crenshaw-dev
Copy link
Collaborator

Glancing at the code, I think it would be quite similar. Looks like Workflows takes apart the id token in the callback handler and then augments it with groups. The same could be done in oidc.go's HandleCallback.

@the-technat
Copy link
Contributor

the-technat commented Sep 5, 2022

Hello there!

As we @swisspost are affected by issue #8272 I'd like to contribute this enhancement to Argo CD. I'm not new to Go but new to OIDC/JWTs so I used this here as reference and started doing the same in argo-cd when I realized that this could require more than just an API call to the userinfo endpoint.

The problem: To add the groups back into the token, I need to create and sign a new JWT token. Spying on how the cli generates JWT tokens for the Argo CD API I used this key to sign my new JWT token. This works as a new valid JWT token is saved on the client side. However once the client tries to authenticate using this token the elephant appears:

time="2022-09-02T11:33:23+02:00" level=info msg="finished unary call with code Unauthenticated" error="rpc error: code = Unauthenticated desc = invalid session: oidc: id token signed with unsupported algorithm, expected [\"RS256\"] got \"HS256\"" grpc.code=Unauthenticated grpc.method=List grpc.service=application.ApplicationService grpc.start_time="2022-09-02T11:33:23+02:00" grpc.time_ms=5.322 span.kind=server system=grpc

Of course I can't resign a token coming from an upstream IDP with a symmetric algorithm and hope that it's valid... Taking a look at how argo-workflows solved this problem I guess to implement #8272 we would need to completely switch Argo CD to issue JWE tokens itself (and also validating them itself rather than using the IDP to validate the JWTs) without passing the oauth2 token from the IDP to the user. But that would be a drastic change in how Argo CD does SSO...

So what does the community (@crenshaw-dev ,@longshine ,@cdobbyn) think about this? Should I try to implement the same approach argo-workflows did or is this considered unacceptable and #8272 can't be implemented?

@cdobbyn
Copy link
Author

cdobbyn commented Sep 19, 2022

I'd have to defer to @crenshaw-dev. I do not use the client at all.

@chetan-rns chetan-rns assigned the-technat and unassigned chetan-rns Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:sso Issues related to Argo CD configurations enhancement New feature or request type:usability Enhancement of an existing feature
Projects
None yet
7 participants