feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs #600

ianhamilton87 · 2024-08-15T14:58:58Z

Enhance did:key functionality to resolve EBSI-style did:keys.

EBSI uses the JWK_JCS-PUB multicodec as noted in the document linked above. JWK_JCS-PUB is not exclusive to EBSI though and may be used elsewhere.

Relevant issues that are related to this PR:
w3c-ccg/did-method-key#63
#536

Re-do of former PR #545

Enhance did:key functionality to resolve [EBSI-style did:keys](https://hub.ebsi.eu/vc-framework/did/natural-person). They use the JWK_JCS-PUB multicodec. Relevant issues that are related to this PR: [w3c-ccg/did-method-key#63](w3c-ccg/did-method-key#63) [#536](#536) Re-do of former PR #545

This reverts commit 43c9843.

Enhance did:key functionality to resolve [EBSI-style did:keys](https://hub.ebsi.eu/vc-framework/did/natural-person). They use the JWK_JCS-PUB multicodec. Relevant issues that are related to this PR: [w3c-ccg/did-method-key#63](w3c-ccg/did-method-key#63) [#536](#536) Re-do of former PR #545

timothee-haudebourg

Hi, thanks for this PR! Is it just about adding the JWK_JCS_PUB decoding format? It doesn't seem directly related to EBSI Natural Person, unless this codec is exclusively used by this specification? Can you clarify and reflect that in the PR title and description?

timothee-haudebourg · 2024-08-23T12:30:21Z

crates/jwk/src/lib.rs

@@ -102,6 +102,10 @@ impl FromStr for JWK {
    }
 }

+pub fn from_bytes(bytes: &[u8]) -> Result<JWK, serde_json::Error> {


Can you make this a constructor of JWK? And also add a TryFrom<&[u8]> implementation?

@timothee-haudebourg - ok. I moved it down to line 313, within the JWK implementation. I hope that's what you mean by make it a constructor.
And I added the TryFrom implementation too.

timothee-haudebourg · 2024-08-23T12:31:49Z

crates/jwk/src/lib.rs

-                    exponent: Some(e1),
-                    ..
-                }),
+                                modulus: Some(n1),


I think there is a formatting issue here. Did you run cargo fmt?

@timothee-haudebourg Yikes! I did. Should be good now.

Enhance did:key functionality to resolve [EBSI-style did:keys](https://hub.ebsi.eu/vc-framework/did/natural-person). They use the JWK_JCS-PUB multicodec. Relevant issues that are related to this PR: [w3c-ccg/did-method-key#63](w3c-ccg/did-method-key#63) [#536](#536) Re-do of former PR #545

code review fixes

ianhamilton87 · 2024-08-28T14:18:52Z

Hi, thanks for this PR! Is it just about adding the JWK_JCS_PUB decoding format? It doesn't seem directly related to EBSI Natural Person, unless this codec is exclusively used by this specification? Can you clarify and reflect that in the PR title and description?

@timothee-haudebourg - I have adjusted the summary and description to make it less EBSI specific.
EBSI proposes using did:key with the JWK_JCS-PUB format, but it has no exclusive use of that multicodec. That multicodec existed before EBSI decided to use it as far as I can see.
There's a link to an EBSI website that explains all this.
I figured mentioning EBSI would get more folks to want this change.
Why they can't juse did:jwk, who knows 🤷 .

timothee-haudebourg

Can you add an implementation of ssi_multicodec::Codec for JWK? Using JWK_JCS_PUB as the codec for JWK. This will also allow use to implement and test encoding. You can use serde_jcs which I think is already a dependency of ssi to convert a JWK into JSON with JCS. It's a bit old now so if you prefer something else like serde_json_canonicalizer it's fine.

timothee-haudebourg · 2024-08-30T08:39:40Z

crates/jwk/src/multicodec.rs

@@ -47,6 +47,9 @@ impl JWK {
            ssi_multicodec::BLS12_381_G2_PUB => {
                crate::bls12381g2_parse(k).map_err(FromMulticodecError::Bls12381G2Pub)
            }
+            ssi_multicodec::JWK_JCS_PUB => {


Can you add a test checking that its able to decode a JWK encoded with the JWK_JCS_PUB multicodec?

Added test for from_multicodec w/ JWK_JCS_PUB. Added an implementation of ssi_multicodec::Codec for JWK_JCS_PUB. Because of that, removed the existing from_bytes() method.

ianhamilton87 · 2024-09-04T18:22:40Z

Can you add an implementation of ssi_multicodec::Codec for JWK? Using JWK_JCS_PUB as the codec for JWK. This will also allow use to implement and test encoding. You can use serde_jcs which I think is already a dependency of ssi to convert a JWK into JSON with JCS. It's a bit old now so if you prefer something else like serde_json_canonicalizer it's fine.

@timothee-haudebourg I have done this. Hopefully, I have done this correctly. This is my first foray into Rust, so thank you for your patience.

timothee-haudebourg

It looks great 😄
Just use MultiEncodedBuf::encode to encode the key in the new test and we're good.

timothee-haudebourg · 2024-09-06T08:03:20Z

crates/jwk/src/multicodec.rs

+        // it will see the P256 key and assign a multicodec of 0x1200.
+        // For jwk_jcs_pub multicodecs, we can only decode them
+        let jwk_buf =
+            MultiEncodedBuf::encode_bytes(ssi_multicodec::JWK_JCS_PUB, jwk_str.as_bytes());


Since you implemented JWK: Codec you can use MultiEncodedBuf::encode(&jwk).

@timothee-haudebourg Done!

Fixed test_multicodec_jwk_jcs_pub()

ianhamilton87 · 2024-09-17T12:55:14Z

@timothee-haudebourg - mind doing one final review + approving?

timothee-haudebourg · 2024-09-18T09:43:39Z

Yes sorry for the delay. Just waiting for the CI to pass and we'll be good.

timothee-haudebourg

The CI workflow is failing because of some missing feature gates.

timothee-haudebourg · 2024-09-18T10:10:03Z

crates/jwk/src/multicodec.rs

+
+    #[test]
+    fn test_multicodec_jwk_jcs_pub() {
+        let jwk = JWK::generate_p256();


This won't work without the secp256r1 feature enabled. You can feature-gate the test.

@timothee-haudebourg - sorry about that. This is my first foray into Rust, can you tell?
Hopefully this fixes everything.

feature gate test

ianhamilton87 · 2024-09-18T14:20:35Z

@timothee-haudebourg - ok. I feature-gated the tests. Mind approving the workflow so the tests can re-run?
(Note: locally the test always ran successfully, so there must be something in my setup that prevents me from recreating this locally)

timothee-haudebourg · 2024-09-18T14:35:56Z

locally the test always ran successfully

That's because some features are enabled by default. The CI workflow is actually testing a range of feature combinations (that's why it takes so much time).

timothee-haudebourg · 2024-09-18T16:08:11Z

All good! Thanks again!

ianhamilton87 · 2024-09-18T17:30:06Z

@timothee-haudebourg Thanks!

ianhamilton87 added 5 commits August 15, 2024 10:49

Merge branch 'spruceid:main' into main

eaeb8a8

Revert "feat(did-key): Add EBSI Natural Person Format"

5faedc3

This reverts commit 43c9843.

Merge remote-tracking branch 'origin/main'

5a03909

ianhamilton87 mentioned this pull request Aug 15, 2024

Looking to get PR approved #546

Closed

timothee-haudebourg requested changes Aug 23, 2024

View reviewed changes

timothee-haudebourg self-assigned this Aug 23, 2024

ianhamilton87 changed the title ~~feat(did-key): Add EBSI Natural Person Format~~ feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs Aug 28, 2024

ianhamilton87 added 5 commits August 28, 2024 09:05

Merge branch 'spruceid:main' into main

79c8969

Merge remote-tracking branch 'origin/did-key-ebsi' into did-key-ebsi

cf6984e

feat(did-key): Add EBSI Natural Person Format

05f0283

code review fixes

feat(did-key): Add EBSI Natural Person Format

0ed793a

code review fixes

ianhamilton87 requested a review from timothee-haudebourg August 28, 2024 14:20

timothee-haudebourg requested changes Aug 30, 2024

View reviewed changes

feat(did-key): Add EBSI Natural Person Format

78a6adb

Added test for from_multicodec w/ JWK_JCS_PUB. Added an implementation of ssi_multicodec::Codec for JWK_JCS_PUB. Because of that, removed the existing from_bytes() method.

ianhamilton87 requested a review from timothee-haudebourg September 4, 2024 18:22

timothee-haudebourg requested changes Sep 6, 2024

View reviewed changes

feat(did-key): Add EBSI Natural Person Format

c9ec5bc

Fixed test_multicodec_jwk_jcs_pub()

ianhamilton87 requested a review from timothee-haudebourg September 9, 2024 16:58

feat(did-key): Add EBSI Natural Person Format

b3b301b

Fixed test_multicodec_jwk_jcs_pub()

timothee-haudebourg requested changes Sep 18, 2024

View reviewed changes

feat(did-key): Add EBSI Natural Person Format

9953e2c

feature gate test

ianhamilton87 requested a review from timothee-haudebourg September 18, 2024 14:18

timothee-haudebourg approved these changes Sep 18, 2024

View reviewed changes

timothee-haudebourg merged commit a585e0c into spruceid:main Sep 18, 2024
4 checks passed

ianhamilton87 deleted the did-key-ebsi branch September 20, 2024 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs #600

feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs #600

ianhamilton87 commented Aug 15, 2024 •

edited

Loading

timothee-haudebourg left a comment

timothee-haudebourg Aug 23, 2024

ianhamilton87 Aug 28, 2024

timothee-haudebourg Aug 23, 2024

ianhamilton87 Aug 28, 2024

ianhamilton87 commented Aug 28, 2024

timothee-haudebourg left a comment •

edited

Loading

timothee-haudebourg Aug 30, 2024

ianhamilton87 Sep 4, 2024

ianhamilton87 commented Sep 4, 2024 •

edited

Loading

timothee-haudebourg left a comment •

edited

Loading

timothee-haudebourg Sep 6, 2024

ianhamilton87 Sep 9, 2024

ianhamilton87 commented Sep 17, 2024

timothee-haudebourg commented Sep 18, 2024

timothee-haudebourg left a comment

timothee-haudebourg Sep 18, 2024

ianhamilton87 Sep 18, 2024

ianhamilton87 commented Sep 18, 2024

timothee-haudebourg commented Sep 18, 2024

timothee-haudebourg commented Sep 18, 2024

ianhamilton87 commented Sep 18, 2024

feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs #600

feat(did-key): Add Support To Resolve JWK_JCS-PUB encoded did:key DIDs #600

Conversation

ianhamilton87 commented Aug 15, 2024 • edited Loading

timothee-haudebourg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianhamilton87 commented Aug 28, 2024

timothee-haudebourg left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianhamilton87 commented Sep 4, 2024 • edited Loading

timothee-haudebourg left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianhamilton87 commented Sep 17, 2024

timothee-haudebourg commented Sep 18, 2024

timothee-haudebourg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianhamilton87 commented Sep 18, 2024

timothee-haudebourg commented Sep 18, 2024

timothee-haudebourg commented Sep 18, 2024

ianhamilton87 commented Sep 18, 2024

ianhamilton87 commented Aug 15, 2024 •

edited

Loading

timothee-haudebourg left a comment •

edited

Loading

ianhamilton87 commented Sep 4, 2024 •

edited

Loading

timothee-haudebourg left a comment •

edited

Loading