Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: didcomm message handler should attempt to pass message to other handlers #1064

Merged
merged 3 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions __tests__/restAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import keyManager from './shared/keyManager'
import didManager from './shared/didManager'
import didCommPacking from './shared/didCommPacking'
import didWithFakeDidFlow from './shared/didCommWithFakeDidFlow'
import didCommAndDataStoreWithCredentials from './shared/didCommAndDataStoreWithCredentials'
import messageHandler from './shared/messageHandler'
import didDiscovery from './shared/didDiscovery'
import utils from './shared/utils'
Expand Down Expand Up @@ -280,6 +281,7 @@ describe('REST integration tests', () => {
messageHandler(testContext)
didCommPacking(testContext)
didWithFakeDidFlow(testContext)
didCommAndDataStoreWithCredentials(testContext)
didDiscovery(testContext)
utils(testContext)
credentialStatus(testContext)
Expand Down
193 changes: 193 additions & 0 deletions __tests__/shared/didCommAndDataStoreWithCredentials.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// noinspection ES6PreferShortImport

import {
IAgentOptions,
IDIDManager,
IEventListener,
IIdentifier,
IKeyManager,
IResolver,
TAgent,
} from '../../packages/core/src'
import { IDIDComm, IPackedDIDCommMessage } from '../../packages/did-comm/src'
import { v4 } from "uuid"
import { VerifiableCredential } from '@veramo/core'

type ConfiguredAgent = TAgent<IDIDManager & IKeyManager & IResolver & IDIDComm>

const DIDCommEventSniffer: IEventListener = {
eventTypes: ['DIDCommV2Message-sent', 'DIDCommV2Message-received'],
onEvent: jest.fn(),
}

export default (testContext: {
getAgent: () => ConfiguredAgent
setup: (options?: IAgentOptions) => Promise<boolean>
tearDown: () => Promise<boolean>
}) => {
describe('DID comm using did:fake flow', () => {
let agent: ConfiguredAgent
let sender: IIdentifier
let receiver: IIdentifier

beforeAll(async () => {
await testContext.setup({ plugins: [DIDCommEventSniffer] })
agent = testContext.getAgent()

sender = await agent.didManagerImport({
did: 'did:fake:z6MkgbqNU4uF9NKSz5BqJQ4XKVHuQZYcUZP8pXGsJC8nTHwo',
keys: [
{
type: 'Ed25519',
kid: 'didcomm-senderKey-1',
publicKeyHex: '1fe9b397c196ab33549041b29cf93be29b9f2bdd27322f05844112fad97ff92a',
privateKeyHex:
'b57103882f7c66512dc96777cbafbeb2d48eca1e7a867f5a17a84e9a6740f7dc1fe9b397c196ab33549041b29cf93be29b9f2bdd27322f05844112fad97ff92a',
kms: 'local',
},
],
services: [
{
id: 'msg1',
type: 'DIDCommMessaging',
serviceEndpoint: 'http://localhost:3002/messaging',
},
],
provider: 'did:fake',
alias: 'sender',
})

receiver = await agent.didManagerImport({
did: 'did:fake:z6MkrPhffVLBZpxH7xvKNyD4sRVZeZsNTWJkLdHdgWbfgNu3',
keys: [
{
type: 'Ed25519',
kid: 'didcomm-receiverKey-1',
publicKeyHex: 'b162e405b6485eff8a57932429b192ec4de13c06813e9028a7cdadf0e2703636',
privateKeyHex:
'19ed9b6949cfd0f9a57e30f0927839a985fa699491886ebcdda6a954d869732ab162e405b6485eff8a57932429b192ec4de13c06813e9028a7cdadf0e2703636',
kms: 'local',
},
],
services: [
{
id: 'msg2',
type: 'DIDCommMessaging',
serviceEndpoint: 'http://localhost:3002/messaging',
},
],
provider: 'did:fake',
alias: 'receiver',
})
return true
})
afterAll(testContext.tearDown)

it('should send a message', async () => {
expect.assertions(2)

const message = {
type: 'test',
to: receiver.did,
from: sender.did,
id: 'test',
body: { hello: 'world' },
}
const packedMessage = await agent.packDIDCommMessage({
packing: 'authcrypt',
message,
})
await agent.sendDIDCommMessage({
messageId: '123',
packedMessage,
recipientDidUrl: receiver.did,
})

const messages = await agent.dataStoreORMGetMessagesCount({})
expect(messages).toEqual(1)
const vcs = await agent.dataStoreORMGetVerifiableCredentialsCount({})
expect(vcs).toEqual(0)
})

const vc = (creator: IIdentifier, proofFormat: string): Promise<VerifiableCredential> => {
return agent.createVerifiableCredential({
credential: {
issuer: { id: creator.did },
'@context': ['https://www.w3.org/2018/credentials/v1', 'https://veramo.io/contexts/profile/v1'],
type: ['VerifiableCredential', 'Profile'],
issuanceDate: new Date().toISOString(),
credentialSubject: {
name: 'No, 33',
},
},
save: false,
proofFormat: proofFormat,
})
}

const packed = (vc: VerifiableCredential): Promise<IPackedDIDCommMessage> => {
return agent.packDIDCommMessage({ packing: 'none', message: {
id: v4(),
type: 'w3c.vc',
from: sender.did,
to: receiver.did,
body: vc
}})
}

it('should save LDS credential found inside DIDCommMessage', async () => {
expect.assertions(2)
const creator = await agent.didManagerGetOrCreate({ alias: 'messageCreator1', provider: 'did:ethr'})

const verifiableCredential = await vc(creator, 'lds')
const packedMessage = await packed(verifiableCredential)

await agent.sendDIDCommMessage({
messageId: 'test-jwt-success',
packedMessage,
recipientDidUrl: sender.did,
})
const messages = await agent.dataStoreORMGetMessagesCount({})
expect(messages).toEqual(2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testing message flow by count is prone to some weird test failures when multiple tests produce messages.

It would be better to use message IDs, or something else to check if they get processed correctly.

const vcs = await agent.dataStoreORMGetVerifiableCredentialsCount({})
expect(vcs).toEqual(1)
})

it('should save JWT credential found inside DIDCommMessage', async () => {
expect.assertions(2)
const creator = await agent.didManagerGetOrCreate({ alias: 'messageCreator1', provider: 'did:ethr'})

const verifiableCredential = await vc(creator, 'jwt')
const packedMessage = await packed(verifiableCredential)

await agent.sendDIDCommMessage({
messageId: 'test-jwt-success',
packedMessage,
recipientDidUrl: sender.did,
})
const messages = await agent.dataStoreORMGetMessagesCount({})
expect(messages).toEqual(3)
const vcs = await agent.dataStoreORMGetVerifiableCredentialsCount({})
expect(vcs).toEqual(2)
})


it('should save JWT credential found inside DIDCommMessage', async () => {
expect.assertions(2)
const creator = await agent.didManagerGetOrCreate({ alias: 'messageCreator1', provider: 'did:ethr'})

const verifiableCredential = await vc(creator, 'EthereumEip712Signature2021')
const packedMessage = await packed(verifiableCredential)

await agent.sendDIDCommMessage({
messageId: 'test-jwt-success',
packedMessage,
recipientDidUrl: sender.did,
})
const messages = await agent.dataStoreORMGetMessagesCount({})
expect(messages).toEqual(4)
const vcs = await agent.dataStoreORMGetVerifiableCredentialsCount({})
expect(vcs).toEqual(3)
})
})
}
1 change: 1 addition & 0 deletions __tests__/shared/messageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export default (testContext: {
const allMessages = await agent.dataStoreORMGetMessages()
const count = await agent.dataStoreORMGetMessagesCount()
expect(allMessages.length).toEqual(count)
expect(count).toEqual(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates some coupling between the tests and the order in which they are executed.
Perhaps the expectation should be that the count is not zero, if another test in this suite produces a message, but not give it a specific value in case other suites create messages of their own.

})
})
}
14 changes: 12 additions & 2 deletions packages/did-comm/src/message-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,19 @@ export class DIDCommMessageHandler extends AbstractMessageHandler {
message.addMetaData({ type: 'didCommMetaData', value: JSON.stringify(unpackedMessage.metaData) })
context.agent.emit('DIDCommV2Message-received', unpackedMessage)

return message
// DIDCommMessageHandler should attempt to forward message to next handler, but
// shouldn't throw an error if other handlers fail
let superHandled
try {
superHandled = await super.handle(message, context)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm seeing here is that a message is parsed by this handler and it could be returned directly (which would also emit the validatedMessage event) but it can also be forwarded to other handlers in case they might be able to extract more value from it.
I like this idea a lot.
I wonder how we can incorporate it as the default way to handle all messages at some point.

Maybe message handlers can flag things as "good enough" so that the top-level handler knows not to throw an error at the end of the pipeline.

} catch (e) {
debug(`Could not handle DIDCommV2Message in downstream handlers: ${e}`)
}

// if downstream message handlers failed, still treat original unpacked DIDCommV2Message as good
return superHandled || message
} catch (e) {
debug(`Could not unpack message as DIDComm v2: ${e}`)
debug(`Could not unpack message as DIDCommV2Message: ${e}`)
}
}
}
Expand Down