Try to fix #3581, make Screenshots work with third party cookies disabled #3601
Conversation
|
@mixedpuppy pointed me to Bug 1295660, which has a patch to expose the XMLHttpRequest, in the way that this code gets that object but without confirming it's the browser's copy of XMLHttpRequest. |
ianb
force-pushed
the
third-party-cookie
branch
from
ca01150 to
43765b2
Compare
ianb
changed the title
ianb
force-pushed
the
third-party-cookie
branch
from
43765b2 to
4b57cf9
Compare
There was a problem hiding this comment.
I wasn't able to trigger the set-login-cookie route after disabling third party cookies.
But modulo a couple nits it looks good to me. 👍
@mozfreddyb do you want to take a look?
Ideally, I think we'd get screenshots to only use cross origin requests for creating shots and maybe login. The approach here: #3581 (comment) looks good but it does sound painful. Instead I think we should keep an eye on https://bugzilla.mozilla.org/show_bug.cgi?id=1295660 since it should let us treat xhrs from content scripts as first party.
| @@ -26,7 +26,8 @@ function isCsrfExemptPath(path) { | |||
| return isAuthPath(path) | |||
| || path.startsWith("/data") | |||
| || path === "/event" | |||
| || path === "/error"; | |||
| || path === "/error" | |||
| || path === "/api/set-login-cookie"; | |||
There was a problem hiding this comment.
If the POST /api/set-login-cookie sends an origin header, we should validate its protocol/scheme is moz-extension:// in the csrf middleware.
There was a problem hiding this comment.
It appears not to send any Origin header at all. I'm a bit surprised, I would have expected it to be the site origin. Or maybe it would only be set for CORS requests?
| accountId: req.accountId, | ||
| userAbTests: req.abTests | ||
| }); | ||
| }); |
There was a problem hiding this comment.
nit: unit tests for this route or an issue to add them
This adds a second attempt to login to wantsauth logins, one that runs in sitehelper.js, and tries to get the cookie set on a request that appears to come from the content page itself. Note this does not firmly protect from the content page overwriting window.XMLHttpRequest and having the add-on use that object.
ianb
force-pushed
the
third-party-cookie
branch
from
4b57cf9 to
ac75a0b
Compare
|
I created a followup to make use of Bug 1295660 in #3626 |
This adds a second attempt to login to wantsauth logins, one that runs in sitehelper.js, and tries to get the cookie set on a request that appears to come from the content page itself.
Note: this now works, but doesn't provide full protection from a screenshots.firefox.com page (that is under attack) from getting the authentication header.