Refactor `@web5/dids` to support first class Key Management #393

frankhinek · 2024-01-31T15:11:21Z

Summary

The primary goal of this PR is to refactor the @web5/dids package to use the refactored @web5/crypto package that substantively changed how key management was approached. In addition to making key management integral to the DID method implementations, it also changes the DID method API to include methods for instantiating DID objects from existing DIDs/keys. The same pattern is being implemented across web5-js, web5-kt, web5-go, etc.

Signed-off-by: Frank Hinek <[email protected]>

…or dids Signed-off-by: Frank Hinek <[email protected]>

Signed-off-by: Frank Hinek <[email protected]>

packages/crypto/src/primitives/xchacha20-poly1305.ts

Signed-off-by: Frank Hinek <[email protected]>

packages/dids/src/did-error.ts

packages/dids/src/did-uri.ts

nitro-neal · 2024-01-31T18:30:49Z

packages/dids/src/methods/did-dht.ts

+    const did = await DidDht.fromPublicKeys({ keyManager, options, ...keySet });
+
+    // By default, publish the DID document to a DHT operations endpoint unless explicitly disabled.
+    did.metadata.published = options.publish ?? true


what happens when someone is on a plane and we try to publish?
timeout exception?

As currently implemented, if the gateway/relay cannot be reached when attempting to publish an exception will be raised.

One option is to pass publish: false so that publishing isn't attempting (when calling create() or fromKeys())... but that assumes the application is able to detect a lack of network connectivity and adapt.

One of the downsides to the current approach is that if a DID is being created for the first time (calling create() to generate keys) and an exception is thrown during publishing, the newly generated keys aren't returned to the caller. By the time the exception is raised, the keys will have already been created and stored in the KeyManager store so they end up "orphaned" since the function raises an error before it can return the DID object.

I went back and forth on this when initially implementing, but another approach is to catch any errors that occur during publishing and return did.metadata.published: false. This way, the caller can detect and attempt to re-publish if they wish... and at least they have the DID URI and key references.

What are your thoughts?

Yea that sounds reasonable.
I can't think of where it will be a huge deal for 'orphaned' keys will mess much up, they can just delete and make a new one.

Also you give them a lot of outs, setting publish to false.

If it turns into a big deal they can publish again later right?

I think leaving it the way it is is fine

Its a great point and the more I think about it... the more I think we ought to look at a results object for DID registration (create, update, and deactivate operations).

I'll leave it out for now and start a design doc to propose an approach.

Early thought is something that looks like: https://github.com/decentralized-identity/did-registration/blob/main/spec/spec.md#didregistrationmetadata

packages/dids/src/methods/did-dht.ts

Signed-off-by: Frank Hinek <[email protected]>

Co-authored-by: nitro-neal <[email protected]>

ianpatton · 2024-01-31T22:16:30Z

packages/dids/src/methods/did-web.ts

+
+    // Replace ":" with "/" in the identifier and prepend "https://" to obtain the fully qualified
+    // domain name and optional path.
+    let baseUrl = `https://${parsedDid.id.replace(/:/g, '/')}`;


Would be nice to use http:// if the uri is localhost, for local dev purposes

@ianpatton Interesting point to consider. The did:web spec is fairly clear about requiring HTTPS. However, perhaps we could do something like a custom resolution option that allows for use of localhost endpoints that aren't secure.

Signed-off-by: Frank Hinek <[email protected]>

packages/dids/tests/methods/did-web.spec.ts

packages/dids/tests/fixtures/test-vectors/did-resolver/resolve.json

packages/dids/src/resolver/resolver-cache-level.ts

packages/dids/src/methods/did-dht.ts

nitro-neal

Wow great job @frankhinek ! 🥇
I look forward to using this, I think it makes a lot of sense. Lots of new code and hard work.

Signed-off-by: Frank Hinek <[email protected]>

* Update Web5 Test Vectors to latest * Fix issue with c8 not reporting code coverage and bump version * Remove unnecessary option in crypto tsconfig and enable strict mode for dids * Add JWK Set to crypto package * Add did:jwk and did:web implementations * Add utility type to @web5/common * Improve DID Utility functions * Add FixedLengthArray utility type * Add support for Base32Z to Convert utility * Improve JWK type * Add InferKeyGeneratorAlgorithm utility type * Remove did-resolver dependency * Temporarily disable old implementations * Update DID JWK and DHT implementations * Rename createFromKeys to fromKeys and change default publishing behavior * Update JWK and DHT implementations * Rename DidKeySet to PortableDid and add uri * Reorganize tests and reintroduce DidResolver * Update package-lock.json * Rename P256 to Secp256r1 and add minimal tests * Add docs for undocumented methods * Finish reintroducing DID resolvers * Support secp256k1 as an algorithm identifier for generateKey() * Improve support for secp256r1 * Rename to LocalKeyManager & AwsKeyManager and improve docs * Fix Ed25519.validatePublicKey method signature * Refactor of DID Key * Complete implementation of DID Key * Add KeyCompressor type to @web5/crypto * Complete refactor of DidIon * Complete adding secp256r1 test vectors * Remove old dependencies and bump versions * Resolve package dependency issue * Remove old test-vectors directory * Remove references to tags in the XChaCha20Poly1305 comments * Remove duplication from DidDhtUtils.identifierToIdentityKey() * Correct typo in comments * Update crypto README * Correct typo, add DHT test, and remove unused file --------- Signed-off-by: Frank Hinek <[email protected]> Co-authored-by: nitro-neal <[email protected]>

frankhinek added 30 commits January 25, 2024 13:27

Update Web5 Test Vectors to latest

073c0f3

Signed-off-by: Frank Hinek <[email protected]>

Fix issue with c8 not reporting code coverage and bump version

22c001f

Signed-off-by: Frank Hinek <[email protected]>

Remove unnecessary option in crypto tsconfig and enable strict mode f…

e17d5e8

…or dids Signed-off-by: Frank Hinek <[email protected]>

Add JWK Set to crypto package

09083b2

Signed-off-by: Frank Hinek <[email protected]>

Add did:jwk and did:web implementations

7b51b76

Signed-off-by: Frank Hinek <[email protected]>

Add utility type to @web5/common

1187a09

Signed-off-by: Frank Hinek <[email protected]>

Improve DID Utility functions

b8cd77e

Signed-off-by: Frank Hinek <[email protected]>

Add FixedLengthArray utility type

b79dfff

Signed-off-by: Frank Hinek <[email protected]>

Add support for Base32Z to Convert utility

42bae59

Signed-off-by: Frank Hinek <[email protected]>

Improve JWK type

197181c

Signed-off-by: Frank Hinek <[email protected]>

Add InferKeyGeneratorAlgorithm utility type

2da26ee

Signed-off-by: Frank Hinek <[email protected]>

Remove did-resolver dependency

cf4cb33

Signed-off-by: Frank Hinek <[email protected]>

Temporarily disable old implementations

80ae0bd

Signed-off-by: Frank Hinek <[email protected]>

Update DID JWK and DHT implementations

de2cf33

Signed-off-by: Frank Hinek <[email protected]>

Rename createFromKeys to fromKeys and change default publishing behavior

6eff583

Signed-off-by: Frank Hinek <[email protected]>

Update JWK and DHT implementations

fcc2796

Signed-off-by: Frank Hinek <[email protected]>

Rename DidKeySet to PortableDid and add uri

0b0693f

Signed-off-by: Frank Hinek <[email protected]>

Reorganize tests and reintroduce DidResolver

be13b71

Signed-off-by: Frank Hinek <[email protected]>

Update package-lock.json

d14076a

Signed-off-by: Frank Hinek <[email protected]>

Rename P256 to Secp256r1 and add minimal tests

4148742

Signed-off-by: Frank Hinek <[email protected]>

Add docs for undocumented methods

53aa79b

Signed-off-by: Frank Hinek <[email protected]>

Finish reintroducing DID resolvers

00700f7

Signed-off-by: Frank Hinek <[email protected]>

Support secp256k1 as an algorithm identifier for generateKey()

cfb8a44

Signed-off-by: Frank Hinek <[email protected]>

Improve support for secp256r1

43f2507

Signed-off-by: Frank Hinek <[email protected]>

Rename to LocalKeyManager & AwsKeyManager and improve docs

0bfbc5b

Signed-off-by: Frank Hinek <[email protected]>

Fix Ed25519.validatePublicKey method signature

b7b8d79

Signed-off-by: Frank Hinek <[email protected]>

Refactor of DID Key

11b69ed

Signed-off-by: Frank Hinek <[email protected]>

Complete implementation of DID Key

7ec833e

Signed-off-by: Frank Hinek <[email protected]>

Add KeyCompressor type to @web5/crypto

29a27ab

Signed-off-by: Frank Hinek <[email protected]>

Complete refactor of DidIon

64120f0

Signed-off-by: Frank Hinek <[email protected]>

frankhinek requested a review from nitro-neal January 31, 2024 15:56

frankhinek added 3 commits January 31, 2024 11:39

Remove old dependencies and bump versions

2ebbd77

Signed-off-by: Frank Hinek <[email protected]>

Resolve package dependency issue

6cb1abb

Signed-off-by: Frank Hinek <[email protected]>

Remove old test-vectors directory

c26d27e

Signed-off-by: Frank Hinek <[email protected]>

frankhinek mentioned this pull request Jan 31, 2024

Confusing to Retrieve or Generate Key Pair for DID Generation #361

Closed