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

[Spec] Only allow SPC authentication if in a foreground tab #238

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

stephenmcgruer
Copy link
Collaborator

@stephenmcgruer stephenmcgruer commented Apr 11, 2023

During PING review of the pre-CR changes to SPC, the PING raised a concern that removing the user activation requirement (see #236) could lead to sites triggering SPC from a background tab. This PR adds logic to the steps to check if a payment can be made to disallow background tabs (and minimized-windows/etc).

It is likely that eventually we will want this specified in Payment Request instead, both because it will be clearer spec text (here we have to refer to a this that is actually from the Payment Request spec), and also because we (in Chrome) already do (afaik) reject Payment Requests from background tabs. (Which is allowable by abusing the Payment Request spec text that says a user agent may reject show() for any security reason).

  • WPT tests - not yet landed
  • Chromium implementation - already done, Chromium doesn't allow Payment Request in a background tab already today
  • Other browsers - N/A, no other browser currently implementing SPC.

Fixes #237


Preview | Diff

@stephenmcgruer stephenmcgruer force-pushed the smcgruer/require-foreground branch from ce4a6e0 to 04a1cf3 Compare April 11, 2023 15:55
@stephenmcgruer stephenmcgruer changed the title [WIP] Add normative text to only allow show() if in foreground tab [Spec] Only allow SPC authentication if in a foreground tab Apr 11, 2023
@stephenmcgruer stephenmcgruer marked this pull request as ready for review April 11, 2023 16:05
@stephenmcgruer
Copy link
Collaborator Author

cc @jyasskin - do you know if I'm holding traversable navigable (and system visibility state) properly here? 🤣

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@ianbjacobs
Copy link
Collaborator

@stephenmcgruer, thanks for creating this. I agree with the direction and will support updated text based on the @jyasskin comments.

spec.bs Outdated Show resolved Hide resolved
Co-authored-by: Jeffrey Yasskin <[email protected]>
@stephenmcgruer
Copy link
Collaborator Author

So I suspect we can fairly easily move this to Payment Request, but in the interest of unblocking the PR for user activationless SPC I'm going to merge this as-is for now. If/when we land the equivalent in Payment Request we can drop this text.

@stephenmcgruer stephenmcgruer merged commit 4a9d883 into main Apr 13, 2023
github-actions bot added a commit that referenced this pull request Apr 13, 2023
SHA: 4a9d883
Reason: push, by stephenmcgruer

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Apr 13, 2023
SHA: 4a9d883
Reason: push, by stephenmcgruer

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@stephenmcgruer stephenmcgruer deleted the smcgruer/require-foreground branch April 17, 2023 14:35
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 18, 2023
… a hidden document, a=testonly

Automatic update from web-platform-tests
[SPC] Add test for SPC authentication in a hidden document (#39507)

See w3c/secure-payment-confirmation#238
--

wpt-commits: ef359117ec74b43acb8472534a43f71e23b4abca
wpt-pr: 39507
stephenmcgruer added a commit that referenced this pull request May 30, 2023
…238)"

This reverts commits 4a9d883 and
fd37ebe

This behavior is now spec'd in Payment Request itself as of
w3c/payment-request@cce8f5e,
and so does not need to additionally be spec'd in SPC.
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.

[PING] Only allow triggering authentication from a foreground tab
4 participants