Skip to content

Commit

Permalink
check for existing keys in BearerDid.import()
Browse files Browse the repository at this point in the history
  • Loading branch information
LiranCohen committed Oct 11, 2024
1 parent 7c33a05 commit 96e25b2
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 38 deletions.
1 change: 1 addition & 0 deletions packages/agent/src/agent-did-resolver-cache.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DidResolutionResult, DidResolverCache, DidResolverCacheLevel, DidResolverCacheLevelParams } from '@web5/dids';
import { Web5PlatformAgent } from './types/agent.js';
import { logger } from '@web5/common';


/**
Expand Down
18 changes: 7 additions & 11 deletions packages/agent/src/local-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,17 +423,13 @@ export class LocalKeyManager implements AgentKeyManager {
// Compute the key URI for the key.
const keyUri = await this.getKeyUri({ key: privateKey });

const existingKey = await this._keyStore.get({ id: keyUri, agent: this.agent, useCache: true });
if (!existingKey) {
// Store the key if it does not already exist in the key store.
await this._keyStore.set({
id : keyUri,
data : privateKey,
agent : this.agent,
preventDuplicates : true,
useCache : true
});
}
await this._keyStore.set({
id : keyUri,
data : privateKey,
agent : this.agent,
preventDuplicates : true,
useCache : true
});

return keyUri;
}
Expand Down
26 changes: 0 additions & 26 deletions packages/agent/tests/local-key-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,6 @@ describe('LocalKeyManager', () => {
expect(importedKey).to.exist;
expect(importedKey).to.deep.equal(key);
});

it('does not attempt to import a key that is already in the key store', async () => {
// scenario: a key already exists within your key store, you attempt to import it again
// the method should not throw an error, but should also not attempt to store the key again

const storeSetSpy = sinon.spy(testHarness.agent.keyManager['_keyStore'], 'set');

// generate a key within the key manager
const keyUri = await testHarness.agent.keyManager.generateKey({ algorithm: 'secp256k1' });
expect(storeSetSpy.callCount).to.equal(1);

// export the key
const exportedKey = await testHarness.agent.keyManager.exportKey({ keyUri });

// reset the spy count
storeSetSpy.resetHistory();

// attempt to import the same key, will not fail
await testHarness.agent.keyManager.importKey({ key: exportedKey });
expect(storeSetSpy.callCount).to.equal(0); //

// sanity import a key generated outside the key manager
const key = await Ed25519.generateKey();
await testHarness.agent.keyManager.importKey({ key });
expect(storeSetSpy.callCount).to.equal(1);
});
});

describe('exportKey()', () => {
Expand Down
9 changes: 8 additions & 1 deletion packages/dids/src/bearer-did.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export class BearerDid {
keyManager?: CryptoApi & KeyImporterExporter<KmsImportKeyParams, KeyIdentifier, KmsExportKeyParams>;
portableDid: PortableDid;
}): Promise<BearerDid> {

// Get all verification methods from the given DID document, including embedded methods.
const verificationMethods = getVerificationMethods({ didDocument: portableDid.document });

Expand All @@ -250,7 +251,13 @@ export class BearerDid {

// If given, import the private key material into the key manager.
for (let key of portableDid.privateKeys ?? []) {
await keyManager.importKey({ key });

// confirm th key does not already exist before importing it to avoid failures from the key manager
const keyUri = await keyManager.getKeyUri({ key });
const keyExists = await keyManager.getPublicKey({ keyUri }).then(() => true).catch(() => false);
if (!keyExists) {
await keyManager.importKey({ key });
}
}

// Validate that the key material for every verification method in the DID document is present
Expand Down
18 changes: 18 additions & 0 deletions packages/dids/tests/bearer-did.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,5 +447,23 @@ describe('BearerDid', () => {
expect(error.message).to.include('Key not found');
}
});

it('does not attempt to import a key that is already in the key manager', async () => {

// create a key manager
const keyManager = new LocalKeyManager();

// Import one of the private keys into the key manager
const privateKey = portableDid.privateKeys![0];
await keyManager.importKey({ key: privateKey });

// spy on the importKey method
const importKeySpy = sinon.spy(keyManager, 'importKey');

// attempt to import the BearerDid with the key manager
const did = await BearerDid.import({ portableDid, keyManager });
expect(did.uri).to.equal(portableDid.uri);
expect(importKeySpy.calledOnce).to.be.false;
});
});
});

0 comments on commit 96e25b2

Please sign in to comment.