Add did-jwk support

[theosirian]

Adds did-jwk support as described in https://github.com/quartzjer/did-jwk/blob/main/spec.md

Although the spec mentions canonicalization is optional, it has been implemented here with serde_jcs. So generally, we can expect the same jwk, with an arbitrary field ordering on input, to generate the same did-jwk identifier when using SSI.

[chunningham]

looks great 👍 Could be worth it to include more tests, although looks like you have covered all the spec-provided test cases (at least one should-fail case could be handy as this did method is just a function)

[cobward]

I'm not familiar with how we use ResolutionMetadata usually, but could you just format the error as a string in the error field?

[sbihel]

Not sure if it's not a specified set https://w3c-ccg.github.io/did-resolution/#errors (but why wouldn't we use an enum if it were)

[cobward]

Aha yes:

The error code from the resolution process. This property is REQUIRED when there is an error in the resolution process. The value of this property MUST be a single keyword ASCII string. The possible property values of this field SHOULD be registered in the DID Specification Registries [DID-SPEC-REGISTRIES].

[cobward]

In this case I think the TODO can just be removed.

[sbihel]

Opened #468

[sbihel]

Do you know if this never fails?

[theosirian]

From docs.rs:

Serialization can fail if T's implementation of Serialize fails.

If JWK is safe to serialize then it shouldn't be a problem, otherwise I can add error handling.

[chunningham]

IMO we should change the .unwrap() to .ok()?

[cobward]

Just noticed something in the spec that might need addressing:

Only JWKs containing public key material may be used, a JWK for a private key must never be used and must be rejected by all implementations when encountered.

From that I would expect that the resolve method should check that the encoded JWK does not contain the private key (and if so return an error instead of resolving?).

Also if we are being very precise about the spec then I would expect generate to return None instead of converting it to a public key.

I might be misunderstanding the strength of the language used in the spec though :)

[sbihel]

Could you rebase please?

[theosirian]

@sbihel rebased.

@cobward I wasn't sure myself, maybe @awoie can help us here.

I chose to go with .to_public in generate because from the outside user perspective in DIDKit that wouldn't be available, the user would have to figure it out themselves.

Hadn't thought of the resolve perspective, it could happen, there could be a .to_public call there too, but there isn't much to do to remediate if the did has been created and shared with the private key material.

[awoie]

i believe the only requirement is that we shouldn't allow private key material in the <JWK> in the did:jwk:<JWK>.

[theosirian]

I have kept the .to_public() call in generation and, in dereference, added a comparison of the decoded JWK against its .to_public() conversion, throwing an ERROR_INVALID_DID if they do not match together with a matching test for this case. I believe that should be enough to cover that requirement. Also rebased to current main.

[sbihel]

Could you still address the leftover TODO and unwrap?

[theosirian]

@sbihel done and rebased.