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

OIDC Connector: Support for IssuerAlias and group claims with maps instead of strings #3676

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

meldsza
Copy link
Contributor

@meldsza meldsza commented Aug 5, 2024

Overview

Support for IssuerAlias and group claims with maps instead of strings in the OIDC Connector

What this PR does / why we need it

  1. Some Identity Providers like IDCS and Azure have different issuer urls in the OIDC discovery endpoint. This leads to the issuer url check failing. This PR adds support for IssuerAlias which would override the issuer check with the value of issuer to be checked in the discovery endpoint.

  2. Many Identity Providers return the groups as a list of maps with the name field. This functionality already exists in the oauth connector but does not currently exist in the OIDC connector. This PR adds support for this scenario and adds relevant tests cases

Special notes for your reviewer

@meldsza meldsza force-pushed the oidc-issuer-alias branch from 0314520 to 9fade21 Compare August 5, 2024 16:23
@meldsza meldsza marked this pull request as ready for review August 5, 2024 16:29
@meldsza meldsza force-pushed the oidc-issuer-alias branch from 9fade21 to c9a434f Compare August 5, 2024 16:43
@meldsza
Copy link
Contributor Author

meldsza commented Oct 8, 2024

Hi @nabokihms,
Please review this pull request and let me know if any changes are required

@nabokihms
Copy link
Member

@meldsza Hi, the PR looks good. Could you:

  1. Rebase it to trigger CI?
  2. Also add the new option to docs https://dexidp.io/docs/connectors/oidc/

@meldsza
Copy link
Contributor Author

meldsza commented Oct 25, 2024

@nabokihms I have rebased the branch and created another PR for the website/docs:
dexidp/website#186

Please approve the workflows so that they can run and provide test coverage

@nabokihms nabokihms merged commit 8b93966 into dexidp:master Oct 31, 2024
10 checks passed
@nabokihms
Copy link
Member

Thanks for this improvement!

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