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: OpenId4VC support #1604

Closed

Conversation

auer-martin
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

Merging #1604 (6eeee9a) into main (a0458fe) will decrease coverage by 8.47%.
Report is 6 commits behind head on main.
The diff coverage is 49.73%.

@@            Coverage Diff             @@
##             main    #1604      +/-   ##
==========================================
- Coverage   85.76%   77.29%   -8.47%     
==========================================
  Files         951      926      -25     
  Lines       22854    22853       -1     
  Branches     4017     4115      +98     
==========================================
- Hits        19601    17665    -1936     
- Misses       3069     4835    +1766     
- Partials      184      353     +169     
Files Coverage Δ
packages/core/src/index.ts 100.00% <100.00%> (ø)
packages/core/src/modules/vc/index.ts 100.00% <100.00%> (ø)
...nid4vc-holder/src/OpenId4VcHolderServiceOptions.ts 100.00% <100.00%> (ø)
packages/openid4vc-holder/src/index.ts 100.00% <100.00%> (ø)
...d4vc-holder/src/issuance/OpenId4VciHolderModule.ts 100.00% <100.00%> (ø)
...kages/openid4vc-holder/src/issuance/utils/index.ts 100.00% <100.00%> (ø)
...ckages/openid4vc-holder/src/presentations/index.ts 100.00% <100.00%> (ø)
...nid4vc-holder/src/presentations/selection/index.ts 100.00% <100.00%> (ø)
packages/openid4vc-holder/tests/fixtures.ts 100.00% <100.00%> (ø)
...odules/vc/data-integrity/SignatureSuiteRegistry.ts 50.00% <0.00%> (ø)
... and 9 more

... and 202 files with indirect coverage changes

@auer-martin auer-martin force-pushed the feat/openid4vc branch 2 times, most recently from 150cc9b to 43a1676 Compare October 10, 2023 07:48
}

// what does this mean
const openId4VcCredentialMetadataKey = '_paradym/openId4VcCredentialMetadata'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a storage key that can be used to retrieve metadata about the credential. Likely for ease of access for displaying. @TimoGlastra likely knows whether we have to keep an equivalent or if this is up to the user to define.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PR is not yet finished. Just pushed some progress

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the display / issuer metadata. I chose to not add it as PR in AFJ as the spec is continuously evolving and thus it will contain different structures in the metadata. Not sure if that's a problem. It is very useful to have this though, but it may be better if we can generalize it and create a universal credential display format (not sure what OCA uses for this) with something like https://identity.foundation/wallet-rendering/

Copy link
Contributor

@MosCD3 MosCD3 Oct 11, 2023

Choose a reason for hiding this comment

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

I am implementing OpenID4VCI in our wallet and in Bifold in the mean time, this PR is a life saver since am not able to use Spheroin for testing with current package version. @TimoGlastra any suggestions on a good testbed (issuer)that supports older draft to use for development until this PR is merged?
For OCA, it will be nice if there is an option to inject an OCA bundle identifier into one of the attributes to use it later for fetching OCA layers for that specific VC. The whole thing is running perfectly in Bifold with the option to remote fetch OCA bundles, the missing part is AFJ handling that as part of credential attributes instead of manually matching with credential/schema ids. Could discuss further

Copy link
Contributor

Choose a reason for hiding this comment

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

@MosCD3 for draft 8 you can I think use https://jff.walt.id/ and for draft 11 you can use https://launchpad.mattrlabs.com/

@auer-martin auer-martin force-pushed the feat/openid4vc branch 2 times, most recently from bc4d47d to ed1641b Compare October 16, 2023 16:02
@auer-martin auer-martin force-pushed the feat/openid4vc branch 3 times, most recently from eaa8e82 to ee8753e Compare October 26, 2023 12:19
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Great work @2mau!!! I think there is some cleanup to do, but thats always expected with such a big PR.

.github/workflows/continuous-deployment.yml Outdated Show resolved Hide resolved
packages/openid4vc-holder/src/index.ts Outdated Show resolved Hide resolved
packages/openid4vc-issuer/package.json Outdated Show resolved Hide resolved
packages/openid4vc-issuer/src/OpenId4VcIssuerApi.ts Outdated Show resolved Hide resolved
packages/openid4vc-verifier/src/OpenId4VcVerifierApi.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Still some minor comments, but nothing blocking from my side.

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