1535763 - browsingData should clear all storage areas

Status: NEW

(WebExtensions :: Storage, enhancement, P3)

Description

[Mike Conca [:mconca]]

This is a spin-off from bug 1531276 where Andrew Sutherland points out that being able to delete localStorage and indexedDB independently may actually create some privacy issues for users if extension authors are not careful.

We should consider making browsingData delete both storage areas at the same time.

Comment 1

[David Durst [:ddurst]]

If the concern is that a privacy-focused extension could incompletely remove storages used by a given domain (and therefore we shouldn't provide that level of granularity), I think a safer path would be to extend our remove() API to be allowed to encompass everything.

There's nothing particularly unique, if I understand correctly, about localStorage and indexedDB any more than there is about other storage area combinations. So there seems to be no good reason to joining the clearing of both to each other.

I would argue that we should, instead, augment the DataTypes for remove() or provide an optional parameter to remove all storages (even those, like indexedDB, that do not have a discrete API like removeLocalStorage or removeCookies).

I don't see how this is a greater risk now than it has been in the past, as any developer who is using the discrete APIs instead of iterating through remove() is already running this risk -- and, conversely, any developer who misses some storage using remove() should already be aware of the other storages that could be involved.

Comment 2

[David Durst [:ddurst]]

(NI asuth to be sure that, across the various bugs that spawned this, I'm not missing some detail about indexedDB vs localStorage that would necessitate conjoining removal methods.)

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

LocalStorage and IndexedDB weren't the focus of my point in bug 1526246; it's just that the way the conversation happened in bug 1531276 due to bullet point structuring that it became the dominant begged question.

The genesis of my point is in bug 1526246 where requesting serviceWorkers to be cleared only cleared ServiceWorkers registrations and their chrome storage, and not the Cache API storage where a ServiceWorker would store its offline data. And in fact there is no way to clear the Cache API as browsingData is implemented in Firefox at this time. Which is much of my point... Firefox's implementation of the API is already fundamentally broken and doesn't have tests that notice this, so how can we expect WebExtension authors to get it right if we can't get it right? If we shift the browsingData to using existing privacy-logic in the same way that Firefox does this for the user and has test coverage for, that makes us less likely to have similar problems in the future.

In terms of IndexedDB and LocalStorage, I think it's important to note that, practically speaking, there are 3 classes of persistent storage we're dealing with for sites. (Note that I'm excluding SessionStorage which is an explicitly ephemeral per-origin-per-tab storage that is made persistent because of the session store subsystem.)

1. Cookies
2. LocalStorage
3. IndexedDB, Cache API, ServiceWorkers, APIs that hang off of ServiceWorker registrations (push, backgroundsync).

Historically speaking, cookies and localstorage have been their own thing and subject to purging independent of each other and the third category. I don't have a major problem if we continue to allow LocalStorage to be cleared separately from the third category about it, but we should be intentional about it. (If only because once it's stored in QuotaManager by LocalStorage NextGen, it is a special-case for us to wipe out the 3rd category but not LocalStorage.)

I do strongly feel that the third category should all be removed together as a group. While I do want to support creativity in WebExtensions, I need to emphasize that much of the point of the move from legacy add-ons to WebExtensions was to create an API surface that we can support. In many cases, this wasn't about reducing complexity for the add-ons API surface, but about reducing complexity for the rest of the browser. I don't think we on the DOM workers & storage team or the members of the webcompat team can support situations where a WebExtension wipes out only IndexedDB but not CacheStorage, so it really begs the question of why we would support WebExtensions doing that in the first place. We have many examples of significant amounts of developer time being consumed in bugzilla on unsupported preferences like the one that allows disabling IndexedDB at an opportunity cost of improving our storage implementations. (Frequently it took many rounds of back and forth to realize that the preference was at play, and I would suspect a similarly costly discovery process for browsingData-related scenarios unless we can have the Bugzilla filing process explicitly be augmented to report when the bug filer's browser is using a webextension that uses the browsingData API. That would change the calculus.)

Comment 4

[Tomislav Jovanovic :zombie]

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #3)

it really begs the question of why we would support WebExtensions doing that in the first place.

The original reason is compatibility with Chrome extensions, which were the basis of web extensions. Changing the behavior of the public api now, after the fact, may introduce unexpected data loss for users, so we should be careful.

Since this originally came from product, Philipp, can you look into what we want to do here?

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to :Tomislav Jovanovic :zombie from comment #4)

The original reason is compatibility with Chrome extensions, which were the basis of web extensions. Changing the behavior of the public api now, after the fact, may introduce unexpected data loss for users, so we should be careful.

Given the context of the motivating bug, I don't think the original API was specified/documented well enough for any party (user, extension developer, browser developer) to reason about what should/would happen. I think it would be handy to enumerate what specific use-case we'd be enabling. There are specific cases where deleting IndexedDB but not Cache API storage for a specific site might work on the day the WebExtension was written, or for the sites the developer tested against, but if the site changes it could break the site.

One approach might be to be to 1) do clear all QuotaManager-stored storage as a unit (IndexedDB, Cache API, ServiceWorkers, everything that hangs off of ServiceWorker registrations), and 2) have the browsingData API trigger a prompt of the UI that's used to clear site data from about:preferences. It could say something like: "Here's what data is going to be cleared. Prompt for this in the future: [always / not for this site anymore, but for other sites I haven't approved / never, let the extension clear this data for any site wheenver it wants]. [Clear] [Don't Clear]." If the user sets to not be prompted in the future, either a permission or other durable setting will be made to avoid annoying the user in the future.

This lets the user understand what's going to happen and to express whether they want to permanently grant this ability or not to the WebExtension. This is also helpful in that I understand it to be a best practice to grant permissions closer to the time of use rather than all at once when an app/extension is downloaded.