-
Notifications
You must be signed in to change notification settings - Fork 883
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 the error_description
and error_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.
Per the OIDC spec only the error
response parameter is required; the error_description
field is optional so we should handle that being blank.
Also there is an error_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 empty error_description
. The Details
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 add error_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.
Providing an invalid scope
-oidc-scopes openid,profile,email,bad
on Okta OIDC provider causes infinite redirect loop.