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

Remove google specific hd / hosted domain claim config from oidc connector #2511

Merged
merged 1 commit into from
May 13, 2022

Conversation

cisco-abrandel
Copy link
Contributor

Signed-off-by: Anthony Brandelli abrandel@cisco.com

Overview

At some point in the evolution of dex it seems like some Google specific stuff got left in the OIDC connector, when there is now a Google specific one.

I would like to gather feedback on this PR whether it makes sense to remove this, given that there could be users out there who are utilizing the OIDC connector but using Google as their IDP, instead of using the Google connector. I don't want to break people, but I also think this stuff is pretty

What this PR does / why we need it

Special notes for your reviewer

Originally added in #974 which is before the Google connector was split out here #1185. I believe that this is just a miss since then, I am unaware of any other IDPs (but of course I am not aware of everyone's configuration) that use this claim.

This is currently also referenced in the docs here https://dexidp.io/docs/connectors/oidc/, I will open a separate PR to update the docs should it be agreed we want to proceed with removing this code.

Does this PR introduce a user-facing change?

The OIDC connector will no longer look for, or respect the hosted domain "hd" claim. User's who are using Google as their IDP should use the Google connector. 

Signed-off-by: Anthony Brandelli <abrandel@cisco.com>
@cisco-abrandel cisco-abrandel changed the title Remove google specific hd / hosted domain claim config Remove google specific hd / hosted domain claim config from oidc connector May 6, 2022
@nabokihms nabokihms self-requested a review May 7, 2022 11:28
@nabokihms nabokihms added this to the v2.32.0 milestone May 7, 2022
Copy link
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! Looks good to me.

@nabokihms
Copy link
Member

We should not forget to mention this in the upper block of the release message.

@nabokihms nabokihms merged commit 9cd29bd into dexidp:master May 13, 2022
cisco-abrandel added a commit to cisco-abrandel/website that referenced this pull request May 13, 2022
…ed from this connector in dexidp/dex#2511

Signed-off-by: Anthony Brandelli <abrandel@cisco.com>
kerberos727 added a commit to kerberos727/dex-project that referenced this pull request Aug 23, 2024
…ed from this connector in dexidp/dex#2511

Signed-off-by: Anthony Brandelli <abrandel@cisco.com>
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