privacycg / storage-access

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

document.requestStorageAccess should consider user explicit settings for unpartitioned data access #186

Closed shuranhuang closed 4 months ago

shuranhuang commented 9 months ago

This change tries to make rSA behavior aligns with hSA and user expectations by including a check of whether the user agent allows the document to access unpartitioned data based on user settings, and move early resolve cases, such as top-level case and same-site case, behind the user settings check. Specifically:

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

johannhof commented 8 months ago

@annevk not sure I understand, the permission state for SAA can be separate from the 3PC access settings that the user has applied, no? Are you saying that they should not be?

@shuranhuang might remember more context on why we're splitting out user settings and permission checks.

annevk commented 8 months ago

I think I don't understand why they need to be different.

Looking at the two use cases that started this in https://github.com/privacycg/storage-access/issues/171#issue-1671816336 the top one requires a change as hasStorageAccess() needs to be able to return false in a top-level context. Similar to cookieEnabled. The second use case however seems equivalent to granting storage access for a particular key.

Maybe @cfredric can weigh in if the second use case has some kind of subtlety that could not be explained by the permission store.

shuranhuang commented 8 months ago

Hi @annevk , I agree with you that this PR is not what #171 (comment) directly translated to regarding "will setting a non-partitioned cookie work". But this PR is to make sure the behavior is aligned between the two methods.

From the two examples in https://github.com/privacycg/storage-access/issues/171#issue-1671816336, in the first use case, if user blocks all cookies (or blocks cookies on specific sites), document.hasStorageAccess will return false (with the change in https://github.com/privacycg/storage-access/pull/174), then document.requestStorageAccess might be called for getting access, which shows user a prompt that could be misleading, i.e. granting the access does not really unblock cookie access in this context. Checking user settings in requestStorageAccess can help avoid this case.

Does that make sense to you?

cfredric commented 8 months ago

Maybe @cfredric can weigh in if the second use case has some kind of subtlety that could not be explained by the permission store.

I haven't read through this PR, but IIUC, you're asking "why wouldn't we just grant the storage-access permission if requestStorageAccess is called and user settings already allow cookie access"?

If so, the reason I didn't want to do that is to preserve the causality/provenance for why cookies are allowed.

E.g.: suppose the user changes their settings to allow 3P cookie access for some site embed.com, then visits a top-level site that embeds embed.com. embed.com then calls requestStorageAccess, gets a new permission grant that lasts 30 days (e.g.), and accesses their cookies.

Now suppose the user changes their mind and decides they don't want to allow cookie access for embed.com anymore. The user therefore modifies their cookie settings again to remove embed.com from the allowlist. But upon future visits, they find that embed.com can still access 3P cookies since the storage-access permission grant hasn't expired, even though the user settings (not permissions) no longer explicitly allow that access.

In my opinion, that's unfortunate because it means the user agent isn't doing what the user intended. That mismatch can be avoided if requestStorageAccess only creates a new permission grant when it needs to, and avoids creating one when a new permission grant isn't necessary to access cookies.

(In theory we could also avoid the mismatch if we decided that user agents weren't allowed to support cookie settings outside of the storage-access permission, but IMO that is not in the purview of this spec. Better to make this spec account for having multiple sources of cookie access info, instead of mandating that there's only one source of info.)

annevk commented 8 months ago

I think the latter thing is what I'm expecting. In particular I don't think there's a need to standardize such UI features as they can be entirely up to the user agent, including ensuring that if the user disables a certain website, it isn't enabled access via some other means.

But the one thing that doesn't get us is the top-level change, which is essentially about aligning with cookieEnabled.

@shuranhuang I do think that if cookieEnabled is false, we should return early from requestStorageAccess() as well. Not just hasStorageAccess().

johannhof commented 8 months ago

@annevk your last sentence sounds like you're okay with what the PR does, then? Or are you saying that we should literally check against https://html.spec.whatwg.org/#dom-navigator-cookieenabled in rSA / hSA?

Personally I don't really like cookieEnabled (it's also not consistent across browsers because e.g. Safari shims it, potentially to avoid breakage) and might prefer to deprecate it.

johannhof commented 5 months ago

@annevk friendly ping on the above, otherwise I'd be happy to (take another look and) merge this and fix potential follow-ups separately, in the interest of moving forward.