Change from Registry to Protocols#401
Conversation
|
Thank you Tim, this is looking like a great start to me. I've filed a Chromium issue to track our impl work and referenced it in your PR description. |
|
PR made to Specref to add these references: tobie/specref#898 |
marcoscaceres
left a comment
There was a problem hiding this comment.
Like where this is going ... some initial feedback.
|
@marcoscaceres I've opened a new pull request, #408, to work on those changes. Once the pull request is ready, I'll request review from you. |
marcoscaceres
left a comment
There was a problem hiding this comment.
Looking good... some suggestions to make this more future proof and better integration with the rest of the API.
marcoscaceres
left a comment
There was a problem hiding this comment.
Couple of additional link suggestions for OpenID...
|
The suggestions is what I've implemented in WebKit WebKit/WebKit#55603 ... and this very closely matches what is in WebKit already, which is great. |
Co-authored-by: Marcos Cáceres <marcosc@apple.com>
| </tr> | ||
| <tr> | ||
| <td> | ||
| [[[ISO18013-7]]] - Annex C |
There was a problem hiding this comment.
I must admit to some ignorance about the tooling stack here but the preview seems to indicate that this results in a normative reference to a document that requires payment for access. Whether through registry or direct listing in this spec, and more so even for the latter, I don't find that acceptable.
[ISO18013-7]
ISO/IEC 18013-7:2025 ISO-compliant driving licence, Part 7: Mobile driving licence (mDL) add-on functions. ISO/IEC JTC 1/SC 17. International Organization for Standardization. May 2025. URL: https://www.iso.org/standard/91154.html
There was a problem hiding this comment.
I heard a while back there was an effort to make it freely available. I’ll see if I can get some status on that.
Also noting that it’s not uncommon for W3C specs to cite ISO specs even though they are behind a paywall. The W3C Process doesn’t prevent that.
There was a problem hiding this comment.
I don't find that acceptable.
There was a problem hiding this comment.
We should probably discuss in #214?
Would it be acceptable to make the table non-normative? There’s no implementation requirement on any user agent to support any one protocol. The protocols just become “Appendix of known protocols”.
Otherwise we drop this section entirely or wait a few weeks until ISO makes 18013-7 available for free?
There was a problem hiding this comment.
We should probably discuss in #214?
That was about inclusion criteria for the registry that is going away and is marked to be closed by this.
Would it be acceptable to make the table non-normative? There’s no implementation requirement on any user agent to support any one protocol. The protocols just become “Appendix of known protocols”.
Not really, no.
Otherwise we drop this section entirely or wait a few weeks until ISO makes 18013-7 available for free?
I thought today @RByers said it already was. But maybe he misspoke or I misheard.
Why cannot this PR proceed without referencing a closed pay-for-access document? And when/if 18013-7 is made available for free, then that can be added in a separate PR? @timcappalli (and maybe @lee) didn't seem to like that idea on the call ~2 hours ago but I'm honestly not seeing why that would be problematic.
There was a problem hiding this comment.
That was about inclusion criteria for the registry that is going away and is marked to be closed by this.
Yeah :( ... because the presumption was that the ISO spec would be available for free already, which renders that discussion mute.
I thought today @RByers said it already was. But maybe he misspoke or I misheard.
If it was we wouldn't be in this predicament, right? An ugly hack would be to just point to their Working Draft PDFs that they put up on GitHub... for example:
https://github.com/ISOWG10/ISO-18013/blob/main/Working%20Documents/
But that's a bit mean... but hey, at least they make something public. 😛
(The above was a joke... let's not cite that!)
I'm honestly not seeing why that would be problematic.
The problem is that it creates developer confusion (if only temporarily)... the spec will list the OpenID ones, but not the ISO one. And because the algorithms and API make use of the enums defined here, things start falling out of sync between implementations and the spec: we want the spec to match reality.
…edentialPresentationProtocol rdar://168641111 https://bugs.webkit.org/show_bug.cgi?id=305989 Reviewed by Anne van Kesteren. Renamed IdentityCredentialProtocol to DigitalCredentialPresentationProtocol. Spec change: w3c-fedid/digital-credentials#401 Relies on existing tests. * Source/WebCore/CMakeLists.txt: * Source/WebCore/DerivedSources-input.xcfilelist: * Source/WebCore/DerivedSources-output.xcfilelist: * Source/WebCore/DerivedSources.make: * Source/WebCore/Headers.cmake: * Source/WebCore/Modules/identity/DigitalCredential.cpp: (WebCore::DigitalCredential::create): (WebCore::DigitalCredential::DigitalCredential): (WebCore::convertProtocolString): (WebCore::jsToCredentialRequest): * Source/WebCore/Modules/identity/DigitalCredential.h: * Source/WebCore/Modules/identity/DigitalCredential.idl: * Source/WebCore/Modules/identity/DigitalCredentialPresentationProtocol.h: Renamed from Source/WebCore/Modules/identity/IdentityCredentialProtocol.h. * Source/WebCore/Modules/identity/DigitalCredentialPresentationProtocol.idl: Renamed from Source/WebCore/Modules/identity/IdentityCredentialProtocol.idl. * Source/WebCore/Modules/identity/DigitalCredentialsResponseData.h: * Source/WebCore/Modules/identity/protocols/DigitalCredentialsProtocols.h: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: * Source/WebKit/UIProcess/DigitalCredentials/WKDigitalCredentialsPicker.mm: (-[WKRequestDataResult initWithRequestDataBytes:protocol:]): (-[WKDigitalCredentialsPicker handlePresentmentCompletionWithResponse:error:]): Canonical link: https://commits.webkit.org/306123@main
|
Just recording for the record that I'm ok for this to land if, and only if, after this PR lands we immediately land the following two PRs:
That would address my concerns, fixes the terminology and citations issues, and allows us to continue with the algorithmic work. If that sounds ok, I can handle the rebasing and land the other PRs. |
There was a problem hiding this comment.
Pull request overview
Removes the protocol registry concept from the spec and replaces it with a “Protocols” section intended to enumerate supported protocols and align the API/prose with that new approach.
Changes:
- Replaces “protocol registry” references with a new “Protocols” section and table.
- Updates spec prose around request
protocolmembers anduserAgentAllowsProtocol(). - Updates cspell word list to include
multisigned.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| index.html | Removes the registry section and introduces a Protocols section/table plus a new presentation-protocol enum and related prose updates. |
| .cspell/misc.txt | Adds multisigned to the spelling allowlist and normalizes an existing entry line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The {{DigitalCredentialCreateRequest/protocol}} member's value is one | ||
| of the protocol identifiers defined in [[[#protocols]]]. |
There was a problem hiding this comment.
DigitalCredentialCreateRequest.protocol is described as being one of the identifiers defined in #protocols, but #protocols currently enumerates presentation protocols only. This makes issuance requests either impossible or underspecified; introduce an issuance protocol enum/table (and reference it here) or rename/split the protocols section so issuance is clearly covered.
| The {{DigitalCredentialCreateRequest/protocol}} member's value is one | |
| of the protocol identifiers defined in [[[#protocols]]]. | |
| The {{DigitalCredentialCreateRequest/protocol}} member's value is a string | |
| identifying the [=digital credential/issuance protocol=] to be used. The set | |
| of valid protocol identifiers and their semantics are defined by the user | |
| agent and/or supporting credential provider ecosystem and are out of scope | |
| for this specification. |
There was a problem hiding this comment.
This will be addressed in a future PR.
There was a problem hiding this comment.
Co-pilot is (sorta) correct here. We can't just point at the table that doesn't contain an issuance value (it doesn't match the use of an enum for presentation).
Honest question: can someone please explain why we can't list [[OPENID4VCI]] and openid4vci-v1? My understanding was that Chrome implemented that (or the openid4vci string, as that's what in the Web Platform Tests).
This removal of things is making me paranoid that [[OPENID4VCI]] and openid4vci-v1 is not actually supported by any user agent? If that's the case, that's ok... but it feels like something weird is going on here.
@leecam or @mohamedamir, can you maybe explain the Chrome situation? In #204 it was indicated that issuance had implementation commitments, but does it not support the openid4vci-v1? How is issuance implemented and does openid4vci-v1 work?
In the Web Platform Tests, we explicitly test for "openid4vci"... was that not correct? Do we need to remove that from the test suite as it's bad value (or change it to openid4vci-v1 rather)?
bc-pi
left a comment
There was a problem hiding this comment.
I maintain an objection to normative reference/requirement to a pay-for-access document.
| </aside> | ||
| <table class="data"> | ||
| <caption> | ||
| <dfn class="export">Table of supported presentation and [=digital credential/issuance protocol|issuance=] protocols</dfn> |
There was a problem hiding this comment.
I thought we agreed not to use the word "supported"? Can it just be?:
| <dfn class="export">Table of supported presentation and [=digital credential/issuance protocol|issuance=] protocols</dfn> | |
| <dfn class="export">Table of presentation and [=digital credential/issuance protocol|issuance=] protocols</dfn> |
marcoscaceres
left a comment
There was a problem hiding this comment.
With issues outlined in https://lists.w3.org/Archives/Public/public-fedid-wg/2026Jan/0015.html
Objection was overruled by the WG Chairs in https://lists.w3.org/Archives/Public/public-fedid-wg/2026Jan/0014.html
SHA: 32c6067 Reason: push, by marcoscaceres Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR makes the following changes:
Closes #396
Closes #345
Closes #257
Closes #256
Closes #240
Closes #215
Closes #214
Closes #213
Closes #212
Closes #211
Closes #136
Closes #69
The following tasks have been completed:
Implementation commitment:
Documentation and checks
Preview | Diff