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 option to always display connector selection even if there's only one #1505

Merged
merged 1 commit into from
Aug 7, 2019

Conversation

MarcDufresne
Copy link
Contributor

We are currently building an auth proxy system that will be used to authenticate users before being redirected to their destination. The fact that Dex automatically forwards to the connector if there's only one has been confusing for our users. They type my-service.com in their browsers and end up on Google login, not a great UX.

This PR allows to specify an option to always show the connector selection screen even if there's only one. That way our users will land on a themed login page, and select "Login with Google" before ending up on Google login.

Let me know what you think!

@srenatus
Copy link
Contributor

srenatus commented Aug 2, 2019

Thank you for the contribution 🎉
Haven't looked closely at the code yet, but I wonder, could the prompt query parameter be used here, too? I haven't looked in the oidc spec of that in a while, it could mean something else.

@MarcDufresne
Copy link
Contributor Author

https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

I'm not sure prompt fits this use case, maybe select_account? But this feels a bit like a stretch

@srenatus
Copy link
Contributor

srenatus commented Aug 5, 2019

@MarcDufresne Ah, I see. Yeah, indeed; but thanks a lot for checking. ✔️

Copy link
Contributor

@srenatus srenatus 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! With a quick test addition, this should be good to go. 🎉

server/handlers.go Show resolved Hide resolved
cmd/dex/config.go Show resolved Hide resolved
@MarcDufresne
Copy link
Contributor Author

Test added!

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Thanks! Good addition.

@srenatus srenatus merged commit e1afe77 into dexidp:master Aug 7, 2019
mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019
Add option to always display connector selection even if there's only one
maxlegault added a commit to maxlegault/charts that referenced this pull request Sep 3, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
maxlegault added a commit to maxlegault/charts that referenced this pull request Sep 3, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
maxlegault added a commit to maxlegault/charts that referenced this pull request Sep 3, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
k8s-ci-robot pushed a commit to helm/charts that referenced this pull request Sep 4, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
kengou pushed a commit to kengou/charts that referenced this pull request Sep 18, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
ramkumarvs pushed a commit to yugabyte/charts-helm-fork that referenced this pull request Sep 30, 2019
The current indentation of the OAuth2 configuration block doesn't allow
for extra properties to be added. This allows to enable the new
configuration that will always display the login screen, even though
only one connector is defined.

The default value (disabled) is also added to the values file for
documentation.

See also: https://github.com/dexidp/dex/releases/tag/v2.18.0
See also: dexidp/dex#1505

Signed-off-by: Maxime Legault-Venne <[email protected]>
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.

2 participants