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] Rename rp --> rpId in CollectedClientAdditionalPaymentData #198

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

stephenmcgruer
Copy link
Collaborator

@stephenmcgruer stephenmcgruer commented Aug 10, 2022

To align with WebAuthn, we should use the term rpId here. This is a breaking
change, but implementations can mitigate the breakage by continuing to include
the old 'rp' name going forwards.

See #191

Test changes: web-platform-tests/wpt#35602
Implementation bugs:


Preview | Diff

To align with WebAuthn, we should use the term rpId here. This is a breaking
change, but implementations can mitigate the breakage by continuing to include
the old 'rp' name going forwards.

See #191
@stephenmcgruer
Copy link
Collaborator Author

This probably warrants discussion in the WPWG, or at least some communication to anyone who may be validating an SPC credential today. On the Chrome side, we would likely have to keep the rp version of the entry around for now, and eventually do a deprecation process to switch over to rpId.

@stephenmcgruer
Copy link
Collaborator Author

Worth noting that it will be actually impossible for us to know if anyone is still relying on rp being included in the assertion (at whatever future point in time we did a deprecation), as it will usually be verified server-side.

Copy link
Collaborator

@rsolomakhin rsolomakhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself LGTM and I agree that it should be discussed in WPWG first.

@ianbjacobs
Copy link
Collaborator

I will add to the 18 August WPWG agenda.

@ianbjacobs
Copy link
Collaborator

cc @stare893

@ianbjacobs
Copy link
Collaborator

See resolution at the 18 August meeting to make this change:
https://www.w3.org/2022/08/18-wpwg-minutes.html#t01

@stare893
Copy link

@stephenmcgruer There is some confusion on the proposed change. From EMV 3DSWG consideration, we already see rpId in use per the SPC API webpage and we have included the same in all 3DS spec references to the SPC API use. Can you please elaborate/point me in the right direction to understand the change.

@ianbjacobs you might get a similar query through the 3dswg official email

@ianbjacobs
Copy link
Collaborator

@stare893 You are correct that SPC input refers to the Web Authentication rpid; see the request dictionary:
https://w3c.github.io/secure-payment-confirmation/#sctn-securepaymentconfirmationrequest-dictionary

This proposal is to change the field name in the output; see the additional payment dictionary:
https://w3c.github.io/secure-payment-confirmation/#sctn-collectedclientadditionalpaymentdata-dictionary

The proposal is that assertions will include the field name "rpid" to align with the Web Authentication name.

The Chrome implementation would support both "rp" and "rpid" field names for some period of time, eventually deprecating "rp".

It was not clear to us on the call today whether the 3DS specification itself would need to change (e.g., because it refers to the "rp" field in the resulting assertion), or if instead it would just be implementations (e.g., ACS) that would need to adapt code to look for "rpid" rather than "rp" in the assertion.

@ianbjacobs
Copy link
Collaborator

@stephenmcgruer, EMVCo folks have confirmed that this change does not impact their 3DS integration (the spec itself). Given the WG's support, let's merge this.

@stephenmcgruer stephenmcgruer merged commit c4962df into main Aug 25, 2022
@stephenmcgruer stephenmcgruer deleted the smcgruer/rpId branch August 25, 2022 14:51
github-actions bot added a commit that referenced this pull request Aug 25, 2022
SHA: c4962df
Reason: push, by @stephenmcgruer

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 25, 2022
The spec has renamed rp to rpId in the
CollectedClientAdditionalPaymentData, to align with how WebAuthn refers
to the ID specifically.
(w3c/secure-payment-confirmation#198)

Simply doing a rename in Chromium would break existing users, so instead
we are introducing the new name first (rpId), and then will have a
switchover period for developers to adapt, and then finally will remove
the old rp parameter.

Bug: 1356224
Change-Id: Ifd51f131402b133f50c4d54bb9da20691a44ce33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3854551
Reviewed-by: Ken Buchanan <[email protected]>
Commit-Queue: Stephen McGruer <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1039319}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 5, 2022
…entAdditionalPaymentData, a=testonly

Automatic update from web-platform-tests
[SPC] Rename rp --> rpId in CollectedClientAdditionalPaymentData (#35602)

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

wpt-commits: 642097fa4e9495f1240c098ce1cefc661ab5c444
wpt-pr: 35602
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.

4 participants