When clearing history, avoid clearing user interaction permissions for sites that still have cookies
Categories
(Toolkit :: Data Sanitization, enhancement, P1)
Tracking
()
People
(Reporter: johannh, Assigned: johannh)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
Bug 1675018 - Part 4 - Use deleteUserInteractionForClearingHistory in Sanitizer.jsm. r=mak,timhuang!
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta-
|
Details |
This is the proper fix to bug 1672394.
Instead of strictly clearing interaction permissions together with history, for each permission we confirm that there are no cookies or site data in store for that origin, and only then clear it. That would still be fine privacy-wise since the user had cookies/site data around anyway, so keeping the permission doesn't reveal more. And it should fix this issue.
It's definitely a lot more complex to implement and expensive to run, so we keep on making bug 1524200 worse, but it's probably worth it.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
This is done in preparation to using the module on Android in order to exclude
certain principals from getting their user interaction permissions cleared.
Assignee | ||
Comment 2•4 years ago
|
||
This function is a helper for clearing all storageAccessAPI permissions
that were modified after a certain date. We need to get them and filter
by principal to clear them.
Depends on D96638
Assignee | ||
Comment 3•4 years ago
|
||
This is a helper function for clear history functionality that needs to ensure that
storageAccessAPI permissions that would mirror history are also deleted without
clearing permissions that keep cookies and site data alive.
Depends on D96639
Assignee | ||
Comment 4•4 years ago
|
||
This uses the new deleteStorageAccessForClearingHistory API in Sanitizer to avoid
clearing all storage access API permissions and thus all cookies and site data when
clearing only history.
Depends on D96640
Assignee | ||
Comment 5•4 years ago
|
||
This uses the new deleteStorageAccessForClearingHistory API in GeckoView to avoid
clearing all storage access API permissions and thus causing purging to clear all cookies
and site data when clearing only history.
Depends on D96641
Assignee | ||
Comment 6•4 years ago
|
||
I'll land this next week after the merge to respect soft freeze, but we're interested in uplifting this to early beta.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 8•4 years ago
|
||
Backed out for linting failure.
backout: https://hg.mozilla.org/integration/autoland/rev/b0db5e0bfda1fb944b8dd98f75f9b99b6bd12512
failure log: https://treeherder.mozilla.org/logviewer?job_id=321958629&repo=autoland&lineNumber=302
Comment 10•4 years ago
|
||
Comment 11•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1e1d21415f6b
https://hg.mozilla.org/mozilla-central/rev/7e58927653ca
https://hg.mozilla.org/mozilla-central/rev/667e91c1e320
https://hg.mozilla.org/mozilla-central/rev/316ac9494453
https://hg.mozilla.org/mozilla-central/rev/44c945afc31b
Updated•4 years ago
|
Assignee | ||
Comment 12•4 years ago
|
||
Comment on attachment 9187026 [details]
Bug 1675018 - Part 1 - Move PrincipalsCollector into its own module. r=mak
Beta/Release Uplift Approval Request
- User impact if declined: Users are logged out of popular sites daily when using clear history on shutdown. We already have a hacky workaround that prevents purging from running for these users, but that's also not great. This is a better solution (I hope).
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce:
- Log into Google, Twitter and/or Facebook
- in about:preferences, enable "Clear History when Nightly closes" and in the "Settings.." sub-menu only enable only Browsing & Download History
- Restart your browser, which will clear your history
- Go to about:preferences and turn off "Clear History when Nightly closes" again (this is needed to work around our short-term fix for v83)
- In the browser toolbox, run
await Components.classes["@mozilla.org/purge-tracker-service;1"].getService(Components.interfaces.nsIPurgeTrackerService).purgeTrackingCookieJars()
- Visit Google, Twitter and/or Facebook again. The bug is that you're logged out now. You should have stayed logged in because you only wanted to clear history, not cookies.
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): This is a relatively complex patch that changes how we clear history, but it's well tested and we've waited with uplifting until the beginning of the beta cycle on purpose.
- String changes made/needed: None
Assignee | ||
Updated•4 years ago
|
Comment 13•4 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #12)
- User impact if declined: Users are logged out of popular sites daily when using clear history on shutdown. We already have a hacky workaround that prevents purging from running for these users, but that's also not great. This is a better solution (I hope).
Given the workaround (in bug 1672394) can't we just let this one ride the trains?
Assignee | ||
Comment 14•4 years ago
|
||
Sorry for the PTO-induced delay, I would still prefer to get this into Beta since it also solves the case for people who are clearing history manually, which bug 1672394 doesn't cover.
Comment 15•4 years ago
|
||
Comment on attachment 9187026 [details]
Bug 1675018 - Part 1 - Move PrincipalsCollector into its own module. r=mak
Given that this hasn't gotten verified by QA yet, is pretty complex, and we're down to our final 2 betas of the cycle, I think it's best to let this ride 85 at this point.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 16•4 years ago
|
||
Hello! Reproduced the issue with Firefox 84.0a1 (20201103213933) on Windows 10x64 and steps from comment 12.
The issue is verified fixed with Firefox 85.0b3 (20201217185930) and 86.0a1 (20201218095607) on Windows 10x64, macOS 10.12 and Ubuntu 18.04. After following the steps from comment 12 and revisiting Facebook, Twitter, and Google I was still logged in.
Description
•