feat: (main) decline request#2353
feat: (main) decline request#2353rmlearney-digicatapult wants to merge 9 commits intoopenwallet-foundation:mainfrom
Conversation
|
rmlearney-digicatapult
changed the title
| // For routing the problem report message | ||
| const routing = | ||
| outOfBandRecord.reusable || outOfBandRecord.outOfBandInvitation.getInlineServices().length === 0 | ||
| ? await this.routingService.getRouting(this.agentContext) | ||
| : undefined | ||
|
|
||
| // Trigger creation of dids in the connectionRecord for sending the problem report | ||
| if (connectionRecord.protocol === HandshakeProtocol.DidExchange) { | ||
| await this.didExchangeProtocol.createResponse( | ||
| this.agentContext, | ||
| connectionRecord, | ||
| outOfBandRecord, | ||
| routing | ||
| ) | ||
| } else { | ||
| throw new CredoError(`Connection protocol is ${connectionRecord.protocol} not didexchange`) | ||
| } |
There was a problem hiding this comment.
I'm not sure we should trigger the whole response creation logic just to decline the request? Since we don't send the response, we don't send the signed attachments from the response, and thus it won't be possible for the other party to verify the problem report came from the inviter.
The only way to do that is by wrapping the problem report using the same key that was included in the invitation.
There was a problem hiding this comment.
Are you able to give me any pointers on how to do that more efficiently? I assumed the creation of the outbound message would automatically wrap/sign the message with the correct keys.
| // Alice's agent reflects this in her OOB record and connection record | ||
| expect(aliceConnectionAfterDecline.state).toBe(DidExchangeState.Abandoned) | ||
| const aliceOobAfterDecline = await aliceAgent.modules.oob.findById(aliceOutOfBandRecord.id) | ||
| expect(aliceOobAfterDecline.state).toBe(OutOfBandState.Done) | ||
|
|
||
| // Bob should receive the problem report correctly | ||
| expect(bobSpy).toHaveBeenCalled() | ||
| const [bobMessage] = bobSpy.mock.calls[0] | ||
|
|
||
| expect(bobSpy2).toHaveBeenCalled() | ||
| const [spy2] = bobSpy2.mock.calls | ||
| // expect(bobConnectionSpy).toHaveBeenCalled() | ||
| // const [spyMessage] = bobConnectionSpy.mock.calls[0] | ||
|
|
||
| console.debug(aliceMessage) | ||
| console.log(bobMessage) | ||
| console.log(spy2) |
There was a problem hiding this comment.
This needs to be clean up. I am curious, how is the decline request currently handled by Bob? Since it should not allow the problem report to be assocaited with the invitation/connection, since we can't verify whether the sender of the problem report is the inviter based on the current code in declineRequest
There was a problem hiding this comment.
The tests were WIP, but I agree the problem report must contain something known to both Alice and Bob which would either be the invitationId from the OOB record, or the threadId from the connectionRecord. I set the threadId of the problem report in the declineRequest method to equal that of the connectionRecord.
TimoGlastra
left a comment
TimoGlastra
left a comment
There was a problem hiding this comment.
Thanks for this PR, i have some concerns on the security of this implementation
2 participants
Linked to issue #2338
Have created a method in ConnectionsAPI.ts for gracefully declining an incoming connection request in the request-received state by sending a problem report to the requester and marking the connection as
abandonedon our side. The OOB invite state is also transitioned toDoneon our side if not reusable to prevent replay.Submitting for your review and insights