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

add regex for oidc group matching #3063

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

jacksonargo
Copy link
Contributor

@jacksonargo jacksonargo commented Aug 4, 2023

Overview

Add a group regex matcher to the OIDC connector config.

What this PR does / why we need it

Sometimes the IDP's can send very long list of groups, not all IDP's support group filters from the IDP side.

Special notes for your reviewer

I tested this in the unit tests, but not sure how to test this in a live environment. and in a live environment, and the filter works as expected.

Does this PR introduce a user-facing change?

It adds a new config field for OIDC: groupsRegex.

ADD groups regex config for oidc connector.

@sagikazarmark
Copy link
Member

@jacksonargo thanks for the contribution!

Two thoughts came to my mind:

  • People generally want to deny authentication with filters like this. I'm not saying it doesn't make sense, but it can be confusing for some, so we may want to structure config/document this feature to avoid any confusion.
  • Would it make sense to support capture groups not just to match, but extract parts of group names?

@jacksonargo
Copy link
Contributor Author

People generally want to deny authentication with filters like this. I'm not saying it doesn't make sense, but it can be confusing for some, so we may want to structure config/document this feature to avoid any confusion.

Ya, makes sense to me. Would groupsFilter be a better name?

Would it make sense to support capture groups not just to match, but extract parts of group names?

Yes probably! Didn't occur to me at the time.

@st-sidjoshi
Copy link

Your thoughts @sagikazarmark ?

@jacksonargo
Copy link
Contributor Author

Hi @sagikazarmark, I've updated the pr to put the regex filter under the claim mutations config.

How does this look?

@sagikazarmark sagikazarmark merged commit 6ceb265 into dexidp:master Aug 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants