-
Notifications
You must be signed in to change notification settings - Fork 887
feat: failed update refresh should redirect to login #9442
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.
This is a great little bit of cleanup 💕
@@ -296,7 +296,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon | |||
}).Token() | |||
if err != nil { | |||
return write(http.StatusUnauthorized, codersdk.Response{ | |||
Message: "Could not refresh expired Oauth token.", | |||
Message: "Could not refresh expired Oauth token. Try re-authenticating to resolve this issue.", |
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.
How will they reauthenticate other than manually wiping their cookies?
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.
The frontend will intercept the "Unauthorized" status code and redirect to the login page. They shouldn't ever even see this message.
If they're using the CLI, they can just run coder login
again.
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.
Oh, duh lol
Our redirect to login was not happening on all 401 errors. I corrected this on the FE and fixed 2 places where we should return a
Forbidden
vsUnauthorized
.Closes https://github.com/coder/v2-customers/issues/280