-
Notifications
You must be signed in to change notification settings - Fork 200
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
feat: Trusted certificates hook for presentations #2052
feat: Trusted certificates hook for presentations #2052
Conversation
Signed-off-by: Tom Lanser <tom@devv.nl>
|
Signed-off-by: Tom Lanser <tom@devv.nl>
caa7b80
to
0fddb85
Compare
@@ -181,6 +181,8 @@ interface W3cVerifyPresentationOptionsBase { | |||
|
|||
export interface W3cJwtVerifyPresentationOptions extends W3cVerifyPresentationOptionsBase { |
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.
I had to think for a bit on what I think of this API. And I think it may be a bit confusing. Especially since correlationId
is something internal to sphereon libraries, and proofRecordId it's not obvious this is for DIDcomm.
So.. what if we make it a verificationContext
, which is optional, and has a unified interface that can be reused.
interface VerificationContext {
/**
* The `id` of the `ProofRecord` that this verification is bound to.
*/
didcommProofRecordId?: string
/**
* The `id` of the `OpenId4VcVerificationSessionRecord` that this verification is bound to.
*/
openId4VcVerificationSessionId?: string
}
I think we could also extract the trustedCertificates
, but it would be good to also add these, as then you can pass them when calling the top-level APIs (w3c verification) (i think Sd-JWT is already possible right?)
And we will only call the get trusted certificates method if you didn't provide certificates (so if you call sdJWt.verify with certificates we won't call it). But otherwise we will
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.
We can also add the already given trustedCertificates here so it's the choice by the user to combine it with the hook ones or completely override them
I like the general appraoch. Just some commetns to make it a bit more specific / unified and so we can extend it with other records as well in the future (e.g. issuance will also need this probably) |
Signed-off-by: Tom Lanser <tom@devv.nl>
one final remark, otherwise LGTM! |
Signed-off-by: Tom Lanser <tom@devv.nl>
08a485b
into
openwallet-foundation:main
…ion#2052) Signed-off-by: Tom Lanser <tom@devv.nl> Signed-off-by: Martin Auer <martin.auer97@gmail.com>
Gives the ability to dynamically set the trusted Certificates for a specific proof request.