- Notifications
You must be signed in to change notification settings - Fork928
fix: Prevent infinite redirects on oidc errors#6550
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Honestly a great find 👍
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.
Nice find Steven. Just a couple comments about how we handle theerror_description anderror_uri parameters.
| // if for example we are providing and invalid scope. | ||
| // We should terminate the OIDC process if we encounter an error. | ||
| oidcError := r.URL.Query().Get("error") | ||
| errorDescription := r.URL.Query().Get("error_description") |
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.
Perthe OIDC spec only theerror response parameter is required; theerror_description field is optional so we should handle that being blank.
Also there is anerror_uri parameter that might be present so we should check for that as well.
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 do handle an emptyerror_description. TheDetails part of the error is extra developer debug info. If it is empty, we provide no extra details, since we have no extra information.
I can adderror_uri though 👍
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.
Woot woot! I've had URI mismatch on my OIDC setup so many times and I always had to inspect. Great change.
Uh oh!
There was an error while loading.Please reload this page.
Providing an invalid scope
-oidc-scopes openid,profile,email,badon Okta OIDC provider causes infinite redirect loop.