-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: respect promptType=none with offline_access scope #4086
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
base: master
Are you sure you want to change the base?
fix: respect promptType=none with offline_access scope #4086
Conversation
4f9f13b
to
3696879
Compare
Signed-off-by: hunter morgan <[email protected]> Signed-off-by: Hunter Morgan <[email protected]>
3696879
to
c50f1e2
Compare
i've now validated i can use argocd, kargo, argocd-cli, and kargo-cli with dex using this patch.
|
@sagikazarmark, would you be so kind as to help me get the ci testing workflow to run against this pr? tysm |
opts = append(opts, oauth2.AccessTypeOffline, oauth2.SetAuthURLParam("prompt", c.promptType)) | ||
opts = append(opts, oauth2.AccessTypeOffline) | ||
// Only add prompt parameter if it's not "none" | ||
if c.promptType != "none" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get one thing. If none is not respected by provider, why is none specified in the configuration? Should it be consent
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get 'consent' to work either 🤷
I'll see if I can get someone closer to OneLogin to comment.
onelogin says:
according to https://developers.onelogin.com/openid-connect/scopes and support:
does this seem like a reasonable workaround PR until such time as the defect is resolved? @nabokihms |
Overview
This PR fixes an issue with the OIDC connector where setting
promptType: "none"
is not respected when requesting theoffline_access
scope. The fix ensures that whenpromptType
is set to "none", no prompt parameter is sent to the identity provider during authentication flows that request refresh tokens.What this PR does / why we need it
When using the OIDC connector with
promptType: "none"
configured and requesting refresh tokens via theoffline_access
scope, Dex incorrectly adds aprompt=none
parameter to the authorization request. This causes issues with identity providers like OneLogin that don't properly handle theprompt=none
parameter when requesting offline access.The problem occurs in the
LoginURL
method of the OIDC connector. Whenoffline_access
scope is requested, Dex forcibly addsprompt=consent
(or whatever the configuredpromptType
is, including"none"
) to the authorization request parameters:This behavior causes authentication failures with error messages like:
consent_required: prompt consent was not resolved
This PR modifies the OIDC connector to respect the
promptType: none
configuration by conditionally adding the prompt parameter only when it's not set to "none".Related issues
Issue #1172: "Disable 'grant access' page" (Dex)
Link: #1172
This issue from 2018 is highly relevant to our fix. The user reported:
"Is there a way to automatically grant access? (skip the page). I tried with the query param prompt=none but I still have to grant access."
This is exactly our issue - the user tried setting prompt=none but it was ineffective when handling consent screens.
In ArgoCD:
Issue #9639: "Unable to login via cli sso" (ArgoCD)
Link: argoproj/argo-cd#9639
This issue involves CLI authentication problems when using external OIDC providers without Dex. While not an exact match, it shows the kind of authentication problems that can occur in the ecosystem.
Issue #2932: "SSO for Existing OIDC Provider returning 401" (ArgoCD)
Link: argoproj/argo-cd#2932
This deals with authentication problems via CLI using SSO, which is related to our issue since CLI authentication often uses PKCE flows that can be affected by prompt parameter issues.
I also found this related comment in the official Dex OIDC connector documentation:
For offline_access, the prompt parameter is set by default to "prompt=consent". However this is not supported by all OIDC providers, some of them support different value for prompt, like "prompt=login" or "prompt=none"
This documentation directly acknowledges the issue we're fixing: that the hardcoded behavior of always adding prompt=consent for offline access doesn't work with all providers.
Based on these findings, the most directly relevant issue is #1172, as it specifically mentions trying prompt=none and finding it ineffective - exactly what our fix addresses.
Special notes for your reviewer
This change has been tested with OneLogin as an identity provider, which previously failed with "consent_required" errors. After implementing this fix, the OIDC flow completes successfully when using
promptType: none
.I'm hoping there is CI upstream to run the tests as I'm not setup to run the tooling in the repo.