fix: do not use request for signup cookie exchange #3071
Conversation
BruceMacD
requested review from
ssoroka,
dnephin,
jmorganca and
pdevine
as code owners
| } | ||
|
|
||
| exp := time.Now().UTC().Add(opts.SessionDuration) |
There was a problem hiding this comment.
Approximating the auth cookie expiry based on the session duration. This should work fine since this will be exchanged within a few minutes.
There was a problem hiding this comment.
Was this longer duration for the signup cookie necessary for the fix, or is this change related to something else?
There was a problem hiding this comment.
Ohh, this is for the new cookie. That makes sense! For some reason I thought this was for the signup cookie, but it's clearly not.
| matched, err := regexp.MatchString("auth=aaa; Path=/; Domain=dev.example.com; Max-Age=\\d\\d; HttpOnly; SameSite=Strict", c.Writer.Header()["Set-Cookie"][0]) | ||
| assert.NilError(t, err) | ||
| assert.Assert(t, matched) |
There was a problem hiding this comment.
One option to avoid the regex would be to use store the httptest.NewRecorder() that we passed into the gin.Context and call Result().Cookies() on it (https://pkg.go.dev/net/http#Response.Cookies).
That should give you the parsed structs, and allow you to use DeepEqual with options to account for unpredictable max age.
Not a blocker, just an option if we need to make further changes some other time.
| } | ||
|
|
||
| exp := time.Now().UTC().Add(opts.SessionDuration) |
There was a problem hiding this comment.
Was this longer duration for the signup cookie necessary for the fix, or is this change related to something else?
- return auth from exchange signup cookie
Summary
#3059 had an issue that it wouldn't set the new auth cookie after sign-up because moving around the cookie logic meant that the key wasn't set where I expected it to be previously.
Checklist