privacycg / storage-access

The Storage Access API
https://privacycg.github.io/storage-access/
212 stars 27 forks source link

Align requestStorageAccess rejection w.r.t lack of user gesture #36

Closed Brandr0id closed 3 years ago

Brandr0id commented 4 years ago

Currently, at least in Firefox, if document.requestStorageAccess is called without a user gesture and storage is not otherwise blocked the API appears to reject. This differs slightly from the wording in the spec draft and I wanted to make sure we pick/align current implementations and the spec to one behaviour here.

Currently the draft of the spec will let the promise resolve/reject immediately if either explicit allow/deny flags/state are set for the document. I also noticed the current draft text doesn't look to take into account the current state of storage access (e.g. let result be running determine if a site has storage access with key and doc and if result istrueresolve p).

I can see both POV on this where if we have a prior signal that access is granted (either access is allowed or the allow/deny flags are set) then we can immediately resolve/reject. I can also see how if we don't have a user gesture, why would we potentially resolve or reject since the API shouldn't be usable without a gesture.

Thoughts on if it matters for the ordering here and depending on the result can we either update the spec language or open bugs on implementers for the behaviour change?

A side note, at least with the current impl logic of rejecting on gesture first we can get some test coverage in WPT to validate the API rejects while https://github.com/w3c/webdriver/issues/1437 is active.

hober commented 4 years ago

cc @privacycg/storage

johannhof commented 4 years ago

Speaking for Firefox we agree that it would be good to align browsers on the same behavior. I'm also not 100% sure about what the right behavior would be, though I'm leaning towards always requiring a user gesture.

In my opinion, it's weird for developers to use an API that will sometimes require interaction and sometimes not, based on criteria that are opaque to them at runtime. When the call to rSA succeeds without user gesture you shouldn't have called it. So maybe we should make that explicit and avoid confusion by always requiring user gesture.

With that said, we've historically added such APIs (that sometimes require interaction and sometimes not) before, for example recently when requiring user interaction for Notification.requestPermission. We did that to avoid breaking too many existing sites mostly, which doesn't seem like a concern here.

If we don't require a user gesture, we would effectively turn rSA into an alternative to hasStorageAccess, when you handle the error from lack of interaction it can be used the same way. There's a small risk for compat issues when browsers don't align on that (as is the case with Safari and Firefox right now), so it does seem important to have this discussion.

johannhof commented 3 years ago

Looking into this again we (@englehardt and I) noticed that the behavior described in https://github.com/privacycg/storage-access/issues/36#issue-619338375 seems outdated, here's what we got in our testing:

Firefox: User interaction is only required when storage access has not been granted (consecutive rSA calls on the same page, after reload, and in frames that don’t have cookies blocked) Safari: Like Firefox Safari Tech preview: Will reject without user interaction, independent of prior storage access state. Edge: Didn't seem to require user interaction at all, is this a bug or a mistake on our end @Brandr0id ?

FWIW we still think that https://github.com/privacycg/storage-access/issues/36#issuecomment-642743857 applies and that Firefox should probably switch to always requiring user interaction, assuming that Webkit folks are looking to ship these changes that we're seeing in Safari Tech Preview. Then (assuming there's no objection from Edge) we can clarify this in the specification.

johannhof commented 3 years ago

An update that I didn't test Edge correctly and that their behavior actually seems to be that the browser will reject without user interaction, independent of prior storage access state. Also, Safari (now?) seems to behave like Safari TP, which makes Firefox the only browser without the "strict" user gesture enforcement that we actually advocate for.

Seems to me like we can resolve this issue by aligning on rejecting immediately when no user gesture is present (Firefox will update our implementation). Does that sound good to you, @Brandr0id? If yes I can make a pull request to the spec.

Brandr0id commented 3 years ago

Thanks @johannhof SGTM.

jackfrankland commented 3 years ago

For future behaviour, the result of https://github.com/privacycg/storage-access/issues/60 could be relevant I think. It could perhaps be confusing for the promise to sometimes reject, or never reject under different circumstances.

In my opinion, it's weird for developers to use an API that will sometimes require interaction and sometimes not, based on criteria that are opaque to them at runtime. When the call to rSA succeeds without user gesture you shouldn't have called it. So maybe we should make that explicit and avoid confusion by always requiring user gesture.

I'm not sure if it exactly relates to the argument, but sticky activation-gated APIs already exist. Also, a user gesture being active could under certain circumstances be opaque to the developer too.

Personally, while I agree that the recommendation should be to first check if storage access has been given, I think it's potentially more confusing that the API can seemingly reject subsequent requests for storage access after it has already been granted within the page's lifecycle.

johannhof commented 3 years ago

Yeah, I agree that "confusing" is a very subjective thing here, depending on the individual expectations of the developer.

In my experience (working on permissions) #60 is a very hard problem to solve, especially trying to balance usability of the API with user protection. I was out when that discussion happened, thanks for putting it on my radar :)

I filed the bug for aligning Firefox here.