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: mdoc selectFrom, evaluation and parsing #179

Conversation

TimoGlastra
Copy link
Contributor

This PR adds support for parsing and evaluation of mdoc. It builds on the already existing work that was done, but now also adds tests and makes sure it works correctly for the following methods:

  • evaluatePresentation
  • selectFrom
  • validateDefinition

I added a new Mdoc.spec.ts file that adds tests from these methods, as well as combining it with sd-jwt. So it allows doing combined presentation evaluation which will be useful for integration into oid4vp :)

I also added some extra checks in the submission extraction logic, as it allowed e.g. path $ while the presentations would be an array messing with the querying. It is now stricter but I tried adding helpful error messages.

One suggestion on making the API more aligned is that if the presentationSubmision returned in the presetnation result uses $ i don't think the presentations should be an array, but rather a single entry. As it now doesn't always match. Because i think if there's only one entry it will always just us $, but still return an array. If you then pass that into the evaluation it will fail (because you try to access $ in an array, selecting the whole array)

Depends on Sphereon-Opensource/SSI-SDK#262, so once that is merged I'll update the dep and release this. There will be another final PR to oid4vp that will integrate this work as well and remove the logic that now bypasses pex!

The only thing for the OID4VP stuff is that we need to fix the deviceSigned parsing to be able to extract the correct nonce

Copy link
Contributor

@sanderPostma sanderPostma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimoGlastra
Can you please rebase against our branch feature/mdoc-parsing
I see changes in the multi-vp behaviour so I want to run all the tests in multiple projects before merging this. (I have another branch which has better tests for multi-vp sd-jwt)

package.json Outdated
@@ -48,7 +48,7 @@
"@sd-jwt/present": "^0.7.2",
"@sd-jwt/types": "^0.7.2",
"@sphereon/pex-models": "^2.3.1",
"@sphereon/ssi-types": "0.30.1",
"@sphereon/ssi-types": "file:/Users/timo/Developer/Work/Projects/Animo/Sphereon/ssi-sdk/packages/ssi-types",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ssi-sdk version is 0.30.2-next.129

@TimoGlastra TimoGlastra changed the base branch from develop to feature/mdoc-parsing October 31, 2024 12:08
@TimoGlastra
Copy link
Contributor Author

I can also split out the multi-vp handling, as it's not required for the mdoc stuff. Just something i ran into while writing the tests. Would that have your preference?

Signed-off-by: Timo Glastra <[email protected]>
@sanderPostma
Copy link
Contributor

Would that have your preference?
Will merge & test now. If the tests are passing it's fine. Otherwise I will revert those and let you know.

@sanderPostma sanderPostma merged commit 22c230c into Sphereon-Opensource:feature/mdoc-parsing Oct 31, 2024
1 check failed
@TimoGlastra
Copy link
Contributor Author

okay removed it before you merged, so her's a pr that adds it back in: #182

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