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

Allow using connector ID as oidc groups prefix #2051

Closed
wants to merge 2 commits into from

Conversation

corest
Copy link

@corest corest commented Mar 19, 2021

Overview

Adds prefixes for OIDC groups. I'm not sure if we want to implement this on a different level (middleware?).
But I've been using this for some time and it works fine for my use-case, which is mostly about the security side, not UX.
E.g. it is not possible to configure the exact OIDC prefix with this, but as connector IDs are unique - it makes OIDC groups unique as well. I'm open for discussion if this approach is not how the community would like to solve this. For me, it works and closes the issue #918, therefore, sharing this solution here.

Usage: "Closes #918"

Special notes for your reviewer

Does this PR introduce a user-facing change?

This change allows enabling OIDC groups prefixes with connector IDs values. Having this makes it impossible for groups from different connectors to impersonate other group access by using the same group names.

This feature can be enabled with oidcGroupsPrefix: true. If this option is missing, it will be disabled by default. So it doesn't require any changes if you're updating from the old version

@corest corest force-pushed the prefix-with-connector-id branch 2 times, most recently from 9d156db to dbffe22 Compare March 19, 2021 15:27
Signed-off-by: vol <[email protected]>
@corest corest force-pushed the prefix-with-connector-id branch from dbffe22 to 230122b Compare March 19, 2021 15:28
@sagikazarmark
Copy link
Member

Thanks @corest!

We might put a hold on it in favor of #1635, but it's definitely a good use case.

@corest
Copy link
Author

corest commented Mar 20, 2021

Ok then. Closing

@corest corest closed this Mar 20, 2021
@corest corest deleted the prefix-with-connector-id branch March 20, 2021 09:45
@marians
Copy link
Contributor

marians commented Feb 18, 2022

For the future visitor: the proposed implementation is lacking the prefixing when refreshing the token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Groups returned in token should be prefixed
3 participants