From bfdfc4c4aa58f4a182d2f3aad8ea6303efb10f42 Mon Sep 17 00:00:00 2001 From: Mircea Nistor Date: Thu, 5 Oct 2023 16:57:42 +0200 Subject: [PATCH] feat(did-manager): matching DIDs by alias should not depend on the provider (#1218) fixes #1215 BREAKING CHANGE: The behavior of `DIDManager` has changed when working with `alias`. It is mostly ignoring `provider` unless it is used to create new identifiers. `AbstractDIDStore` APIs have been adapted and implementations have changed. --- __tests__/shared/didManager.ts | 112 ++++++++++-------- __tests__/shared/webDidFlow.ts | 6 - .../src/identifier/did-store.ts | 6 +- .../data-store/src/identifier/did-store.ts | 6 +- .../src/abstract-identifier-store.ts | 2 +- packages/did-manager/src/id-manager.ts | 26 ++-- packages/did-manager/src/memory-did-store.ts | 4 +- 7 files changed, 90 insertions(+), 72 deletions(-) diff --git a/__tests__/shared/didManager.ts b/__tests__/shared/didManager.ts index 286775d5b..77dd750cd 100644 --- a/__tests__/shared/didManager.ts +++ b/__tests__/shared/didManager.ts @@ -37,7 +37,7 @@ export default (testContext: { identifier = await agent.didManagerCreate({ // this expects the `did:ethr` provider to matchPrefix and use the `arbitrum:goerli` network specifier provider: 'did:pkh', - options: { chainId: "1"} + options: { chainId: '1' }, }) expect(identifier.provider).toEqual('did:pkh') //expect(identifier.did).toMatch(/^did:pkh:eip155:*$/) @@ -81,7 +81,7 @@ export default (testContext: { provider: 'did:jwk', options: { keyType, - } + }, }) expect(identifier.provider).toEqual('did:jwk') expect(identifier.keys[0].type).toEqual(keyType) @@ -94,8 +94,9 @@ export default (testContext: { provider: 'did:jwk', options: { keyType, - privateKeyHex: 'f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b0f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b0', - } + privateKeyHex: + 'f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b0f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b0', + }, }) expect(identifier.provider).toEqual('did:jwk') expect(identifier.keys[0].type).toEqual(keyType) @@ -108,40 +109,46 @@ export default (testContext: { provider: 'did:jwk', options: { privateKeyHex: 'f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b0', - } + }, }) expect(identifier.provider).toEqual('did:jwk') expect(identifier.keys[0].type).toEqual(keyType) expect(identifier.controllerKeyId).toEqual(identifier.keys[0].kid) }) it('should throw error for invalid privateKEyHex', async () => { - await expect( agent.didManagerCreate({ - provider: 'did:jwk', - options: { - privateKeyHex: '1234', - } - })).rejects.toThrow() + await expect( + agent.didManagerCreate({ + provider: 'did:jwk', + options: { + privateKeyHex: '1234', + }, + }), + ).rejects.toThrow() expect(identifier.provider).toEqual('did:jwk') }) it('should throw error for invalid keyUse parameter', async () => { - await expect( agent.didManagerCreate({ - provider: 'did:jwk', - options: { - keyType: 'Secp256k1', - keyUse: 'signing', - } - })).rejects.toThrow('illegal_argument: Key use must be sig or enc') + await expect( + agent.didManagerCreate({ + provider: 'did:jwk', + options: { + keyType: 'Secp256k1', + keyUse: 'signing', + }, + }), + ).rejects.toThrow('illegal_argument: Key use must be sig or enc') expect(identifier.provider).toEqual('did:jwk') }) it('should throw error for invalid Ed25519 key use', async () => { - await expect( agent.didManagerCreate({ - provider: 'did:jwk', - alias: 'test1', - options: { - keyType: 'Ed25519', - keyUse: 'enc', - } - })).rejects.toThrow('illegal_argument: Ed25519 keys cannot be used for encryption') + await expect( + agent.didManagerCreate({ + provider: 'did:jwk', + alias: 'test1', + options: { + keyType: 'Ed25519', + keyUse: 'enc', + }, + }), + ).rejects.toThrow('illegal_argument: Ed25519 keys cannot be used for encryption') expect(identifier.provider).toEqual('did:jwk') }) @@ -154,7 +161,7 @@ export default (testContext: { options: { keyType, privateKeyHex: 'f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b1', - } + }, }) expect(identifier.provider).toEqual('did:key') expect(identifier.keys[0].type).toEqual(keyType) @@ -167,7 +174,7 @@ export default (testContext: { provider: 'did:web', alias: 'example.com', }), - ).rejects.toThrow('Identifier with alias: example.com, provider: did:web already exists') + ).rejects.toThrow(/Identifier with alias: example.com already exists/) }) it('should get identifier', async () => { @@ -187,38 +194,36 @@ export default (testContext: { it('should get or create identifier', async () => { const identifier3 = await agent.didManagerGetOrCreate({ - alias: 'alice', - provider: 'did:ethr:goerli', + alias: 'aliceDID11', + provider: 'did:ethr:mainnet', }) const identifier4 = await agent.didManagerGetOrCreate({ - alias: 'alice', - provider: 'did:ethr:goerli', + alias: 'aliceDID11', }) expect(identifier3).toEqual(identifier4) const identifierKey1 = await agent.didManagerGetOrCreate({ - alias: 'carol', + alias: 'caroline', provider: 'did:key', }) const identifierKey2 = await agent.didManagerGetOrCreate({ - alias: 'carol', - provider: 'did:key', + alias: 'caroline', }) expect(identifierKey1).toEqual(identifierKey2) const identifier5 = await agent.didManagerGetOrCreate({ - alias: 'alice', + alias: 'aliceDID11', provider: 'did:ethr', }) - expect(identifier5).not.toEqual(identifier4) + expect(identifier5).toEqual(identifier4) const identifier6 = await agent.didManagerGetByAlias({ - alias: 'alice', + alias: 'aliceDID11', provider: 'did:ethr', }) @@ -230,22 +235,22 @@ export default (testContext: { expect(allIdentifiers.length).toBeGreaterThanOrEqual(5) const aliceIdentifiers = await agent.didManagerFind({ - alias: 'alice', + alias: 'aliceDID11', }) - expect(aliceIdentifiers.length).toEqual(2) + expect(aliceIdentifiers.length).toEqual(1) - const goerliIdentifiers = await agent.didManagerFind({ - provider: 'did:ethr:goerli', + const ethrIdentifiers = await agent.didManagerFind({ + provider: 'did:ethr', }) - expect(goerliIdentifiers.length).toBeGreaterThanOrEqual(1) + expect(ethrIdentifiers.length).toBeGreaterThanOrEqual(1) // Default provider 'did:ethr:goerli' - await agent.didManagerCreate({ provider: 'did:ethr:goerli' }) + await agent.didManagerCreate({ provider: 'did:ethr' }) - const goerliIdentifiers2 = await agent.didManagerFind({ - provider: 'did:ethr:goerli', + const ethrIdentifiers2 = await agent.didManagerFind({ + provider: 'did:ethr', }) - expect(goerliIdentifiers2.length).toEqual(goerliIdentifiers.length + 1) + expect(ethrIdentifiers2.length).toEqual(ethrIdentifiers.length + 1) }) it('should delete identifier', async () => { @@ -436,5 +441,18 @@ export default (testContext: { expect(identifier2).toEqual({ ...identifier, alias: 'dave' }) }) + + it('should refuse to getOrCreate identifier with existing alias but different provider', async () => { + expect.assertions(1) + await agent.didManagerGetOrCreate({ alias: 'I am the same', provider: 'did:ethr' }) + await expect( + agent.didManagerGetOrCreate({ + alias: 'I am the same', + provider: 'did:key', + }), + ).rejects.toThrow( + /illegal_argument: Identifier with alias:.*already exists.*but was created with a different provider.*/, + ) + }) }) } diff --git a/__tests__/shared/webDidFlow.ts b/__tests__/shared/webDidFlow.ts index 61e47cf87..560ada324 100644 --- a/__tests__/shared/webDidFlow.ts +++ b/__tests__/shared/webDidFlow.ts @@ -66,10 +66,8 @@ export default (testContext: { it('should create identifier with alias: alice', async () => { alice = await agent.didManagerGetOrCreate({ alias: 'alice', - provider: 'did:ethr:goerli', }) - expect(alice.provider).toEqual('did:ethr:goerli') expect(alice.alias).toEqual('alice') expect(alice.did).toBeDefined() }) @@ -77,10 +75,8 @@ export default (testContext: { it('should create identifier with alias: bob', async () => { bob = await agent.didManagerGetOrCreate({ alias: 'bob', - provider: 'did:ethr:goerli', }) - expect(bob.provider).toEqual('did:ethr:goerli') expect(bob.alias).toEqual('bob') expect(bob.did).toBeDefined() }) @@ -115,12 +111,10 @@ export default (testContext: { it('issuer - Alice, subject - Bob', async () => { const a = await agent.didManagerGetOrCreate({ alias: 'alice', - provider: 'did:ethr:goerli', }) const b = await agent.didManagerGetOrCreate({ alias: 'bob', - provider: 'did:ethr:goerli', }) const verifiableCredential = await agent.createVerifiableCredential({ diff --git a/packages/data-store-json/src/identifier/did-store.ts b/packages/data-store-json/src/identifier/did-store.ts index 1186e2a8d..f7d253a7f 100644 --- a/packages/data-store-json/src/identifier/did-store.ts +++ b/packages/data-store-json/src/identifier/did-store.ts @@ -50,8 +50,8 @@ export class DIDStoreJson extends AbstractDIDStore { if (did !== undefined && alias === undefined) { where = { did } - } else if (did === undefined && alias !== undefined && provider !== undefined) { - where = { alias, provider } + } else if (did === undefined && alias !== undefined) { + where = { alias } } else { throw Error('invalid_arguments: DidStoreJson.get requires did or (alias and provider)') } @@ -61,7 +61,7 @@ export class DIDStoreJson extends AbstractDIDStore { identifier = this.cacheTree.dids[where.did] } else { identifier = Object.values(this.cacheTree.dids).find( - (iid: IIdentifier) => iid.provider === where.provider && iid.alias === where.alias, + (iid: IIdentifier) => iid.alias === where.alias, ) } diff --git a/packages/data-store/src/identifier/did-store.ts b/packages/data-store/src/identifier/did-store.ts index f7636f72f..c9f845d57 100644 --- a/packages/data-store/src/identifier/did-store.ts +++ b/packages/data-store/src/identifier/did-store.ts @@ -39,11 +39,11 @@ export class DIDStore extends AbstractDIDStore { alias: string provider: string }): Promise { - let where = {} + let where: { did?: string; alias?: string; provider?: string } = {} if (did !== undefined && alias === undefined) { where = { did } - } else if (did === undefined && alias !== undefined && provider !== undefined) { - where = { alias, provider } + } else if (did === undefined && alias !== undefined) { + where = { alias } } else { throw Error('[veramo:data-store:identifier-store] Get requires did or (alias and provider)') } diff --git a/packages/did-manager/src/abstract-identifier-store.ts b/packages/did-manager/src/abstract-identifier-store.ts index 28e291100..0c4e17f58 100644 --- a/packages/did-manager/src/abstract-identifier-store.ts +++ b/packages/did-manager/src/abstract-identifier-store.ts @@ -7,7 +7,7 @@ import { IIdentifier } from '@veramo/core-types' export abstract class AbstractDIDStore { abstract importDID(args: IIdentifier): Promise abstract getDID(args: { did: string }): Promise - abstract getDID(args: { alias: string; provider: string }): Promise + abstract getDID(args: { alias: string }): Promise abstract deleteDID(args: { did: string }): Promise abstract listDIDs(args: { alias?: string; provider?: string }): Promise } diff --git a/packages/did-manager/src/id-manager.ts b/packages/did-manager/src/id-manager.ts index 78fea219d..116b5e303 100644 --- a/packages/did-manager/src/id-manager.ts +++ b/packages/did-manager/src/id-manager.ts @@ -93,9 +93,8 @@ export class DIDManager implements IAgentPlugin { } /** {@inheritDoc @veramo/core-types#IDIDManager.didManagerGetByAlias} */ - async didManagerGetByAlias({ alias, provider }: IDIDManagerGetByAliasArgs): Promise { - const providerName = provider || this.defaultProvider - return this.store.getDID({ alias, provider: providerName }) + async didManagerGetByAlias({ alias }: IDIDManagerGetByAliasArgs): Promise { + return this.store.getDID({ alias }) } /** {@inheritDoc @veramo/core-types#IDIDManager.didManagerCreate} */ @@ -107,11 +106,11 @@ export class DIDManager implements IAgentPlugin { if (args?.alias !== undefined) { let existingIdentifier try { - existingIdentifier = await this.store.getDID({ alias: args.alias, provider: providerName }) + existingIdentifier = await this.store.getDID({ alias: args.alias }) } catch (e) {} if (existingIdentifier) { throw Error( - `illegal_argument: Identifier with alias: ${args.alias}, provider: ${providerName} already exists: ${existingIdentifier.did}`, + `illegal_argument: Identifier with alias: ${args.alias} already exists: ${existingIdentifier.did}`, ) } } @@ -133,14 +132,21 @@ export class DIDManager implements IAgentPlugin { { provider, alias, kms, options }: IDIDManagerGetOrCreateArgs, context: IAgentContext, ): Promise { + let identifier: IIdentifier | undefined try { - const providerName = provider || this.defaultProvider - // @ts-ignore - const identifier = await this.store.getDID({ alias, provider: providerName }) - return identifier + identifier = await this.store.getDID({ alias }) } catch { - return this.didManagerCreate({ provider, alias, kms, options }, context) + const providerName = provider || this.defaultProvider + return this.didManagerCreate({ provider: providerName, alias, kms, options }, context) } + if (identifier && provider && identifier.provider !== provider) { + if (this.getProvider(identifier.provider) !== this.getProvider(provider)) { + throw Error( + `illegal_argument: Identifier with alias: ${alias}, already exists ${identifier.did}, but was created with a different provider: ${identifier.provider}!==${provider}`, + ) + } + } + return identifier } /** {@inheritDoc @veramo/core-types#IDIDManager.didManagerUpdate} */ diff --git a/packages/did-manager/src/memory-did-store.ts b/packages/did-manager/src/memory-did-store.ts index b552217c4..1845439ce 100644 --- a/packages/did-manager/src/memory-did-store.ts +++ b/packages/did-manager/src/memory-did-store.ts @@ -21,9 +21,9 @@ export class MemoryDIDStore extends AbstractDIDStore { if (did && !alias) { if (!this.identifiers[did]) throw Error(`not_found: IIdentifier not found with did=${did}`) return this.identifiers[did] - } else if (!did && alias && provider) { + } else if (!did && alias) { for (const key of Object.keys(this.identifiers)) { - if (this.identifiers[key].alias === alias && this.identifiers[key].provider === provider) { + if (this.identifiers[key].alias === alias) { return this.identifiers[key] } }