Adds support for SameSite cookie attribute#165
Merged
Conversation
aefa750 to
600b387
Compare
Contributor
Author
|
gofmt issue, fixed and re-running CI. |
Contributor
Author
|
Waiting on #167 to build this against Go 1.11.x - running into the Go 1.4.x vet issue. |
600b387 to
594db95
Compare
Contributor
Author
|
Rebased to run against latest CI config. |
|
Maybe I'm missing something, but shouldn't |
Contributor
Author
|
Good catch. Let me add tests to catch this in future (copying structs,
field by field, is wonderfully error prone).
…On Mon, Sep 24, 2018 at 12:51 PM Niels Widger ***@***.***> wrote:
Maybe I'm missing something, but shouldn't sessions.NewCookie set SameSite
in the cookie it returns based on the value of options.SameSite?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#165 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABIcHUuvRxCY5nLNmPKi77mJurLMF9Dks5ueTfNgaJpZM4WXuvN>
.
|
|
Sounds good, let me know if I can be of any help. I'm happy to submit a PR to fix this if you're busy. |
Contributor
Author
|
Would be great if you could submit a PR for the fix + test.
You'll also need to take into account Go 1.11 vs. earlier versions, which
means two versions of NewCookie + build flags.
…On Thu, Sep 27, 2018 at 5:34 AM Niels Widger ***@***.***> wrote:
Sounds good, let me know if I can be of any help. I'm happy to submit a PR
to fix this if you're busy.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#165 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABIcJV_MJDMdC0ger3_LZu4g6K2zeQDks5ufMW7gaJpZM4WXuvN>
.
|
|
Yea, I was thinking that was going to be necessary. I'll see what I can do! |
nwidger
pushed a commit
to nwidger/sessions
that referenced
this pull request
Sep 28, 2018
Set the returned http.Cookie's SameSite field to the value of the SameSite field in the Options struct passed into NewCookie. This fixes an oversight made in PR gorilla#165 which was made to address issue Add a newCookieFromOptions function which takes a name, value and Options struct and returns an http.Cookie. There are two newCookieFromOptions implementations, one for Go 1.11 and later which sets SameSite and one for earlier Go versions which does not. Add new tests TestNewCookieFromOptions and TestNewCookieFromOptionsSameSite which ensure that the values passed to newCookieFromOptions are properly set in the returned cookie. The test TestNewCookieFromOptionsSameSite only runs if running Go 1.11 and later.
kisielk
pushed a commit
that referenced
this pull request
Sep 28, 2018
Set the returned http.Cookie's SameSite field to the value of the SameSite field in the Options struct passed into NewCookie. This fixes an oversight made in PR #165 which was made to address issue Add a newCookieFromOptions function which takes a name, value and Options struct and returns an http.Cookie. There are two newCookieFromOptions implementations, one for Go 1.11 and later which sets SameSite and one for earlier Go versions which does not. Add new tests TestNewCookieFromOptions and TestNewCookieFromOptionsSameSite which ensure that the values passed to newCookieFromOptions are properly set in the returned cookie. The test TestNewCookieFromOptionsSameSite only runs if running Go 1.11 and later.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #164
Adds support for the
SameSiteattribute introduced in Go 1.11 - a new cookie attribute introduced to further prevent CSRF attacks: https://blog.mozilla.org/security/2018/04/24/same-site-cookies-in-firefox-60/