Add support for partitioned cookies to CookieStore. #206

DCtheTall · 2021-09-15T19:53:33Z

Preview | Diff

DCtheTall · 2021-09-15T19:58:11Z

This PR is a change to the CookieStore API spec to support partitioned cookies.

These changes could be used to support the Partitioned attribute, a.k.a. CHIPS.

There is an open PR in the CHIPS repository adding a note about these changes to the explainer.

index.bs

DCtheTall · 2021-09-16T16:49:25Z

One quick addendum: if user agents implement service worker partitioning (ref) then the cookie's partition key should be the site of the worker's partition key.

If user agents don't implement storage partitioning, then partitioned cookies may need to be blocked in service workers. Otherwise, service workers could use partitioned cookies as a cross-site identifier. See the CHIPS explainer for more detail.

Service worker partitioning is also mentioned in more detail in this proposal.

DCtheTall · 2021-10-07T21:40:56Z

I put together a CL that implements these changes in Chromium.

This CL adds support for Partitioned cookies* to the CookieStore API. It implements the changes proposed in WICG/cookie-store#206 in Chromium. Cookie partitioning will only be active and the `partitioned` field is only considered by `CookieStore.set` or `CookieStore.delete` when the --partitioned-cookies flag is set. *: see https://github.com/WICG/CHIPS Bug:1225444 Change-Id: I4bdde833c28fa192bb0b5e011a6005a45029b481

This CL adds support for Partitioned cookies* to the CookieStore API. It implements the changes proposed in WICG/cookie-store#206 in Chromium. Cookie partitioning will only be active and the `partitioned` field is only considered by `CookieStore.set` or `CookieStore.delete` when the --partitioned-cookies flag is set. *: see https://github.com/WICG/CHIPS Bug: 1225444 Change-Id: I4bdde833c28fa192bb0b5e011a6005a45029b481 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212611 Commit-Queue: Dylan Cutler <dylancutler@google.com> Reviewed-by: Ayu Ishii <ayui@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Maksim Orlovich <morlovich@chromium.org> Cr-Commit-Position: refs/heads/main@{#937656}

…Store API, a=testonly Automatic update from web-platform-tests Add Partitioned cookie support to CookieStore API This CL adds support for Partitioned cookies* to the CookieStore API. It implements the changes proposed in WICG/cookie-store#206 in Chromium. Cookie partitioning will only be active and the `partitioned` field is only considered by `CookieStore.set` or `CookieStore.delete` when the --partitioned-cookies flag is set. *: see https://github.com/WICG/CHIPS Bug: 1225444 Change-Id: I4bdde833c28fa192bb0b5e011a6005a45029b481 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212611 Commit-Queue: Dylan Cutler <dylancutler@google.com> Reviewed-by: Ayu Ishii <ayui@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Maksim Orlovich <morlovich@chromium.org> Cr-Commit-Position: refs/heads/main@{#937656} -- wpt-commits: bb30d118adc1c7154c548a28e81d30723e0aaaa8 wpt-pr: 31374

This CL adds support for Partitioned cookies* to the CookieStore API. It implements the changes proposed in WICG/cookie-store#206 in Chromium. Cookie partitioning will only be active and the `partitioned` field is only considered by `CookieStore.set` or `CookieStore.delete` when the --partitioned-cookies flag is set. *: see https://github.com/WICG/CHIPS Bug: 1225444 Change-Id: I4bdde833c28fa192bb0b5e011a6005a45029b481 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212611 Commit-Queue: Dylan Cutler <dylancutler@google.com> Reviewed-by: Ayu Ishii <ayui@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Maksim Orlovich <morlovich@chromium.org> Cr-Commit-Position: refs/heads/main@{#937656}

This CL adds support for Partitioned cookies* to the CookieStore API. It implements the changes proposed in WICG/cookie-store#206 in Chromium. Cookie partitioning will only be active and the `partitioned` field is only considered by `CookieStore.set` or `CookieStore.delete` when the --partitioned-cookies flag is set. *: see https://github.com/WICG/CHIPS Bug: 1225444 Change-Id: I4bdde833c28fa192bb0b5e011a6005a45029b481 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212611 Commit-Queue: Dylan Cutler <dylancutler@google.com> Reviewed-by: Ayu Ishii <ayui@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Maksim Orlovich <morlovich@chromium.org> Cr-Commit-Position: refs/heads/main@{#937656} NOKEYCHECK=True GitOrigin-RevId: 4a461f1bb88cd8e12231512641dafcfa7f1cd48b

inexorabletash · 2023-06-15T18:27:37Z

Wow, this fell off my radar. Is this just waiting for final review/merge, or is there further discussion that should happen first?

inexorabletash · 2021-11-03T15:48:53Z

index.bs

@@ -689,6 +692,9 @@ The <dfn method for=CookieStore>set(|options|)</dfn> method steps are:
        |options|["{{CookieInit/domain}}"],
        |options|["{{CookieInit/path}}"], and
        |options|["{{CookieInit/sameSite}}"].
+    1. If the |options|["{{CookieInit/partitioned}}"] attribute is present and the user agent supports [cookie partitioning](https://github.com/WICG/CHIPS), then
+      1. If the current browsing context is a service worker, set the cookie's partition key be |origin|'s [=site=].


Should linkify "current browsing context" and "service worker"

Also.. this is run after the cookie is set, which seems unusual. ISTM the partition key should be captured before "set a cookie" and passed into it.

And should we toss a definition of "partition key" into the cookie-concept section?

I think the partition key can be up to RFC6265bis to define, since the CookieStore API is not actually concerned with partition key values and the cookie RFC will (some day) specify how to select which partitioned cookies to include in a request. So I do not think this spec needs to be concerned with that?

Yes... but https://wicg.github.io/cookie-store/#cookie-concept basically has "proxy" definitions for RFC6265bis terms, since I don't think we can link directly to them. We can add it later though.

explainer.md

index.bs

explainer.md

index.bs

explainer.md

index.bs

inexorabletash

Almost there! Just tiny nitpicks.

index.bs

inexorabletash · 2023-08-01T20:32:11Z

BTW, I was playing with cookieStore/document.cookie interop and it took me a while to realize that with document.cookie "partitioned" is ignored unless "path" and "secure" are also specified, which are the default with cookieStore but of course not document.cookie. Do you think we should add an example in the "Modifying Cookies" section, or is this well known?

DCtheTall · 2023-08-04T21:30:30Z

BTW, I was playing with cookieStore/document.cookie interop and it took me a while to realize that with document.cookie "partitioned" is ignored unless "path" and "secure" are also specified

Tad correction, you only need the "secure" attribute set. The "path" attribute is not necessary 😄

Do you think we should add an example in the "Modifying Cookies" section, or is this well known?

The Secure requirement is a well-known part of the proposal for the definition of Partitioned for RFC6265bis.

SHA: 880b5ea Reason: push, by inexorabletash Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

inexorabletash · 2023-08-04T23:47:57Z

BTW, I was playing with cookieStore/document.cookie interop and it took me a while to realize that with document.cookie "partitioned" is ignored unless "path" and "secure" are also specified

Tad correction, you only need the "secure" attribute set. The "path" attribute is not necessary 😄

Thanks for clarifying! https://source.chromium.org/chromium/chromium/src/+/main:net/cookies/canonical_cookie_unittest.cc;l=731 mislead me a bit I guess.

DCtheTall · 2023-08-15T19:54:09Z

Thanks for clarifying! source.chromium.org/chromium/chromium/src/+/main:net/cookies/canonical_cookie_unittest.cc;l=731 mislead me a bit I guess.

Ah, this is because Path=/ is required for the __Host- prefix, which that cookie also uses. I can see why that is initially confusing, especially considering Partitioned was originally designed to require __Host- but we loosened it to only require Secure 😄

Add support for partitioned cookies to CookieStore.

4df7a86

domenic reviewed Sep 15, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

add examples

b25de90

DCtheTall marked this pull request as ready for review October 7, 2021 21:38

add default value to delete

625fb51

chromium-wpt-export-bot mentioned this pull request Oct 25, 2021

Add Partitioned cookie support to CookieStore API web-platform-tests/wpt#31374

Merged

Add partitioned to set and get

976425a

DCtheTall mentioned this pull request Jan 10, 2023

Add CHIPS docs mdn/content#23495

Merged

inexorabletash reviewed Jun 15, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

explainer.md Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

inexorabletash mentioned this pull request Jul 14, 2023

Add note about duplicate cookies #221

Open

Merge branch 'WICG:main' into partitioned

ec91ee9

dmdabbs reviewed Jul 18, 2023

View reviewed changes

explainer.md Show resolved Hide resolved

DCtheTall added 2 commits July 19, 2023 13:41

review comments

aa12ba3

Modify set cookie algo for partitioned

b4504f9

DCtheTall requested review from inexorabletash and domenic July 19, 2023 17:53

inexorabletash reviewed Jul 20, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Review comments

023a7cf

DCtheTall requested a review from inexorabletash July 31, 2023 16:50

inexorabletash requested changes Aug 1, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

review comments

28de9d0

DCtheTall requested a review from inexorabletash August 4, 2023 21:30

inexorabletash merged commit 880b5ea into WICG:main Aug 4, 2023

github-actions bot added a commit that referenced this pull request Aug 4, 2023

Add support for partitioned cookies to CookieStore. (#206)

7419065

SHA: 880b5ea Reason: push, by inexorabletash Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for partitioned cookies to CookieStore. #206

Add support for partitioned cookies to CookieStore. #206

DCtheTall commented Sep 15, 2021 •

edited by pr-preview bot

Loading

DCtheTall commented Sep 15, 2021

DCtheTall commented Sep 16, 2021 •

edited

Loading

DCtheTall commented Oct 7, 2021 •

edited

Loading

inexorabletash commented Jun 15, 2023

inexorabletash Nov 3, 2021

inexorabletash Jun 15, 2023

inexorabletash Jun 15, 2023

DCtheTall Jul 31, 2023

inexorabletash Aug 1, 2023

inexorabletash left a comment

inexorabletash commented Aug 1, 2023

DCtheTall commented Aug 4, 2023 •

edited

Loading

inexorabletash commented Aug 4, 2023

DCtheTall commented Aug 15, 2023

Add support for partitioned cookies to CookieStore. #206

Add support for partitioned cookies to CookieStore. #206

Conversation

DCtheTall commented Sep 15, 2021 • edited by pr-preview bot Loading

DCtheTall commented Sep 15, 2021

DCtheTall commented Sep 16, 2021 • edited Loading

DCtheTall commented Oct 7, 2021 • edited Loading

inexorabletash commented Jun 15, 2023

inexorabletash Nov 3, 2021

Choose a reason for hiding this comment

inexorabletash Jun 15, 2023

Choose a reason for hiding this comment

inexorabletash Jun 15, 2023

Choose a reason for hiding this comment

DCtheTall Jul 31, 2023

Choose a reason for hiding this comment

inexorabletash Aug 1, 2023

Choose a reason for hiding this comment

inexorabletash left a comment

Choose a reason for hiding this comment

inexorabletash commented Aug 1, 2023

DCtheTall commented Aug 4, 2023 • edited Loading

inexorabletash commented Aug 4, 2023

DCtheTall commented Aug 15, 2023

DCtheTall commented Sep 15, 2021 •

edited by pr-preview bot

Loading

DCtheTall commented Sep 16, 2021 •

edited

Loading

DCtheTall commented Oct 7, 2021 •

edited

Loading

DCtheTall commented Aug 4, 2023 •

edited

Loading