From db027423d709930dccfb7246738670726a33ab9f Mon Sep 17 00:00:00 2001 From: Niels Klomp Date: Wed, 26 Oct 2022 05:01:46 +0200 Subject: [PATCH] fix(did-provider-ion): delete new keys if addKey fails(#1045) --- .../__tests__/canonicalizer.test.ts | 10 +++--- .../__tests__/ion-did-provider.test.ts | 28 +++++++-------- packages/did-provider-ion/package.json | 4 +-- .../did-provider-ion/src/ion-did-provider.ts | 34 +++++++++++++++---- .../did-provider-ion/src/ion-did-resolver.ts | 8 +++-- 5 files changed, 54 insertions(+), 30 deletions(-) diff --git a/packages/did-provider-ion/__tests__/canonicalizer.test.ts b/packages/did-provider-ion/__tests__/canonicalizer.test.ts index 84646dc3d..e1a06ba1d 100644 --- a/packages/did-provider-ion/__tests__/canonicalizer.test.ts +++ b/packages/did-provider-ion/__tests__/canonicalizer.test.ts @@ -15,7 +15,7 @@ describe('canonicalizer result should be', () => { JsonCanonicalizer.asString({ test1: 'test1', test2: 'test2', - }) + }), ).toEqual('{"test1":"test1","test2":"test2"}') }) it('a json string with properties reordered', () => { @@ -23,7 +23,7 @@ describe('canonicalizer result should be', () => { JsonCanonicalizer.asString({ test2: 'test2', test1: 'test1', - }) + }), ).toEqual('{"test1":"test1","test2":"test2"}') }) it('a json string with null values intact', () => { @@ -31,7 +31,7 @@ describe('canonicalizer result should be', () => { JsonCanonicalizer.asString({ test1: 'test1', test2: null, - }) + }), ).toEqual('{"test1":"test1","test2":null}') }) it('a json string with undefined property values removed', () => { @@ -39,7 +39,7 @@ describe('canonicalizer result should be', () => { 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"}}') }) }) diff --git a/packages/did-provider-ion/__tests__/ion-did-provider.test.ts b/packages/did-provider-ion/__tests__/ion-did-provider.test.ts index 27a7d254d..286cdb1fe 100644 --- a/packages/did-provider-ion/__tests__/ion-did-provider.test.ts +++ b/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') } }) }) diff --git a/packages/did-provider-ion/package.json b/packages/did-provider-ion/package.json index 9d08991ea..1f4104788 100644 --- a/packages/did-provider-ion/package.json +++ b/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 ", "license": "Apache-2.0", - "keywords": [] + "keywords": ["Veramo", "ION", "DID"] } diff --git a/packages/did-provider-ion/src/ion-did-provider.ts b/packages/did-provider-ion/src/ion-did-provider.ts index 95b972ca3..d045814ef 100644 --- a/packages/did-provider-ion/src/ion-did-provider.ts +++ b/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 ): Promise { 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 ): Promise { 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) { + try { + await context.agent.keyManagerDelete({ kid }) + } catch (ignore) { + debug(ignore.message) + } + } } diff --git a/packages/did-provider-ion/src/ion-did-resolver.ts b/packages/did-provider-ion/src/ion-did-resolver.ts index a53348511..9b1720645 100644 --- a/packages/did-provider-ion/src/ion-did-resolver.ts +++ b/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() }) }