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: sd-jwt support #132

Merged
merged 7 commits into from
Jan 13, 2024
Merged

Conversation

TimoGlastra
Copy link
Contributor

@TimoGlastra TimoGlastra commented Dec 5, 2023

Adds support for creating and evaluating presentations using SD-JWT credentials. It is dependant on Sphereon-Opensource/SSI-SDK#145 to be merged and released first.

The library is able to limit disclosures based on the presentation definition, and will construct the base KB jwt (keybinding jwt) to be added to the SD-JWT in a presentation.

Everything should work as it did before, except that we now have more types in the interfaces. So same as with other libraries, it might be nice to add an abstraction layer here as well (e.g. in the sign options we split it into jwt, json-ld and sd-jwt as the sign options differ quite a lot, and then the user has more insights on what the actual different payloads can be)

I've added SdJwt.spec.ts to test the new SD-JWT releated functionalities.

@TimoGlastra TimoGlastra changed the title update presentation definition types feat: sd-jwt support Dec 5, 2023
Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra
Copy link
Contributor Author

Also updated this PR to released version of SSI-types, should be ready to merge now

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2023

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (b41460e) 94.70% compared to head (79ba6f3) 94.16%.
Report is 5 commits behind head on develop.

❗ Current head 79ba6f3 differs from pull request most recent head 0869e92. Consider uploading reports for the commit 0869e92 to get more accurate results

Files Patch % Lines
lib/PEX.ts 83.33% 13 Missing ⚠️
...ation/handlers/limitDisclosureEvaluationHandler.ts 83.33% 5 Missing ⚠️
lib/evaluation/handlers/uriEvaluationHandler.ts 91.66% 2 Missing ⚠️
lib/PEXv1.ts 83.33% 1 Missing ⚠️
lib/PEXv2.ts 83.33% 1 Missing ⚠️
lib/evaluation/evaluationClientWrapper.ts 94.44% 1 Missing ⚠️
...uation/handlers/didRestrictionEvaluationHandler.ts 91.66% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #132      +/-   ##
===========================================
- Coverage    94.70%   94.16%   -0.55%     
===========================================
  Files           63       64       +1     
  Lines         2247     2364     +117     
  Branches       488      545      +57     
===========================================
+ Hits          2128     2226      +98     
- Misses         114      133      +19     
  Partials         5        5              
Flag Coverage Δ
unittest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Hey @nklomp also commented this on Sphereon-Opensource/OID4VC#80, but do you have by any chance some time to merge and release this PR this week? It's beginning to be a blocker on our side for finishing the OID4VCI integration in AFJ

Copy link
Contributor

@nklomp nklomp left a comment

Choose a reason for hiding this comment

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

2 remarks, for which I will create separate issues.

  • The sync and async hasher asymmetry from the user perspective
  • The need to type results from CredentialMapper after the introduction.

I will create separate tickets for these, to look at later, to not block the PR for now. We will be doing some refactoring and simplification also for PEX (removing v1 of PEX)

import { PresentationDefinitionV1VB, PresentationDefinitionV2VB, PresentationSubmissionVB, Validated, ValidationEngine } from './validation';

export interface PEXOptions {
/**
* Hasher implementation, can be used for tasks such as decoding a compact SD-JWT VC to it's encoded variant.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the exact reason. I do not immediately see why async and sync could not behave in the same way. When looking up the Hasher implementation a week ago, I noticed they have 2 types/interfaces. One basically being the async version the other the sync version. Is there a specific reason for it? Right now it is just one function, so why not have a type that is either one or the other using an OrPromise<> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the difference as otherwise a lot of top-level, now-sync, API methods have to become asynchronous in PEX, which I wanted to be cautious of.

If that's ok with you then i can combine them

@nklomp nklomp merged commit c99b1c2 into Sphereon-Opensource:develop Jan 13, 2024
1 check passed
@TimoGlastra
Copy link
Contributor Author

The need to type results from CredentialMapper after the introduction.

What do you mean with this exactly?

@TimoGlastra
Copy link
Contributor Author

removing v1 of PEX

We're still actively using v1. But as the changes are so minimal I think we can use your library with v2 and do some small transformations to map between v1/v2

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.

3 participants