Skip to content
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

sessions pkg: omit domain attribute in session cookie #329

Merged
merged 3 commits into from
Dec 17, 2021

Conversation

Jusshersmith
Copy link
Contributor

Problem

By default, we set the Domain attribute in the session cookie to the request's host value. This causes the cookie to be valid for the provided domain, and all subdomains. I don't believe this is expected behaviour, and shouldn't be the default.

For reference: https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.2.3

The Domain attribute specifies those hosts to which the cookie will
be sent. For example, if the value of the Domain attribute is
"example.com", the user agent will include the cookie in the Cookie
header when making HTTP requests to example.com, www.example.com, and
www.corp.example.com.
...
If the server omits the Domain attribute, the user
agent will return the cookie only to the origin server.

A leading . no longer has any impact on where the cookie is shared.

More references: mozilla domain attribute, mozilla set-cookie

Solution

Unless a domain is explicitly specified in the configuration, set the domain value to "" so that the cookie isn't shared with subdomains. Instead, it automatically defaults to only the request's domain

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #329 (8b33030) into main (7018b6f) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #329      +/-   ##
==========================================
- Coverage   62.94%   62.93%   -0.01%     
==========================================
  Files          58       58              
  Lines        4758     4757       -1     
==========================================
- Hits         2995     2994       -1     
  Misses       1546     1546              
  Partials      217      217              
Impacted Files Coverage Δ
internal/pkg/sessions/cookie_store.go 85.29% <60.00%> (-0.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7018b6f...8b33030. Read the comment docs.

benjsto
benjsto previously approved these changes Dec 15, 2021
@Jusshersmith Jusshersmith merged commit 8f2d3ac into main Dec 17, 2021
@Jusshersmith Jusshersmith deleted the jusshersmith-duplicate-cookie-names branch December 17, 2021 13:07
danbf added a commit that referenced this pull request Apr 24, 2024
danbf added a commit that referenced this pull request Apr 24, 2024
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