-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Propagate groups from upstream OIDC provider #1065
Comments
This is blocked on #863 Since so many providers implement refresh tokens differently, we don't actually re-query the upstream provider when a dex client refresh its token. So we don't have any way of keeping the groups list fresh. |
I see. But how does groups differ from the other properties that are propagated in the current version (username, email, email verified)? Does those never change? |
Groups are much more likely to change dynamically. I'm not saying that it's good that we don't update the other claims, but we definitely want to address this before expanding our features to groups |
Yeah, going to re-open for tracking this issue. Thanks for opening and sorry we don't have a better answer today. |
@ericchiang I have recently run into this a few times, would you guys be open to merging an option to pull groups? I understand that they don't get update when using a refresh token, but as it stands nothing else does either. My proposal would be to (1) add an option to enable/disable groups for OIDC or (2) allow it to be enabled through config (as is) -- either way it sounds like we should specifically call out the behavior in the docs. |
I've opened #1434 with my proposed workaround. |
Any updates here? Concourse forked dex and managed to pull in groups. I am with @jacksontj on this one: offer documentation and recommendations (if using groups set short ttl). |
There's been some discussion in dexidp#1065 regarding what to do about refreshing groups. As it stands today dex doesn't update any of the claims on refresh (groups would just be another one). The main concern with enabling it is that group claims may change more frequently. While we continue to wait on the upstream refresh flows, this adds an option to enable the group claim. This is disabled by default (so no behavioral change) but enables those that are willing to have the delay in group claim change to use oidc IDPs. Workaround to dexidp#1065
There's been some discussion in dexidp#1065 regarding what to do about refreshing groups. As it stands today dex doesn't update any of the claims on refresh (groups would just be another one). The main concern with enabling it is that group claims may change more frequently. While we continue to wait on the upstream refresh flows, this adds an option to enable the group claim. This is disabled by default (so no behavioral change) but enables those that are willing to have the delay in group claim change to use oidc IDPs. Workaround to dexidp#1065
This builds on the terrific work in https://github.com/dexidp/dex/pull/1180/files and dexidp#1065. This makes some minor changes that bring the approach up-to-date with current dex versions.
Hey Guys, |
Now that Dex v.2.21.0 has been released with both Refresh Tokens and Groups claims, is this still an issue? And if not, could the flag "insecureEnableGroups" be renamed "enableGroups"? In the use-cases that I am testing, groups claims are refreshed pretty quickly < 3 minutes in most cases which satisfies requirements. From my perspective there does not seem to be a security issue anymore. |
@dskatz any guidance how you enabled groups? for me Google always returns |
Guys. I'm trying to enable groups for google. Was added serviceAccountFilePath, adminEmail, insecureEnableGroups: true, but I'm still getting groups=[]. Where am I wrong? |
Dene14, I found. you need to ask extra-claim groups but you need to use a connector type google, not oidc |
hi folks, so does groups now works for oidc or not? I'm trying to use oidc with okta and I tried insecureEnableGroups: true but still getting groups=[]. any idea? |
@abdulsalama Did you make sure to add the |
@JoelSpeed yes I did. I had this in the config: and I see that groups is being requested in the initial call. |
Hi @abdulsalama, With Okta specifically, groups aren't returned in the idtoken unless you pay for that feature. You will need to make a second call to the userinfo endpoint after requesting the claims initially. |
@abdulsalama If you still have the problem, you can fix it specifying With the enabled option, dex will make a second call to the useinfo endpoint like @dskatz suggest. I tested it yesterday. |
Thanks @dskatz and @nabokihms for the hints. I will give that a try and see if it works. |
hi @nabokihms & @dskatz . If I'm installing Concourse through Helm, where exactly would I edit this It's become a nuisance not being able to get a user's groups through OIDC or OAUTH =[ |
#1634 |
@lentzi90 you still need to use |
Could the issue be rescoped to also include upstream OAuth2 login flows? We are interested in passing on The title could then read like something along the lines of:
|
@almereyda it seems it's in already: https://github.com/dexidp/dex/blob/master/connector/github/github.go#L317 haven't used it yet, hoping to get this working soon |
The OIDC connectors currently discards the groups claim. I would like this to be propagated.
The text was updated successfully, but these errors were encountered: