Implementing BearerDid / PortableDid, exposing our own JWS, JWT, JWK types #262

jiyoonie9 · 2024-03-07T19:25:02Z

Motivation

Align public API surface of DID creation and credential signing to be consistent with other web5 repos

Overview

BearerDid and PortableDid types used in place of Did(uri, keyManager) abstract class. Now all existing DID implementations' DidXyz.create() to return BearerDid.
Our own Jws, Jwt, Jwk types are used in place of nimbusds types. We are still creating private key internally using nimbusds (see Ed25519.kt for example), but all public surface APIs no longer expose the third party dependency.

How Has This Been Tested?

Ensured existing tests compiled and passed
Wrote new tests for new types and methods

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

…earerDid. changed DidUri to Did

…t how to call didxyz.import() from BearerDID. also figure out how to call didDhtApi methods after creating BearerDID via DidDht.create()

…import() as a static method. not sure if right path, might reverse

dids/src/main/kotlin/web5/sdk/dids/did/BearerDID.kt

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

dids/src/main/kotlin/web5/sdk/dids/extensions/DidExtensions.kt

… related types

…not being used

…de review comments

nitro-neal · 2024-03-25T15:42:30Z

credentials/src/main/kotlin/web5/sdk/credentials/VerifiablePresentation.kt

+  public fun sign(bearerDid: BearerDid, assertionMethodId: String? = null): String {
+    val payload = JwtClaimsSet.Builder()
+      .issuer(bearerDid.uri)
+      .issueTime(Date().time)


if this is iat may need to change to seconds, currently this is milliseconds I think

fixed, and updated the ktdoc

dids/src/main/kotlin/web5/sdk/dids/did/BearerDid.kt

jose/src/main/kotlin/web5/sdk/jose/jws/Jws.kt

nitro-neal · 2024-03-25T16:03:32Z

jose/src/main/kotlin/web5/sdk/jose/jwt/Jwt.kt

+  public val iss: String? = null,
+  public val sub: String? = null,
+  public val aud: String? = null,
+  public val exp: Long? = null,


I think long is the right impl here but I think i see other libs with int

i agree it should stay as a long - can you tell me which lib has it as an int so we can write an issue for it? i checked js (number), go(int64) and swift (ISO8601Date)

nitro-neal

Wow great job ! Lots of hard work here.

diehuxx

great work!

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

decentralgabe · 2024-03-27T01:12:37Z

crypto/src/main/kotlin/web5/sdk/crypto/AwsKeyManager.kt

   * @throws [AWSKMSException] for any error originating from the [AWSKMS] client
   */
-  override fun getPublicKey(keyAlias: String): JWK {
+  override fun getPublicKey(keyAlias: String): Jwk {
    val getPublicKeyRequest = GetPublicKeyRequest().withKeyId(keyAlias)
    val publicKeyResponse = kmsClient.getPublicKey(getPublicKeyRequest)
    val publicKey = convertToJavaPublicKey(publicKeyResponse.publicKey)

    val algorithmDetails = getAlgorithmDetails(publicKeyResponse.keySpec.enum())
    val jwkBuilder = when (publicKey) {


does this handle uncompressing compressed EC keys?

good question not sure. it does handle converting what aws returns into a JWK correctly

crypto/src/main/kotlin/web5/sdk/crypto/Crypto.kt

decentralgabe · 2024-03-27T01:14:31Z

crypto/src/main/kotlin/web5/sdk/crypto/Ed25519.kt

  }

-  override fun privateKeyToBytes(privateKey: JWK): ByteArray {
+  override fun privateKeyToBytes(privateKey: Jwk): ByteArray {


it would be good to have methods that are able to represent compressed keys

for Ed25519 @decentralgabe ? or are you talking about Secp256k1 or more generally EC?

would like gabe to weigh in on this / edit this issue to clarify what's missing here

#280

also are you talking about compressed public keys? bc presumably if you have d you can deterministically derive x and therefore y. so i'd say privateKeyToBytes already returns the most compressed key?

dids/src/main/kotlin/web5/sdk/dids/methods/key/DidKey.kt

dids/src/test/kotlin/web5/sdk/dids/methods/jwk/DidJwkTest.kt

jiyoonie9 · 2024-03-27T02:05:29Z

a reminder to myself to first merge TBD54566975/web5-spec#130 and then change the submodule commit to the merge commit

also do the same for TBD54566975/web5-js#466

Co-authored-by: Gabe <[email protected]>

…d parse test

…4-did-impl

tomdaffurn

Approved with comment.

Huge amount of work, well done Jiyoon. Good riddance nimbus

tomdaffurn · 2024-03-27T04:44:21Z

crypto/src/main/kotlin/web5/sdk/crypto/AwsKeyManager.kt

-      is ECPublicKey -> ECKey.Builder(JwaCurve.toJwkCurve(algorithmDetails.curve), publicKey)
+      is ECPublicKey -> {
+        val key = ECKey.Builder(JwaCurve.toNimbusCurve(algorithmDetails.curve), publicKey).build()
+        Jwk.Builder("EC", key.curve.name)


Does this defs populate algorithm, keyID, keyUse? I think at least keyID was needed when first implemented

alg and kid is now not populated by default - relevant comment here

kid can be computed when needed via Jwk#computeThumbprint()

keyUse hmm... i'm pretty sure we're also not populating these by default, and it is optional per JWK spec
https://datatracker.ietf.org/doc/html/rfc7517#section-4.2

Values defined by this specification are:

o "sig" (signature)
o "enc" (encryption)

Other values MAY be used. The "use" value is a case-sensitive
string. Use of the "use" member is OPTIONAL, unless the application
requires its presence.

the only fields that are required to build a Jwk are kty and crv

…r and the corresponding test

jiyoonie9 added 8 commits March 7, 2024 01:18

create bearerdid and portable did, keyexporter and keyimporter

9eca633

changing Did to ChangemeDid to make it easier to spot and replace w B…

12cb845

…earerDid. changed DidUri to Did

removing didmethod interface

28e683a

trying on BearerDID for did jwk

0106920

removing create options

9c5fbbf

getting did:dht and did:key to return BearerDID. next step: figure ou…

f348127

…t how to call didxyz.import() from BearerDID. also figure out how to call didDhtApi methods after creating BearerDID via DidDht.create()

moved bearerdid import() to companion object so i can call bearerdid.…

cb309ee

…import() as a static method. not sure if right path, might reverse

updated codeowners doc

1f32e5a

jiyoonie9 commented Mar 10, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/did/BearerDID.kt Outdated Show resolved Hide resolved

jiyoonie9 commented Mar 10, 2024

View reviewed changes

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

jiyoonie9 commented Mar 11, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/extensions/DidExtensions.kt Outdated Show resolved Hide resolved

changing type names to be true CamelCase. implemented Jws methods and…

a3ac9b5

… related types

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

Refactor DID method implementations to match design from web5-go, web5-swift, web5-js #234

Closed

jiyoonie9 mentioned this pull request Mar 11, 2024

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

Closed

jiyoonie9 added 4 commits March 11, 2024 15:05

wrote jwt class. modify jws class to include builder for jwsheader

90fa517

sub out JwtUtils.sign() for Jwt.sign()

584a36b

implemented jwt. removed jwtutil class

a9e1494

using hand rolled JwsHeader and JwtClaimsSet instead of nimbusds

d387f2c

jiyoonie9 added 6 commits March 12, 2024 13:02

adding jwk impl, in progress

b681b7c

adding jose package. fixing some todos

33852b9

goodbye, changemedid

9a77e89

updating tests to test import instead of load

9945824

wrote kt docs, fixed didresolvers, removed resolvedidoptions because …

f522197

…not being used

removing unnecessary tests

e92dbd1

jiyoonie9 added 3 commits March 23, 2024 16:27

adding wording around not supporting did web creation

cda57a0

adding uri as a property to BearerDid as it was missed. addressing co…

58db293

…de review comments

Merge branch 'main' of github.com:TBD54566975/web5-kt into 234-did-impl

55e652c

nitro-neal reviewed Mar 25, 2024

View reviewed changes

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

nitro-neal reviewed Mar 25, 2024

View reviewed changes

jose/src/main/kotlin/web5/sdk/jose/jws/Jws.kt Show resolved Hide resolved

nitro-neal reviewed Mar 25, 2024

View reviewed changes

nitro-neal approved these changes Mar 25, 2024

View reviewed changes

mistermoe approved these changes Mar 27, 2024

View reviewed changes

diehuxx approved these changes Mar 27, 2024

View reviewed changes

decentralgabe reviewed Mar 27, 2024

View reviewed changes

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

decentralgabe reviewed Mar 27, 2024

View reviewed changes

crypto/src/main/kotlin/web5/sdk/crypto/Crypto.kt Show resolved Hide resolved

decentralgabe reviewed Mar 27, 2024

View reviewed changes

dids/src/main/kotlin/web5/sdk/dids/methods/key/DidKey.kt Outdated Show resolved Hide resolved

decentralgabe reviewed Mar 27, 2024

View reviewed changes

dids/src/test/kotlin/web5/sdk/dids/methods/jwk/DidJwkTest.kt Outdated Show resolved Hide resolved

changing how portabledid is constructed in vc test vector test

5ab978d

jiyoonie9 and others added 5 commits March 26, 2024 22:31

addressing review comments

030678d

Update dids/src/test/kotlin/web5/sdk/dids/methods/jwk/DidJwkTest.kt

6004707

Co-authored-by: Gabe <[email protected]>

making error in testvectorinput to default to false, added portabledi…

b472990

…d parse test

Merge branch '234-did-impl' of github.com:TBD54566975/web5-kt into 23…

aa90cd4

…4-did-impl

adding comment about supporting keys that use ECC

c5bba53

tomdaffurn approved these changes Mar 27, 2024

View reviewed changes

jiyoonie9 added 2 commits March 27, 2024 12:34

reverting back to include portabledid concept in vc create test vecto…

1d7e0a5

…r and the corresponding test

bumping web5-spec commit

870cc12

This was referenced Mar 27, 2024

Should we use misc in JwtClaimsSet to store the vc object? TBD54566975/web5-spec#143

Closed

Add support for compressed keys #280

Open

jiyoonie9 merged commit f5a0488 into main Mar 27, 2024
6 of 7 checks passed

jiyoonie9 deleted the 234-did-impl branch March 27, 2024 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing BearerDid / PortableDid, exposing our own JWS, JWT, JWK types #262

Implementing BearerDid / PortableDid, exposing our own JWS, JWT, JWK types #262

jiyoonie9 commented Mar 7, 2024 •

edited

Loading

nitro-neal Mar 25, 2024

jiyoonie9 Mar 27, 2024

nitro-neal Mar 25, 2024

jiyoonie9 Mar 27, 2024

nitro-neal left a comment

diehuxx left a comment

decentralgabe Mar 27, 2024

mistermoe Mar 27, 2024

decentralgabe Mar 27, 2024

mistermoe Mar 27, 2024

jiyoonie9 Mar 27, 2024

mistermoe Mar 27, 2024

jiyoonie9 commented Mar 27, 2024 •

edited

Loading

tomdaffurn left a comment

tomdaffurn Mar 27, 2024 •

edited

Loading

jiyoonie9 Mar 27, 2024

jiyoonie9 Mar 27, 2024

Implementing BearerDid / PortableDid, exposing our own JWS, JWT, JWK types #262

Implementing BearerDid / PortableDid, exposing our own JWS, JWT, JWK types #262

Conversation

jiyoonie9 commented Mar 7, 2024 • edited Loading

Motivation

Overview

How Has This Been Tested?

Checklist

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

nitro-neal left a comment

Choose a reason for hiding this comment

diehuxx 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

jiyoonie9 commented Mar 27, 2024 • edited Loading

tomdaffurn left a comment

Choose a reason for hiding this comment

tomdaffurn Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiyoonie9 commented Mar 7, 2024 •

edited

Loading

jiyoonie9 commented Mar 27, 2024 •

edited

Loading

tomdaffurn Mar 27, 2024 •

edited

Loading