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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]> Signed-off-by: Ariel Gentile <[email protected]>

Signed-off-by: Ariel Gentile <[email protected]> feat: deliver messages individually, not fetching from the queue every time Signed-off-by: Ariel Gentile <[email protected]> chore: revert to free runners (openwallet-foundation#1662) Signed-off-by: Ry Jones <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> chore: create settings.yml (openwallet-foundation#1663) Signed-off-by: Ry Jones <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> chore: fix ci and add note to readme (openwallet-foundation#1669) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> docs: update active maintainers (openwallet-foundation#1664) Signed-off-by: Karim Stekelenburg <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> feat: did:peer:2 and did:peer:4 support in DID Exchange (openwallet-foundation#1550) Signed-off-by: Ariel Gentile <[email protected]> feat(presentation-exchange): added PresentationExchangeService (openwallet-foundation#1672) Signed-off-by: Berend Sliedrecht <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> chore: removed jan as maintainer (openwallet-foundation#1678) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> ci: change secret (openwallet-foundation#1679) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> chore: add meta to rxjs timeout errors (openwallet-foundation#1683) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> build(deps): bump ws and @types/ws (openwallet-foundation#1686) Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> build(deps): bump follow-redirects from 1.15.2 to 1.15.4 (openwallet-foundation#1694) Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> chore: update shared components libraries (openwallet-foundation#1691) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> fix: properly print key class (openwallet-foundation#1684) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> feat(present-proof): add support for aries RFC 510 (openwallet-foundation#1676) Signed-off-by: Berend Sliedrecht <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> fix(present-proof): isolated tests (openwallet-foundation#1696) Signed-off-by: Ariel Gentile <[email protected]> feat(indy-vdr): register revocation registry definitions and status list (openwallet-foundation#1693) Signed-off-by: Ariel Gentile <[email protected]> chore: rename to credo-ts (openwallet-foundation#1703) Signed-off-by: Ry Jones <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> ci: fix git checkout path and update setup-node (openwallet-foundation#1709) Signed-off-by: Ariel Gentile <[email protected]> fix: remove check for DifPresentationExchangeService dependency (openwallet-foundation#1702) Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> docs: update zoom meeting link (openwallet-foundation#1706) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> refactor(oob)!: make label optional (openwallet-foundation#1680) Signed-off-by: Timo Glastra <[email protected]> Co-authored-by: Ariel Gentile <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> feat: support short legacy connectionless invitations (openwallet-foundation#1705) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> feat(dids)!: did caching (openwallet-foundation#1710) feat: did caching Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> fix: jsonld document loader node 18 (openwallet-foundation#1454) Signed-off-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> 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] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Timo Glastra <[email protected]> Signed-off-by: Ariel Gentile <[email protected]> feat: did rotate (openwallet-foundation#1699) Signed-off-by: Ariel Gentile <[email protected]> refactor: pickup protocol method names Signed-off-by: Ariel Gentile <[email protected]>

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 <[email protected]>

feat(proofs): initial boiler plate for ppv2

3dd4e61

Signed-off-by: Berend Sliedrecht <[email protected]>

feat(proofs): added PresentationExchangeModule

8ef8f2d

Signed-off-by: Berend Sliedrecht <[email protected]>

feat(present-proof): initial implementation without tests

528d31b

Signed-off-by: Berend Sliedrecht <[email protected]>

feat(pex): created core PEX module

e4a41c2

Signed-off-by: Berend Sliedrecht <[email protected]>

chore(proofs): use core pex implementation

12ca53b

Signed-off-by: Berend Sliedrecht <[email protected]>

feat(present-proof): working initial full flow

802a016

Signed-off-by: Berend Sliedrecht <[email protected]>

fix(present-proof): fixed the pex tests

2df672f

Signed-off-by: Berend Sliedrecht <[email protected]>

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

985b19b

Signed-off-by: Berend Sliedrecht <[email protected]>

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

f3e1534

Signed-off-by: Berend Sliedrecht <[email protected]>

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 <[email protected]>

TimoGlastra added 6 commits January 8, 2024 11:58

cleanup and ppv2 only supports pex v1

59ed3d3

Signed-off-by: Timo Glastra <[email protected]>

refactor: add dif prefix

04b17b6

Signed-off-by: Timo Glastra <[email protected]>

fix: json-ld issues ... :(

d19ccb5

Signed-off-by: Timo Glastra <[email protected]>

style: formatting

e2a67b8

Signed-off-by: Timo Glastra <[email protected]>

remove console log

1e5fe20

Signed-off-by: Timo Glastra <[email protected]>

fix test

80bbc8b

Signed-off-by: Timo Glastra <[email protected]>

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

genaris pushed a commit to genaris/credo-ts that referenced this pull request Jan 29, 2024

feat(present-proof): add support for aries RFC 510 (openwallet-founda…

2f7caa5

…tion#1676) Signed-off-by: Berend Sliedrecht <[email protected]> Signed-off-by: Ariel Gentile <[email protected]>

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