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

Request storage access in state partitioned contexts #85

Merged
merged 2 commits into from
Jul 21, 2021

Conversation

matheus23
Copy link
Member

TODO: Test this.

A user on discord told us he got this error: https://developer.mozilla.org/en-US/docs/Web/Privacy/Storage_Access_Policy/Errors/CookiePartitionedForeign

To reproduce this issue, we can use certain versions of firefox:

State Partitioning is currently turned on by default in the Firefox Nightly channel. A subset of the state partitioning efforts (namely network partitioning) has been enabled by default in the release channel of Firefox since version 85.

In the same document, there's also a debugging section with helpful info: https://developer.mozilla.org/en-US/docs/Web/Privacy/State_Partitioning#debugging

@matheus23
Copy link
Member Author

Really hard for me to test this. According to the debugging section, I disabled all redirect heuristics and enabled state partitioning by setting

  • privacy.restrict3rdpartystorage.heuristic.recently_visited to false
  • privacy.restrict3rdpartystorage.heuristic.redirect to false
  • network.cookie.cookieBehavior to 5

But I'm still able to log into the auth lobby just fine...

The good news is: I also tested the auth lobby with this PR and it doesN't seem to break anything. 😅

I'll try another test with brave next

@matheus23
Copy link
Member Author

Also working in brave.

I've added some logging to see whether state partitioning stuff is triggered, but as it seems, it isn't (in brave). Not sure what's going on.

In firefox, it detects state partitioning support, but it already has access (despite the settings I mentioned in my last post).

If anyone has some ideas how to test this, feel free to tell them.

Otherwise, I'm fairly sure the code won't break anything and can be merged.

@matheus23 matheus23 marked this pull request as ready for review July 20, 2021 16:02
@matheus23 matheus23 requested a review from icidasset July 20, 2021 16:02
@icidasset
Copy link
Contributor

It doesn't look like this is implemented yet in Brave is it? Regarding Firefox, maybe https://developer.mozilla.org/en-US/docs/Web/API/StorageManager/persist has something to do with it? Which we call on the lobby side when authorising apps. Or maybe it just grants access automatically because you did so in the past? (even if you revoked access later on)

@matheus23
Copy link
Member Author

matheus23 commented Jul 21, 2021

It doesn't look like this is implemented yet in Brave is it?

Yeah right, requestStorageAccess isn't there in brave. I can't find anything about brave and state partitioning online, and it runs fine for me, even with the shield enabled. We've had bug reports from other people who were using brave with the shield up, though. Not sure what's up with that.

Regarding Firefox, maybe https://developer.mozilla.org/en-US/docs/Web/API/StorageManager/persist has something to do with it?

Maybe? 🤷‍♂️

Or maybe it just grants access automatically because you did so in the past?

Also possible :/ I've tried turn off everything as described in https://developer.mozilla.org/en-US/docs/Web/Privacy/State_Partitioning#debugging


Should we merge this?

If we don't, then here's a proposal: Let's publish this to another subdomain on production. And if someone comes to us with a state-partitioning issue (and we can't reproduce it), we can send them over to the 'alternative' auth lobby that contains this PR and they check whether it works?
Although we'd also need to publish an app that is configured to the right lobby for that to work.

Copy link
Contributor

@icidasset icidasset left a comment

Choose a reason for hiding this comment

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

Merge it 👍

@matheus23 matheus23 merged commit 5de51b7 into main Jul 21, 2021
@matheus23 matheus23 deleted the matheus23/state-partitioning branch July 21, 2021 19:33
@matheus23
Copy link
Member Author

I've kept the state partitioning settings in my firefox. Now I'm getting this (only on staging!):
image

Can publish the current version to staging and see whether that is doing something?

@icidasset
Copy link
Contributor

@matheus23 Deployed to staging 🚢

@matheus23
Copy link
Member Author

Ok so this PR didn't help :(

image

Notice this message:

document.requestStorageAccess() may only be requested from inside a short running user-generated event handler.

But that'd mean a different UI flow completely: We'd have to have a button or something included in the iframe. :/ (and the iframe would have to be visible in the app & the user needs to interact with it!)

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

Successfully merging this pull request may close these issues.

2 participants