Add showOptOut option to spec #215

nickburris · 2022-11-14T22:44:29Z

This PR adds spec text for a new showOptOut option in the SecurePaymentConfirmationRequest API. See the original proposal and discussion in #172. This spec change reflects the prototype currently implemented behind a flag in Chrome, which is available via origin trial and used in this test page.

The OptOutError mentioned in this change is being added to WebIDL in whatwg/webidl#1231.

Opt-out web platform test: web-platform-tests/wpt#37012

Preview | Diff

stephenmcgruer

Thanks Nick! Some immediate comments about how we phrase this (sorry, specifications around UX are tricky :D)

spec.bs

stephenmcgruer · 2022-11-15T21:43:10Z

(cc @jcemer as an fyi)

stephenmcgruer

Thanks Nick! I think this LGTM modulo a few nits.

Can we link to the WPT test PR (once available) in the PR description here?

spec.bs

stephenmcgruer · 2022-11-17T18:25:00Z

+@ianbjacobs to review as well. Ian - I expect we will want to talk about this one in the WPWG?

ianbjacobs

Thank you for the pull request. I have a clarification request (must v. should) and suggest a privacy consideration addition. Thanks!

ianbjacobs · 2022-11-17T19:26:07Z

@stephenmcgruer, I will put this on the 8 December agenda.

ianbjacobs · 2022-11-17T19:39:08Z

Some day I will learn to use GitHub. In the meantime, here were the comments I thought I left:

The proposal refers to this new feature using both "should" (twice) and "must." I recommend keeping the one "must" requirement and rewriting the other ones to be declarative, such as "provides the developer a way to ask the user agent to provide the user with an opportunity to indicate they wish to opt out of the relying party's storage of information."
I think the privacy considerations section should speak more directly to the fact that the opt out error lets the caller know the user has credentials, even though the user did not complete an authentication. I recommend including a statement about mechanisms that mitigate this (e.g., the user has taken explicit action via dedicated browser-owned UX).
Also, there is a "that that" in the proposal; drop one that. :)

nickburris · 2022-11-17T21:58:50Z

Thanks! See updates to the privacy considerations section.

I think the privacy considerations section should speak more directly to the fact that the opt out error lets the caller know the user has credentials, even though the user did not complete an authentication. I recommend including a statement about mechanisms that mitigate this (e.g., the user has taken explicit action via dedicated browser-owned UX).

In our implementation we actually do not leak this information, since we also show the opt out option on the no-credential-found UX which already exists to mitigate credential probing attacks. I've added text to the privacy considerations section on this, good catch!

spec.bs

ianbjacobs

See one more editorial suggestion. Thanks!

Adds chromedriver support and web platform test for opt-out for SPC. The WPT passes locally with SPC opt out enabled (currently disabled behind a flag). There's a small change in the SPC MVC logic, since opt-out can now happen by calling SPCController::OnOptOut directly, which resulted in the dialog view calling back to OnCancel when the dialog closes since it didn't know opt-out was programmatically invoked. This is fixed by having the controller set a bit on the model which the view checks. Spec PR: w3c/secure-payment-confirmation#215 Bug: 1385865 Change-Id: I777e92b5110785415de68ef9f0497905598e4897

nickburris · 2022-11-17T22:17:00Z

Thanks again! As mentioned we'll leave this open until discussion at WPWG 8 December meeting.

Adds chromedriver support and web platform test for opt-out for SPC. The WPT passes locally with SPC opt out enabled (currently disabled behind a flag). There's a small change in the SPC MVC logic, since opt-out can now happen by calling SPCController::OnOptOut directly, which resulted in the dialog view calling back to OnCancel when the dialog closes since it didn't know opt-out was programmatically invoked. This is fixed by having the controller set a bit on the model which the view checks. Spec PR: w3c/secure-payment-confirmation#215 Bug: 1385865 Change-Id: I777e92b5110785415de68ef9f0497905598e4897 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4030688 Commit-Queue: Nick Burris <[email protected]> Reviewed-by: Stephen McGruer <[email protected]> Reviewed-by: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1073462}

Adds chromedriver support and web platform test for opt-out for SPC. The WPT passes locally with SPC opt out enabled (currently disabled behind a flag). There's a small change in the SPC MVC logic, since opt-out can now happen by calling SPCController::OnOptOut directly, which resulted in the dialog view calling back to OnCancel when the dialog closes since it didn't know opt-out was programmatically invoked. This is fixed by having the controller set a bit on the model which the view checks. Spec PR: w3c/secure-payment-confirmation#215 Bug: 1385865 Change-Id: I777e92b5110785415de68ef9f0497905598e4897 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4030688 Commit-Queue: Nick Burris <[email protected]> Reviewed-by: Stephen McGruer <[email protected]> Reviewed-by: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1073462}

nickburris · 2022-11-18T19:48:52Z

FYI I made a small change to update the naming convention in the TransactionAutomationMode enum to camelCase, per a recommendation in this comment which I'll follow up with an implementation change.

…t out WPT, a=testonly Automatic update from web-platform-tests [SPC] Add Secure Payment Confirmation opt out WPT Adds chromedriver support and web platform test for opt-out for SPC. The WPT passes locally with SPC opt out enabled (currently disabled behind a flag). There's a small change in the SPC MVC logic, since opt-out can now happen by calling SPCController::OnOptOut directly, which resulted in the dialog view calling back to OnCancel when the dialog closes since it didn't know opt-out was programmatically invoked. This is fixed by having the controller set a bit on the model which the view checks. Spec PR: w3c/secure-payment-confirmation#215 Bug: 1385865 Change-Id: I777e92b5110785415de68ef9f0497905598e4897 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4030688 Commit-Queue: Nick Burris <[email protected]> Reviewed-by: Stephen McGruer <[email protected]> Reviewed-by: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1073462} -- wpt-commits: 29d4f9513ad84c09f54199ffc157a0f9ce7d1e90 wpt-pr: 37012

stephenmcgruer · 2022-11-23T21:44:30Z

@nickburris - fyi that this PR should also add a reference to the newly added WPT test (see c096e3b where I landed it as wpt hidden for now; we should un-hide it and move it wherever in the spec seems most relevant!)

stephenmcgruer · 2022-12-08T20:41:41Z

This was discussed today at the WPWG (minutes), and a resolution was taken to land it (once updated for the WPT test comment). We also heard interest in exploring clearer cancellation states for SPC, which we intend to discuss internally - but consider separate from this PR.

nickburris · 2022-12-08T20:59:30Z

Sorry about the force push mess, one day I will get rebasing a fork right...

@nickburris - fyi that this PR should also add a reference to the newly added WPT test (see c096e3b where I landed it as wpt hidden for now; we should un-hide it and move it wherever in the spec seems most relevant!)

Done

stephenmcgruer · 2022-12-09T13:52:53Z

Sorry about the force push mess, one day I will get rebasing a fork right...

No problem! I also just rebased and forced-pushed it again, to pick up a fix I landed in main: 06a08c0

stephenmcgruer · 2022-12-09T13:54:02Z

@ianbjacobs - I think we're good to merge here, but will wait on you to confirm before I squash-merge it :)

SHA: c24f564 Reason: push, by stephenmcgruer Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

nickburris marked this pull request as ready for review November 15, 2022 21:25

nickburris requested a review from stephenmcgruer November 15, 2022 21:25

stephenmcgruer requested changes Nov 15, 2022

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

stephenmcgruer self-requested a review November 16, 2022 20:43

stephenmcgruer approved these changes Nov 17, 2022

View reviewed changes

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

stephenmcgruer requested a review from ianbjacobs November 17, 2022 18:24

ianbjacobs requested changes Nov 17, 2022

View reviewed changes

nickburris force-pushed the optout branch from 8335c0e to 4259ab5 Compare November 17, 2022 21:55

ianbjacobs reviewed Nov 17, 2022

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

ianbjacobs approved these changes Nov 17, 2022

View reviewed changes

chromium-wpt-export-bot mentioned this pull request Nov 17, 2022

[SPC] Add Secure Payment Confirmation opt out WPT web-platform-tests/wpt#37012

Merged

nickburris force-pushed the optout branch from 9a32899 to 7cd811f Compare December 8, 2022 20:55

Nick Burris added 2 commits December 9, 2022 08:48

Add showOptOut option to spec

5d7f9b3

Generalize references to opt-out UX

30359ac

Nick Burris added 5 commits December 9, 2022 08:48

Specify opt out party

db2ef42

Add to privacy considerations

3c158a5

Address editorial comment

06eacaa

Update TransactionAutomationMode naming convention

f4577f3

Move and show authentication-optout wpt results

78e2553

stephenmcgruer force-pushed the optout branch from 161c849 to 78e2553 Compare December 9, 2022 13:49

stephenmcgruer merged commit c24f564 into w3c:main Dec 9, 2022

github-actions bot added a commit that referenced this pull request Dec 9, 2022

Add showOptOut option to spec (#215)

fdc40e7

SHA: c24f564 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 Dec 9, 2022

Add showOptOut option to spec (#215)

eff8656

SHA: c24f564 Reason: push, by stephenmcgruer Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

dontcallmedom mentioned this pull request Dec 15, 2022

Use lowercase values in enum #219

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add showOptOut option to spec #215

Add showOptOut option to spec #215

nickburris commented Nov 14, 2022 •

edited by pr-preview bot

Loading

stephenmcgruer left a comment

stephenmcgruer commented Nov 15, 2022

stephenmcgruer left a comment

stephenmcgruer commented Nov 17, 2022

ianbjacobs left a comment

ianbjacobs commented Nov 17, 2022

ianbjacobs commented Nov 17, 2022 •

edited

Loading

nickburris commented Nov 17, 2022

ianbjacobs left a comment

nickburris commented Nov 17, 2022

nickburris commented Nov 18, 2022

stephenmcgruer commented Nov 23, 2022

stephenmcgruer commented Dec 8, 2022

nickburris commented Dec 8, 2022

stephenmcgruer commented Dec 9, 2022

stephenmcgruer commented Dec 9, 2022

Add showOptOut option to spec #215

Add showOptOut option to spec #215

Conversation

nickburris commented Nov 14, 2022 • edited by pr-preview bot Loading

stephenmcgruer left a comment

Choose a reason for hiding this comment

stephenmcgruer commented Nov 15, 2022

stephenmcgruer left a comment

Choose a reason for hiding this comment

stephenmcgruer commented Nov 17, 2022

ianbjacobs left a comment

Choose a reason for hiding this comment

ianbjacobs commented Nov 17, 2022

ianbjacobs commented Nov 17, 2022 • edited Loading

nickburris commented Nov 17, 2022

ianbjacobs left a comment

Choose a reason for hiding this comment

nickburris commented Nov 17, 2022

nickburris commented Nov 18, 2022

stephenmcgruer commented Nov 23, 2022

stephenmcgruer commented Dec 8, 2022

nickburris commented Dec 8, 2022

stephenmcgruer commented Dec 9, 2022

stephenmcgruer commented Dec 9, 2022

nickburris commented Nov 14, 2022 •

edited by pr-preview bot

Loading

ianbjacobs commented Nov 17, 2022 •

edited

Loading