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

OpenID4VCI Plugin #47

Merged
merged 99 commits into from
Dec 6, 2023
Merged

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Dec 1, 2023

This PR adds a plugin implementing the OpenID4VCI protocol (Issuance only -- future work will address OpenID4VP and I'm not sure if that should be in this plugin or a separate one). See the README for details on the implementation.

A couple of things to call out:

  • This Plugin Implements Draft 11 of OpenID4VCI (https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-11.html).
  • We have been testing against the Sphereon Wallet. Kudos to the Sphereon team for their work on that!
  • To interoperate with Sphereon, we needed to add support for did:jwk and verifying ES256K signatures. This implementation is within this plugin right now (as a resolver and a helper method). We plan to contribute this to ACA-Py Core.
  • We plan to have automated tests against AFJ's OpenID4VCI package as soon as PR OpenId4Vc Support credo-ts#1639 is merged and released.
  • Not implemented (yet):
    • ldp_vc, sd_jwt_vc
    • Authorization Code Flow
    • Only signature suite supported by ACA-Py for jwt-vc right now is EdDSA
    • GET /.well-known/openid-configuration
    • GET /.well-known/oauth-authorization-server
    • Batch Credential Issuance
    • We're limited to DID Methods that ACA-Py supports for issuance (more can be added by Plugin, e.g. DID Web); did:sov, did:key
    • Multitenancy Support; we need to figure out the right way to identify a sub-wallet from OpenID4VCI endpoints.

@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 1, 2023

@swcurran We'd like to demo this at the next ACA-PUG call!
@jamshale Quick ACK that we have some structural things to adjust to meet the expectations of the GHA (and some failing unit tests, right this minute)

@swcurran
Copy link
Contributor

swcurran commented Dec 1, 2023

Definitely want to see the demo — great stuff! Looking forward to it!

@jamshale
Copy link
Contributor

jamshale commented Dec 1, 2023

@dbluhm I think the unit tests may be failing just because the integration test folder needs to be renamed from int --> integration or it tries to run the integration tests as unit tests.

dbluhm added a commit to dbluhm/aries-cloudagent-python that referenced this pull request Dec 1, 2023
This implements a did:jwk resolver. See
openwallet-foundation/acapy-plugins#47 for context.

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
oid4vci/pyproject.toml Outdated Show resolved Hide resolved
@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 1, 2023

Currently failing unit tests are legit failures; we'll get them taken care of (probably Monday 😄)

burdettadam and others added 22 commits December 4, 2023 14:48
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Char Howland <char@indicio.tech>
Signed-off-by: Char Howland <char@indicio.tech>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Char Howland <char@indicio.tech>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
dbluhm and others added 19 commits December 4, 2023 14:48
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 4, 2023

We'll have more developments on this plugin coming soon but this PR is ready for review!

Copy link
Contributor

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

@dbluhm, Have a look at the way we're updating the ACA-Py dependencies over here. I'd recommend updating before the PR is merged.

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 5, 2023

@dbluhm, Have a look at the way we're updating the ACA-Py dependencies over here. I'd recommend updating before the PR is merged.

Good point! Updates pushed implementing the ACA-Py as an optional dependency change adopted in other plugins.

Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @dbluhm!
Just a comment on an outdated dependency: I realize it is "just" the demo, but having an old base image will likely trigger vulnerability alerts pretty quickly after the first scan.

oid4vci/demo/frontend/Dockerfile Outdated Show resolved Hide resolved
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 5, 2023

Node version in the demo dockerfile updated. I attempted to bump it to 20 (latest LTS) but was having some weird issues. We're going to be working on the demo soon so I just bumped to 18, which worked fine on the first try, and we'll revisit when we come back around.

Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

👍🏻

@dbluhm dbluhm merged commit 2f5d32f into openwallet-foundation:main Dec 6, 2023
3 checks passed
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.

7 participants