Removing `did-common-java` dependency #248

jiyoonie9 · 2024-02-16T05:15:17Z

Overview

Introducing our own types for DID and related things like DIDDocument, Service, VerificationMethod, etc, and ejecting from did-common-java dependency. Some refactoring of DidDht implementation.

Blocked from merging by updates to web5-spec test vectors that also changed decentralized-identity/web5-spec#118

Description

Motivation (per original issue)

Not published to maven central. consumers have to add danubetech's repository

introduces CVEs (though this may have been remedied in more recent versions)

heavy on JSON-LD

Follow up tasks

Rename Did abstract class to BearerDid

Note

DidUri name in this PR is temporary - will be changed back to Did after above renaming. See #254

Refactor DidDht

How Has This Been Tested?

Wrote tests for newly added types such as DIDDocument
Ensured all existing tests pass

Checklist

Before submitting this PR, please make sure:

I have read the CONTRIBUTING document.
My code is consistent with the rest of the project
I have tagged the relevant reviewers and/or interested parties
I have updated the READMEs and other documentation of affected packages

References

https://www.w3.org/TR/did-core/
https://github.com/TBD54566975/web5-go/tree/main/dids

…e-did-common-java

…n-java in comments

credentials/build.gradle.kts

dids/build.gradle.kts

dids/src/main/kotlin/web5/sdk/dids/DidMethod.kt

…iddoc verif method and service types

…jsonldobject inherited methods from did common java. replacing didurl with a verificationMethodId splitting on #

…questions as todos

… didion stuff

…did methods inherit from, and the new DID type

… specific methods in favor of the generic one

jiyoonie9 · 2024-02-26T20:39:38Z

blocker: need to merge decentralized-identity/web5-spec#118 and pull in latest

dids/src/main/kotlin/web5/sdk/dids/DidResolutionResult.kt

…e-did-common-java

… reference. in the middle of refactoring did dht slightly. only 7-8 tests failing now...

…g. making verificationMethod a set to avoid duplication. 1 test remains to be fixed

jiyoonie9 · 2024-02-29T00:37:14Z

config/detekt.yml

@@ -82,7 +83,7 @@ formatting:
  MaximumLineLength:
    active: true
  ImportOrdering:
-    active: true
+    active: false


with this true, i cannot order import in a way that makes detekt happy. detekt does not like how Square style formatter in intellij does import ordering :(

dids/src/main/kotlin/web5/sdk/dids/methods/dht/DidDht.kt

build.gradle.kts

mistermoe

great work @jiyoontbd! i know the effort required here was significant and that it's the first of a few PRs needed to get the dids package to parity with our SDKs

credentials/src/test/kotlin/web5/sdk/credentials/VerifiableCredentialTest.kt

dids/build.gradle.kts

KendallWeihe · 2024-03-05T16:27:51Z

dids/src/main/kotlin/web5/sdk/dids/didcore/DIDDocument.kt

+import java.security.SignatureException
+
+/**
+ * DIDDocument represents a set of data describing the DID subject including mechanisms such as:


Idea: maybe link to the spec doc? https://www.w3.org/TR/did-core/#core-properties

KendallWeihe · 2024-03-05T16:30:17Z

dids/src/main/kotlin/web5/sdk/dids/didcore/DIDDocument.kt

+    }
+
+    /**
+     * Adds Controllers.


Might be better to use the word "Sets" rather than "Adds" as with arrays/lists "Add" may be confused with append or concat

KendallWeihe · 2024-03-05T16:32:25Z

dids/src/main/kotlin/web5/sdk/dids/didcore/DIDDocumentMetadata.kt

+package web5.sdk.dids.didcore
+
+/**
+ * Contains metadata about the DID document.


maybe link to spec? https://www.w3.org/TR/did-core/#did-document-metadata

KendallWeihe · 2024-03-05T16:34:07Z

dids/src/main/kotlin/web5/sdk/dids/didcore/Service.kt

+/**
+ * Service is used in DID documents to express ways of communicating with
+ * the DID subject or associated entities.
+ * A service can be any type of service the DID subject wants to advertise.


again, another spec link? https://www.w3.org/TR/did-core/#services

oh I see you're linking to the spec in the @property comment below (and that may have been true to previous comments) ... do whatevs you think is best, I'm moving quickly

KendallWeihe · 2024-03-05T16:40:06Z

dids/src/main/kotlin/web5/sdk/dids/didcore/VerificationMethod.kt

+    return type == this.type
+  }
+
+  override fun toString(): String {


@jiyoontbd do you think we should introduce toJSON() functions on all of these types? I know we have dids/src/main/kotlin/web5/sdk/dids/Json.kt but another idea would be to add the JSON functionality to each DID type, to-and-from JSON strings. idk just a random thought I figured worth considering

i think for now i'd like to just have Json.kt i sniped from tbdex-kt to do all the stringify work instead of distributing it to be for each type

KendallWeihe · 2024-03-05T16:43:58Z

dids/src/main/kotlin/web5/sdk/dids/methods/DidUtil.kt

+ * Utl methods for DIDs.
+ *
+ */
+public object DidUtil {


I generally a little allergic to anything named "utility" because it's a bit amorphous in my mind

What is a SimpleDid? It may warrant it's own place in the code base next to PortableDid and BearerDid? (doesn't need to be addressed here, food for thought)

(which I know PortableDid and BearerDid aren't here yet, so looking ahead)

actually i don't have to use this class or its methods anymore

i can use DidUri instead of SimpleDid!

i also dislike utility anything but alas my brainjuice had ran out by the time i wrote this

KendallWeihe · 2024-03-05T16:44:29Z

dids/src/main/kotlin/web5/sdk/dids/methods/dht/DidDhtDocumentMetadata.kt

+import web5.sdk.dids.didcore.DidDocumentMetadata
+
+/**
+ * Did document metadata for did:dht that extends the base did document metadata.


again, links to spec?

hmm, i can't seem to find where type is defined in did dht document spec - @decentralgabe ? best i can find is https://did-dht.com/#type-indexing but that's about how to use types, not where types are defined in the did dht doc metadata section

KendallWeihe · 2024-03-05T16:46:23Z

dids/src/main/kotlin/web5/sdk/dids/methods/jwk/DidJwk.kt

-        .id(URI(did))
-        .verificationMethod(verificationMethod)
+      val didDocumentBuilder = DIDDocument.Builder()
+        .context("https://www.w3.org/ns/did/v1")


since "https://www.w3.org/ns/did/v1" is always to be included in a DID Document we could encapsulate that within the DIDDocument abstraction

the spec is confusing here, but this is not the case. the section you pointed to is for JSON-LD; however, there is a section above on a pure JSON representation that does not have this restriction.

for DID DHT I've chosen to follow the JSON representation and explicitly excluded the context property

KendallWeihe · 2024-03-05T16:47:12Z

dids/src/main/kotlin/web5/sdk/dids/didcore/DIDDocument.kt

+ * @property capabilityInvocation specifies a verification method used by the DID subject to invoke a
+ * 	         cryptographic capability, such as the authorization to update the DID Document.
+ */
+public class DIDDocument(


In some places we have DidDocument casing and other places (like here) we have DIDDocument casing. Any plans to make consistent?

you probably saw DidDocumentMetadata - i changed that to DIDDocumentMetadata to make it consistent, ty!

KendallWeihe · 2024-03-05T16:51:32Z

dids/src/test/kotlin/web5/sdk/dids/methods/dht/DhtTest.kt

@@ -136,15 +136,17 @@ class DhtTest {
  inner class DhtTest {
    @Test
    fun `create and parse a bep44 put request`() {
+      print("WITCHCRAFT")


🧙‍♀️

KendallWeihe · 2024-03-05T16:52:38Z

dids/src/test/kotlin/web5/sdk/dids/methods/dht/DhtTest.kt


      require(did.didDocument != null)

-      val kid = did.didDocument!!.verificationMethods?.first()?.publicKeyJwk?.get("kid")?.toString()
+      val kid = did.didDocument!!.verificationMethod?.first()?.publicKeyJwk?.keyID?.toString()


just noticing casing on keyID here as opposed to keyId, does Kotlin have an idiomatic casing style?

ah. that's from the nimbusdssd JWK library that provides that field with the casing. we are going to also roll our own JWK so i'll keep that in mind when i'm working on that issue

KendallWeihe

added comments but nothing blocking, well done!

decentralgabe · 2024-03-05T17:34:46Z

dids/src/test/resources/did_jwk_x25519_document.json

  "id": "did:jwk:eyJrdHkiOiJPS1AiLCJjcnYiOiJYMjU1MTkiLCJ1c2UiOiJlbmMiLCJ4IjoiM3A3YmZYdDl3YlRUVzJIQzdPUTFOei1EUThoYmVHZE5yZngtRkctSUswOCJ9",
  "verificationMethod": [
    {
      "id": "did:jwk:eyJrdHkiOiJPS1AiLCJjcnYiOiJYMjU1MTkiLCJ1c2UiOiJlbmMiLCJ4IjoiM3A3YmZYdDl3YlRUVzJIQzdPUTFOei1EUThoYmVHZE5yZngtRkctSUswOCJ9#0",
-      "type": "JsonWebKey2020",
+      "type": "JsonWebKey",


I believe this should be reverted, but practically it doesn't matter as long as we're internally consistent

the original did jwk spec was created before the JsonWebKey type existed

yea. i did this coz i thought we were internally moving towards aligning on JsonWebKey, so i changed the type for verificationmethod of DidJwk during resolve() to be JsonWebKey - https://github.com/TBD54566975/web5-kt/pull/248/files#diff-5537230f4c2b63c726999e7c345be0afee8bad05f7d49154d9897013f550ae7aR152

should i keep it as JsonWebKey2020? then i can revert this change.

…with this change

…e-did-common-java

jiyoonie9 added 3 commits February 14, 2024 18:23

removing did common java dep

3e60407

Merge branch 'main' of github.com:TBD54566975/web5-kt into 185-replac…

f54507c

…e-did-common-java

adding new types. wip - some difference between web5-go and did-commo…

9a0d068

…n-java in comments

jiyoonie9 commented Feb 21, 2024

View reviewed changes

credentials/build.gradle.kts Outdated Show resolved Hide resolved

jiyoonie9 commented Feb 21, 2024

View reviewed changes

dids/build.gradle.kts Outdated Show resolved Hide resolved

jiyoonie9 commented Feb 21, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/DidMethod.kt Show resolved Hide resolved

jiyoonie9 added 2 commits February 21, 2024 14:19

adding ktdocs. changing out id of type URI for id of type String in d…

948427c

…iddoc verif method and service types

removing todos and just using string for id instead of uri

17e2ed0

This was linked to issues Feb 21, 2024

Handroll replacement for decentralized-identity:did-common-java:1.9.0 #185

Closed

API Leak: decentralized-identity:did-common-java #231

Closed

jiyoonie9 added 5 commits February 24, 2024 15:24

pulling in Json from tbdex-kt for stringifying instead of relying on …

812d341

…jsonldobject inherited methods from did common java. replacing didurl with a verificationMethodId splitting on #

goodbye didion. cleaning up diddocument. renaming some methods. more …

7a2db55

…questions as todos

cleaning up. pulling documentmetadata back in from previously deleted…

7f8c2b1

… didion stuff

changing Did to BaseDid to reduce confusion between the Did that all …

882957a

…did methods inherit from, and the new DID type

using verificationMethod() in the builder class, removing all purpose…

4808b17

… specific methods in favor of the generic one

jiyoonie9 added 2 commits February 26, 2024 18:45

addressing some todos

7f2546a

reverting renaming, to be done later

6306e57

jiyoonie9 commented Feb 26, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/DidResolutionResult.kt Show resolved Hide resolved

jiyoonie9 added 8 commits February 26, 2024 18:53

removing duplicate documentmetadata

de48004

hella kt docs

c13c135

stuff from pairing

0df84ce

fixing tests

a67b4fd

Merge branch 'main' of github.com:TBD54566975/web5-kt into 185-replac…

078440f

…e-did-common-java

pulling in work done in PR#225 by @grahnj as part of removing did ion…

5f42234

… reference. in the middle of refactoring did dht slightly. only 7-8 tests failing now...

wrote tests for diddocument

223216a

renaming Did.kt file to DidUri.kt to be consistent with current namin…

1b2c995

…g. making verificationMethod a set to avoid duplication. 1 test remains to be fixed

jiyoonie9 changed the title ~~removing did common java dep~~ Removing did-common-java dependency Feb 28, 2024

jiyoonie9 commented Feb 29, 2024

View reviewed changes

decentralgabe reviewed Feb 29, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/methods/dht/DidDht.kt Outdated Show resolved Hide resolved

KendallWeihe reviewed Mar 4, 2024

View reviewed changes

build.gradle.kts Show resolved Hide resolved

mistermoe approved these changes Mar 4, 2024

View reviewed changes

KendallWeihe reviewed Mar 5, 2024

View reviewed changes

credentials/src/test/kotlin/web5/sdk/credentials/VerifiableCredentialTest.kt Show resolved Hide resolved

KendallWeihe reviewed Mar 5, 2024

View reviewed changes

dids/build.gradle.kts Show resolved Hide resolved

KendallWeihe reviewed Mar 5, 2024

View reviewed changes

KendallWeihe approved these changes Mar 5, 2024

View reviewed changes

decentralgabe reviewed Mar 5, 2024

View reviewed changes

diehuxx approved these changes Mar 5, 2024

View reviewed changes

jiyoonie9 added 4 commits March 5, 2024 14:47

addressing code review comments

21b6dd6

adding more tests to please codecov

749492b

moving Json out to commons package

73d351c

fixing import errors

0e25c3f

jiyoonie9 mentioned this pull request Mar 6, 2024

enforcing controller type to be a list of string decentralized-identity/web5-spec#118

Merged

jiyoonie9 added 3 commits March 5, 2024 20:18

changing context field to be a list, changing test vectors to comply …

61062b0

…with this change

update web5-spec commit to be latest in main

55bff30

Merge branch 'main' of github.com:TBD54566975/web5-kt into 185-replac…

3b05af9

…e-did-common-java

jiyoonie9 merged commit 2e4946d into main Mar 6, 2024
5 of 7 checks passed

jiyoonie9 deleted the 185-replace-did-common-java branch March 6, 2024 01:27

jiyoonie9 mentioned this pull request Mar 6, 2024

mark DidIon as @Deprecated #206

Closed

jiyoonie9 linked an issue Mar 11, 2024 that may be closed by this pull request

Replace usage of did:ion in tests with another DID method #218

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing `did-common-java` dependency #248

Removing `did-common-java` dependency #248

jiyoonie9 commented Feb 16, 2024 •

edited

Loading

jiyoonie9 commented Feb 26, 2024 •

edited

Loading

jiyoonie9 Feb 29, 2024

mistermoe left a comment

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

jiyoonie9 Mar 6, 2024

KendallWeihe Mar 6, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024

jiyoonie9 Mar 5, 2024

jiyoonie9 Mar 6, 2024

KendallWeihe Mar 5, 2024

jiyoonie9 Mar 5, 2024

KendallWeihe Mar 5, 2024

decentralgabe Mar 5, 2024

KendallWeihe Mar 5, 2024

KendallWeihe Mar 5, 2024 •

edited

Loading

jiyoonie9 Mar 5, 2024

KendallWeihe Mar 5, 2024

jiyoonie9 Mar 5, 2024

KendallWeihe Mar 5, 2024

jiyoonie9 Mar 5, 2024

KendallWeihe left a comment

decentralgabe Mar 5, 2024

jiyoonie9 Mar 5, 2024

Removing did-common-java dependency #248

Removing did-common-java dependency #248

Conversation

jiyoonie9 commented Feb 16, 2024 • edited Loading

Overview

Description

Motivation (per original issue)

Follow up tasks

How Has This Been Tested?

Checklist

References

jiyoonie9 commented Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

mistermoe 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KendallWeihe Mar 5, 2024 • edited Loading

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

Choose a reason for hiding this comment

KendallWeihe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Removing `did-common-java` dependency #248

Removing `did-common-java` dependency #248

jiyoonie9 commented Feb 16, 2024 •

edited

Loading

jiyoonie9 commented Feb 26, 2024 •

edited

Loading

KendallWeihe Mar 5, 2024 •

edited

Loading