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: support more key types in jws service #1453

Merged
merged 7 commits into from
May 17, 2023

Conversation

TimoGlastra
Copy link
Contributor

@TimoGlastra TimoGlastra commented May 10, 2023

NOTE: this pr is dependant on #1446 and thus should be merged after that PR is merged.

This PR refactors the JWK and JWS implementation to have better support for JWK / JWA, and also support more algs/key types in the JWS service. Basically the JWS service now supports all JWKs supported in AFJ, although it's limited by the algs and key types supported by the wallet (for askar we support EdDsa and ES256)

@TimoGlastra TimoGlastra requested a review from a team as a code owner May 10, 2023 19:29
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2023

Codecov Report

Merging #1453 (8829207) into main (9a43afe) will increase coverage by 0.29%.
The diff coverage is 95.38%.

@@            Coverage Diff             @@
##             main    #1453      +/-   ##
==========================================
+ Coverage   85.54%   85.84%   +0.29%     
==========================================
  Files         886      910      +24     
  Lines       21150    21676     +526     
  Branches     3635     3709      +74     
==========================================
+ Hits        18093    18607     +514     
- Misses       2882     2890       +8     
- Partials      175      179       +4     
Impacted Files Coverage Δ
...ds/domain/verificationMethod/VerificationMethod.ts 100.00% <ø> (ø)
packages/askar/src/wallet/AskarWallet.ts 65.71% <50.00%> (+0.31%) ⬆️
...es/core/src/modules/vc/libraries/documentLoader.ts 33.33% <50.00%> (+3.92%) ⬆️
packages/core/src/crypto/JwsService.ts 73.91% <63.63%> (-8.23%) ⬇️
.../src/modules/dids/domain/key-type/keyDidMapping.ts 83.33% <70.00%> (-7.58%) ⬇️
packages/core/src/crypto/jose/jwk/Jwk.ts 80.00% <80.00%> (ø)
packages/core/src/crypto/jose/jwk/transform.ts 83.33% <83.33%> (ø)
packages/core/src/crypto/keyUtils.ts 85.71% <87.50%> (+10.71%) ⬆️
...ore/src/modules/dids/methods/jwk/JwkDidResolver.ts 88.88% <88.88%> (ø)
...ges/core/src/modules/dids/domain/keyDidDocument.ts 98.14% <94.11%> (-1.86%) ⬇️
... and 52 more

Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
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.

Left a couple of small comments again that should be relatively easy to resolve. Great work Timo!

packages/core/src/crypto/JwsService.ts Show resolved Hide resolved
packages/core/src/crypto/JwsService.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/JwsService.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/__tests__/JwsService.test.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/jose/jwk/P_256Jwk.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/jose/jwk/P_256Jwk.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/jose/jwk/P_256Jwk.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/jose/jwk/P_521Jwk.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/keyUtils.ts Show resolved Hide resolved
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
@TimoGlastra TimoGlastra merged commit 8a3f03e into openwallet-foundation:main May 17, 2023
@TimoGlastra TimoGlastra deleted the feat/jws-service branch May 17, 2023 09:11
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