-
Notifications
You must be signed in to change notification settings - Fork 594
Add SameSite attribute to Cookies #2928
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
Changes from 9 commits
048a5dc
f7dd008
89f445c
0e19351
68c263a
4ec36b5
6241af4
0edd269
ebece1b
fc39135
f1ddee4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /* | ||
| * Copyright (C) 2020 Lightbend Inc. <https://www.lightbend.com> | ||
| */ | ||
|
|
||
| package akka.http.javadsl.model.headers; | ||
|
|
||
| /** | ||
| * The Cookie SameSite attribute as defined by <a href="https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00">RFC6265bis</a> | ||
| * and <a href="https://tools.ietf.org/html/draft-west-cookie-incrementalism-00">Incrementally Better Cookies</a>. | ||
| */ | ||
| public enum SameSite { | ||
| Strict, | ||
| Lax, | ||
| // SameSite.None is different from not adding the SameSite attribute in a cookie. | ||
jrudolph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // - Cookies without a SameSite attribute will be treated as SameSite=Lax. | ||
| // - Cookies for cross-site usage must specify `SameSite=None; Secure` to enable inclusion in third party | ||
| // context. We are not enforcing `; Secure` when `SameSite=None`, but users should. | ||
marcospereira marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| None; | ||
|
|
||
| public akka.http.scaladsl.model.headers.SameSite asScala() { | ||
| if (this == Strict) return akka.http.scaladsl.model.headers.SameSite.Strict$.MODULE$; | ||
| if (this == Lax) return akka.http.scaladsl.model.headers.SameSite.Lax$.MODULE$; | ||
| return akka.http.scaladsl.model.headers.SameSite.None$.MODULE$; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Add SameSite attribute to HttpCookie | ||
|
|
||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.javadsl.model.headers.HttpCookie.getSameSite") | ||
jrudolph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.javadsl.model.headers.HttpCookie.withSameSite") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -266,19 +266,19 @@ private[parser] trait CommonRules { this: Parser with StringBuilding => | |
| } | ||
|
|
||
| def `cookie-av` = rule { | ||
| `expires-av` | `max-age-av` | `domain-av` | `path-av` | `secure-av` | `httponly-av` | `extension-av` | ||
| `expires-av` | `max-age-av` | `domain-av` | `path-av` | `same-site-av` | `secure-av` | `httponly-av` | `extension-av` | ||
| } | ||
|
|
||
| def `expires-av` = rule { | ||
| ignoreCase("expires=") ~ OWS ~ `expires-date` ~> { (c: HttpCookie, dt: DateTime) => c.copy(expires = Some(dt)) } | ||
| ignoreCase("expires=") ~ OWS ~ `expires-date` ~> { (c: HttpCookie, dt: DateTime) => c.withExpires(dt) } | ||
| } | ||
|
|
||
| def `max-age-av` = rule { | ||
| ignoreCase("max-age=") ~ OWS ~ longNumberCappedAtIntMaxValue ~> { (c: HttpCookie, seconds: Long) => c.copy(maxAge = Some(seconds)) } | ||
| ignoreCase("max-age=") ~ OWS ~ longNumberCappedAtIntMaxValue ~> { (c: HttpCookie, seconds: Long) => c.withMaxAge(seconds) } | ||
| } | ||
|
|
||
| def `domain-av` = rule { | ||
| ignoreCase("domain=") ~ OWS ~ `domain-value` ~> { (c: HttpCookie, domainName: String) => c.copy(domain = Some(domainName)) } | ||
| ignoreCase("domain=") ~ OWS ~ `domain-value` ~> { (c: HttpCookie, domainName: String) => c.withDomain(domainName) } | ||
| } | ||
|
|
||
| // https://tools.ietf.org/html/rfc1034#section-3.5 relaxed by https://tools.ietf.org/html/rfc1123#section-2 | ||
|
|
@@ -288,20 +288,28 @@ private[parser] trait CommonRules { this: Parser with StringBuilding => | |
| } | ||
|
|
||
| def `path-av` = rule { | ||
| ignoreCase("path=") ~ OWS ~ `path-value` ~> { (c: HttpCookie, pathValue: String) => c.copy(path = Some(pathValue)) } | ||
| ignoreCase("path=") ~ OWS ~ `path-value` ~> { (c: HttpCookie, pathValue: String) => c.withPath(pathValue) } | ||
| } | ||
|
|
||
| // http://www.rfc-editor.org/errata_search.php?rfc=6265 | ||
| def `path-value` = rule { | ||
| capture(zeroOrMore(`av-octet`)) ~ OWS | ||
| } | ||
|
|
||
| def `same-site-av` = rule { | ||
| ignoreCase("samesite=") ~ OWS ~ `same-site-value` ~> { (c: HttpCookie, sameSiteValue: String) => c.withSameSite(sameSite = SameSite(sameSiteValue)) } | ||
| } | ||
|
|
||
| def `same-site-value` = rule { | ||
| capture(ignoreCase("lax") | ignoreCase("strict") | ignoreCase("none")) ~ OWS | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be ok I think for the first version. If people run into that problem often enough, we can think about providing alternatives.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, since it is the server-side sending those, only our client-side would be affected where cookie usage currently isn't as important.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, you're right. Let's keep it like this then. I'll update the tests - do you think we should call this out in the release/migration notes?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't hurt to mention it. It's not really worse than before, though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, let's leave it like this. |
||
| } | ||
|
|
||
| def `secure-av` = rule { | ||
| ignoreCase("secure") ~ OWS ~> { (cookie: HttpCookie) => cookie.copy(secure = true) } | ||
| ignoreCase("secure") ~ OWS ~> { (cookie: HttpCookie) => cookie.withSecure(true) } | ||
| } | ||
|
|
||
| def `httponly-av` = rule { | ||
| ignoreCase("httponly") ~ OWS ~> { (cookie: HttpCookie) => cookie.copy(httpOnly = true) } | ||
| ignoreCase("httponly") ~ OWS ~> { (cookie: HttpCookie) => cookie.withHttpOnly(true) } | ||
| } | ||
|
|
||
| // http://www.rfc-editor.org/errata_search.php?rfc=6265 | ||
|
|
@@ -310,9 +318,10 @@ private[parser] trait CommonRules { this: Parser with StringBuilding => | |
| | ignoreCase("max-age=") | ||
| | ignoreCase("domain=") | ||
| | ignoreCase("path=") | ||
| | ignoreCase("samesite=") | ||
| | ignoreCase("secure") | ||
| | ignoreCase("httponly")) ~ | ||
| capture(zeroOrMore(`av-octet`)) ~ OWS ~> { (c: HttpCookie, s: String) => c.copy(extension = Some(s)) } | ||
| capture(zeroOrMore(`av-octet`)) ~ OWS ~> { (c: HttpCookie, s: String) => c.withExtension(s) } | ||
| } | ||
|
|
||
| // ****************************************************************************************** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.