Skip to content

chore: Deprecate old cookie value #4336

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 2 commits into from
Oct 3, 2022
Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Oct 3, 2022

Older clis will need to be updated.
Modern clis cannot communicate with <8.15 coderd

Future work

Remove logout legacy cookies:

coder/coderd/users.go

Lines 1021 to 1037 in 0d1ab74

// This code should be removed after Jan 1 2023.
// This code logs out of the old session cookie before we renamed it
// if it is a valid coder token. Otherwise, this old cookie hangs around
// and we never log out of the user.
oldCookie, err := r.Cookie("session_token")
if err == nil && oldCookie != nil {
_, _, err := httpmw.SplitAPIToken(oldCookie.Value)
if err == nil {
cookie := &http.Cookie{
// MaxAge < 0 means to delete the cookie now.
MaxAge: -1,
Name: "session_token",
Path: "/",
}
http.SetCookie(rw, cookie)
}
}

Enable CSRF:

// Enable CSRF in November 2022 by deleting this "return true" line.
// CSRF is not enforced to ensure backwards compatibility with older
// cli versions.
//nolint:revive
return true

Note

I began writing code to return nice errors Coder cli version out of date to communicate with..., but the error code was messy.

I think an easier way to do this is somehow write some generic code that says what version of coderd a cli can communicate with and vis-versa.

It would be nice as a dev to say "Coder cli version 8.15+" is required for a coderd, so the cli can lookup the supported list. We can do this generically rather than a bunch of 1 off code in random spots accumulating debt.

Older clis will need to be updated.
Modern clis cannot communicate with <8.15 coderd
Comment on lines -86 to -91
// Delete this custom cookie set in November 2022. This is just to remain
// backwards compatible with older versions of Coder.
req.AddCookie(&http.Cookie{
Name: "session_token",
Value: c.SessionToken,
})
Copy link
Member Author

Choose a reason for hiding this comment

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

Close enough to November. New clis cannot communicate with <8.15 coderds.

@Emyrk Emyrk merged commit 0a95ba6 into main Oct 3, 2022
@Emyrk Emyrk deleted the stevenmasley/deprecate_old_cookie branch October 3, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants