Add missing same-site check in document.hasStorageAccess

[cfredric]

This adds the missing check for an A(B(A)) embedding case in document.hasStorageAccess(), to match the corresponding check in document.requestStorageAccess() (see step 16.7). The effect is that for ABA embeds, after document.requestStorageAccess() is called and resolves, invocations of document.hasStorageAccess() resolve to true.

Fixes #234.

• At least two implementers are interested (and none opposed):
  • Chromium
  • WebKit (Clarify intended semantics of document.hasStorageAccess #171 (comment))
  • Gecko (Clarify intended semantics of document.hasStorageAccess #171 (comment))
• Tests are written and can be reviewed and commented upon at:
  • https://crrev.com/c/7795672
  • Note: the case where document.hasStorageAccess() should return false in the ABA embed (i.e. when document.requestStorageAccess() has not been called) is already tested at https://github.com/web-platform-tests/wpt/blob/af3b02cb13c11ad6299bdda16813bd8e5048aa60/storage-access-api/hasStorageAccess.sub.https.window.js#L26.
• Implementation bugs are filed:
  • Chromium: N/A
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=2035026
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=313328

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

---

Preview | Diff

[annevk]

I'm not sure I understand how this enforces that requestStorageAccess() has previously been invoked. And if this is the right place it seems we can get rid of the previous step as this is a less narrow check.

[cfredric]

requestStorageAccess() is the main place that sets the environment's has storage access bool, so the new step returns true iff requestStorageAccess() has been called and resolved previously.

Step 10.3.5 (the "same authority" check) handles A(A) embeds, where there is no cross-site ancestor (and therefore third-party cookie blocking isn't relevant). So that check is different and should be kept separately.

[annevk]

I think I understand now. This seems okay, but one thing we should fix is that we should add "abort these steps" when we resolve while in parallel because it makes no sense to run the subsequent steps as well.

[cfredric]

Good point, done.