auth: Redirect github oauth login attempt back if user cancels #23083
Conversation
Before, we would show a raw plaintext error. Instead, we detect the specifc `access_denied` error returned from GitHub and redirect the user back.
ryanslade
changed the title
ryanslade
changed the title
|
Notifying subscribers in CODENOTIFY files for diff 28e524c...7898a49.
|
| ctx := r.Context() | ||
| encodedState, err := goauth2.StateFromContext(ctx) | ||
| if err != nil { | ||
| log15.Error("OAuth failed: could not get state from context.", "error", err) | ||
| http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get OAuth state from context.", http.StatusInternalServerError) | ||
| return | ||
| } | ||
| state, err := oauth.DecodeState(encodedState) | ||
| if err != nil { | ||
| log15.Error("OAuth failed: could not decode state.", "error", err) | ||
| http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get decode OAuth state.", http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
I'm not sure all of these are actually needed because we know we got "access_denied" exactly here.
My suggestion would be put a warning log saying OAuth flow is failing because of "access_denied" and do the redirect.
There was a problem hiding this comment.
We need to get the state so that we can extract the URL we want to redirect back to
There was a problem hiding this comment.
Or do you think it's safe to always assume we're redirecting back to the sign in page?
There was a problem hiding this comment.
Ah, I see. Assuming you've verified that we always get "state" back even when the user cencals the authorization on GitHub side.
There was a problem hiding this comment.
Yep, the state is populated.
| ctx := r.Context() | ||
| encodedState, err := goauth2.StateFromContext(ctx) | ||
| if err != nil { | ||
| log15.Error("OAuth failed: could not get state from context.", "error", err) | ||
| http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get OAuth state from context.", http.StatusInternalServerError) | ||
| return | ||
| } | ||
| state, err := oauth.DecodeState(encodedState) | ||
| if err != nil { | ||
| log15.Error("OAuth failed: could not decode state.", "error", err) | ||
| http.Error(w, "Authentication failed. Try signing in again (and clearing cookies for the current site). The error was: could not get decode OAuth state.", http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
Ah, I see. Assuming you've verified that we always get "state" back even when the user cencals the authorization on GitHub side.
Before, we would show a raw plaintext error. Instead, we detect the
specifc
access_deniederror returned from GitHub and redirect the userback.
Closes: https://sourcegraph.atlassian.net/browse/COREAPP-25