From 61f51b2b84a1d2f41384e9cbf7abccae7165bbce Mon Sep 17 00:00:00 2001 From: Sai Ranjit Tummalapalli Date: Wed, 8 May 2024 19:47:52 +0530 Subject: [PATCH 1/5] fix: remove connection id from query in problem report Signed-off-by: Sai Ranjit Tummalapalli --- .../credentials/v1/__tests__/V1CredentialProtocolCred.test.ts | 1 - .../src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts | 1 - .../src/modules/credentials/protocol/BaseCredentialProtocol.ts | 3 --- .../protocol/v2/__tests__/V2CredentialProtocolCred.test.ts | 1 - packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts | 1 - .../proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts | 1 - 6 files changed, 8 deletions(-) diff --git a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts index 8912207417..c2d9e81203 100644 --- a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts +++ b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts @@ -736,7 +736,6 @@ describe('V1CredentialProtocol', () => { } expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts index 46ae4cfc1b..5d7a2cde62 100644 --- a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts +++ b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts @@ -245,7 +245,6 @@ describe('V1ProofProtocol', () => { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts index 6da4bdc9fb..1ea5accac9 100644 --- a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts +++ b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts @@ -138,13 +138,10 @@ export abstract class BaseCredentialProtocol { const { message: credentialProblemReportMessage, agentContext } = messageContext - const connection = messageContext.assertReadyConnection() - agentContext.config.logger.debug(`Processing problem report with message id ${credentialProblemReportMessage.id}`) const credentialRecord = await this.getByProperties(agentContext, { threadId: credentialProblemReportMessage.threadId, - connectionId: connection.id, }) // Update record diff --git a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts index 3ac571e708..54d270accb 100644 --- a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts +++ b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts @@ -684,7 +684,6 @@ describe('credentialProtocol', () => { expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: '123', }) expect(credentialRepository.update).toHaveBeenCalled() expect(returnedCredentialRecord.errorMessage).toBe('issuance-abandoned: Indy error') diff --git a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts index 6729e94796..94bb11f3d6 100644 --- a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts +++ b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts @@ -116,7 +116,6 @@ export abstract class BaseProofProtocol { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls From de155e00a3dc91849113f7fe19b3157f14247046 Mon Sep 17 00:00:00 2001 From: Sai Ranjit Tummalapalli Date: Wed, 8 May 2024 22:31:33 +0530 Subject: [PATCH 2/5] refactor: add checks to process problem report Signed-off-by: Sai Ranjit Tummalapalli --- .../V1CredentialProtocolCred.test.ts | 1 + .../v1/__tests__/V1ProofProtocol.test.ts | 1 + .../protocol/BaseCredentialProtocol.ts | 17 +++++++++++++++++ .../V2CredentialProtocolCred.test.ts | 1 + .../proofs/protocol/BaseProofProtocol.ts | 19 ++++++++++++++++++- .../v2/__tests__/V2ProofProtocol.test.ts | 1 + 6 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts index c2d9e81203..8912207417 100644 --- a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts +++ b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts @@ -736,6 +736,7 @@ describe('V1CredentialProtocol', () => { } expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', + connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts index 5d7a2cde62..46ae4cfc1b 100644 --- a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts +++ b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts @@ -245,6 +245,7 @@ describe('V1ProofProtocol', () => { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', + connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts index 1ea5accac9..97208168b0 100644 --- a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts +++ b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts @@ -28,6 +28,7 @@ import type { CredentialExchangeRecord } from '../repository' import { EventEmitter } from '../../../agent/EventEmitter' import { DidCommMessageRepository } from '../../../storage/didcomm' +import { ConnectionService } from '../../connections' import { CredentialEventTypes } from '../CredentialEvents' import { CredentialState } from '../models/CredentialState' import { CredentialRepository } from '../repository' @@ -138,12 +139,28 @@ export abstract class BaseCredentialProtocol { const { message: credentialProblemReportMessage, agentContext } = messageContext + const connectionService = agentContext.dependencyManager.resolve(ConnectionService) + + const connection = messageContext.assertReadyConnection() + agentContext.config.logger.debug(`Processing problem report with message id ${credentialProblemReportMessage.id}`) const credentialRecord = await this.getByProperties(agentContext, { threadId: credentialProblemReportMessage.threadId, }) + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) + + // This makes sure that the sender of the incoming message is authorized to do so. + if (!credentialRecord?.connectionId) { + await connectionService.matchIncomingMessageToRequestMessageInOutOfBandExchange(messageContext, { + expectedConnectionId: credentialRecord?.connectionId, + }) + + credentialRecord.connectionId = connection?.id + } + // Update record credentialRecord.errorMessage = `${credentialProblemReportMessage.description.code}: ${credentialProblemReportMessage.description.en}` await this.updateState(agentContext, credentialRecord, CredentialState.Abandoned) diff --git a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts index 54d270accb..3ac571e708 100644 --- a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts +++ b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts @@ -684,6 +684,7 @@ describe('credentialProtocol', () => { expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', + connectionId: '123', }) expect(credentialRepository.update).toHaveBeenCalled() expect(returnedCredentialRecord.errorMessage).toBe('issuance-abandoned: Indy error') diff --git a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts index 94bb11f3d6..21867d6e34 100644 --- a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts +++ b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts @@ -30,6 +30,7 @@ import type { ProofExchangeRecord } from '../repository' import { EventEmitter } from '../../../agent/EventEmitter' import { DidCommMessageRepository } from '../../../storage/didcomm' +import { ConnectionService } from '../../connections' import { ProofEventTypes } from '../ProofEvents' import { ProofState } from '../models/ProofState' import { ProofRepository } from '../repository' @@ -110,14 +111,30 @@ export abstract class BaseProofProtocol ): Promise { - const { message: proofProblemReportMessage, agentContext, connection } = messageContext + const { message: proofProblemReportMessage, agentContext } = messageContext + + const connectionService = agentContext.dependencyManager.resolve(ConnectionService) agentContext.config.logger.debug(`Processing problem report with message id ${proofProblemReportMessage.id}`) + const connection = messageContext.assertReadyConnection() + const proofRecord = await this.getByProperties(agentContext, { threadId: proofProblemReportMessage.threadId, }) + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) + + // This makes sure that the sender of the incoming message is authorized to do so. + if (!proofRecord?.connectionId) { + await connectionService.matchIncomingMessageToRequestMessageInOutOfBandExchange(messageContext, { + expectedConnectionId: proofRecord?.connectionId, + }) + + proofRecord.connectionId = connection?.id + } + // Update record proofRecord.errorMessage = `${proofProblemReportMessage.description.code}: ${proofProblemReportMessage.description.en}` await this.updateState(agentContext, proofRecord, ProofState.Abandoned) diff --git a/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts b/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts index d0fabec0a2..b917ad90f9 100644 --- a/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts +++ b/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts @@ -231,6 +231,7 @@ describe('V2ProofProtocol', () => { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', + connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls From c3ef229cf983ed869b2f8cb601bc84cae235e753 Mon Sep 17 00:00:00 2001 From: Sai Ranjit Tummalapalli Date: Wed, 8 May 2024 22:39:58 +0530 Subject: [PATCH 3/5] fix: tests Signed-off-by: Sai Ranjit Tummalapalli --- .../credentials/v1/__tests__/V1CredentialProtocolCred.test.ts | 1 - .../src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts | 1 - .../protocol/v2/__tests__/V2CredentialProtocolCred.test.ts | 1 - .../modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts | 1 - 4 files changed, 4 deletions(-) diff --git a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts index 8912207417..c2d9e81203 100644 --- a/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts +++ b/packages/anoncreds/src/protocols/credentials/v1/__tests__/V1CredentialProtocolCred.test.ts @@ -736,7 +736,6 @@ describe('V1CredentialProtocol', () => { } expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts index 46ae4cfc1b..5d7a2cde62 100644 --- a/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts +++ b/packages/anoncreds/src/protocols/proofs/v1/__tests__/V1ProofProtocol.test.ts @@ -245,7 +245,6 @@ describe('V1ProofProtocol', () => { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls diff --git a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts index 3ac571e708..54d270accb 100644 --- a/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts +++ b/packages/core/src/modules/credentials/protocol/v2/__tests__/V2CredentialProtocolCred.test.ts @@ -684,7 +684,6 @@ describe('credentialProtocol', () => { expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: '123', }) expect(credentialRepository.update).toHaveBeenCalled() expect(returnedCredentialRecord.errorMessage).toBe('issuance-abandoned: Indy error') diff --git a/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts b/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts index b917ad90f9..d0fabec0a2 100644 --- a/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts +++ b/packages/core/src/modules/proofs/protocol/v2/__tests__/V2ProofProtocol.test.ts @@ -231,7 +231,6 @@ describe('V2ProofProtocol', () => { } expect(proofRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, agentContext, { threadId: 'somethreadid', - connectionId: connection.id, }) expect(repositoryUpdateSpy).toHaveBeenCalledTimes(1) const [[, updatedCredentialRecord]] = repositoryUpdateSpy.mock.calls From 7c6285438f1ec2294a71674e12e934dfb5b5d53b Mon Sep 17 00:00:00 2001 From: Timo Glastra Date: Thu, 9 May 2024 13:34:40 +0800 Subject: [PATCH 4/5] improve check for incoming message connection relation Signed-off-by: Timo Glastra --- .../credentials/v1/V1CredentialProtocol.ts | 17 ++++++----- .../protocols/proofs/v1/V1ProofProtocol.ts | 15 ++++++---- .../__tests__/ConnectionService.test.ts | 29 +++++++++++++++++++ .../connections/services/ConnectionService.ts | 15 +++++++++- .../protocol/BaseCredentialProtocol.ts | 4 ++- .../protocol/v2/V2CredentialProtocol.ts | 5 ++++ .../proofs/protocol/BaseProofProtocol.ts | 4 ++- .../proofs/protocol/v2/V2ProofProtocol.ts | 4 +++ 8 files changed, 77 insertions(+), 16 deletions(-) diff --git a/packages/anoncreds/src/protocols/credentials/v1/V1CredentialProtocol.ts b/packages/anoncreds/src/protocols/credentials/v1/V1CredentialProtocol.ts index ce3a306704..1937a7c20c 100644 --- a/packages/anoncreds/src/protocols/credentials/v1/V1CredentialProtocol.ts +++ b/packages/anoncreds/src/protocols/credentials/v1/V1CredentialProtocol.ts @@ -231,6 +231,7 @@ export class V1CredentialProtocol await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage, lastSentMessage, + expectedConnectionId: credentialRecord.connectionId, }) await this.indyCredentialFormat.processProposal(messageContext.agentContext, { @@ -251,6 +252,8 @@ export class V1CredentialProtocol }) } else { agentContext.config.logger.debug('Credential record does not exists yet for incoming proposal') + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) // No credential record exists with thread id credentialRecord = new CredentialExchangeRecord({ @@ -261,9 +264,6 @@ export class V1CredentialProtocol protocolVersion: 'v1', }) - // Assert - await connectionService.assertConnectionOrOutOfBandExchange(messageContext) - // Save record await credentialRepository.save(messageContext.agentContext, credentialRecord) this.emitStateChangedEvent(messageContext.agentContext, credentialRecord, null) @@ -532,6 +532,7 @@ export class V1CredentialProtocol await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage, lastSentMessage, + expectedConnectionId: credentialRecord.connectionId, }) await this.indyCredentialFormat.processOffer(messageContext.agentContext, { @@ -548,6 +549,9 @@ export class V1CredentialProtocol return credentialRecord } else { + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) + // No credential record exists with thread id credentialRecord = new CredentialExchangeRecord({ connectionId: connection?.id, @@ -558,9 +562,6 @@ export class V1CredentialProtocol protocolVersion: 'v1', }) - // Assert - await connectionService.assertConnectionOrOutOfBandExchange(messageContext) - await this.indyCredentialFormat.processOffer(messageContext.agentContext, { credentialRecord, attachment: offerAttachment, @@ -767,6 +768,7 @@ export class V1CredentialProtocol await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage: proposalMessage ?? undefined, lastSentMessage: offerMessage ?? undefined, + expectedConnectionId: credentialRecord.connectionId, }) // This makes sure that the sender of the incoming message is authorized to do so. @@ -774,7 +776,6 @@ export class V1CredentialProtocol await connectionService.matchIncomingMessageToRequestMessageInOutOfBandExchange(messageContext, { expectedConnectionId: credentialRecord.connectionId, }) - credentialRecord.connectionId = connection?.id } @@ -916,6 +917,7 @@ export class V1CredentialProtocol await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage: offerCredentialMessage, lastSentMessage: requestCredentialMessage, + expectedConnectionId: credentialRecord.connectionId, }) const issueAttachment = issueMessage.getCredentialAttachmentById(INDY_CREDENTIAL_ATTACHMENT_ID) @@ -1022,6 +1024,7 @@ export class V1CredentialProtocol await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage: requestCredentialMessage, lastSentMessage: issueCredentialMessage, + expectedConnectionId: credentialRecord.connectionId, }) // Update record diff --git a/packages/anoncreds/src/protocols/proofs/v1/V1ProofProtocol.ts b/packages/anoncreds/src/protocols/proofs/v1/V1ProofProtocol.ts index dafb943139..df5c931712 100644 --- a/packages/anoncreds/src/protocols/proofs/v1/V1ProofProtocol.ts +++ b/packages/anoncreds/src/protocols/proofs/v1/V1ProofProtocol.ts @@ -198,6 +198,7 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage, lastSentMessage, + expectedConnectionId: proofRecord.connectionId, }) // Update record @@ -209,6 +210,8 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< await this.updateState(agentContext, proofRecord, ProofState.ProposalReceived) } else { agentContext.config.logger.debug('Proof record does not exist yet for incoming proposal') + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) // No proof record exists with thread id proofRecord = new ProofExchangeRecord({ @@ -220,9 +223,6 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< protocolVersion: 'v1', }) - // Assert - await connectionService.assertConnectionOrOutOfBandExchange(messageContext) - await didCommMessageRepository.saveOrUpdateAgentMessage(agentContext, { agentMessage: proposalMessage, associatedRecordId: proofRecord.id, @@ -456,6 +456,7 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage, lastSentMessage, + expectedConnectionId: proofRecord.connectionId, }) await this.indyProofFormat.processRequest(agentContext, { @@ -470,6 +471,9 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< }) await this.updateState(agentContext, proofRecord, ProofState.RequestReceived) } else { + // Assert + await connectionService.assertConnectionOrOutOfBandExchange(messageContext) + // No proof record exists with thread id proofRecord = new ProofExchangeRecord({ connectionId: connection?.id, @@ -491,9 +495,6 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< role: DidCommMessageRole.Receiver, }) - // Assert - await connectionService.assertConnectionOrOutOfBandExchange(messageContext) - // Save in repository await proofRepository.save(agentContext, proofRecord) this.emitStateChangedEvent(agentContext, proofRecord, null) @@ -791,6 +792,7 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage: proposalMessage, lastSentMessage: requestMessage, + expectedConnectionId: proofRecord.connectionId, }) // This makes sure that the sender of the incoming message is authorized to do so. @@ -922,6 +924,7 @@ export class V1ProofProtocol extends BaseProofProtocol implements ProofProtocol< await connectionService.assertConnectionOrOutOfBandExchange(messageContext, { lastReceivedMessage, lastSentMessage, + expectedConnectionId: proofRecord.connectionId, }) // Update record diff --git a/packages/core/src/modules/connections/__tests__/ConnectionService.test.ts b/packages/core/src/modules/connections/__tests__/ConnectionService.test.ts index 0f2d58ff05..24256e561e 100644 --- a/packages/core/src/modules/connections/__tests__/ConnectionService.test.ts +++ b/packages/core/src/modules/connections/__tests__/ConnectionService.test.ts @@ -753,6 +753,35 @@ describe('ConnectionService', () => { }) describe('assertConnectionOrOutOfBandExchange', () => { + it('should throw an error when a expectedConnectionId is present, but no connection is present in the messageContext', async () => { + expect.assertions(1) + + const messageContext = new InboundMessageContext(new AgentMessage(), { + agentContext, + }) + + await expect( + connectionService.assertConnectionOrOutOfBandExchange(messageContext, { + expectedConnectionId: '123', + }) + ).rejects.toThrow('Expected incoming message to be from connection 123 but no connection found.') + }) + + it('should throw an error when a expectedConnectionId is present, but does not match with connection id present in the messageContext', async () => { + expect.assertions(1) + + const messageContext = new InboundMessageContext(new AgentMessage(), { + agentContext, + connection: getMockConnection({ state: DidExchangeState.InvitationReceived, id: 'something' }), + }) + + await expect( + connectionService.assertConnectionOrOutOfBandExchange(messageContext, { + expectedConnectionId: 'something-else', + }) + ).rejects.toThrow('Expected incoming message to be from connection something-else but connection is something.') + }) + it('should not throw an error when a connection record with state complete is present in the messageContext', async () => { expect.assertions(1) diff --git a/packages/core/src/modules/connections/services/ConnectionService.ts b/packages/core/src/modules/connections/services/ConnectionService.ts index f823ade7c0..34e06982f1 100644 --- a/packages/core/src/modules/connections/services/ConnectionService.ts +++ b/packages/core/src/modules/connections/services/ConnectionService.ts @@ -459,13 +459,26 @@ export class ConnectionService { { lastSentMessage, lastReceivedMessage, + expectedConnectionId, }: { lastSentMessage?: AgentMessage | null lastReceivedMessage?: AgentMessage | null + expectedConnectionId?: string } = {} ) { const { connection, message } = messageContext + if (expectedConnectionId && !connection) { + throw new CredoError( + `Expected incoming message to be from connection ${expectedConnectionId} but no connection found.` + ) + } + if (expectedConnectionId && connection?.id !== expectedConnectionId) { + throw new CredoError( + `Expected incoming message to be from connection ${expectedConnectionId} but connection is ${connection?.id}.` + ) + } + // Check if we have a ready connection. Verification is already done somewhere else. Return if (connection) { connection.assertReady() @@ -559,7 +572,7 @@ export class ConnectionService { messageContext: InboundMessageContext, { expectedConnectionId }: { expectedConnectionId?: string } ) { - if (expectedConnectionId && messageContext.connection?.id === expectedConnectionId) { + if (expectedConnectionId && messageContext.connection?.id !== expectedConnectionId) { throw new CredoError( `Expecting incoming message to have connection ${expectedConnectionId}, but incoming connection is ${ messageContext.connection?.id ?? 'undefined' diff --git a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts index 97208168b0..71f3d8ceab 100644 --- a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts +++ b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts @@ -150,7 +150,9 @@ export abstract class BaseCredentialProtocol Date: Thu, 9 May 2024 12:19:11 +0530 Subject: [PATCH 5/5] fix: connection ready check for connection less flows Signed-off-by: Sai Ranjit Tummalapalli --- .../modules/credentials/protocol/BaseCredentialProtocol.ts | 4 +--- .../core/src/modules/proofs/protocol/BaseProofProtocol.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts index 71f3d8ceab..5e9259ee20 100644 --- a/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts +++ b/packages/core/src/modules/credentials/protocol/BaseCredentialProtocol.ts @@ -137,12 +137,10 @@ export abstract class BaseCredentialProtocol ): Promise { - const { message: credentialProblemReportMessage, agentContext } = messageContext + const { message: credentialProblemReportMessage, agentContext, connection } = messageContext const connectionService = agentContext.dependencyManager.resolve(ConnectionService) - const connection = messageContext.assertReadyConnection() - agentContext.config.logger.debug(`Processing problem report with message id ${credentialProblemReportMessage.id}`) const credentialRecord = await this.getByProperties(agentContext, { diff --git a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts index 61590255fa..fa146221ff 100644 --- a/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts +++ b/packages/core/src/modules/proofs/protocol/BaseProofProtocol.ts @@ -111,14 +111,12 @@ export abstract class BaseProofProtocol ): Promise { - const { message: proofProblemReportMessage, agentContext } = messageContext + const { message: proofProblemReportMessage, agentContext, connection } = messageContext const connectionService = agentContext.dependencyManager.resolve(ConnectionService) agentContext.config.logger.debug(`Processing problem report with message id ${proofProblemReportMessage.id}`) - const connection = messageContext.assertReadyConnection() - const proofRecord = await this.getByProperties(agentContext, { threadId: proofProblemReportMessage.threadId, })