feat(present-proof): add support for aries RFC 510 #1676

berendsliedrecht · 2023-12-19T12:53:25Z

Adds basic support for dif presentation exchange over DIDComm
- According to RFC 0454 and 510
Would be good to include some more tests, but code is ready to review
There is still an open issue I have with the credential selection. Right now if we use PEXv2, we basically call getAll on the w3c credentials which is not really great... For V1 we could use the schema attribute, but that is not included anymore. For now, it might be fine to just call the getAll function and use those credentials and resolve this later when we want to revamp the storage.

TimoGlastra

Overall looks good, but it's missing some things

packages/core/src/modules/presentation-exchange/PresentationExchangeService.ts

TimoGlastra · 2023-12-22T05:38:17Z

packages/core/src/modules/presentation-exchange/PresentationExchangeService.ts

-          proofType: this.getProofTypeForLdpVc(agentContext, presentationDefinition, verificationMethod),
+          proofType: this.getProofTypeForLdpVc(
+            agentContext,
+            presentationDefinition as PresentationDefinition,


TimoGlastra · 2023-12-22T05:41:54Z

packages/core/src/modules/presentation-exchange/utils/credentialSelection.ts

+      const credentialRecord = credentialRecords.find((record) => {
+        const originalVc = getSphereonOriginalVerifiableCredential(record.credential)
+
+        return deepEquality(originalVc, encoded)
+      })


It feels a bit expensive to do a deepEquality on every credential to find the original record that was associated with VC. Is there no other way to do the matching?

Because we have the encodedCredenitals, we can do a === check on it right, as the object references are the same?

So in that case const credentialIndex = encodedCredentials.indexOf(encoded) should still work

But maybe I'm missing something

I tried it again, but the indexOf does not seem to work. Likely somewhere deeper in the code a clone is being done (could be in the sphereon library). I will leave it like this for now.

TimoGlastra · 2023-12-22T05:55:29Z

...e/src/modules/proofs/formats/presentation-exchange/PresentationExchangeProofFormatService.ts

+        ps.validatePresentationSubmission(presentation.presentation_submission)
+      }
+
+      ps.validatePresentation(presentationDefinition, presentation as unknown as VerifiablePresentation)


Is the presentation submission not used to verify a presentation? Does it just check if it matches the presentationDefinition, but not the presentationSubmission?

Also -- should we make validatePresentation take a W3CVerifiablePresentation instead of a sphereon interface?

TimoGlastra · 2023-12-22T05:56:17Z

...e/src/modules/proofs/formats/presentation-exchange/PresentationExchangeProofFormatService.ts

+      presentation_definition: PresentationDefinition
+    }>()
+    const presentation = attachment.getDataAsJson<
+      W3cVerifiablePresentation & { presentation_submission: PresentationSubmission }


W3cVerifiablePresentation is incorrect I think, as it's a class, while we just take the object here. So the type will think it has context property, but it will actually have @context property. We need to use json transformer on it as well.

TimoGlastra · 2023-12-22T05:57:08Z

...e/src/modules/proofs/formats/presentation-exchange/PresentationExchangeProofFormatService.ts

+      ps.validatePresentation(presentationDefinition, presentation as unknown as VerifiablePresentation)
+      return true
+    } catch (e) {
+      agentContext.config.logger.error(e)


Suggested change

agentContext.config.logger.error(e)

agentContext.config.logger.error(`Failed to verify presentation in PEX proof format service: ${e.message}`, { cause: e })

TimoGlastra · 2023-12-22T05:57:53Z

...e/src/modules/proofs/formats/presentation-exchange/PresentationExchangeProofFormatService.ts

+    const credentials = presentationSubmission.requirements.flatMap((r) =>
+      r.submissionEntry.flatMap((e) => e.verifiableCredentials)
+    )
+
+    return credentials
+  }
+
+  public async selectCredentialsForRequest(
+    agentContext: AgentContext,
+    { requestAttachment }: ProofFormatSelectCredentialsForRequestOptions<PresentationExchangeProofFormat>
+  ): Promise<Array<W3cCredentialRecord>> {
+    const ps = this.presentationExchangeService(agentContext)
+    const { presentation_definition: presentationDefinition } = requestAttachment.getDataAsJson<{
+      presentation_definition: PresentationDefinition
+    }>()
+
+    ps.validatePresentationDefinition(presentationDefinition)
+
+    const presentationSubmission = await ps.selectCredentialsForRequest(agentContext, presentationDefinition)
+
+    const credentials = presentationSubmission.requirements.flatMap((r) =>
+      r.submissionEntry.flatMap((e) => e.verifiableCredentials)
+    )


As mentioned in other comments, the structures returned by the service adds a lot of needed context. I think we should return them as is, rather than flatmapping

TimoGlastra · 2023-12-22T05:58:50Z

packages/core/tests/jsonld.ts

    }),
    cache: new CacheModule({
      cache: new InMemoryLruCache({ limit: 100 }),
    }),
    indySdk: new IndySdkModule({
      indySdk,
    }),
+    pex: new PresentationExchangeModule(),


Is this registered by default? As it's in core, I think we should register it by default

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

berendsliedrecht · 2024-01-04T11:00:24Z

@TimoGlastra I resolved your feedback, but I am now encountering an error that the PresentationExchangeProofFormatData.formatData.presentation does not contain the proof object. Do you know if we have this already defined somewhere as a type that I could reuse? I could not find it myself.

TimoGlastra · 2024-01-08T04:32:56Z

@TimoGlastra I resolved your feedback, but I am now encountering an error that the PresentationExchangeProofFormatData.formatData.presentation does not contain the proof object. Do you know if we have this already defined somewhere as a type that I could reuse? I could not find it myself.

I was able to fix this by using presentation.encoded. This return either the JSON variant of the w3c presentation or the serialized JWT as string (in case of JWT VP). You were directly using the class, which is not suitable for directly sending over the wire (it first needs to be serialized to JSON).

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra · 2024-01-08T04:44:15Z

@berendsliedrecht I have pushed a fix for the above + several other fixes (please take a look at the commit so you can see which changes I made).

Could you maybe to wrap this up, update the tests in packages/core/src/modules/proofs/protocol/v2/__tests__/v2-presentation-exchange-presentation.e2e.test.ts to make it either all standalone tests or a single test?

Currently it's multiple tests but they depend on each other. It will result in that you can't run the tests separately, and if one fails the others will fail too. So in that case it's better to make it just one test or properly split them up.

Otherwise I think we're good here, really nice work and happy that it's finally ready to be integrated into AFJ :)

Once merged I'll update it in the openid4vc branch

BTW -- added a non-spec compliant way to also support JWT VPs. Haven't added tests for it yet, but I think it is really nice to also support JWT VPs (although it only supports once and you can't add multiple proofs, see suggestion in this issue on some changes I want to propose for V2 of the attachment format: hyperledger/aries-rfcs#807)

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra · 2024-01-10T07:53:57Z

Merging, would be great if you can still do a small follow up PR @berendsliedrecht

berendsliedrecht · 2024-01-10T10:38:03Z

Should be fixed in #1696

…tion#1676) Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io> Signed-off-by: Ariel Gentile <gentilester@gmail.com>

Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat: deliver messages individually, not fetching from the queue every time Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: revert to free runners (openwallet-foundation#1662) Signed-off-by: Ry Jones <ry@linux.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: create settings.yml (openwallet-foundation#1663) Signed-off-by: Ry Jones <ry@linux.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: fix ci and add note to readme (openwallet-foundation#1669) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> docs: update active maintainers (openwallet-foundation#1664) Signed-off-by: Karim Stekelenburg <karim@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat: did:peer:2 and did:peer:4 support in DID Exchange (openwallet-foundation#1550) Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat(presentation-exchange): added PresentationExchangeService (openwallet-foundation#1672) Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io> Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: removed jan as maintainer (openwallet-foundation#1678) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> ci: change secret (openwallet-foundation#1679) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: add meta to rxjs timeout errors (openwallet-foundation#1683) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> build(deps): bump ws and @types/ws (openwallet-foundation#1686) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> build(deps): bump follow-redirects from 1.15.2 to 1.15.4 (openwallet-foundation#1694) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: update shared components libraries (openwallet-foundation#1691) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> fix: properly print key class (openwallet-foundation#1684) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat(present-proof): add support for aries RFC 510 (openwallet-foundation#1676) Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io> Signed-off-by: Ariel Gentile <gentilester@gmail.com> fix(present-proof): isolated tests (openwallet-foundation#1696) Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat(indy-vdr): register revocation registry definitions and status list (openwallet-foundation#1693) Signed-off-by: Ariel Gentile <gentilester@gmail.com> chore: rename to credo-ts (openwallet-foundation#1703) Signed-off-by: Ry Jones <ry@linux.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> ci: fix git checkout path and update setup-node (openwallet-foundation#1709) Signed-off-by: Ariel Gentile <gentilester@gmail.com> fix: remove check for DifPresentationExchangeService dependency (openwallet-foundation#1702) Signed-off-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> docs: update zoom meeting link (openwallet-foundation#1706) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> refactor(oob)!: make label optional (openwallet-foundation#1680) Signed-off-by: Timo Glastra <timo@animo.id> Co-authored-by: Ariel Gentile <gentilester@gmail.com> Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat: support short legacy connectionless invitations (openwallet-foundation#1705) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat(dids)!: did caching (openwallet-foundation#1710) feat: did caching Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> fix: jsonld document loader node 18 (openwallet-foundation#1454) Signed-off-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> build(deps): bump amannn/action-semantic-pull-request from 5.3.0 to 5.4.0 (openwallet-foundation#1656) build(deps): bump amannn/action-semantic-pull-request Bumps [amannn/action-semantic-pull-request](https://github.com/amannn/action-semantic-pull-request) from 5.3.0 to 5.4.0. - [Release notes](https://github.com/amannn/action-semantic-pull-request/releases) - [Changelog](https://github.com/amannn/action-semantic-pull-request/blob/main/CHANGELOG.md) - [Commits](amannn/action-semantic-pull-request@v5.3.0...v5.4.0) --- updated-dependencies: - dependency-name: amannn/action-semantic-pull-request dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Timo Glastra <timo@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com> feat: did rotate (openwallet-foundation#1699) Signed-off-by: Ariel Gentile <gentilester@gmail.com> refactor: pickup protocol method names Signed-off-by: Ariel Gentile <gentilester@gmail.com>

berendsliedrecht force-pushed the present-proof-v2 branch 3 times, most recently from 436b922 to 27fb71a Compare December 21, 2023 09:04

berendsliedrecht marked this pull request as ready for review December 21, 2023 12:24

berendsliedrecht requested a review from a team as a code owner December 21, 2023 12:24

berendsliedrecht force-pushed the present-proof-v2 branch from 3931e50 to 5551267 Compare December 21, 2023 13:38

TimoGlastra requested changes Dec 22, 2023

View reviewed changes

berendsliedrecht added 10 commits January 3, 2024 15:15

feat(proofs): added PresentationExchangeService

ead1020

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

feat(proofs): initial boiler plate for ppv2

3dd4e61

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

feat(proofs): added PresentationExchangeModule

8ef8f2d

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

feat(present-proof): initial implementation without tests

528d31b

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

feat(pex): created core PEX module

e4a41c2

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

chore(proofs): use core pex implementation

12ca53b

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

feat(present-proof): working initial full flow

802a016

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

fix(present-proof): fixed the pex tests

2df672f

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

fix(present-proof): do not take credentials as an input

985b19b

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

fix(present-proof): resolved feedback but proof is not included

f3e1534

Signed-off-by: Berend Sliedrecht <sliedrecht@berend.io>

berendsliedrecht force-pushed the present-proof-v2 branch from a2931c3 to f3e1534 Compare January 3, 2024 14:16

updates to PEX flow

6757add

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra added 6 commits January 8, 2024 11:58

cleanup and ppv2 only supports pex v1

59ed3d3

Signed-off-by: Timo Glastra <timo@animo.id>

refactor: add dif prefix

04b17b6

Signed-off-by: Timo Glastra <timo@animo.id>

fix: json-ld issues ... :(

d19ccb5

Signed-off-by: Timo Glastra <timo@animo.id>

style: formatting

e2a67b8

Signed-off-by: Timo Glastra <timo@animo.id>

remove console log

1e5fe20

Signed-off-by: Timo Glastra <timo@animo.id>

fix test

80bbc8b

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra self-requested a review January 8, 2024 08:42

TimoGlastra approved these changes Jan 8, 2024

View reviewed changes

TimoGlastra merged commit 40c9bb6 into openwallet-foundation:main Jan 10, 2024
7 checks passed

berendsliedrecht deleted the present-proof-v2 branch January 10, 2024 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(present-proof): add support for aries RFC 510 #1676

feat(present-proof): add support for aries RFC 510 #1676

berendsliedrecht commented Dec 19, 2023 •

edited

Loading

TimoGlastra left a comment

TimoGlastra Dec 22, 2023

TimoGlastra Dec 22, 2023

berendsliedrecht Dec 22, 2023

TimoGlastra Dec 22, 2023

TimoGlastra Dec 22, 2023

TimoGlastra Dec 22, 2023

TimoGlastra Dec 22, 2023

TimoGlastra Dec 22, 2023

berendsliedrecht commented Jan 4, 2024

TimoGlastra commented Jan 8, 2024

TimoGlastra commented Jan 8, 2024

TimoGlastra commented Jan 10, 2024

berendsliedrecht commented Jan 10, 2024

	agentContext.config.logger.error(e)
	agentContext.config.logger.error(`Failed to verify presentation in PEX proof format service: ${e.message}`, { cause: e })

feat(present-proof): add support for aries RFC 510 #1676

feat(present-proof): add support for aries RFC 510 #1676

Conversation

berendsliedrecht commented Dec 19, 2023 • edited Loading

TimoGlastra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berendsliedrecht commented Jan 4, 2024

TimoGlastra commented Jan 8, 2024

TimoGlastra commented Jan 8, 2024

TimoGlastra commented Jan 10, 2024

berendsliedrecht commented Jan 10, 2024

berendsliedrecht commented Dec 19, 2023 •

edited

Loading