#469: Cookie Store API

Visit on Github.

Opened Jan 29, 2020

Hello TAG!

I'm requesting a TAG review of Cookie Store API.

Cookie Store API offers an asynchronous alternative to document.cookie, and exposes HTTP cookies to service workers. This allows sites to react to cookie changes, such as session cookie changes during authentication, and update the current page UI in response. It also allows service workers to react to these changes and could initiate a cleanup of private cached data.

Further details:

We'd prefer the TAG provide feedback as:

💬 leave review feedback as a comment in this issue and @-notify @ayuishii

Discussions

Discussed Feb 1, 2020 (See Github)

Kenneth: Looks pretty sensible, I don't know all the details.

Sangwhan: Does this guarantee transactionality?

Kenneth: Not sure. There's an onchange event listener...

... some shorthands... guess that's fine...

... Will leave a comment about ??

Comment by @kenchris Feb 11, 2020 (See Github)
 Promise<void> set(USVString name, USVString value,
                   optional CookieStoreSetOptions options = {});

explicitly forces to write name (not part of options dict) with optional options, but in the getter

Promise<CookieListItem?> get(optional CookieStoreGetOptions options = {});

dictionary CookieStoreGetOptions {
  USVString name;
  USVString url;
  CookieMatchType matchType = "equals";
};

it is inside the options dict. That seems a bit inconsistent.

Actually, the setter is a bit weird because there are two different setter options dicts, and the other does "allow" setting the name as part of the options dict - a bit confusing

Promise<void> set(USVString name, USVString value,
                    optional CookieStoreSetOptions options = {});
  Promise<void> set(CookieStoreSetExtraOptions options);
Comment by @torgo Feb 12, 2020 (See Github)

@lknik are you able to take a look at this from a privacy PoV (in your capacity as a TAG alumni)?

Comment by @pwnall Feb 13, 2020 (See Github)

@kenchris Thank you very much for the feedback!

We tried to balance two concerns, and ended up in an admittedly odd situation. We'd appreciate the TAG's guidance here!

On one hand, we wanted to match get, set, and delete in the ES6 Map. So, we wanted to be able to support cookieStore.get(cookieName), cookieStore.set(cookieName, cookieValue) and cookieStore.delete(cookieName).

On the other hand, we wanted to let the users get a cookie dictionary from get or getAll, modify it, and be able to pass it back to set. In other words, we wanted to be able to support const cookieData = cookieStore.get(cookieName); cookieStore.set(cookieData);

One more issue I wanted to highlight is that the name argument / option to get and getAll isn't necessarily the full name of the cookie. When the matchType option is "starts-with", the name is a prefix.

We're looking forward to your guidance!

Comment by @lknik Feb 16, 2020 (See Github)

@torgo depends on the deadline, when would you need this done? ;)

Comment by @lknik Feb 25, 2020 (See Github)

@ayuishii I see that the questionnaire filled as it stands now, does not address cookie capability in service workers (which currently have no access to cookies), did you look at it and perhaps it would be possible to amend that?

(is the API implemented already?)

Comment by @ayuishii Mar 3, 2020 (See Github)

@lknik Thank you for the feedback!

I see that the questionnaire filled as it stands now, does not address cookie capability in service workers (which currently have no access to cookies), did you look at it and perhaps it would be possible to amend that?

We've updated the questionnaire (https://github.com/WICG/cookie-store/blob/master/security-privacy-self-assessment.md#33-does-this-specification-introduce-new-state-for-an-origin-that-persists-across-browsing-sessions).

Thank you for pointing out this gap in our explanation.

As a guiding principle, we want the Cookie Store API to match the access level of the existing document.cookie API. We do not want to expand a page's acess to cookie data, and we're OK with reducing access in some edge cases, such as non-secure contexts.

We think that Service Workers can currently access the cookies of any URL under their scope. For example, a service worker could respond to any top-level request with an HTML document embedding an <iframe> pointing to the desired URL. When responding to the request for that URL, the Service Worker can respond with an HTML document containing a <script> that proxies the Service Worker's access to the document.cookie API using postMessage.

For this reason, we think that our API doesn't change the security properties of cookies on the Web Platform. We only intend to provide high-performance alternatives to the current cookie access methods. We hope this allows developers to improve the user experience on sites that rely on cookies.

is the API implemented already?

We have implemented the API behind a flag in Chrome. The API will undergo at least one more Origin Trial before we consider shipping it. We look forward to the TAG's feedback, and are eager to improve the API based on it.

Comment by @kenchris Mar 5, 2020 (See Github)
const cookieData = cookieStore.get(cookieName); cookieStore.set(cookieData);

should be

const cookieData = await cookieStore.get(cookieName); cookieStore.set(cookieData);

given the idea is that this is async :)

For me it seems like a bit of magic. The getter returns more than just the { name, value } pair and it is not really a performance optimization due to being a common operation as it is already async, so I am not a bit fan of the magic and you can just do

const { name, value } = await cookieStore.get(cookieName); cookieStore.set(name, value);

You can even make such an example in the spec.

Also as @plinss points out to me, we [TAG] are not promoting the use of function overloads.

Comment by @ayuishii Mar 16, 2020 (See Github)

@kenchris Thank you for the feedback!

We tried providing an ergonomic option for usages such as below.

let cookie = await cookieStore.get(cookieName);
cookie.expires = new Date('Wed, 1 Jan 2025 00:00:00 GMT');
cookieStore.set(cookie); 

However, we defer to your expertise on API ergonomics. If there are other variants of set that are preferred, please show us the way! We would be happy to adopt your recommendation.

Comment by @lknik Mar 27, 2020 (See Github)

@lknik Thank you for the feedback!

I see that the questionnaire filled as it stands now, does not address cookie capability in service workers (which currently have no access to cookies), did you look at it and perhaps it would be possible to amend that?

We've updated the questionnaire (https://github.com/WICG/cookie-store/blob/master/security-privacy-self-assessment.md#33-does-this-specification-introduce-new-state-for-an-origin-that-persists-across-browsing-sessions).

Thank you for pointing out this gap in our explanation.

So can Service Workers access cookies now (i.e. read/write)? I'm concerned with a scenario where SW goes into background, the user clears site cookies, but SW still runs and has the state & reinstate the cookie. Did you consider such case?

Comment by @wanderview Mar 27, 2020 (See Github)

I'm concerned with a scenario where SW goes into background, the user clears site cookies, but SW still runs and has the state & reinstate the cookie. Did you consider such case?

At least in chrome (and I believe firefox) when a user "clears cookies" for an origin it wipes all storage for that origin; including service workers.

Comment by @pwnall Mar 27, 2020 (See Github)

I think we could argue that "clear cookies" needs to clear service workers. Otherwise, a SW that outlives "clear cookies" can use one of the already exposed persistent storage APIs (IndexedDB, Cache API, maybe more) to store a user identifier that outlives "clear cookies".

Comment by @lknik Mar 28, 2020 (See Github)

I think we could argue that "clear cookies" needs to clear service workers. Otherwise, a SW that outlives "clear cookies" can use one of the already exposed persistent storage APIs (IndexedDB, Cache API, maybe more) to store a user identifier that outlives "clear cookies".

Anyhow, in that case, this should be written down somewhere ;) and definitely in the considerations of this api? Just let's be clear about this.

Subsequent to that, for the moment, I think we can wait a bit @torgo unless you have some more time :)

Comment by @wanderview Mar 30, 2020 (See Github)

I think we could argue that "clear cookies" needs to clear service workers. Otherwise, a SW that outlives "clear cookies" can use one of the already exposed persistent storage APIs (IndexedDB, Cache API, maybe more) to store a user identifier that outlives "clear cookies".

Anyhow, in that case, this should be written down somewhere ;) and definitely in the considerations of this api? Just let's be clear about this.

I think that would have to be non-normative. There is no spec AFAIK that covers browser UI and what it means when a user clicks something like "clear cookies".

Comment by @lknik May 6, 2020 (See Github)

@wanderview so do it non-normatively then, but have it documented? either in the spec or in the questionnaire - or both. Assuming your proposed feature might produce a potential new abuse vector, it's good to be aware of it.

Comment by @kenchris May 26, 2020 (See Github)

@ayuishii

let cookie = await cookieStore.get(cookieName);
cookie.expires = new Date('Wed, 1 Jan 2025 00:00:00 GMT');
cookieStore.set(cookie); 

how does that work?

  Promise<CookieListItem?> get(USVString name);
  Promise<CookieListItem?> get(optional CookieStoreGetOptions options = {});

await + get returns a CookieListItem, so cookie will be of that type,

  Promise<void> set(USVString name, USVString value,
                    optional CookieStoreSetOptions options = {});
  Promise<void> set(CookieStoreSetExtraOptions options);

But there is no set that takes a CookieListItem?

However, we defer to your expertise on API ergonomics. If there are other variants of set that are preferred, please show us the way! We would be happy to adopt your recommendation.

Your code example above makes a lot of sense, but I just don't see how it would work with the specced API

Comment by @torgo May 26, 2020 (See Github)

@lknik @wanderview I think a non-normative note on this cookie clearing issue makes sense. In line with the guidance in the design principles doc you may want to be explicit about how this technology operates in incognito / private mode as well?

Comment by @kenchris May 26, 2020 (See Github)

So the way the above works (your example) is because you have two dictionaries which are very similar, but not named the same way and one is called *Options.

The Options ending is wrong here as it is more a CookieListItemInit as used by other specs instead of actually options to change the behavior of the setter.

If you compare CookieStoreSetExtraOptions with CookieListItem, the only difference is the secure field.

dictionary CookieListItem {
  required USVString name;
  USVString value;
  USVString? domain = null;
  USVString path = "/";
  DOMTimeStamp? expires = null;
  boolean secure = true;
  CookieSameSite sameSite = "strict";
};
dictionary CookieStoreSetExtraOptions : CookieStoreSetOptions {
  required USVString name;
  required USVString value;
};

dictionary CookieStoreSetOptions {
  DOMTimeStamp? expires = null;
  USVString? domain = null;
  USVString path = "/";
  CookieSameSite sameSite = "strict";
};

The CookieStoreGetOptions is actually options as they change the behavior of the get function

dictionary CookieStoreGetOptions {
  USVString name;
  USVString url;
  CookieMatchType matchType = "equals";
};

I don't find following method very useful as it splits out name and value but keeps the rest in the options dict, but in JS this is much easier to just write an a initDict anyway

  Promise<void> set(USVString name, USVString value,
                    optional CookieStoreSetOptions options = {});
   cookieStore.set({ name, value, path: '/'})
Comment by @kenchris May 26, 2020 (See Github)

So what I am suggesting is the following

Change this

 Promise<void> set(USVString name, USVString value,
                    optional CookieStoreSetOptions options = {});
 Promise<void> set(CookieStoreSetExtraOptions options);

dictionary CookieStoreSetOptions {
  DOMTimeStamp? expires = null;
  USVString? domain = null;
  USVString path = "/";
  CookieSameSite sameSite = "strict";
};

dictionary CookieStoreSetExtraOptions : CookieStoreSetOptions {
  required USVString name;
  required USVString value;
};

to this:

  Promise<void> set(USVString name, USVString value);
  Promise<void> set(CookieListItemInit cookieListItemInit);

dictionary CookieListItemInit {
  required USVString name;
  USVString value;
  USVString? domain = null;
  USVString path = "/";
  DOMTimeStamp? expires = null;
  CookieSameSite sameSite = "strict";
};
Comment by @kenchris May 26, 2020 (See Github)
 Promise<CookieListItem?> get(optional CookieStoreGetOptions options = {});

The above means that I can call get() (with nothing) and that doesn't seem to get my anything (throws TypeError if I read the algo right). Throwing makes sense, but it is not a type error as I am doing something that is valid according to the IDL :-)

So you should really remove the "optional ... = {}" part - those options are not optional as they throw

Comment by @wanderview May 26, 2020 (See Github)

@lknik @wanderview I think a non-normative note on this cookie clearing issue makes sense. In line with the guidance in the design principles doc you may want to be explicit about how this technology operates in incognito / private mode as well?

Sounds reasonable to me, but I defer to @ayuishii on this. I'm not directly working on this API and was more just making a drive-by comment. Sorry for any confusion.

Comment by @ayuishii May 28, 2020 (See Github)

Thanks @kenchris for the thorough advice.

For the set() method change, I agree, I think this seems more straight forward without having 2 dict options. I filed a tracking bug for track Chrome's implementation.

Removing optional from CookieStoreGetOptions seems reasonable as well. Currently the behavior is with no options, get()returns the first cookie (or null if no cookies are set). This is fairly arbitrary, and only makes sense from the lens of being consistent with getAll(). (Bug for tracking Chrome's implementation).

Thanks!

Comment by @ayuishii May 28, 2020 (See Github)

@lknik @wanderview I think a non-normative note on this cookie clearing issue makes sense. In line with the guidance in the design principles doc you may want to be explicit about how this technology operates in incognito / private mode as well?

Sounds reasonable to me, but I defer to @ayuishii on this. I'm not directly working on this API and was more just making a drive-by comment. Sorry for any confusion.

Thanks all for discussing this point on "clear cookies". I've created an issue to add/track this note addition to the spec.

Discussed Jun 1, 2020 (See Github)

Kenneth: Seems like they implemented my previous suggestions.

... Added some other options..? Disallow required dictionary, no required fields.

Dan: They're saying that they're looking at the clear cookies issue, created an issue on their repo to track it.

... If appropriate, maybe we can add their issue to our tracker and close the review issue.

... Setting to propose close, and will add the issue to our tracker once I remember how.

Yves: I can help with that.

Comment by @kenchris Jun 9, 2020 (See Github)

Thanks for incorporating out feedback! Looks like you filed issues for all our points, so looks like we should be able to close this issue!

Comment by @kenchris Jun 10, 2020 (See Github)

Thanks for consulting with the TAG - hope our feedback has been useful! Closing for now, feel free to reopen if there is anything else you need us to take a look at

Comment by @pwnall Jun 11, 2020 (See Github)

We thank the TAG for the review! This has been very helpful!

I tried to credit the folks who contributed in https://github.com/WICG/cookie-store/pull/147 -- please let me know if I've missed anyone!

Comment by @ayuishii Jun 11, 2020 (See Github)

Thank you very much for the attention and guidance!

@kenchris I apologize for the delayed comment, but was wondering if I can ask for your guidance on one last thing.

While implementation of the removal of optional from CookieStoreGetOptions, I came across and idl rule issue (dic-arg-optional) that disallows me from making a dictionary required if it does not have any required fields. Does it seem reasonable to keep this optional as long as it does not error on retrieval if there are no cookies? Or do you recommendation something different?

Thank you, and sorry again for the delayed question.