Skip to content

feat(openid4vc-client): openid authorization flow #1384

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

Merged
merged 20 commits into from
Mar 22, 2023

Conversation

karimStekelenburg
Copy link
Contributor

@karimStekelenburg karimStekelenburg commented Mar 17, 2023

This PR contains the logic for the OpenID For Verifiable Credentials Issuance authorization flow.

Funded by the Ontario Government

Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Nice! Mostly some small stylistic suggestions

@TimoGlastra
Copy link
Contributor

Seems DCO is failing BTW

@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2023

Codecov Report

Merging #1384 (24003e2) into main (d6e2ea2) will increase coverage by 0.00%.
The diff coverage is 86.00%.

@@           Coverage Diff           @@
##             main    #1384   +/-   ##
=======================================
  Coverage   81.84%   81.85%           
=======================================
  Files         788      788           
  Lines       19408    19445   +37     
  Branches     3151     3162   +11     
=======================================
+ Hits        15884    15916   +32     
- Misses       3517     3522    +5     
  Partials        7        7           
Impacted Files Coverage Δ
...ckages/core/src/modules/vc/W3cCredentialService.ts 79.41% <66.66%> (+0.15%) ⬆️
...ges/openid4vc-client/src/OpenId4VcClientService.ts 82.52% <84.61%> (+0.57%) ⬆️
...ackages/openid4vc-client/src/OpenId4VcClientApi.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@karimStekelenburg
Copy link
Contributor Author

Thanks, @TimoGlastra. I've processed your comments.

You were right, the two implementations had huge overlaps, and I've merged them together in the service. I did create two separate methods in the API. I figured that is a clearer API than optional values depending on the flow type.

Regarding the code_verifier stuff, does the following description resolve your questions?

In OpenID Connect (OIDC) with Proof Key for Code Exchange (PKCE), the code_verifier is a random string used as a cryptographic challenge when requesting an authorization code from the authorization server.

PKCE is a security mechanism designed to prevent certain types of attacks that can occur when using the Authorization Code flow in OAuth 2.0. Specifically, PKCE is intended to protect against code injection attacks where an attacker intercepts the authorization code and uses it to obtain access tokens without the user's knowledge.

The PKCE flow involves creating a random code_verifier string, calculating a hash of this string, and then sending both the code_verifier and the hash (called the code_challenge) to the authorization server when requesting an authorization code. Later, when the client exchanges the authorization code for an access token, it must again send the code_verifier to the authorization server, which will use it to verify that the code_challenge matches the original code_verifier.

By requiring the code_verifier in addition to the code_challenge, PKCE ensures that only the client that initiated the authorization request can obtain an access token, even if an attacker is able to intercept the authorization code.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
The "@sphereon/openid4vci-client" dependency has been updated from version 0.3.6 to 0.4.0, and "@sphereon/ssi-types" has been updated from version 0.8.1-next.123 to 0.9.0. This update includes bug fixes and new features.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
This commit adds a method to request a credential using the authorization flow.

Additionally, the following to helper methods are added:
- to generate a code verifier
- to construct the authorization url (this url is to be visited by the user)

Signed-off-by: Karim Stekelenburg <karim@animo.id>
The package.json file now includes two new dependencies, "@stablelib/random" and "@stablelib/sha256". These dependencies are added to improve the security of the application. In the openid4vc-client.e2e.test.ts file, a typo in the "acquireAccessTokenResponse" variable name has been fixed.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
The ts-ignore comment was removed as it is not recommended to use it in the codebase. The code was refactored to remove the comment and ensure that the code is type-safe.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
…thod call on codeVerifier property in debug log message

Signed-off-by: Karim Stekelenburg <karim@animo.id>
feat(openid4vc-client): use Hasher from @aries-framework/core to hash codeVerifier

Signed-off-by: Karim Stekelenburg <karim@animo.id>
…alidation before assertCredentialHasFormat call

Signed-off-by: Karim Stekelenburg <karim@animo.id>
…t with a variable named credentialFormat in acquireCredentialsUsingProof method.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
…in acquireCredentialsUsingProof method call

Signed-off-by: Karim Stekelenburg <karim@animo.id>
…eter from verifyCredential method

feat(W3cCredentialServiceOptions.ts): add verifyRevocationState parameter to VerifyCredentialOptions interface
refactor(OpenId4VcClientService.ts): pass verifyRevocationState parameter as an object property to verifyCredential method in W3cCredentialService

Signed-off-by: Karim Stekelenburg <karim@animo.id>
… arguments to an object for better readability

Signed-off-by: Karim Stekelenburg <karim@animo.id>
… in TypedArrayEncoder.toBase64URL() method call

Signed-off-by: Karim Stekelenburg <karim@animo.id>
Signed-off-by: Karim Stekelenburg <karim@animo.id>
…d to requestCredentialUsingPreAuthorizedCode and add flowType parameter

feat(OpenId4VcClientApi.ts): add requestCredentialUsingAuthorizationCode method and flowType parameter
refactor(OpenId4VcClientService.ts): rename requestCredential2 to requestCredential and remove unused variable

Signed-off-by: Karim Stekelenburg <karim@animo.id>
Signed-off-by: Karim Stekelenburg <karim@animo.id>
feat(openid4vc-client.e2e.test.ts): replace requestCredential with requestCredentialUsingPreAuthorizedCode and requestCredentialUsingAuthorizationCode methods

Signed-off-by: Karim Stekelenburg <karim@animo.id>
fix(vc): make verifyRevocationState optional in VerifyCredentialOptions
feat(vc): set verifyRevocationState to true by default in W3cCredentialService

Signed-off-by: Karim Stekelenburg <karim@animo.id>
Removed method to generate codeVerifier from the API.

Signed-off-by: Karim Stekelenburg <karim@animo.id>
Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Some final nits for consistency, but LGTM!!

Signed-off-by: Karim Stekelenburg <karim@animo.id>
@TimoGlastra
Copy link
Contributor

Nice !!

@TimoGlastra TimoGlastra changed the title feat: OpenID Authorization Flow feat(openid4vc-client): openid authorization flow Mar 22, 2023
@TimoGlastra TimoGlastra enabled auto-merge (squash) March 22, 2023 13:46
@TimoGlastra TimoGlastra merged commit 996c08f into openwallet-foundation:main Mar 22, 2023
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