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.

Sign up for GitHub

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

Jump to bottom

Proposal: add cookies.removeAll() method #690

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Proposal: add cookies.removeAll() method #690

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e34be81
Create Proposal: add cookies.removeAll() method.md
aselya Sep 18, 2024
e8dae2e
Update proposals/Proposal: add cookies.removeAll() method.md
aselya Oct 1, 2024
d42833a
Update Proposal: add cookies.removeAll() method.md
aselya Oct 1, 2024
dac145f
Update Proposal: add cookies.removeAll() method.md
aselya Oct 1, 2024
5682b4c
Update Proposal: add cookies.removeAll() method.md
aselya Oct 1, 2024
bf51fe9
Update Proposal: add cookies.removeAll() method.md
aselya Oct 2, 2024
1c7ffcf
Update Proposal: add cookies.removeAll() method.md
aselya Oct 2, 2024
d7d953b
Update Proposal: add cookies.removeAll() method.md
aselya Oct 3, 2024
76f57be
Update Proposal: add cookies.removeAll() method.md
aselya Oct 8, 2024
8f17da2
Update Proposal: add cookies.removeAll() method.md
aselya Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions proposals/Proposal: add cookies.removeAll() method.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
**Summary**

Adds a new API `cookies.removeAll()` to allow for the removal of multiple cookies with a single call.

**Document Metadata**

**Author:** [aselya](https://github.com/aselya)

**Sponsoring Browser:** Chrome

**Contributors:**

**Created:** 2024-09-18

**Related Issues:** [PrivacyCG/CHIPS issue 6 - How do Partitioned cookies interact with browser extensions?](https://github.com/privacycg/CHIPS/issues/6)
aselya marked this conversation as resolved.
Show resolved Hide resolved

## Motivation

Prior to the introduction of [partitioned cookies](https://github.com/privacycg/CHIPS), the required parameters (`url` and `name`) used by the existing `cookies.remove()` api would be unique to a single cookie. The inclusion of an optional `partitionKey` means that there can be more than one cookie that matches the currently required parameters for `cookies.remove()`. Because of this, handling partitioned cookies with `cookies.remove()` requires additional steps which can be simplified by the inclusion of this new API.
aselya marked this conversation as resolved.
Show resolved Hide resolved

### Objective

This new API addresses two workflows for developers:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the examples you provided is how extensions would have to achieve the functionality these days. Can you rephrase to emphasize that? To someone not deeply aware of the details, it can be read as part of the proposal for something net, not of existing behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the language and I would appreciate feedback on if my rephrasing addresses the issue you brought up.

##### Removal of cookies with the same url and name combination but different partitionKey values:
Since the introduction of partitioned cookies, to remove all the cookies with the same `url` and `name` a developer must first call `cookies.getAll()` to get all of the partitioned and unpartitioned cookies associated with the values. Then use the results of that call to make individual calls to `cookies.remove()` to delete each cookie.

##### Removal of cookies associated with a topLevelSite:
To remove all the cookies associated with a `topLevelSite`, a developer must first call `cookies.getAll({})` to retrieve all partitioned and unpartitioned cookies. Then filter the results to identify cookies that have a value for `topLevelSite` of their `partitionKey` match the desired `topLevelSite`. Finally the developer will need to make individual calls to `cookies.remove()` to delete each cookie.

#### Use Cases

##### Cookie Manager:

Let's say a cookie manager extension (with host permissions) is used by users to remove their cookies. Using `cookies.removeAll()` would ensure that the cookie manager extension is removing all the cookies associated with a `topLevelSite` correctly. Instead of relying on the developer of the extension to make a call to `cookies.getAll()` followed by call(s) to `cookies.remove()`. Reducing complexity for the developers while improving performance of the extension.

### Known Consumers

All extensions that access and/or modify cookies with awareness of partitioned cookies, through the use of the `partitionKey` property in the `cookies` extension API.

## Specification


### Schema

##### Syntax

```
let removed = await browser.cookies.removeAll(
details // object
)
```
##### Parameters
An `object` containing information to identify the cookie(s) to remove. It contains the following properties:
aselya marked this conversation as resolved.
Show resolved Hide resolved

>[`name`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/cookies/remove#name) optional:
>
>A `string` representing the name of the cookie to remove.
>
>[`partitionKey`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/cookies/remove#partitionkey) optional:
>
>An object representing the storage partition containing the cookie.
>
>[`topLevelSite`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/cookies/remove#toplevelsite) optional:
>
>A `string` representing the first-party URL of the top-level site storage partition containing the cookie.
>
>[`storeId`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/cookies/remove#storeid) optional:
>
>A `string` representing the ID of the cookie store to find the cookie in. If unspecified, the cookie is looked for by default in the current execution context's cookie store.
>
>[`url`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/cookies/remove#url) optional:
>
>A `string` representing the URL associated with the cookie.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Return value" is not documented in this proposal. What are you going to return?

The remove method returns the deleted cookie, and that could be reasonable to do the same in cookies.removeAll. A concrete use case is to allow for extensions to undo the removal. Currently extensions have to call cookies.get or cookies.getAll and keep track of the cookie. This tracking requires extensions to accurately predict which cookie would be removed for a given cookies.removeAll call, but it is not easy to determine which cookies would be removed.

Another option is to return nothing (undefined) and encourage extension developers to call getAll with the given parameters, under the assumption that the cookies.getAll call would return exactly the same set of cookies as matched by cookies.removeAll. In theory this is prone to TOCTOU issues, but I think that this is not that big of a deal in practice.

Yet another option is to add an option to the cookies.removeCookies call to flag whether deleted cookies should be returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think returning a list of cookies that were removed makes sense here. In addition to addressing the use case you mentioned it also matches the behavior of cookies.getAll().

### Behavior

The API will remove all cookies that match the `details` object parameter with the exception of the cases outlined in the implementation details.
If the extension does not have [host permissions](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#host_permissions) for this URL, the API call will fail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we allow calling this API without a url (see below) we should add a note here about the behavior if you only have some host permissions. Presumably, we would just remove the cookies for the sites you have access to.


### New Permissions
No new permissions are required.

### Manifest File Changes
No new manifest fields are required.

## Security and Privacy
Incorrect usage can lead to the removal of cookies that are not intended.

### Exposed Sensitive Data
None

### Abuse Mitigations
To remove a cookie, the extension must have host permissions for the `url` associated with the cookie.


## Alternatives
None

### Existing Workarounds

The behavior introduced by this API can be replicated by using a combination of `cookies.getAll()` and `cookies.remove()`.

### Open Web API

The cookies API is specific to extensions

## Implementation Notes

Host permissions are required for this API.
#### Special cases:
```cookies.removeAll({})```

>An empty details object will result in all cookies being removed.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like some input on this. My thought was to keep the behavior the same between cookies.getAll and this implementation but it might make sense to block it or direct developers to use the already existing browserData API

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open to thoughts, but I'd prefer to make this an error. As you mentioned, there is already the browsingData API for this, and even just during development it would be frustrating to accidentally clear all cookies. This would also allow us to match the CookieDetails object from the remove() API where name and url are currently required.

Copy link
Collaborator

@xeenon xeenon Sep 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also like you make {} be an error. In Safari, we only return cookies you have host permissions for in getAll({}), so that is what would be removed if we supported it with removeAll({}). However, it does seem to be too much of a foot gun.

I think requiring one key is still useful and safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback @xeenon and @oliverdunk , having {} be an error works for me.

I'm inclined to agree with @xeenon, that having at least one key is useful and safe. Would requiring the caller to provide at least a url or a topLevelSite sound good?

A url allows for a similar requirement as the existing cookies.remove() and it's inclusion allows for preserving the functionality. While a topLevelSite enables the new functionality of removing all cookies associated with a single topLevelSite introduced by the API. Keeping both of these parameters optional but requiring at least one of them, matches existing APIs such as webNavigation.getFrame() that already have the pattern of having all optional parameters but returning an error when no parameters or an invalid combination of parameters are provided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm comfortable with that. Thanks!


```
cookies.removeAll({
partitionKey:{}})
```

>An empty `partitionKey` object as the only value in the details object, will result in all partitioned cookies being removed.
aselya marked this conversation as resolved.
Show resolved Hide resolved

```
cookies.removeAll({
name: “example”,
url: “https://example.com”,
partitionKey:{}
})
```

>An empty `partitionKey` object with other values populated, will result in both unpartitioned and partitioned cookies that also match the other values provided will be removed.

```
cookies.removeAll({
topLevelSite: “https://example.com”
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is topLevelSite a separate key? I would expect it to be a member of partitionKey.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having topLevelSite be a separate (optional) parameter addresses the second motivation "Removal of cookies associated with a topLevelSite".

This could be replaced by having the developer pass in a partitionKey that contains the toplevelSite and no value for hasCrossSiteAncestor, such aspartitionKey:{topLevelSite: "https/example.com"}. But I wanted to provide an easier option for developers who may not be as familiar with the intricacies of partitioned cookies.

})
```

>If `topLevelSite` is the only argument, it will result in all cookies that have that value as the `topLevelSite` in their `partitionKey` being removed.

```
cookies.removeAll({
topLevelSite: “https://example.com”,
partitionKey:{topLevelSite: “https://foo.com”}
})
```

>If the `topLevelSite` differs from the `topLevelSite`, if present, in the `partitionKey` an error will be returned.