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.

Sign up for GitHub

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

Jump to bottom

JwkStorageDocument & JwtCredential validation #1152

Merged
merged 34 commits into from
May 12, 2023
Merged

JwkStorageDocument & JwtCredential validation #1152

merged 34 commits into from
May 12, 2023

Conversation

abdulmth
Copy link
Contributor

@abdulmth abdulmth commented Apr 5, 2023

Description of change

see #1148

Links to any relevant issues

Be sure to reference any related issues by adding fixes issue #.

Type of change

Add an x to the boxes that are relevant to your changes.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Describe the tests that you ran to verify your changes.
Make sure to provide instructions for the maintainer as well as any relevant configurations.

Change checklist

Add an x to the boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@abdulmth abdulmth force-pushed the feat/jwk-storage-document-ext-2 branch from f1d8aa0 to 7b7a2d9 Compare April 5, 2023 17:25
@abdulmth abdulmth added Wasm Related to Wasm bindings. Becomes part of the Wasm changelog Breaking change A change to the API that requires a major release. Part of "Changed" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. labels Apr 6, 2023
PhilippGackstatter
PhilippGackstatter reviewed Apr 23, 2023
View reviewed changes
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

Preliminary comments on the Rust part, will do Wasm next.

identity_core/src/error.rs Outdated Show resolved Hide resolved
identity_core/src/error.rs Outdated Show resolved Hide resolved
identity_credential/src/credential/jwt_serialization.rs Outdated Show resolved Hide resolved
identity_credential/src/validator/mod.rs Show resolved Hide resolved
identity_credential/src/validator/vc_jwt_validation/credential_jwt_validation_options.rs Show resolved Hide resolved
identity_storage/src/storage/jwk_storage_document_ext.rs Outdated Show resolved Hide resolved
identity_storage/src/storage/jwk_storage_document_ext.rs Outdated Show resolved Hide resolved
identity_storage/src/storage/jwk_storage_document_ext.rs Show resolved Hide resolved
libjose/src/jwk/key_params.rs Show resolved Hide resolved
identity_storage/src/storage/tests/credential_validation.rs Outdated Show resolved Hide resolved
PhilippGackstatter
PhilippGackstatter approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter 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 to me. We should take care of the TODOs in the code and then remove them to clean up. We should remove the old JCS implementation and the old credential and presentation validator.

bindings/wasm/src/credential/jwt_credential_validation/credential_token.rs Outdated Show resolved Hide resolved
bindings/wasm/src/credential/jwt_credential_validation/jwt_credential_validator.rs Outdated Show resolved Hide resolved
bindings/wasm/src/credential/jwt_credential_validation/options.rs Outdated Show resolved Hide resolved
bindings/wasm/src/credential/jwt_credential_validation/options.rs Outdated Show resolved Hide resolved
bindings/wasm/src/did/wasm_core_document.rs Outdated Show resolved Hide resolved
bindings/wasm/src/iota/iota_document.rs Show resolved Hide resolved
bindings/wasm/src/iota/iota_document.rs Show resolved Hide resolved
bindings/wasm/src/storage/jwk_storage.rs Outdated Show resolved Hide resolved
bindings/wasm/src/verification/custom_verification.rs Outdated Show resolved Hide resolved
bindings/wasm/tests/jwk_storage.ts Outdated Show resolved Hide resolved
Abdulrahim Al Methiab and others added 23 commits April 26, 2023 22:24
minor comments
aa4b485
add constructor for WasmJwtCredentialValidationOptions
50317cb
minor fixes
0aa75bb
@PhilippGackstatter
Fix RevocationBitmap typo
4626476
@PhilippGackstatter
Return CredentialToken<T> where possible
366afab
@PhilippGackstatter
Improve error messages
6976c16
@PhilippGackstatter
Improve method name
71c1a1d
@PhilippGackstatter
Improve error messages, remove resolved TODOs
7111b21
@PhilippGackstatter
Wasm cleanup
4314d50
@PhilippGackstatter
Remove the removed JwkStorage.public method
43b0798
@PhilippGackstatter
Fix clippy lints
7ff1674
@PhilippGackstatter
Test some cases for credential validation
8ff09e3
@PhilippGackstatter
Update b64 docs in WasmJwsSignatureOptions
7f6bf8d
@PhilippGackstatter
Add nonce verification during credential verif.
f4c1c97
@PhilippGackstatter
Add b64=false and detached=true test
5b6ca9c
@PhilippGackstatter
Remove user-facing crit JOSE parameter
00f1b45
@PhilippGackstatter
Add test setup and organize tests
2638b4b
@abdulmth
Add wrappers for JWT and JWS (#1160)
9c0c071
@PhilippGackstatter
Add more credential validator tests
9eb5599
@PhilippGackstatter
Merge remote-tracking branch 'origin/feat/jwk-storage-document-ext-2'…
36dafff 
… into feat/jwk-storage-document-ext-2
@PhilippGackstatter
Add custom error for external extensibility
d39a054
@PhilippGackstatter
Make source in custom error optional
58bb4b8
@PhilippGackstatter
Merge remote-tracking branch 'origin/main' into feat/jwk-storage-docu…
21eca06 
…ment-ext-2
@PhilippGackstatter PhilippGackstatter merged commit a6c22f5 into main May 12, 2023
@PhilippGackstatter PhilippGackstatter deleted the feat/jwk-storage-document-ext-2 branch May 12, 2023 09:39
@eike-hass eike-hass mentioned this pull request May 12, 2023
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilippGackstatter PhilippGackstatter PhilippGackstatter approved these changes

Assignees
No one assigned
Labels
Breaking change A change to the API that requires a major release. Part of "Changed" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abdulmth @PhilippGackstatter @eike-hass