Skip to content
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: add support for verifying multi-vp presentations #148

Merged

Conversation

TimoGlastra
Copy link
Collaborator

Adds support for verifying multi-vp presentations based on the recent addition to PEX. it does not add methods yet to create responses including multiple VPs, as this would require more changes to PEX as well. Currently how we do it in Credo is we determine the presentations we need to create based on the credentials (each sd-jwt one, each credentialSubject.id in jwt-vp one, etc..). And then we can call the evaluatePresentations to generate the submission. We should probably contribute that to PEX?

But this PR at least allows the verifying and handling of multi-vp responses. Currently based on unstable version of PEX, need a new release as well.

@TimoGlastra
Copy link
Collaborator Author

@sksadjad / @sanderPostma if you also have time to look at this one, that'll be helpful. It will unblock quite a few things for us :)

@sanderPostma
Copy link
Contributor

No comments on the code itself, but going to build & release a new pex library first on which this branch depends on.
Also the tests are failing, please see if this is related to your changes, otherwise I will also create a branch feat/multi-presentation in our repo it base the PR against.

@TimoGlastra
Copy link
Collaborator Author

Also the tests are failing, please see if this is related to your changes

Yes I coded this in the plane and some tests were failing due to missing internet, but I see now that there's some issues in those tests still. I will fix them!

Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra
Copy link
Collaborator Author

Fixed the test @sanderPostma, passes locally now

@TimoGlastra
Copy link
Collaborator Author

Ah and also in the CI now 🎉

Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra
Copy link
Collaborator Author

Updated with latest version of PEX. It now needs Sphereon-Opensource/PEX#169 and Sphereon-Opensource/PEX#168 to be merged/release first

Signed-off-by: Timo Glastra <[email protected]>
@sanderPostma sanderPostma merged commit d9a79e2 into Sphereon-Opensource:develop Sep 18, 2024
1 check failed
@sanderPostma
Copy link
Contributor

@TimoGlastra
I merged it, but apparently we had an increase in test coverage because tsc is now complaining about a lot of things when compiling from AuthenticationResponse.response.spec.ts
(working on it in develop...fix/post-merge )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants