fix(did-provider-ion): delete new keys if addKey fails(#1045)

packages/did-provider-ion/__tests__/canonicalizer.test.ts

@@ -15,31 +15,31 @@ describe('canonicalizer result should be', () => {
       JsonCanonicalizer.asString({
         test1: 'test1',
         test2: 'test2',
-      })
+      }),
     ).toEqual('{"test1":"test1","test2":"test2"}')
   })
   it('a json string with properties reordered', () => {
     expect(
       JsonCanonicalizer.asString({
         test2: 'test2',
         test1: 'test1',
-      })
+      }),
     ).toEqual('{"test1":"test1","test2":"test2"}')
   })
   it('a json string with null values intact', () => {
     expect(
       JsonCanonicalizer.asString({
         test1: 'test1',
         test2: null,
-      })
+      }),
     ).toEqual('{"test1":"test1","test2":null}')
   })
   it('a json string with undefined property values removed', () => {
     expect(
       JsonCanonicalizer.asString({
         test1: 'test1',
         test2: undefined,
-      })
+      }),
     ).toEqual('{"test1":"test1"}')
   })
   it('a json string with undefined property values removed if nested', () => {
@@ -49,7 +49,7 @@ describe('canonicalizer result should be', () => {
           test2: 'test2',
           test3: undefined,
         },
-      })
+      }),
     ).toEqual('{"test1":{"test2":"test2"}}')
   })
 })

packages/did-provider-ion/__tests__/ion-did-provider.test.ts

@@ -66,12 +66,12 @@ describe('@sphereon/ion-did-provider', () => {
       did: identifier.did,
       key: newKey,
       kid: 'test-add-key-' + Date.now(),
-      options: { purposes: [IonPublicKeyPurpose.AssertionMethod, IonPublicKeyPurpose.Authentication], anchor: true },
+      options: { purposes: [IonPublicKeyPurpose.AssertionMethod, IonPublicKeyPurpose.Authentication], anchor: false },
     })
     try {
-      await expect(resultPromise).resolves.toMatchObject({})
+      expect(await resultPromise).toMatchObject({})
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
   })
 
@@ -92,12 +92,12 @@ describe('@sphereon/ion-did-provider', () => {
     const resultPromise = agent.didManagerAddService({
       did: identifier.did,
       service,
-      options: { anchor: true },
+      options: { anchor: false },
     })
     try {
-      await expect(resultPromise).resolves.toMatchObject({})
+      expect(await resultPromise).toMatchObject({})
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
   })
 
@@ -115,9 +115,9 @@ describe('@sphereon/ion-did-provider', () => {
       options: { anchor: false },
     })
     try {
-      await expect(resultPromise).resolves.toMatchObject({})
+      expect(await resultPromise).toMatchObject({})
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
   })
 
@@ -141,9 +141,9 @@ describe('@sphereon/ion-did-provider', () => {
       options: { anchor: false },
     })
     try {
-      await expect(addPromise).resolves.toMatchObject({})
+      expect(await addPromise).toMatchObject({})
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
 
     const removePromise = agent.didManagerRemoveService({
@@ -152,9 +152,9 @@ describe('@sphereon/ion-did-provider', () => {
       options: { anchor: false },
     })
     try {
-      await expect(removePromise).resolves.toMatchObject({})
+      expect(await removePromise).toMatchObject({})
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
   })
 
@@ -166,9 +166,9 @@ describe('@sphereon/ion-did-provider', () => {
 
     const deletePromise = agent.didManagerDelete({ did: identifier.did, options: { anchor: false } })
     try {
-      await expect(deletePromise).resolves.toBeTruthy()
+      expect(await deletePromise).toBeTruthy()
     } catch (error) {
-      expect(JSON.stringify(error)).toMatch('An operation request already exists in queue for DID')
+      await expect(error.message).toMatch('An operation request already exists in queue for DID')
     }
   })
 })

packages/did-provider-ion/package.json

@@ -38,8 +38,8 @@
   "publishConfig": {
     "access": "public"
   },
-  "repository": "git@github.com:Sphereon-OpenSource/ssi-sdk.git",
+  "repository": "git@github.com:uport-project/veramo.git",
   "author": "Sphereon <dev@sphereon.com>",
   "license": "Apache-2.0",
-  "keywords": []
+  "keywords": ["Veramo", "ION", "DID"]
 }

packages/did-provider-ion/src/ion-did-provider.ts

@@ -53,7 +53,7 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
     const challengeEnabled = options?.challengeEnabled === undefined ? true : options.challengeEnabled;
     const challengeEndpoint = options?.challengeEndpoint
     const solutionEndpoint = options?.solutionEndpoint
-    this.ionPoW = new IonPoW({challengeEnabled, challengeEndpoint, solutionEndpoint})
+    this.ionPoW = new IonPoW({ challengeEnabled, challengeEndpoint, solutionEndpoint })
   }
 
   /** {@inheritDoc @veramo/core#IDIDManager.didManagerCreate} */
@@ -179,7 +179,7 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
       return request
     } catch (error) {
       // It would have been nicer if we hadn't stored the new update key yet
-      await context.agent.keyManagerDelete({ kid: rotation.nextVeramoKey.kid })
+      await this.deleteKeyOnError(rotation.nextVeramoKey.kid, context)
       throw error
     }
   }
@@ -208,7 +208,7 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
       return request
     } catch (error) {
       // It would have been nicer if we hadn't stored the new update key yet
-      await context.agent.keyManagerDelete({ kid: rotation.nextVeramoKey.kid })
+      await this.deleteKeyOnError(rotation.nextVeramoKey.kid, context)
       throw error
     }
   }
@@ -230,7 +230,7 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
       return request
     } catch (error) {
       // It would have been nicer if we hadn't stored the new update key yet
-      await context.agent.keyManagerDelete({ kid: rotation.nextVeramoKey.kid })
+      await this.deleteKeyOnError(rotation.nextVeramoKey.kid, context)
       throw error
     }
   }
@@ -252,7 +252,7 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
       return request
     } catch (error) {
       // It would have been nicer if we hadn't stored the new update key yet
-      await context.agent.keyManagerDelete({ kid: rotation.nextVeramoKey.kid })
+      await this.deleteKeyOnError(rotation.nextVeramoKey.kid, context)
       throw error
     }
   }
@@ -306,7 +306,9 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
     context: IAgentContext<IKeyManager>
   ): Promise<IKeyRotation> {
     const currentVeramoKey =
-      relation == KeyIdentifierRelation.UPDATE ? getVeramoUpdateKey(identifier.keys, commitment) : getVeramoRecoveryKey(identifier.keys, commitment)
+      relation == KeyIdentifierRelation.UPDATE
+        ? getVeramoUpdateKey(identifier.keys, commitment)
+        : getVeramoRecoveryKey(identifier.keys, commitment)
     const currentIonKey = toIonPublicKey(currentVeramoKey)
     const currentJwk = toIonPublicKeyJwk(currentVeramoKey.publicKeyHex)
     //todo alias?
@@ -386,7 +388,11 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
     context: IAgentContext<IKeyManager>
   ): Promise<IKey> {
     const kid = options?.kid ? options.kid : options?.key?.kid
-    const type = options?.type ? options.type : options?.key?.type ? (options.key.type as KeyType) : KeyType.Secp256k1
+    const type = options?.type
+      ? options.type
+      : options?.key?.type
+      ? (options.key.type as KeyType)
+      : KeyType.Secp256k1
 
     const meta = options?.key?.meta ? options.key.meta : {}
     const ionMeta: IonKeyMetadata = {
@@ -442,4 +448,18 @@ export class IonDIDProvider extends AbstractIdentifierProvider {
       debug(`Not anchoring as anchoring was not enabled`)
     }
   }
+
+  /**
+   * Deletes a key (typically a rotation key) on error. As this happens in an exception flow, any issues with deletion are only debug logged.
+   *
+   * @param kid
+   * @private
+   */
+  private async deleteKeyOnError(kid: string, context: IAgentContext<IKeyManager>) {
+    try {
+      await context.agent.keyManagerDelete({ kid })
+    } catch (ignore) {
+      debug(ignore.message)
+    }
+  }
 }

packages/did-provider-ion/src/ion-did-resolver.ts

@@ -16,8 +16,12 @@ export const resolveDidIon: DIDResolver = async (didUrl: string, options?: DIDRe
 }
 
 const resolve = async (didUrl: string, options?: DIDResolutionOptions) => {
-  return fetch((options?.nodeEndpoint || 'https://beta.discover.did.microsoft.com/1.0/identifiers/') + didUrl).then((response) => {
-    if (response.status >= 400) throw new Error('Not Found')
+  return fetch(
+    (options?.nodeEndpoint || 'https://beta.discover.did.microsoft.com/1.0/identifiers/') + didUrl,
+  ).then(async (response) => {
+    if (response.status >= 400) {
+      throw new Error(`Not Found:\r\n${didUrl}\r\n${JSON.stringify(await response.json())}`)
+    }
     return response.json()
   })
 }