Skip to content

Commit

Permalink
feat(did-manager): matching DIDs by alias should not depend on the pr…
Browse files Browse the repository at this point in the history
…ovider (#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.
  • Loading branch information
mirceanis authored Oct 5, 2023
1 parent fbf5c69 commit bfdfc4c
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 72 deletions.
112 changes: 65 additions & 47 deletions __tests__/shared/didManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:*$/)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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')
})

Expand All @@ -154,7 +161,7 @@ export default (testContext: {
options: {
keyType,
privateKeyHex: 'f3157fbbb356a0d56a84a1a9752f81d0638cce4153168bd1b46f68a6e62b82b1',
}
},
})
expect(identifier.provider).toEqual('did:key')
expect(identifier.keys[0].type).toEqual(keyType)
Expand All @@ -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 () => {
Expand All @@ -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',
})

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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.*/,
)
})
})
}
6 changes: 0 additions & 6 deletions __tests__/shared/webDidFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,17 @@ 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()
})

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()
})
Expand Down Expand Up @@ -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({
Expand Down
6 changes: 3 additions & 3 deletions packages/data-store-json/src/identifier/did-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)')
}
Expand All @@ -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,
)
}

Expand Down
6 changes: 3 additions & 3 deletions packages/data-store/src/identifier/did-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ export class DIDStore extends AbstractDIDStore {
alias: string
provider: string
}): Promise<IIdentifier> {
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)')
}
Expand Down
2 changes: 1 addition & 1 deletion packages/did-manager/src/abstract-identifier-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { IIdentifier } from '@veramo/core-types'
export abstract class AbstractDIDStore {
abstract importDID(args: IIdentifier): Promise<boolean>
abstract getDID(args: { did: string }): Promise<IIdentifier>
abstract getDID(args: { alias: string; provider: string }): Promise<IIdentifier>
abstract getDID(args: { alias: string }): Promise<IIdentifier>
abstract deleteDID(args: { did: string }): Promise<boolean>
abstract listDIDs(args: { alias?: string; provider?: string }): Promise<IIdentifier[]>
}
26 changes: 16 additions & 10 deletions packages/did-manager/src/id-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,8 @@ export class DIDManager implements IAgentPlugin {
}

/** {@inheritDoc @veramo/core-types#IDIDManager.didManagerGetByAlias} */
async didManagerGetByAlias({ alias, provider }: IDIDManagerGetByAliasArgs): Promise<IIdentifier> {
const providerName = provider || this.defaultProvider
return this.store.getDID({ alias, provider: providerName })
async didManagerGetByAlias({ alias }: IDIDManagerGetByAliasArgs): Promise<IIdentifier> {
return this.store.getDID({ alias })
}

/** {@inheritDoc @veramo/core-types#IDIDManager.didManagerCreate} */
Expand All @@ -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}`,
)
}
}
Expand All @@ -133,14 +132,21 @@ export class DIDManager implements IAgentPlugin {
{ provider, alias, kms, options }: IDIDManagerGetOrCreateArgs,
context: IAgentContext<IKeyManager>,
): Promise<IIdentifier> {
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} */
Expand Down
4 changes: 2 additions & 2 deletions packages/did-manager/src/memory-did-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
}
Expand Down

0 comments on commit bfdfc4c

Please sign in to comment.