Add support for setting, getting, and deleting cookies

[jgraham]

No description provided.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Cookies.

The full IRC log of that discussion

<AutomatedTester> topic: Cookies
<AutomatedTester> jgraham (IRC): This is something that doesn't have an open issue
<jgraham> https://github.com//issues/287
<jgraham> github: https://github.com//issues/287
<AutomatedTester> the high level item for this topic is that we need ot have something here like webdriver classic
<AutomatedTester> ... I assume that people want this
<AutomatedTester> ... how do we want this to work?
<AutomatedTester> ... and then we have partitioned cookies do we want the API to return everything or just for the part of the partition?
<AutomatedTester> Devin Rousso: in web inspector we have both
<AutomatedTester> ... in CDP network domain returns a list of cookies that could have been sent, including ones that could have been blocked
<AutomatedTester> ... e.g 3rd party cookies blocked by policy
<AutomatedTester> Devin Rousso: I think we should go for the simpler option here as all cookies seems heavy handed
<AutomatedTester> q?
<AutomatedTester> q+
<BrandonWalderman> q+
<DevinRousso> q+
<jgraham> AutomatedTester: One of the things that comes up when dealing with cookies is the ability to set before you navigate to a page. IN WebDriver classic you can only set for the current page. It would be good to have the ability to interact directly with the cookie store without requiring a navigation.
<AutomatedTester> ack next
<AutomatedTester> ack next
<AutomatedTester> Brandon Walderman: we're debating whether to show all cookies on a page or to a domain?
<AutomatedTester> ... if we're talking about adding a cookie domain that could straight to the cookie jar and doesn't need a network reques?
<AutomatedTester> jgraham (IRC): I think like AutomatedTester_ (IRC) said, we want an aPi that allows us to interact with the cookie jar
<AutomatedTester> ... the comment about network request was more 'do we want to emulate CDP'? or do our own? i can't see use cases yet?
<AutomatedTester> Brandon Walderman: a web dev might want to test their site with different level s of tracking being blocked during testing
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> Devin Rousso: as far your point AutomatedTester_ (IRC) I thinkj this will be handled by network interception
<jgraham> q+
<AutomatedTester> ... a user could set this while setting up the network interception and they would set the cookie and away they go
<AutomatedTester> ack next
<AutomatedTester> jgraham (IRC): in interception there a way to handle that and we should and I think that we should also add a cookie API. Interception could be overkill for some usecases
<JimEvans> q+
<AutomatedTester> ... I think that this isn't provin to be uncontroversial
<AutomatedTester> ... we can discuss more in the PR
<AutomatedTester> ack next
<DevinRousso> q+
<AutomatedTester> Jim Evans: it feels like the design goal of being able to implement classic on top of bidi not having an API that directly accessing it
<AutomatedTester> ack next
<AutomatedTester> Devin Rousso: my comment on network interception is that was purely for AutomatedTester_ (IRC) 's example but I do think that we need an API for interaction with the cookie jar

[whimboo]

Given that we haven't had an issue for deleting cookies yet and that it actually fits into this issue I've updated the summary of the issue to reflect that as well.

[whimboo]

See as well some ongoing discussions on #423.

[lolaodelola]

Hi folks, my colleague @jugglinmike and I have been thinking about ways we could implement Cookies (get, set & delete) and have come up with three possible solutions and would love to hear what folks think.

1. We specify every way that a browser does cookies and include this in the API. So getCookie("foo", container: containerName) would get the named foo from the Firefox container named bar, and getCookie("foo", requestSourceOrigin: example.com) would get the named cookie from the request source origin, etc.

2. We specify a mechanism to include generic properties for your requests. getCookie("foo", source: container | requestSourceOrigin) So you can add whatever you want and implementers can define what those things are and what the browser does in response. This method would mean we don’t have to get into the details of the request, source origin, etc & allow the browsers to interpret as they will.

3. We specify that when getCookie("foo") returns any and everything, for every request source origin, every container. "get" could return an array (which could be empty in some instances), while "set" could create a cookie that applied to all contexts, and "delete" would remove the named cookie in any & all contexts where it was defined.

There are pros and cons to each of these methods, so none is perfect, and I’m not particularly attached to any, so it’d be great to hear what others think.
cc: @mathiasbynens

[OrKoN]

@jgraham and I were discussing cookies right now on the webdriver channel. It looks like the option 3 would be preferable and in line with the discussion there but we need to include the cookie key into the matching cookies and probably add a way to compute a cookie key for a given context.

[jgraham]

So the problem is forwards compatibility.

Let's say we adopt a design where you have browser-specific partition keys, and any key the client is missing is interpreted as a wildcard.

For getting cookies that's not totally terrible, but there are still sharp edges. For example if initially cookies are only keyed on destination origin (the default), then getCookies("name", host="example.org") is going to always just return one cookie. But if it then adds partitioning by source origin, there may well be more than one cookie returned, and that would almost certainly lead to unexpected results in automated tests, even though the site under test would in fact be working correctly.

For setting it may well not work at all. The issue is that you can't know the values that different key parts might have in the future. So if I setCookie("name", "value", host="example.org") in a browser with only destination-keyed cookies, it will naturally work when example.com tries to send data to example.org. But if a later version of that browser adds source origin keyed cookies, then at the time I set the cookie the browser doesn't know that it will later be asked to make a request from example.com, so it doesn't know to insert the cookie with that key. So you either need to allow wildcard keys in the underlying cookie store which it isn't obvious that you should (given the entire point is state partitioning), or you somehow need to set the cookie just in time whenever something happens that would correspond to a new key. I can check in detail what Firefox does, but I'd be surprised if we could support this.

[jgraham]

The other problem with "set is a wildcard" is that if we later want to support multiple isolated browsing contexts (without requiring multiple top-level browser processes), it would be rather surprising if the default behaviour of setting state is to write it to all the different isolates.

[OrKoN]

I imagined something like this:

type Cookie {
   name: string;
   value: string;
 
   host: string;
   ....
   partitionKey: string|undefined; // used for partitioning, clients have no insights into the structure of the key.
}

getCookies(): Cookies[]; // a way to get all cookies stored in the browser; 
// perhaps with filter by name/key/etc.

addCookie(cookies: Cookie[]); // adds cookies to the storage; 
 // UnsupportedOperation if the partition key is wrong; loosing future compatibility I guess if browsingContext.getCookiePartitionKey is not used? 
 // but I don't see a way if browsers can make breaking changes here.

removeCookie(cookies: Cookie[]); // perhaps only a subset of data is needed for removal?

browsingContext.getCookiePartitionKey(): string // returns a cookie key for the context

This would not support the wildcards (should we? based on James' comments I am not sure we can/should). It should cover the potential triple keying by the container/profile in the sense that the bidi implementations would need to rewrite the partition keys using the browser-specific algorithms so the clients would get some partitionKey token that does not match 1 to 1 to what is actually in the storage.

[jgraham]

Yeah, so that design is more or less the best idea I have as well. But it doesn't work if you want to be able to set a cookie before the first request to a site. If the partition keys are fixed between sessions people will absolutely try to get around that by hardcoding known keys in their tests (which will then break again when the browser version changes). On the other hand, I guess browsers must have a way of dealing with cookies set in older versions after updates, so maybe they already have to do something sensible for missing fields. But that might work better for user-scenarios (where no one is directly inspecting the cookies, and they can just depend on the server to set any missing cookies) compared to tests scenarios where direct inspection is the point.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Support for cookie handling.

The full IRC log of that discussion

<AutomatedTester> topic: Support for cookie handling
<AutomatedTester> github: https://github.com//issues/287
<AutomatedTester> jgraham: This is something that we need to make progress on this. It is a high priority on the roadmap
<AutomatedTester> ... this is the biggest missing piece so far
<orkon> q+
<AutomatedTester> ... I think because in the previous webdriver it causes issues for our users
<AutomatedTester> ... and we need to make sure that we have it work well in all browsers
<orkon> q-
<AutomatedTester> ... There is a draft PR up
<AutomatedTester> ... could everyone start reading the draft PR so we can have a much longer discussion at TPAC

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Webdriver BiDi Cookies, and agreed to the following:

• ACTION: give feedback on the PR

The full IRC log of that discussion

<AutomatedTester> topic: Webdriver BiDi Cookies
<AutomatedTester> github: https://github.com//issues/287
<AutomatedTester> Lola: We are working on the cookies implementationm
<AutomatedTester> ... we wanted some clarification on the design
<Lola> https://github.com//pull/501
<AutomatedTester> ... I am not sure if people have reviewed jugglinmike (IRC) last comment in 501
<AutomatedTester> ... the question is "Do we need storage.cookie since there is network.cookie?
<shs96c> q+
<AutomatedTester> ack next
<jgraham> q+
<AutomatedTester> shs96c: I think we should only have 1 thing for cookie. I dont know if it should be storage or network but I feel there should only be 1
<AutomatedTester> ack next
<Lola> q+
<AutomatedTester> jgraham: on the naming there should be one. There might be oberservable to clients. If they are observable to clients they same then it should be the same thing
<gsnedders> q+ to point out correspondence to sandboxing discussion earlier
<AutomatedTester> ... if you were to do things through network interaction that we should eb able to set cookies easily
<AutomatedTester> ... with 1 simple way
<Lola> q-
<AutomatedTester> ... for context on cookies. browsers partition cookies according to the site that it has set it
<AutomatedTester> ... browsers are now starting to allow a different way of setting cookies. So in firefox cookies are being partitioned from where they come.
<AutomatedTester> ... and containers and private browsing can set these in different ways
<AutomatedTester> ... we are not standardised on partitioned.
<AutomatedTester> ... I've looked at Mike's draft that it tries to set what needs to be used and hints what the values could be
<AutomatedTester> ... we could see about setting the values in the spec
<AutomatedTester> ... and we can mention about setting the partition key
<AutomatedTester> ... and we should leave it open for future development
<AutomatedTester> q?
<AutomatedTester> ... and I will stop talking. Dont hold me to it... or may be do
<AutomatedTester> ack next
<Zakim> gsnedders, you wanted to point out correspondence to sandboxing discussion earlier
<AutomatedTester> Sam Sneddon [:gsnedders]: I want to build on what jgraham said about sandboxing.
<jgraham> q+
<gsnedders> ... I don't think it makes sense to consider storage (or cookies) separate to the earlier sandboxing discussion.
<jugglinmike> q+
<gsnedders> ... We probably can't enumerate all the keys that every implementation might use to key their storage (and network, etc.) into different partitions, because that's something where implementations continue to try and innovate.
<gsnedders> ... So we probably, to use the language of Infra, need a way to select a User Agent that exists within that Implementation. Which is exactly what the sandboxing discussion was about earlier.
<AutomatedTester> ack next
<AutomatedTester> jgraham: I think make sense for nameable partitions
<AutomatedTester> ... I don't know how that works for cookies keyed on Source Origin
<AutomatedTester> ... if we have partioned containers like 1 and 2 and they can be keyed to it
<AutomatedTester> ... there are cases where example.org and example.net being stored differently in different browsers
<AutomatedTester> ... and we cant enumerate things since it's unbounded implicit partitions
<shs96c> q+
<AutomatedTester> ... if 1st party cookies this can be easy but how each browser treats 3rd party cookies differently
<AutomatedTester> ... and not being obvious to users
<AutomatedTester> q?
<AutomatedTester> ack next
<AutomatedTester> jugglinmike (IRC): I wonder if there a presendence of storage. I wonder if there cookies on top of containers is it our job to explain how this works to users
<AutomatedTester> .... <explains different ways of containers and urls>
<AutomatedTester> ... is there any concern on this from a standardisation point of view on how this interacts with the containers
<AutomatedTester> ack next
<jgraham> q+
<AutomatedTester> shs96c: I think the real concern here that we will make something the average tester will have no idea how to use
<AutomatedTester> ... they wont understand all the differences in these keys
<AutomatedTester> ... I think that we create a situation where it confuses folks
<AutomatedTester> ... I think we should go back to the use case of "I visit a site and then ask for cookies:" that it makes sense
<AutomatedTester> ... so that we dont have users needing to know what browsers they are using
<AutomatedTester> ack next
<AutomatedTester> ... what is the bare minimum we can put in the spec
<AutomatedTester> ... and then iterate
<AutomatedTester> jgraham: I agree we should have reasonable defaults
<AutomatedTester> ... for the cases we know we have reasonable defaults
<orkon> q+
<AutomatedTester> .... once we have containers we will make sure that it makes sense for the user
<AutomatedTester> ... for the origin paritioned cookies that we default to all be same
<AutomatedTester> ... in the case for SSO this is likely where things "break"
<AutomatedTester> ... e.g. auth.org is doing the auth for corp.com and people will want to set auth.org
<AutomatedTester> shs96c: if we pregenerate the cookie and then allow people to set it
<jugglinmike> q+
<AutomatedTester> jgraham: we will try do the simple case well. There will also be cases wher ethey need to understand what they doing with cookies and partiions and they will do it
<AutomatedTester> shs96c: can we do the bare minimum or should we do more
<AutomatedTester> jgraham: <explains example. SOrry someone asked a question and lsot track>
<AutomatedTester> ... if we think that containers and SO make sense acorss all browsers then we could encode in the spec and if they cant do it just document it
<AutomatedTester> ... e.g. you might be able to set it in Firefox and Safari and Chrome ignores it
<AutomatedTester> shs96c: can we do this so we dont slam the door on iterating over this
<AutomatedTester> jgraham: yes. we can have a key for container that could eb the session and then we have the source origin
<sadym> s/<explains example. SOrry someone asked a question and lsot track>/if user really wants, they can set up network interception for the specific auth request and return set cookie.
<AutomatedTester> ack next
<AutomatedTester> orkon: I wanted to say that this capapbility based negotiation can allow us to build useful defaults
<AutomatedTester> ... if we wanted to allow people to set source origin via a caps
<AutomatedTester> ... I think this is a workable solution in the draft PR and what jgraham suggests
<AutomatedTester> ... and some of the container stuff could be automatically computed
<AutomatedTester> ack next
<AutomatedTester> jugglinmike (IRC): to shs96c point on simplifying... would it work to have this part of the storage container?
<AutomatedTester> ... or is this going to paint us into a corner?
<orkon> q+
<AutomatedTester> ack next
<AutomatedTester> jgraham: the organisation of modules is arbritary
<AutomatedTester> ... as orkon said people want to set this near the beginning
<AutomatedTester> ... it can be in browser... is it about controling the browser? I don't think it matters
<AutomatedTester> q?
<shs96c> q?
<orkon> q+
<AutomatedTester> ack next
<AutomatedTester> orkon: we could maybe put containers into storage too?
<AutomatedTester> ACTION: give feedback on the PR
<jugglinmike> s/have this part of the storage container/define this command on the browsingContext module/
<AutomatedTester> jgraham: the summary is we should try a model where we have 2 partion keys (container name from the containers discussion and source origin and both are optional)
<jgraham> I think the model is:... (full message at <https://matrix.org/_matrix/media/v3/download/matrix.org/qrEAcyDbsFrPNbBWwmGhaDJE>)
<AutomatedTester> rrsagent, make minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2023/09/15-webdriver-minutes.html AutomatedTester

[jugglinmike]

We've just started iterating on specification text in gh-501, but we've already made some assumptions for which I haven't yet found justification in the WebDriver BiDi project's public records. I'm seeking confirmation in this thread because these details will fundamental to any solution--not just the one we're currently working on.

• Cookie retrieval
  • Should we specify getCookie (i.e. "get the named cookie"), getCookies (i.e. "get any number of named cookies"), getAllCookies (i.e. "get all the cookies"), or some combination?
  • Precedence

    platform	get the named cookie	get any number of named cookies	get all the cookies
    DOM	❌	❌	✔️
    WebDriver Classic	✔️	❌	✔️
    Chrome Devtools Protocol	❌	❌	✔️

  • Assumptions
    • getAllCookies is meaningfully more powerful because it allows developers to discover cookies (one potential use case: writing a test which fails in the presence of any unexpected cookie).
    • getCookie and getCookies are superfluous in the presence of getAllCookies (the waste of transmitting unneeded data is negligible, and library authors can build getCookie and getCookies on top of it)
• Cookie modification
  • Should we specify setCookie (i.e. "modify one cookie") or setCookies (i.e. "modify any number of cookies")?
  • Precedence

    platform	modify one cookie	modify any number of cookies
    DOM	❌	✔️
    WebDriver Classic	✔️	❌
    Chrome Devtools Protocol	❌	✔️

  • Assumptions
    • setCookies is more versatile since it allows developers to set a single cookie
    • setCookie is superfluous in the presence of setCookies (the syntax overhead on developers will be minor)
• Cookie deletion
  • Should we specify deleteCookie (i.e. "delete the named cookie"), deleteCookies (i.e. "delete any number of named cookies"), deleteAllCookies (i.e. "delete all the cookies"), or some combination?
  • Precedence

    platform	delete the named cookie	delete any number of named cookies	delete all the cookies
    DOM	❌	✔️	❌
    WebDriver Classic	✔️	❌	✔️
    Chrome Devtools Protocol	❌	❌	✔️

  • Assumptions
    • deleteCookie and deleteCookies are superfluous in the presence of setCookie or setCookies
    • deleteAllCookies is superfluous in the presence of getCookies and either "set" API ("get all cookies" followed by "modify any number of cookies" is an acceptable approximation despite being non-atomic)

[OrKoN]

I would be in favour of starting with get(all)Cookies and set(all)Cookies with setCookies(cookies=[]) supporting the deletion use case. @jgraham @sadym-chromium what's your opinion?

[sadym-chromium]

getAllCookies SGTM.
WRT setCookies(cookies: Cookie[]), I would expect it to append the cookies to the list of currently set cookies. Having that, I'd be in favour of having addCookies(cookies: Cookie[]), deleteCookies(cookieNames: String[]) and deleteAllCookies() to provide atomicity.

[jgraham]

I'd have getCookies which takes an optional list of names (or list of cookie properties, perhaps). If it's not supplied then all cookies are returned.

I'd also expect setCookies to modify/append but not delete (I think the right case to optimise for is "add one cookie to whatever the existing set is", so that should take one operation).

deleteCookies should work like getCookies i.e. it takes an optional list of properties to filter on, but the default is to delete all cookies.

[OrKoN]

After a discussion offline, I am with Maksim and James that setCookies should not delete.

[jugglinmike]

deleteAllCookies() to provide atomicity.

@sadym-chromium Since everything that occurs in this protocol is asynchronous, I'm having trouble coming up with a situation where atomicity is meaningful. A command like storage.deleteAllCookies could induce a browser to clear its cookie store, but because every BiDi client invariably needs to wait for a subsequent WebSocket message, it seems like they still won't be able to guarantee that the store is empty. Can you help me understand how atomicity is important in this context?

I'd also expect setCookies to modify/append but not delete

After a discussion offline, I am with Maksim and James that setCookies should not delete.

@jgraham @OrKoN It's my understanding that changing an existing cookie's expiry-time to a date in the past is functionally equivalent to deleting that cookie. From RFC6265:

The user agent MUST evict all expired cookies from the cookie store if, at any time, an expired cookie exists in the cookie store.

From my perspective, the decision is not whether "set cookie" should allow deletion, but rather whether BiDi needs a dedicated "delete cookie" command given that "set cookie" will inherently allow deletion. I suppose we could design BiDi's "set cookie" command to be incapable of deletion (e.g. by ignoring the expires field of network.Cookie or perhaps failing in its presence), though since this kind of conflicts with the existing design of BiDi's types (and also precludes non-deletion usage of that field), I feel I should verify: is that what you folks have in mind?

[OrKoN]

@jugglinmike what I meant is that if there are the following cookies in the storage (I guess identified by name + partition keys): (cookie1, cookie2) then calling setCookies(cookies: [cookie3]) should not remove cookie1 and cookie2 from storage. I believe if you call setCookies(cookies: [cookie2]) with an expiration date that leads to its deletion, I think that would be expected. I would probably expect that you can still add an expired cookie to test that the user agent actually deletes it (e.g., in a WPT test).

[jgraham]

whether BiDi needs a dedicated "delete cookie" command given that "set cookie" will inherently allow deletion.

Yes. Although it's possible to use setCookie to update a cookie so that it's already expired, I don't think we should expect end users to use this rather roundabout method to perform the simple operation of removing a cookie from the cookie jar. I also think that having a dedicated command will make it easier to understand logs and so on, since the intent of the command will be explicit and obvious.

[jugglinmike]

Thank you both!

[gsnedders]

So, AIUI, there's roughly three cookie models currently in use (sorry, existing discussion here is split between here and PR comments, but let's capture this all in the issue):

• Firefox (always? or maybe still has a denylist of sites where this doesn't apply?) partitions cookies by first-party, third-party
• Chrome by default provides unpartitioned cookies to both first and third parties, and allows cookies to opt-in to partitioning via CHIPS (similarly first-party, third-party), with the end-goal of stopping sending cookies to third-parties if they don't opt-in
• Safari by default provides cookies to only first parties, and allows third-party sites to opt-in to unpartitioned cookie access via Document#requestStorageAccess (which requires user interaction)

There's potentially other layers of partitioning, but these are more akin to different top-level cookie stores (essentially what Infra calls a "user agent"). An obvious example is private browsing (in all browsers), but also "containers" in Firefox, "profiles" in Safari, etc. These are really different types of partitioning, as these are linked to UI features to segment storage domains.

[jgraham]

@gsnedders I think the design in #501 (comment) allows for all those scenarios? It is basically up to the implementation to decide which parts of the key it wants to use for a specific request (or reject the request if it doesn't have a complete key). In Safari that might depend on which permissions have already been granted.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Cookies.

The full IRC log of that discussion

<AutomatedTester_> Topic: Cookies
<jgraham_> (don't want to extend this topic further, but I always assumed BiDi would move to an event-based model for permissions i.e. you'd get a permissions.Request event with some information)
<AutomatedTester_> github: https://github.com//issues/287
<AutomatedTester_> orkon___: I was wondering if there are any open issues/blockers for the cookies proposal?
<AutomatedTester_> q?
<orkon___> https://github.com//pull/501
<lola_> q+
<AutomatedTester_> ack next
<AutomatedTester_> lola_: Mike isn't here today but he will be working on them this week. We should hopefully have those comments handled and ready for review later this week

[whimboo]

This is done via PR #501.