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

Add cookie accessor/setter methods? #707

Closed
slightlyoff opened this issue Jun 9, 2015 · 52 comments
Closed

Add cookie accessor/setter methods? #707

slightlyoff opened this issue Jun 9, 2015 · 52 comments

Comments

@slightlyoff
Copy link
Contributor

In keeping with our approach of only adding APIs as-needed, we've had a request come in from @cramforce for access to cookies from the SW context.

We'll need something asynchronous; e.g.:

self.getCookies().then(...);
self.setCookies().then(...);

I'm afraid it won't lock, so might get out of sync with documents more easily than script access does today, but we should perhaps try it out and see how it goes.

While we're at it: do we want more structured cookie types for both these records and the cookies entries in Response objects?

@cramforce
Copy link

Would this we SW only or also go into other global scopes?
Non-worst-API-ever document.cookie style magic strings would be great, although it makes little difference for everyone who can already handle them.

@jakearchibald
Copy link
Contributor

I think this is an opportunity to introduce a decent cookie API. Async, so it can be accessed from worker contexts. I imagine this would be part of the fetch spec, @annevk?

partial interface Navigator {
  [SameObject] readonly attribute CookiesContainer cookies;
};

partial interface WorkerNavigator {
  [SameObject] readonly attribute CookiesContainer cookies;
};

[Exposed=(Window,Worker)]
interface CookiesContainer {
  [NewObject] Promise<Cookie> get(USVString name);
  [NewObject] Promise<sequence<Cookie>> getAll();
  [NewObject] Promise<void> set((USVString or Cookie) init, optional USVString value = "", optional CookieOptions options);
};

[Constructor((USVString or Cookie) init, optional USVString value = "", optional CookieOptions options),
Exposed=(Window,Worker)]
interface Cookie {
  attribute USVString name;
  attribute USVString value;
  attribute USVString domain;
  attribute USVString path;
  [SameObject] attribute Date expires;
  attribute Boolean secure;
}

dictionary CookieOptions {
  USVString domain; // defaults to location.host
  USVString path = "/";
  Date expires;
  Boolean secure = false;
}

Usage:

// read cookies value
navigator.cookies.get('hello').then(c => c && c.value).then(value => {
  if (value) {
    // …
  }
});

// create/overwrite cookie value
navigator.cookies.set('hello', 'world', {
  path: '/foo/'
});

// change cookie value
navigator.cookies.get('viewCount').then(viewCookie => {
  var viewCount = (viewCookie ? Number(viewCookie.value) : 0) + 1;
  var date = new Date();
  date.setFullYear(d.getFullYear() + 20);

  return navigator.cookies.set('viewCount', viewCount, {
    expires: date
  });
});

// delete cookie
navigator.cookies.delete('viewCount').then(cookieWasDeleted => {
  // …
});

// output all cookie name/vals as urlencoded string
navigator.cookies.getAll().then(cookies => {
  var params = new URLSearchParams();
  for (var cookie of cookies) {
    params.append(cookie.name, cookie.value);
  }
  return params.toString();
});

@jakearchibald
Copy link
Contributor

I guess get, set, getAll, delete could be static methods of Cookie rather than navigator.cookies.

Cookie.set('hello', 'world')

@RReverser
Copy link
Member

Looks awesome!

Just couple of questions:

  1. Shouldn't be Cookie.name Readonly?
  2. What is the purpose of USVString name parameter in getAll()? Aren't names unique?
  3. If yes is the answer to both above, maybe we can do Promise<Map<string, Cookie>> getAll()?

@hsablonniere
Copy link

I prefer self.cookies but I imagine that if it gets to the Window scope one day, navigator.cookies would be better to avoid global conflicts.

I'm not a great fan of first-letter-capital object names, it makes me feel I can new it.

@jakearchibald
Copy link
Contributor

@RReverser

  1. Shouldn't be Cookie.name Readonly?

Is there any benefit in:

navigator.cookies.get('hello').then(cookie => {
  cookie.value += ' foobar';
  return navigator.cookies.set(cookie);
});

…if not, all cookie properties should be readonly.

  1. What is the purpose of USVString name parameter in getAll()? Aren't names unique?

Copy/paste error, sorry!

  1. If yes is the answer to both above, maybe we can do Promise<Map<string, Cookie>> getAll()?

Interesting. If we're going to remove name from Cookie it may be better to remove getAll and go with cookies.keys() which returns an array of cookie names, which you can get with .get if you need to.

@jakearchibald
Copy link
Contributor

I prefer self.cookies but I imagine that if it gets to the Window scope one day

If we come up with a better API, it should be accessible from window straight away.

I'm not a great fan of first-letter-capital object names, it makes me feel I can new it.

You can in the design above.

@jakearchibald
Copy link
Contributor

@pornel points out (on Twitter) that maybe "secure" should default to true on https pages. I'm open to this. I simply went with the cookie default & wanted to avoid a property with a non-static default.

@RReverser
Copy link
Member

Is there any benefit in:

Do you mean that we can't make values writable because setting them should be asynchronous? If so, then yes, all of them are readonly.

Interesting. If we're going to remove name from Cookie it may be better to remove getAll and go with cookies.keys() which returns an array of cookie names, which you can get with .get if you need to.

I like the idea, although removing name is not necessary since it's immutable in the design above anyway.

The only downside of this is that if someone really needs to get all the cookies, he needs to use more complex and slower construction like

function cookieEntries() {
  return navigator.cookies.keys().then(
    keys => Promise.all(keys.map(
      key => navigator.cookies.get(key).then(value => [key, value])
    ))
  );
}

So I'd propose to add both keys() and entries() / getAll().

@jakearchibald
Copy link
Contributor

Perhaps CookieOptions should support maxAge too, which would be a number. If expires and maxAge are both set, it should throw.

@annevk
Copy link
Member

annevk commented Jun 9, 2015

I wonder if we should wait until same-origin cookies are more established before exposing a new API. Exposing anything but same-origin cookies would be another sidechannel of sorts. Also, I suspect we would restrict a new API, especially for cookies, to secure contexts these days. So only generating secure cookies makes sense.

@lozandier
Copy link

@jakearchibald For educational purposes, why throw when both are set? AFAIK, it's not uncommon for devs to set both, or receive cookies that have only expires set instead of maxAge because of the older browsers their application or services support.

Nonetheless, the older browsers that accept only expires like IE 6, 7, & 8 are EOL'd (the latter January 12, 2016)…

@RReverser
Copy link
Member

For educational purposes, why throw when both are set?

Because it's ambiguous which one to use for determining deadline.

@annevk
Copy link
Member

annevk commented Jun 9, 2015

I don't think we should even expose expires. max-age is much better and you need a new browser to make use of this API anyway.

@lozandier
Copy link

@RReverser @annevk Ahh, that makes sense. I agree, max-age is much better; it's why I mentioned it to @jakearchibald on Twitter.

@bsittler
Copy link

bsittler commented Jun 9, 2015

Cookie names are not unique. Only the combination of name, domain-or-implicit-pseudo-origin, and path is unique, and even that combination is unique only at a single point in time and only with a consistent cookie store.

Also, shouldn't set also be asynchronous, and perhaps even result in promise rejection if a later-initiated update collided with it before it completed? In any case, shouldn't it work asynchronously and only finish once the change is synchronized across the session?

@domenic
Copy link
Contributor

domenic commented Jun 9, 2015

I think we should shoot for something async-map-like, which #707 (comment) diverges from.

Agreed it should be secure-only. Probably should be same-origin cookies only as well.

Using dates in APIs is always tricky, especially since they are mutable. (Not just a question of "can you set .expires = ..., but also what happens when you do .expires.setDay(...).) I think the convention is to use timestamps instead. (ISO strings seem nicer?) I am not sure what the convention for accepting them is; maybe anything the Date constructor accepts?

Unsure why USVString. What are the restrictions on cookie names anyway?

Fuller proposal to follow.

@bsittler
Copy link

bsittler commented Jun 9, 2015

Also, perhaps self.cookies.match and self.cookies.matchAll might make more sense since it's an asynchronous operation rather than a simple getter. I think the name should be optional in each case.

 self.cookies.match().then(function(cookie){
     /* gets the first cookie, if any are visible to scripts on this origin
        (one of the narrowest-scoped ones) */ })
 self.cookies.matchAll().then(function(cookies) {
     /* gets all cookies visible to scripts on this origin, ordered from
         narrowest-scoped to broadest-scoped */ })
 self.cookies.match('SAPISID').then(function(cookie){
     /* gets the narrowest-scoped cookie named 'SAPISID' visible
         to scripts on this origin, if at least one exists */ })
 self.cookies.matchAll('SAPISID').then(function(cookies){
     /* gets all cookies named 'SAPISID' visible to scripts on this
         origin, ordered from narrowest-scoped to broadest-scoped */ }

This lookup interface provides the same capabilities as document.cookie. If more capabilities are declared fit for web consumption they might be implemented by adding a second optional parameter with matching options.

Would you expose parameters on read cookies? The document.cookie interface does not, and simply concatenates all the in-scope cookies in a fairly well-defined but not standardized order.

@bsittler
Copy link

bsittler commented Jun 9, 2015

"Same-origin cookies only" explicitly won't work for SAPISID and other domain-wide synchronization cookies.

Also, being able to subscribe to script-visible cookie jar changes would be super-awesome and would vastly reduce wakeups and power usage for apps synchronizing with sign-in state. Right now many web apps do this by periodic polling, typically being forced into a bad compromise between laggy session synchronization and shortened battery life.

Something like this might work well:

https://developer.chrome.com/extensions/cookies#event-onChanged

(This has been used successfully for power-efficient session synchronization in extensions.)

@domenic
Copy link
Contributor

domenic commented Jun 9, 2015

It does occur to me that restricting to same-origin cookies might prevent @cramforce's original use case :-/.

Also as @bsittler reminds us it sounds like the key is not id, but instead id + path + other stuff.

With that in mind I have two proposals. One is based on a simplified model: path is always "/", secure is always true, httponly is always false, and same-origin means no domain. So the keys can just be strings. The other is more full-featured, adding path and domain and dropping the same-origin restriction (but keeping secure-only).

Simplified

[Exposed=(Window,Worker)]
interface CookieJar {
  Promise<Cookie> get(DOMString id);
  Promise<boolean> has(DOMString id);
  Promise<void> set(DOMString id, Cookie cookie);
  Promise<void> delete(DOMString id);

  // These are annoying because maybe they should be async iterators instead...
  Promise<sequence<DOMString>> keys();
  Promise<sequence<Cookie>> values();
  Promise<sequence<sequence<DOMString or Cookie>>> entries(); // haha WebIDL
};

dictionary CookieOptions {
  DOMTimeStamp expires; // incoming Dates will be auto-converted
}

[Constructor(DOMString value, optional CookieOptions options),
Exposed=(Window,Worker)]
interface Cookie {
  readonly attribute DOMString value;
  readonly attribute DOMTimeStamp expires;

  // Immutable for simplicity.
}

// usage:
const original = window.cookies.get("key");
const updated = new Cookie(
    original.value + "\nanother week",
    { expires: original.expires + 7 * 24 * 60 * 60 * 1000 }
 );

window.cookies.set("key", updated);

This looks kind of like async local storage with expiration built-in, heh.

Full-Featured

This version could potentially be the same just with domain and path:

partial dictionary CookieOptions {
  USVString domain; // defaults to location.origin (*not* host)
  USVString path = "/";
}

partial interface Cookie {
  readonly attribute USVString domain;
  readonly attribute USVString path;
}

However I am unsure how well the string-keyed get/set/has/delete can work given that the key is actually something like { id, domain, path }.

  • Maybe we add a extra optional { domain, path } parameter to get/has/delete?
    • If that's present do we do exact matching, or some kind of what-you're-allowed-to-access matching?
    • E.g., does passing { path: "/" } match cookies set with path: "/a/b/c" when used inside /index.js?
  • Do we need a getAll so that given an ID you can get the various domain/path-varying cookies that match it?
  • Does the keys/values/entries thing work well here or fall down flat? (Probably the latter.)

Alternately we can just say that cookies are stupid and putting lipstick on that pig is not a good idea.

@annevk
Copy link
Member

annevk commented Jun 10, 2015

Alternately we can just say that cookies are stupid and putting lipstick on that pig is not a good idea.

Given that we so far did not introduce them in workers I think that might be okay. Just use cookies as synchronization token, and that's it...

@cramforce
Copy link

In a Worker I can write a 10 minute polyfill for all APIs described in this
thread by sending a message to the owning document. There is no comparable
polyfill for SWs.

As of now cookies are the only mechanism that allow for cross sub domain
login state propagation and they are used for this use case with little
alternative on the horizon. When a SW is offline there is currently no good
way for it to find out whether the stored offline content still belongs to
the session the user is logged into.

E.g. you lock into foo.slack.com as user X. At the end of the session you
go to slack.com and logout. Now the next person goes to this internet cafe
work station and goes to foo.slack.com – they should not see stored offline
content for user X. For all hosts that have N sub domains governed by a
single login session (which is at least true for all big online properties)
there is currently no scalable way to get rid of SWs on all those sub
domains on central logout.

Comparing a cookie value stored in IndexedDB to the actual cookie value is
a straight forward way to discard a cache with a few lines of code that has
high confidence to be correct and is not subject to questionable timing
(such as opening N iframes that spawn M > N SWs to delete themselves).

On Tue, Jun 9, 2015 at 10:12 PM Anne van Kesteren notifications@github.com
wrote:

Alternately we can just say that cookies are stupid and putting lipstick
on that pig is not a good idea.

Given that we so far did not introduce them in workers I think that might
be okay. Just use cookies as synchronization token, and that's it...


Reply to this email directly or view it on GitHub
#707 (comment)
.

@annevk
Copy link
Member

annevk commented Jun 10, 2015

That should be considerably easier once you can use service workers as a service (through navigator.connect).

@cramforce
Copy link

That isn't going to change that spinning up 3, 5, 20 or even 100 SWs on
logout on a device with 256 MB of RAM will not work in a timely fashion.

Maybe we could make the API read-only initially to reduce complexity.
On Jun 10, 2015 6:42 AM, "Anne van Kesteren" notifications@github.com
wrote:

That should be considerably easier once you can use service workers as a
service (through navigator.connect).


Reply to this email directly or view it on GitHub
#707 (comment)
.

@cramforce
Copy link

With navigator.connect one would need to implement a flow like:

  • SW only ever serves a bootstrap page
  • Bootstrap page signals to SW it is now ready to answer questions about
    cookies or sends current cookies to SW
  • SW replies with actual response
  • Bootstrap upgrades to full page based on that.

Please lets not require hacks like this as the first thing privacy
sensitive people have to do when using SWs.

On Wed, Jun 10, 2015 at 6:55 AM Malte Ubl malte.ubl@gmail.com wrote:

That isn't going to change that spinning up 3, 5, 20 or even 100 SWs on
logout on a device with 256 MB of RAM will not work in a timely fashion.

Maybe we could make the API read-only initially to reduce complexity.
On Jun 10, 2015 6:42 AM, "Anne van Kesteren" notifications@github.com
wrote:

That should be considerably easier once you can use service workers as a
service (through navigator.connect).


Reply to this email directly or view it on GitHub
#707 (comment)
.

@annevk
Copy link
Member

annevk commented Jun 10, 2015

That isn't going to change that spinning up 3, 5, 20 or even 100 SWs on logout on a device with 256 MB of RAM will not work in a timely fashion.

How is a cookie API going to change that? I have the feeling the scenario here is not complete.

@cramforce
Copy link

If you have access to cookies (read cross sub domain information) workers
can detect changes to session state lazily upon next invocation and act
accordingly.

On Wed, Jun 10, 2015 at 7:07 AM Anne van Kesteren notifications@github.com
wrote:

That isn't going to change that spinning up 3, 5, 20 or even 100 SWs on
logout on a device with 256 MB of RAM will not work in a timely fashion.

How is a cookie API going to change that? I have the feeling the scenario
here is not complete.


Reply to this email directly or view it on GitHub
#707 (comment)
.

@slightlyoff
Copy link
Contributor Author

A few points:

  • having access to cookies is a no-brainer. We can't prevent it (if there's a document, there's an API that already does it already); can we just get over our bad selves and get on with a design that does cookies better?
  • extra restrictions over what the current API can do need to be justified; the burden of proof is on removal, not equality of functionality
  • setting should be symmetric; if reading is async, setting should be too

@bsittler
Copy link

Given that, async map with full access to exactly the same data as
document.cookie with no new functionality (e.g. it would reveal on;y names
and values you could see in document.cookie, and setting would allow you
the same domain, path, name, value, maxage, secure, httponly and even
expires control [the cookie expiration may in fact need to be a precise
date and time!) and no new functionality (e.g. reading interface only
exposes in-scope cookie names and values in the same order as
document.cookie and does not tell you their domain, path, expiration time,
or secure flag) and just a cleaner API (no string parsing, no
concatenation, allow a date object for expiration) would seem to me like
the obvious starting point.
On Jun 10, 2015 15:52, "Alex Russell" notifications@github.com wrote:

A few points:

  • having access to cookies is a no-brainer. We can't prevent it (if
    there's a document, there's an API that already does it already); can we
    just get over our bad selves and get on with a design that does cookies
    better?
  • extra restrictions over what the current API can do need to be
    justified; the burden of proof is on removal, not equality of functionality
  • setting should be symmetric; if reading is async, setting should be
    too


Reply to this email directly or view it on GitHub
#707 (comment)
.

@bsittler
Copy link

[Apologies for the typos, I pressed send too soon.]

@owencm
Copy link

owencm commented Feb 22, 2016

FYI, as noted in issue #837, the ability to read cookies is fundamental to extending signed in sessions on a number of major websites.

I'd encourage this group to prioritize this feature.

@jakearchibald
Copy link
Contributor

I like @domenic's full-featured proposal, and agree that id should be a compound thing, but maybe { name, url } is enough?

.get(name, url = '/') - get the cookie with name foo, and the domain/path from the url.

.matchAll({ name: 'foo', url: '/bar/' }) - get the cookies that would be sent to a fetch to /bar/, then filter by name if provided. url defaults to location.href.

.match({ name: 'foo', url: '/bar/' }) - as above but returns the first result.

.getAll() would be all cookies for the domain.

If I wanted to find all the (non-http-only) cookies that would be sent to /hello/world/, that's .matchAll({ url: '/hello/world/' }).

.set(cookie)

This means making "name" a property of the cookie.

@mkruisselbrink
Copy link
Collaborator

So I fully agree that we probably want some kind of new and improved cookie API that works in service workers, but I don't believe that such an API should be part of the service worker spec, so this repository is probably not the best place to discuss details about the exact shape of the API.

Not sure where such an API should live though (as part of HTML? as a separate spec in the web platform WG? As a proposal/spec in the WICG?) Personally I like small specs for well defined features so having a separate spec for this seems like it would make sense.

@inexorabletash
Copy link
Member

After discussing w/ @domenic, @bsittler was planning to spin up a repo for it for iterating/issue tracking, though we imagined it would end up as part of HTML since it needs to interop w/ the existing Cookie behavior. (Otherwise I prefer "small specs" too.)

And WICG probably has the lowest bar...?

@mkruisselbrink
Copy link
Collaborator

@inexorabletash not quite sure how needing to interop with the existing Cookie behavior would imply this being part of HTML? The part of the HTML spec that deal with cookies is pretty much two lines of spec text calling out to the ietf cookie spec to get/set cookies. Referring to the same IETF spec from a cookie API spec seems like all that would be needed to achieve interop (of course a cookie API spec will have to reference parts of HTML and SW to properly specify its API).

Aside from that, spinning up a github repo for now and later migrate that to WICG sounds good to me.

@inexorabletash
Copy link
Member

@mkruisselbrink is probably right that there's no dependency. For the record, I was thinking of consistency issues but we've given up on that anyway - see the big warning near https://html.spec.whatwg.org/#dom-document-cookie

@jakearchibald
Copy link
Contributor

WICG probably has the lowest bar...?

Agreed

bsittler added a commit to WICG/cookie-store that referenced this issue Mar 4, 2016
This is also a design sketch for the new API and is loosely based on/inspired by w3c/ServiceWorker#707
@bsittler
Copy link

bsittler commented Mar 4, 2016

Just created a repo: https://github.com/bsittler/async-cookies-api

It doesn't have a useful explainer yet but perhaps it would be useful to move further cookie API discussion there, file issues, etc.?

@annevk
Copy link
Member

annevk commented Mar 5, 2016

@inexorabletash I think implementers have not quite given up on consistency issues and we shouldn't either. It's just no longer in the form of the storage mutex. We should be able to work something out though between the HTML Standard and @bsittler's work.

@bsittler
Copy link

bsittler commented Jul 27, 2016

There's an explainer for the asynchronous cookies API proposal now: https://github.com/bsittler/async-cookies-api/blob/gh-pages/explainer.md (but the polyfill there is out of date and should be ignored for now)

I welcome any and all of your comments, feedback, pull requests, recommendations for change in style or venue, and issue reports!

@bsittler
Copy link

bsittler commented Jul 27, 2016

Also, re: consistency, the proposed API does not introduce any new consistency requirements but does leave open the possibility of eventually introducing a stronger consistency guarantee

@bsittler
Copy link

The explainer and polyfill are updated and synchronized and I've started a thread on the WICG discourse forum for the proposal.

@jimmywarting
Copy link

jimmywarting commented Apr 27, 2017

Just thought about one time cookies...
I had to make a request only one time to get a bearer token with a cookie included, but i didn't have to save it and i didn't want to have it afterwards either to avoid it being sent all the time to the public/static requests

Just wondering if something like this would be useful

let cookie = new Cookie(...)
fetch(url, {
  cookies: [cookie]
})

or:

let cookieJar = new CookieJar()
let cookie = new Cookie(...)
cookieJar.put(cookie)

fetch(url, {
  cookieJar: cookieJar
})

@triblondon
Copy link
Collaborator

I have just had a use case for adding cookies to a response in a serviceworker before allowing the response to proceed to the browser tab. This doesn't currently work because Set-Cookie headers are prohibited on manually constructed responses. This is counter to my expectations, and doesn't raise any errors when tested in Chrome. See

https://stackoverflow.com/questions/44424231/how-to-set-cookies-on-a-response-in-a-serviceworker/44445217

@frvge
Copy link

frvge commented Jan 11, 2019

Currently our end-users are sending cookies for static files because our asset origin is a subdomain and we can't specify a list of allowed subdomains in the Set-Cookie header. Now the idea was, instead of using a new cookie-free domain, to use a service-worker to intercept the outgoing request, and strip the cookies.

So I'd like to be able to modify/drop cookies on outgoing requests in service workers.

@Bettelstab
Copy link

@frvge Here you go:

self.addEventListener('fetch', event => {
    if (event.request.url.startsWith(self.location.origin)) { // adapt this line
        // forward request
        event.respondWith(fetch(event.request, {
            // omit cookie transmission
            credentials: 'omit'
        }));
    }
});

@NekR
Copy link

NekR commented Mar 16, 2019

Use case for cookies in SW:
Errors/statistics logs via cookie. It is being set to cookie and server grabs it on whatever request and removes. Not possible with SW to collect stats offline/when recovered from bg-sync this way -- you need main thread window to set cookie

@wanderview
Copy link
Member

Chrome did an original trial of the cookie store api recently, but I don't think it's still running:

https://groups.google.com/a/chromium.org/forum/m/#!msg/blink-dev/pdxkBoURmaA/vOTkwUBCBAAJ

If you are interested in that feature you may want to post something there.

@mfalken
Copy link
Member

mfalken commented May 22, 2019

Yep, I think this issue is superseded by https://wicg.github.io/cookie-store/.

@joelpro2
Copy link

So, still not available?

@jimmywarting
Copy link

@joelpro2 globalThis.cookieStore will be able to read the cookies from a service worker (that don't have access to document.cookie)

cookieStore is also available in the main thread (but only in secure origins). but it's also not implemented by no other than Blink... see comp table here: https://developer.mozilla.org/en-US/docs/Web/API/CookieStore

@jcubic
Copy link

jcubic commented Apr 16, 2024

Is it possible to modify the cookies from Service Worker based on different URL? It's not useful if can't set cookies per request and only to origin that register Service Worker.

I was thinking of creating service worker that block cookies when user don't give consent. I just want to see what is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests