From 1708050fa2cbcdd9861ef289bb306505c8671194 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Fri, 12 Jul 2024 21:35:27 -0400 Subject: [PATCH] Delete Identity/DID/Key from DWN Stores (#744) * ability to delete keys from key manager, identity data and did data - `delete` method added to `DidApi` which deletes the `portableDid` stored in the `DidStore` - Optionally, if the `deleteKey` option is set to false the keys associated with the DID stored by the `KeyStore` are deleted. **default is true** - `delete` method added to `IdentityApi` which deletes the `IdentityMetadata` stored in the `IdentityStore` - `deleteKey` method added to the `LocalKeyManager` and `AgentKeyManager` interface. --- .changeset/mean-ducks-deny.md | 5 + packages/agent/.c8rc.json | 5 +- packages/agent/src/did-api.ts | 33 +++++ packages/agent/src/identity-api.ts | 14 ++ packages/agent/src/local-key-manager.ts | 10 ++ .../src/prototyping/crypto/types/key-io.ts | 13 ++ .../prototyping/crypto/types/params-kms.ts | 5 + packages/agent/src/types/key-manager.ts | 5 +- packages/agent/tests/did-api.spec.ts | 127 +++++++++++++++++- packages/agent/tests/identity-api.spec.ts | 42 ++++++ .../agent/tests/local-key-manager.spec.ts | 34 +++++ 11 files changed, 287 insertions(+), 6 deletions(-) create mode 100644 .changeset/mean-ducks-deny.md diff --git a/.changeset/mean-ducks-deny.md b/.changeset/mean-ducks-deny.md new file mode 100644 index 000000000..8d311a6a8 --- /dev/null +++ b/.changeset/mean-ducks-deny.md @@ -0,0 +1,5 @@ +--- +"@web5/agent": patch +--- + +Add ability to delete IdentityMetadata, PortableDid, and Jwk from Dwn Stores in Agent diff --git a/packages/agent/.c8rc.json b/packages/agent/.c8rc.json index e0b57f53e..cfbbb06bf 100644 --- a/packages/agent/.c8rc.json +++ b/packages/agent/.c8rc.json @@ -11,10 +11,11 @@ "tests/compiled/**/src/index.js", "tests/compiled/**/src/types.js", "tests/compiled/**/src/types/**", - "tests/compiled/**/src/prototyping/clients/*-types.js" + "tests/compiled/**/src/prototyping/clients/*-types.js", + "tests/compiled/**/src/prototyping/**/types/**" ], "reporter": [ "cobertura", "text" ] -} \ No newline at end of file +} diff --git a/packages/agent/src/did-api.ts b/packages/agent/src/did-api.ts index ff5fac51b..8b533b635 100644 --- a/packages/agent/src/did-api.ts +++ b/packages/agent/src/did-api.ts @@ -268,6 +268,39 @@ export class AgentDidApi return bearerDid; } + public async delete({ didUri, tenant, deleteKey = true }: { + didUri: string; + tenant?: string; + deleteKey?: boolean; + }): Promise { + const portableDid = await this._store.get({ id: didUri, agent: this.agent, tenant, useCache: false }); + if(!portableDid) { + throw new Error('AgentDidApi: Could not delete, DID not found'); + } + + // Delete the data before deleting the associated keys. + await this._store.delete({ id: didUri, agent: this.agent, tenant }); + + if (deleteKey) { + // Delete the keys associated with the DID + // TODO: this could be made a static method on `BearerDid` class + await this.deleteKeys({ portableDid }); + } + } + + public async deleteKeys({ portableDid }: { + portableDid: PortableDid; + }): Promise { + for (const verificationMethod of portableDid.document.verificationMethod || []) { + if (!verificationMethod.publicKeyJwk) { + continue; + } + // Compute the key URI of the verification method's public key. + const keyUri = await this.agent.keyManager.getKeyUri({ key: verificationMethod.publicKeyJwk }); + await this.agent.keyManager.deleteKey({ keyUri }); + } + } + public async processRequest( request: DidRequest ): Promise> { diff --git a/packages/agent/src/identity-api.ts b/packages/agent/src/identity-api.ts index adc8ded35..530fc1129 100644 --- a/packages/agent/src/identity-api.ts +++ b/packages/agent/src/identity-api.ts @@ -221,4 +221,18 @@ export class AgentIdentityApi { + // Attempt to retrieve the Identity from the Agent's Identity store. + const storedIdentity = await this._store.get({ id: didUri, agent: this.agent, tenant, useCache: true }); + if (!storedIdentity) { + throw new Error(`AgentIdentityApi: Failed to purge due to Identity not found: ${didUri}`); + } + + // Delete the Identity from the Agent's Identity store. + await this._store.delete({ id: didUri, agent: this.agent, tenant }); + } } \ No newline at end of file diff --git a/packages/agent/src/local-key-manager.ts b/packages/agent/src/local-key-manager.ts index eeae50030..7e46b33db 100644 --- a/packages/agent/src/local-key-manager.ts +++ b/packages/agent/src/local-key-manager.ts @@ -552,6 +552,16 @@ export class LocalKeyManager implements AgentKeyManager { return wrappedKeyBytes; } + public async deleteKey({ keyUri }:{ keyUri: KeyIdentifier }): Promise { + // Get the private key from the key store. + const jwk = await this._keyStore.get({ id: keyUri, agent: this.agent, useCache: true }); + if (!jwk) { + throw new Error(`Key not found: ${keyUri}`); + } + + await this._keyStore.delete({ id: keyUri, agent: this.agent }); + } + /** * Retrieves an algorithm implementation instance based on the provided algorithm name. * diff --git a/packages/agent/src/prototyping/crypto/types/key-io.ts b/packages/agent/src/prototyping/crypto/types/key-io.ts index e53a909b2..06c139564 100644 --- a/packages/agent/src/prototyping/crypto/types/key-io.ts +++ b/packages/agent/src/prototyping/crypto/types/key-io.ts @@ -35,4 +35,17 @@ export interface KeyImporter { * @returns A Promise resolving to the key identifier of the imported key. */ importKey(params: ImportKeyInput): Promise; +} + +export interface KeyDeleter { + /** + * Deletes a cryptographic key. + * + * @remarks + * The `deleteKey()` method of the {@link KeyDeleter | `KeyDeleter`} interface deletes a cryptographic + * key from the key store. + * + * @param params - The parameters for the key deletion operation. + */ + deleteKey(params: DeleteKeyInput): Promise; } \ No newline at end of file diff --git a/packages/agent/src/prototyping/crypto/types/params-kms.ts b/packages/agent/src/prototyping/crypto/types/params-kms.ts index 4c202b68b..085907741 100644 --- a/packages/agent/src/prototyping/crypto/types/params-kms.ts +++ b/packages/agent/src/prototyping/crypto/types/params-kms.ts @@ -68,4 +68,9 @@ export interface KmsWrapKeyParams { /** An object defining the algorithm-specific parameters for encrypting the `unwrappedKey`. */ encryptParams?: unknown +} + +export interface KmsDeleteKeyParams { + /** Identifier for the key to be deleted in the KMS. */ + keyUri: KeyIdentifier; } \ No newline at end of file diff --git a/packages/agent/src/types/key-manager.ts b/packages/agent/src/types/key-manager.ts index f5bacd21a..587a88c00 100644 --- a/packages/agent/src/types/key-manager.ts +++ b/packages/agent/src/types/key-manager.ts @@ -2,12 +2,13 @@ import type { Cipher, Jwk, KeyIdentifier, KeyWrapper, KmsExportKeyParams, KmsImp import type { Web5PlatformAgent } from './agent.js'; import type { KeyManager } from '../prototyping/crypto/types/key-manager.js'; -import type { KeyExporter, KeyImporter } from '../prototyping/crypto/types/key-io.js'; -import type { KmsCipherParams, KmsUnwrapKeyParams, KmsWrapKeyParams } from '../prototyping/crypto/types/params-kms.js'; +import type { KeyDeleter, KeyExporter, KeyImporter } from '../prototyping/crypto/types/key-io.js'; +import type { KmsCipherParams, KmsDeleteKeyParams, KmsUnwrapKeyParams, KmsWrapKeyParams } from '../prototyping/crypto/types/params-kms.js'; export interface AgentKeyManager extends KeyManager, Cipher, KeyImporter, KeyExporter, + KeyDeleter, KeyWrapper { agent: Web5PlatformAgent; diff --git a/packages/agent/tests/did-api.spec.ts b/packages/agent/tests/did-api.spec.ts index 6d041991f..1ef1bda25 100644 --- a/packages/agent/tests/did-api.spec.ts +++ b/packages/agent/tests/did-api.spec.ts @@ -1,4 +1,4 @@ - +import sinon from 'sinon'; import { expect } from 'chai'; import { BearerDid, DidJwk } from '@web5/dids'; @@ -60,11 +60,13 @@ describe('AgentDidApi', () => { }); beforeEach(async () => { + sinon.restore(); await testHarness.clearStorage(); await testHarness.createAgentDid(); }); after(async () => { + sinon.restore(); await testHarness.clearStorage(); await testHarness.closeStorage(); }); @@ -174,7 +176,128 @@ describe('AgentDidApi', () => { }); describe('delete()', () => { - xit('should be implemented'); + it('should delete a DID', async () => { + // we use the agentDid as the tenant for this test + // that way when we delete the DID we cna still issue a `get()` and agent's key is still there + const agentDid = testHarness.agent.agentDid.uri; + // Generate a new DID. + const did = await testHarness.agent.did.create({tenant: agentDid, method: 'jwk', store: true }); // store + + // attempt to get the DID + let storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: agentDid }); + expect(storedDid).to.not.be.undefined; + expect(storedDid!.uri).to.equal(did.uri); + + // delete the DID + await testHarness.agent.did.delete({ didUri: did.uri, tenant: agentDid }); + + // attempt to get the DID again + storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: agentDid }); + expect(storedDid).to.be.undefined; + }); + + it('should throw not found if the DID does not exist', async () => { + try { + await testHarness.agent.did.delete({ didUri: 'did:method:abc123', tenant: testHarness.agent.agentDid.uri }); + expect.fail('Expected an error to be thrown'); + } catch(error: any) { + expect(error.message).to.include('AgentDidApi: Could not delete, DID not found'); + } + }); + + it('should not be able to get signer for tenant after the tenant DID is deleted and the deleteKey parameter is set not false', async function () { + // This test is only relevant for the DWN store as it needs a signer to perform storage operations + if (agentStoreType !== 'dwn') { + this.skip(); + } + // Generate a new DID, since no tenant is provided it will be stored under its own tenant + const did = await testHarness.agent.did.create({ method: 'jwk', store: true }); // store + + // attempt to get the DID + let storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: did.uri }); + expect(storedDid).to.not.be.undefined; + expect(storedDid!.uri).to.equal(did.uri); + + // delete the DID + await testHarness.agent.did.delete({ didUri: did.uri, tenant: did.uri }); + + console.log('deleted'); + // attempt to get the DID again + try { + storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: did.uri }); + expect.fail('Expected an error to be thrown'); + } catch(error:any) { + expect(error.message).to.include('Unable to get signer for author'); + } + }); + + it('should keep key if deleteKey parameter is false', async () => { + // Generate a new DID. + const did = await testHarness.agent.did.create({ method: 'jwk', store: true }); // store + + // attempt to get the DID + let storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: did.uri}); + expect(storedDid).to.not.be.undefined; + expect(storedDid!.uri).to.equal(did.uri); + + // spy on deleteKey + const keyManagerSpy = sinon.spy(testHarness.agent.keyManager, 'deleteKey'); + + // delete the DID without deleting the key + await testHarness.agent.did.delete({ didUri: did.uri, tenant: did.uri, deleteKey: false }); + + expect(keyManagerSpy.called).to.be.false; + + // attempt to get the DID again this will not fail because the key still exists + storedDid = await testHarness.agent.did.get({ didUri: did.uri, tenant: did.uri }); + expect(storedDid).to.be.undefined; + }); + + it('should skip non Jwk encoded verification methods', async () => { + // stub store to return a portable did with non-jwk verification methods + sinon.stub(testHarness.agent.did['_store'], 'get').resolves({ + uri : 'did:method:abc123', + metadata : {}, + document : { + id : 'did:method:abc123', + verificationMethod : [{ + id : 'did:method:abc123#key1', + type : 'Ed25519VerificationKey2018', + controller : 'did:method:abc123', + publicKeyMultibase : 'z6Mkq' + }] + } + }); + + sinon.stub(testHarness.agent.did['_store'], 'delete').resolves(); + + // spy on deleteKey + const keyManagerSpy = sinon.spy(testHarness.agent.keyManager, 'deleteKey'); + // delete the DID + await testHarness.agent.did.delete({ didUri: 'did:example:123' }); + + expect(keyManagerSpy.called).to.be.false; + }); + + it('skips if verificationMethod is not defined', async () => { + // stub store to return a portable did with non-jwk verification methods + sinon.stub(testHarness.agent.did['_store'], 'get').resolves({ + uri : 'did:method:abc123', + metadata : {}, + document : { + id: 'did:method:abc123', + } + }); + + sinon.stub(testHarness.agent.did['_store'], 'delete').resolves(); + + // spy on deleteKey + const keyManagerDeleteSpy = sinon.spy(testHarness.agent.keyManager, 'deleteKey'); + // delete the DID + await testHarness.agent.did.delete({ didUri: 'did:example:123' }); + + expect(keyManagerDeleteSpy.called).to.be.false; + }); }); describe('export()', () => { diff --git a/packages/agent/tests/identity-api.spec.ts b/packages/agent/tests/identity-api.spec.ts index 95ab7a7f7..7d2814634 100644 --- a/packages/agent/tests/identity-api.spec.ts +++ b/packages/agent/tests/identity-api.spec.ts @@ -1,3 +1,4 @@ +import sinon from 'sinon'; import { expect } from 'chai'; import { TestAgent } from './utils/test-agent.js'; @@ -51,11 +52,13 @@ describe('AgentIdentityApi', () => { }); beforeEach(async () => { + sinon.restore(); await testHarness.clearStorage(); await testHarness.createAgentDid(); }); after(async () => { + sinon.restore(); await testHarness.clearStorage(); await testHarness.closeStorage(); }); @@ -145,6 +148,45 @@ describe('AgentIdentityApi', () => { expect(storedIdentities).to.be.empty; }); }); + + describe('delete()', () => { + it('deletes an Identity', async () => { + // Create a new Identity. + const identity = await testHarness.agent.identity.create({ + didMethod : 'jwk', + metadata : { name: 'Test Identity' }, + store : true + }); + + // Verify that the Identity exists. + let storedIdentity = await testHarness.agent.identity.get({ didUri: identity.did.uri, tenant: identity.did.uri }); + expect(storedIdentity).to.exist; + expect(storedIdentity?.did.uri).to.equal(identity.did.uri); + + // Delete the Identity. + await testHarness.agent.identity.delete({ didUri: identity.did.uri, tenant: identity.did.uri }); + + // Verify that the Identity no longer exists. + storedIdentity = await testHarness.agent.identity.get({ didUri: identity.did.uri, tenant: identity.did.uri }); + expect(storedIdentity).to.not.exist; + + // Verify that the DID still exists + const storedDid = await testHarness.agent.did.get({ didUri: identity.did.uri, tenant: identity.did.uri }); + expect(storedDid).to.not.be.undefined; + expect(storedDid!.uri).to.equal(identity.did.uri); + }); + + it('fails with not found error if the Identity does not exist', async () => { + // Delete an Identity that does not exist. + const didUri = 'did:method:xyz123'; + try { + await testHarness.agent.identity.delete({ didUri }); + expect.fail('Expected an error to be thrown'); + } catch (error: any) { + expect(error.message).to.include('AgentIdentityApi: Failed to purge due to Identity not found'); + } + }); + }); }); } }); \ No newline at end of file diff --git a/packages/agent/tests/local-key-manager.spec.ts b/packages/agent/tests/local-key-manager.spec.ts index a857bd65d..997b3d6a8 100644 --- a/packages/agent/tests/local-key-manager.spec.ts +++ b/packages/agent/tests/local-key-manager.spec.ts @@ -375,6 +375,40 @@ describe('LocalKeyManager', () => { expect(wrappedKeyBytes).to.deep.equal(expectedOutput); }); }); + + describe('deleteKey()', () => { + it('deletes a key', async () => { + // create key that will be stored in the keyStore + const keyUri = await testHarness.agent.keyManager.generateKey({ algorithm: 'secp256k1' }); + + // verify that you can get the key + let key = await testHarness.agent.keyManager.getPublicKey({ keyUri }); + const computedKeyUri = await testHarness.agent.keyManager.getKeyUri({ key }); + expect(computedKeyUri).to.equal(keyUri); + + // delete the key + await testHarness.agent.keyManager.deleteKey({ keyUri }); + + try { + // verify that the key is no longer in the keyStore + key = await testHarness.agent.keyManager.getPublicKey({ keyUri }); + expect.fail('Expected an error to be thrown.'); + } catch(error: any) { + expect(error.message).to.include('Key not found'); + } + }); + + it('errors if key is not found', async () => { + const keyUri = 'urn:jwk:does-not-exist'; + + try { + await testHarness.agent.keyManager.deleteKey({ keyUri }); + expect.fail('Expected an error to be thrown.'); + } catch(error: any) { + expect(error.message).to.include('Key not found'); + } + }); + }); }); }); }); \ No newline at end of file