Skip to content

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

Merged
merged 8 commits into from
Aug 30, 2023

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Aug 30, 2023

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 vs Unauthorized.

Closes https://github.com/coder/v2-customers/issues/280

@Emyrk Emyrk requested a review from aslilac August 30, 2023 18:24
@Emyrk Emyrk requested a review from deansheather August 30, 2023 18:27
Copy link
Member

@aslilac aslilac left a 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.",
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, duh lol

@Emyrk Emyrk merged commit e827278 into main Aug 30, 2023
@Emyrk Emyrk deleted the stevenmasley/expire_refresh branch August 30, 2023 21:14
@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants