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: jarm #147

Merged
merged 13 commits into from
Oct 3, 2024
Merged

feat: jarm #147

merged 13 commits into from
Oct 3, 2024

Conversation

auer-martin
Copy link
Contributor

@auer-martin auer-martin commented Sep 10, 2024

Some left todos.

  • Either
    • Allow passing in the encrypted response as string to the lib and decrypt inside the vp library
    • Decrypt outside
  • Move MdocVerifiablePresentation to the SSI SDK. (Should not be a class)
    The version is currently not inferred correctly. This is probably due to a type issue with the MDOC presentation definition input.
  • No Revocation Verification for Mdoc
  • Nonce handling
  • Authorization Response Encryption key extraction. (Maybe we should just allow passing in the jwk instead of extracting the encryption jwk from the metadata)
  • Pex Integration

@TimoGlastra TimoGlastra changed the title feat: jarm (not finished) feat: jarm and mdoc openid4vp (not finished) Sep 10, 2024
@sanderPostma
Copy link
Contributor

This is going to clash with our mdoc support branch which is soon to be merged.
See https://github.com/Sphereon-Opensource/SSI-SDK/blob/f6f9cace14a1cee5340ecf0db1c0606d42159084/packages/ssi-types/src/types/vc.ts#L21

@TimoGlastra
Copy link
Collaborator

This is going to clash with our mdoc support branch which is soon to be merged.

I think it's best if we update to your mdoc support then as the todo comments also mentions "Move MdocVerifiablePresentation to the SSI SDK. (Should not be a class)". So if you added the needed types to the ssi-sdk we can cross that one off :)

@TimoGlastra
Copy link
Collaborator

Either

  • Allow passing in the encrypted response as string to the lib and decrypt inside the vp library
  • Decrypt outside

I think we should allow passing in the encrypted response. Same like we allow passing the signed request.

  • Authorization Response Encryption key extraction. (Maybe we should just allow passing in the jwk instead of extracting the encryption jwk from the metadata)

I think for the Funke we need to extract it from jwks in the client_metadata. So maybe we can allow to pass a key, but if it's not provided we try to extract it from the metadata? I think for most normal cases it'll then work out of the box, as long as you provide a decryption callback.

Pex Integration

@sanderPostma I saw you also had some MDOC integration work in the PEX library. Is the idea that PEX will support MDOC natively, and that it can also process e.g. a submission including both mdoc and sd-jwt presentations?

@auer-martin
Copy link
Contributor Author

auer-martin commented Sep 27, 2024

I will start moving the external depdency. Are you okay with the general abstractions?

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.

See remark about kid in the callback interface. Rest looks okay to proceed

packages/siop-oid4vp/lib/rp/RP.ts Outdated Show resolved Hide resolved
@auer-martin auer-martin changed the title feat: jarm and mdoc openid4vp (not finished) feat: jarm Oct 1, 2024
@auer-martin auer-martin marked this pull request as ready for review October 1, 2024 10:11
@TimoGlastra
Copy link
Collaborator

It seems this test is failing @auer-martin

succeed in a full flow with the client using OpenID4VCI version 11 and jwt_vc_json

@TimoGlastra TimoGlastra requested review from nklomp and jcmelati October 1, 2024 14:45
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.

Some small remarks. The client_id aud validation seems to be incorrect

@TimoGlastra
Copy link
Collaborator

Is this ready to merge now @nklomp?

@nklomp
Copy link
Contributor

nklomp commented Oct 3, 2024

Yes, needed to do a round of testing

@nklomp nklomp merged commit 6b2cd09 into Sphereon-Opensource:develop Oct 3, 2024
1 check failed
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.

5 participants