-
Notifications
You must be signed in to change notification settings - Fork 9
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
change providers to requests #165
Conversation
Something that occurred to me that feels a bit off is that we'd have a Here: await navigator.credentials.get({
digital: {
requests: [{ <-----------
protocol: "openid4vp",
request: { <----------- is this weird?
nonce: "foo",
client_id_metadata: "bar",
// ... more stuff
},
}]
}
}); It is not terrible, but felt like worth noting. WDYT? |
14f10d8
to
bfbe94d
Compare
bfbe94d
to
cd8764f
Compare
Yeah, I spotted that too... I can live with it though. How about |
Some alternatives:
|
To add to the a bit odd but not terrible observation, due to the layering and reuse of prior RFCs, the top level thing[1] of a
[1] I can't even keep track of what things at what layer are called anymore |
Can someone remind me what the rational for the |
The layers and potential options for various cardinalities is making my little mind hurt. Kinda related is openid/OpenID4VP#248 not to mention that whatever query language(s) in openid4vp can themselves express asking for multiple things and/or combinatorial options. |
Yeah, @bc-pi , that's one of the things that I think we may be worth revisiting: I'm not quite sure anymore we need the level of indirection of To answer your question, originally, At this point, I think it is not clear to me that we would run into this situation: when a verifier makes a request, it doesn't have to connect with a variety of wallets that support different protocols. |
I wouldn't mind dropping We should then add a. static setlike With the sequence, a developer can easily filter with standard Set methods. Proposed API additioninterface DigitalCredentialsSupportedProtocols {
readonly setlike<DOMString>; // user agent pre-populates
};
partial interface DigitalCredential {
readonly attribute DigitalCredentialsSupportedProtocols supportedProtocols;
} Usage: if (DigitalCredential.supportedProtocols.has("openid4vp")) {
// let's make an openid4vp request
}
// Or you can do...
const supported = Array.from(DigitalCredential.supportedProtocols).filter(typesTheRPSupports);
// Or whatever developers want:
for (const supported in DigitalCredential.supportedProtocols.values()) {
// do things with supported
}
const requests = [...DigitalCredential.supportedProtocols].map(toRequestWeKnowHowToMake);
// or even...
switch (true) {
case DigitalCredential.supportedProtocols.has("openid4vp"):
makeOpenIDRequest(data);
break;
case DigitalCredential.supportedProtocols.has("whatever"):
makeWhateverRequest(data);
break;
default:
throw TypeError("Oh noes! they don't support our favorite protocol!")
} That gives a ton of flexibility. You can even use it will all the new fancy JavaScript set comparison operations: // DigitalCredential.supportedProtocols is a Set
const supportedProtocols = DigitalCredential.supportedProtocols;
// Example Set of protocols to compare
const protocolsToCheck = new Set(['openid4vp', 'someOtherProtocol']);
// Union: Combining both sets
const union = supportedProtocols.union(protocolsToCheck);
console.log(union); // Set { 'openid4vp', 'someOtherProtocol', ... }
// Intersection: Getting common protocols between the sets
const intersection = supportedProtocols.intersection(protocolsToCheck);
console.log(intersection); // Set { 'openid4vp' }
// Difference: Getting protocols supported by DigitalCredential but not in protocolsToCheck
const difference = supportedProtocols.difference(protocolsToCheck);
console.log(difference); // Set { ... } (protocols supported by DigitalCredential but not in protocolsToCheck)
// Symmetric Difference: Getting protocols that are in either set, but not in both
const symmetricDifference = supportedProtocols.symmetricDifference(protocolsToCheck);
console.log(symmetricDifference); // Set { 'someOtherProtocol', ... } |
@samuelgoto wrote:
Hmm, no, we definitely still have that requirement that a Verifier might ask for the same sort of thing over multiple protocols (because it won't know what the wallet supports when it asks). For example, in CA DMV, we might have a future where mDoc is requested over a different protocol (or protocol version) than other VCs. We cannot presume that there will only be one protocol. To be clear, it sounds like @marcoscaceres proposal above would work just fine. |
Right, @msporny, but the future format would need to be known to the browser, so "mdoc" would need to be a registered protocol (i.e., you can't sent up anything as that would expose what is installed on the device, so for obvious user privacy and security reasons you can't send "wallet-custom-format"). The developer can check and do the filtering on their most preferred standardized type there. |
Moved the proposal here: #168 |
Yes, agreed. I wasn't making an argument against having a set of standardized protocols or standardized formats. It sounded like @samuelgoto was saying: "We don't need to contemplate a set of standardized protocols that a verifier might use to request a credential, because there will only ever be one that a verifier will use to request a credential." ... and that's not true today and is unlikely to be true in the future (as much as we'd all like that to be true from an interoperability and implementation complexity standpoint). |
Ok, talked with @samuelgoto and he also convinced me that we should keep the array. for the member names, how about we do So |
Thanks for backstory @samuelgoto. My general intuition is that there are too many levels of indirection in all the various layers of all this. But that's easy to say and not so easy to clearly say what should be removed. |
I like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a backwards incompatible change, so we are going to need to update the OpenID4VP spec as well as verifiers. It is still possible, but we shouldn't take this lightly.
Can we run this by the community group to make sure that we are on all board with this trade-off?
Co-authored-by: Sam Goto <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a backwards incompatible change, but a good one in case we still have time. Lets run this by the CG before merging.
Just for transparency, the other alternatives that we considered for |
2024-09-18 call: good to merge once we have links |
Updated the tests #165 |
Filed issues on WebKit side... |
Going ahead and merging, as we have tests and implementation commitments and it's mostly a cosmetic change. |
Chromium bug filed: |
Closes #155
The following tasks have been completed:
Implementation commitment:
Documentation and checks
Preview | Diff