-
Notifications
You must be signed in to change notification settings - Fork 510
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
Added did:web support to holder for JSON-LD proof presentation #3138
Conversation
Signed-off-by: aritroCoder <[email protected]>
I think these changes are acceptable but agree that there might be better approaches to determining which verification method ID to use based on the DID. For this particular scenario, we should know the VM id because the credential proof should contain a VM ID. @PatStLouis might be a good idea to keep this scenario in mind as you work on DataIntegrity and VCDM 2.0 updates. |
Would it make sense to use the first VM referenced or embedded in the |
I'm currently not convinced by this solution as this assumes that the first verification method corresponds to the keypair used to sign the document. I would rather see a field for user provided @aritroCoder could you expand on the issue you are having and how I can reproduce it? What is the |
@dbluhm wasn't there a way to bind a |
Yes, that was a typo from me and I corrected it. It should be Steps to reproduce:
{
"auto_issue": false,
"auto_remove": false,
"comment": "Credential from minimal example",
"connection_id": "df82e639-c15c-4374-8e9a-f820be2a1c6f", #
"filter": {
"ld_proof": {
"credential": {
"@context": [
"https://www.w3.org/2018/credentials/v1",
"https://w3id.org/citizenship/v1"
],
"credentialSubject": {
"birthCountry": "Bahamas",
"birthDate": "1958-07-17",
"familyName": "Builder",
"gender": "Male",
"givenName": "Bob",
"id": "did:web:aritrocoder.github.io:didwebholder", #
"type": [
"PermanentResident"
]
},
"issuanceDate": "2024-05-24",
"issuer": "did:web:aritra-issuer-faber.nborbit.ca", #
"type": [
"VerifiableCredential",
"PermanentResident"
]
},
"options": {
"proofType": "Ed25519Signature2018",
"proofPurpose": "assertionMethod",
"verificationMethod": "did:web:aritra-issuer-faber.nborbit.ca#key-1" #
}
}
},
"trace": false
} In the above body, replace fields marked
{
"auto_verify": false,
"comment": "Presentation request from minimal",
"connection_id": "6a54d6e2-f371-403a-84ab-0e633b4939f3", # get from conn id
"presentation_request": {
"dif": {
"options": {
"challenge": "bdf0c9e7-2d2b-45f7-aae6-961329835d85",
"domain": "test-degree"
},
"presentation_definition": {
"format": {
"ldp_vp": {
"proof_type": [
"Ed25519Signature2018"
]
}
},
"id": "a5487d6d-50e6-4bfe-bb94-52c2deb8d2a2",
"input_descriptors": [
{
"constraints": {
"fields": [
{
"filter": {
"const": "Builder"
},
"id": "1f44d55f-f161-4938-a659-f8026467f126",
"path": [
"$.credentialSubject.familyName"
],
"purpose": "The claim must be from one of the specified issuers"
},
{
"path": [
"$.credentialSubject.givenName"
],
"purpose": "The claim must be from one of the specified issuers"
}
],
"is_holder": [
{
"directive": "required",
"field_id": [
"1f44d55f-f161-4938-a659-f8026467f126"
]
}
]
},
"id": "citizenship_input_1",
"name": "EU Driver's License",
"schema": [
{
"uri": "https://www.w3.org/2018/credentials#VerifiableCredential"
},
{
"uri": "https://w3id.org/citizenship#PermanentResident"
}
]
}
]
}
}
},
"trace": false
}
{
"dif": {
"presentation_definition": {
"format": {
"ldp_vp": {
"proof_type": [
"Ed25519Signature2018"
]
}
},
"id": "a5487d6d-50e6-4bfe-bb94-52c2deb8d2a2",
"input_descriptors": [
{
"constraints": {
"fields": [
{
"filter": {
"const": "Builder"
},
"id": "1f44d55f-f161-4938-a659-f8026467f126",
"path": [
"$.credentialSubject.familyName"
],
"purpose": "The claim must be from one of the specified issuers"
},
{
"path": [
"$.credentialSubject.givenName"
],
"purpose": "The claim must be from one of the specified issuers"
}
],
"is_holder": [
{
"directive": "required",
"field_id": [
"1f44d55f-f161-4938-a659-f8026467f126"
]
}
]
},
"id": "citizenship_input_1",
"name": "EU Driver's License",
"schema": [
{
"uri": "https://www.w3.org/2018/credentials#VerifiableCredential"
},
{
"uri": "https://w3id.org/citizenship#PermanentResident"
}
]
}
]
}
}
} At this step the error occurs |
Thanks for clarifying. One way to create a binding is to ensure that the Here's so pseudo code: key_types_mapping = {
"Ed25519Signature2018": "Ed25519VerificationKey2018",
"Ed25519Signature2020": "Ed25519VerificationKey2020",
}
proof_type = "Ed25519Signature2018"
verification_method_type = key_types_mapping[proof_type]
verification_method_list = did_document.get("verificationMethod", {})
verification_method_id = next(
(
verification_method["id"]
for verification_method in verification_method_list
if verification_method["type"] == verification_method_type
),
None,
) |
In the current state, this feature is undocumented and is inserting a behavior which could be problematic in some cases. But this is definitely something that needs to be addressed and is a valid error/issue to raise |
Signed-off-by: aritroCoder <[email protected]>
@PatStLouis I implemented the change you suggested, you can have a look at it. I did the preliminary testing and it was working. |
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.
Make verification method types an array to support multikey
This should do the trick, however I still want to point out I'm not convinced by this solution as it makes a lot of opinions and is currently undocumented. Let's wait before 1.0 release before looking at merging this. I would also strongly consider alternatives such as binding a @dbluhm I think the issue here is that this isn't a verification but the holder looking to sign the presentation, so it's unrelated to the proof of the VC. With the issuance we can provide a We could add a
This would serve a similar purpose, providing the holder the ability to select which keypair to use for the presentation proof. |
added multikey support Co-authored-by: Patrick St-Louis <[email protected]> Signed-off-by: Aritra Bhaduri <[email protected]>
added multikey support Co-authored-by: Patrick St-Louis <[email protected]> Signed-off-by: Aritra Bhaduri <[email protected]>
added multikey support Co-authored-by: Patrick St-Louis <[email protected]> Signed-off-by: Aritra Bhaduri <[email protected]>
Hi @PatStLouis you can take your time and suggest best way this can be done according to you, and I can implement it. I can also add the verification method with the issuance object if that looks better. We can finalize it after a discussion with the community. |
I thought I heard on one of the calls that this might be better as a plugin. Is that the approach that should be taken? |
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
@aritroCoder it's not the issuance, but the presentation request. Issuance already supports passing the verification method. The issue is that the step where the holder signs the credential is midway through the protocol and cannot be set deterministically, if the protocol is automated unless the verifier knows in advance which verification method to request. I'm assuming you are doing a business to business use case since the dif presentation exchange implementation was designed around did:key / did:sov where each did is self resolvable to 1 verification method only. Did web introduces verification method entropy. I think your suggestion is the best current way to automate this, unless we make it a requirement for did:web to have the protocol request a "manual" presentation step, requiring the holder to declare which verification method to use. This being said, if you have a use case that you need to leverage present-proof-v2 where the holder is using did:web (highly correlatable, no privacy), this is the simplest way I can think of achieving this. The other option is to have a "default" If you are willing to spend some more time with this, I would look at adding an optional field to the did:web registration call to provide a default
@dbluhm how do you feel about this approach? This would require a modificaiton to the didInfo class, adding a |
Closing this issue as it is covered by #3279. Thanks for the contribution. Nice work! |
Problem description
When the holder uses a did:web in a credential, it is unable to present proofs for that credential. The
/present-proof-2.0/records/{pres_ex_id}/send-presentation
API endpoint terminates with the message: Unable to retrieve verification method for did.Where the problem occurs
When the
send-presentation
endpoint is called, it builds the VP by calling the following functions:present_proof_send_presentation
ataries_cloudagent/protocols/present_proof/v2_0/routes.py
create_pres
ataries_cloudagent/protocols/present_proof/v2_0/manager.py
create_pres
ataries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py
create_vp
ataries_cloudagent/protocols/present_proof/dif/pres_exch_handler.py
_get_issue_suite
ataries_cloudagent/protocols/present_proof/dif/pres_exch_handler.py
get_verification_method_id_for_did
ataries_cloudagent/wallet/default_verification_key_strategy.py
. This is where the error occurs.How we are fixing it:
default_verification_key_strategy
.resolve
method, look at the services, and use the verification key for a service.Interoperability and performance:
NOTE
When checking for verification method id for did:web, we resolve the DID and take the first key available in the DID document. There might be better approaches to this which the contributors can suggest.