Skip to content

Commit ef6b8bc

Browse files
nbAmitnbAmit
authored
refactor: resolve feedback for problem report (openwallet-foundation#584)
Signed-off-by: Amit Padmani <[email protected]>
1 parent 4d7d3c1 commit ef6b8bc

File tree

20 files changed

+63
-92
lines changed

20 files changed

+63
-92
lines changed

‎packages/core/src/agent/MessageReceiver.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@ import { Lifecycle, scoped } from 'tsyringe'
99

1010
import { AriesFrameworkError } from '../error'
1111
import { ConnectionService } from '../modules/connections/services/ConnectionService'
12-
import { ProblemReportError, ProblemReportMessage } from '../modules/problem-reports'
12+
import { ProblemReportError, ProblemReportMessage, ProblemReportReason } from '../modules/problem-reports'
1313
import { JsonTransformer } from '../utils/JsonTransformer'
1414
import { MessageValidator } from '../utils/MessageValidator'
1515
import { replaceLegacyDidSovPrefixOnMessage } from '../utils/messageType'
1616

17-
import { CommonMessageType } from './../modules/common/messages/CommonMessageType'
1817
import { AgentConfig } from './AgentConfig'
1918
import { Dispatcher } from './Dispatcher'
2019
import { EnvelopeService } from './EnvelopeService'
@@ -23,9 +22,6 @@ import { TransportService } from './TransportService'
2322
import { createOutboundMessage } from './helpers'
2423
import { InboundMessageContext } from './models/InboundMessageContext'
2524

26-
export enum ProblemReportReason {
27-
MessageParseFailure = 'message-parse-failure',
28-
}
2925
@scoped(Lifecycle.ContainerScoped)
3026
export class MessageReceiver {
3127
private config: AgentConfig
@@ -226,7 +222,7 @@ export class MessageReceiver {
226222
connection: ConnectionRecord,
227223
plaintextMessage: PlaintextMessage
228224
) {
229-
if (plaintextMessage['@type'] === CommonMessageType.ProblemReport) {
225+
if (plaintextMessage['@type'] === ProblemReportMessage.type) {
230226
throw new AriesFrameworkError(message)
231227
}
232228
const problemReportMessage = new ProblemReportMessage({

‎packages/core/src/decorators/signature/SignatureDecoratorUtils.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Wallet } from '../../wallet/Wallet'
22

3-
import { ConnectionProblemReportError, ConnectionProblemReportReason } from '../../modules/connections/errors'
3+
import { AriesFrameworkError } from '../../error'
44
import { BufferEncoder } from '../../utils/BufferEncoder'
55
import { JsonEncoder } from '../../utils/JsonEncoder'
66
import { Buffer } from '../../utils/buffer'
@@ -29,9 +29,7 @@ export async function unpackAndVerifySignatureDecorator(
2929
const isValid = await wallet.verify(signerVerkey, signedData, signature)
3030

3131
if (!isValid) {
32-
throw new ConnectionProblemReportError('Signature is not valid', {
33-
problemCode: ConnectionProblemReportReason.RequestProcessingError,
34-
})
32+
throw new AriesFrameworkError('Signature is not valid')
3533
}
3634

3735
// TODO: return Connection instance instead of raw json

‎packages/core/src/modules/common/messages/CommonMessageType.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
export enum CommonMessageType {
22
Ack = 'https://didcomm.org/notification/1.0/ack',
3-
ProblemReport = 'https://didcomm.org/notification/1.0/problem-report',
43
}

‎packages/core/src/modules/connections/models/ConnectionState.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,4 @@ export enum ConnectionState {
1010
Requested = 'requested',
1111
Responded = 'responded',
1212
Complete = 'complete',
13-
None = 'none',
1413
}

‎packages/core/src/modules/connections/repository/ConnectionRecord.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export interface ConnectionRecordProps {
2929
imageUrl?: string
3030
multiUseInvitation: boolean
3131
mediatorId?: string
32-
errorMsg?: string
32+
errorMessage?: string
3333
}
3434

3535
export type CustomConnectionTags = TagsBase
@@ -69,7 +69,7 @@ export class ConnectionRecord
6969

7070
public threadId?: string
7171
public mediatorId?: string
72-
public errorMsg?: string
72+
public errorMessage?: string
7373

7474
public static readonly type = 'ConnectionRecord'
7575
public readonly type = ConnectionRecord.type
@@ -96,7 +96,7 @@ export class ConnectionRecord
9696
this.imageUrl = props.imageUrl
9797
this.multiUseInvitation = props.multiUseInvitation
9898
this.mediatorId = props.mediatorId
99-
this.errorMsg = props.errorMsg
99+
this.errorMessage = props.errorMessage
100100
}
101101
}
102102

‎packages/core/src/modules/connections/services/ConnectionService.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,16 @@ export class ConnectionService {
323323
connectionRecord.assertState(ConnectionState.Requested)
324324
connectionRecord.assertRole(ConnectionRole.Invitee)
325325

326-
const connectionJson = await unpackAndVerifySignatureDecorator(message.connectionSig, this.wallet)
326+
let connectionJson = null
327+
try {
328+
connectionJson = await unpackAndVerifySignatureDecorator(message.connectionSig, this.wallet)
329+
} catch (error) {
330+
if (error instanceof AriesFrameworkError) {
331+
throw new ConnectionProblemReportError(error.message, {
332+
problemCode: ConnectionProblemReportReason.RequestProcessingError,
333+
})
334+
}
335+
}
327336

328337
const connection = JsonTransformer.fromJSON(connectionJson, Connection)
329338
await MessageValidator.validate(connection)
@@ -417,7 +426,7 @@ export class ConnectionService {
417426
public async processProblemReport(
418427
messageContext: InboundMessageContext<ConnectionProblemReportMessage>
419428
): Promise<ConnectionRecord> {
420-
const { message: connectionProblemReportMessage, recipientVerkey } = messageContext
429+
const { message: connectionProblemReportMessage, recipientVerkey, senderVerkey } = messageContext
421430

422431
this.logger.debug(`Processing connection problem report for verkey ${recipientVerkey}`)
423432

@@ -433,8 +442,12 @@ export class ConnectionService {
433442
)
434443
}
435444

436-
connectionRecord.errorMsg = `${connectionProblemReportMessage.description.code} : ${connectionProblemReportMessage.description.en}`
437-
await this.updateState(connectionRecord, ConnectionState.None)
445+
if (connectionRecord.theirKey && connectionRecord.theirKey !== senderVerkey) {
446+
throw new AriesFrameworkError("Sender verkey doesn't match verkey of connection record")
447+
}
448+
449+
connectionRecord.errorMessage = `${connectionProblemReportMessage.description.code} : ${connectionProblemReportMessage.description.en}`
450+
await this.update(connectionRecord)
438451
return connectionRecord
439452
}
440453

@@ -529,6 +542,10 @@ export class ConnectionService {
529542
})
530543
}
531544

545+
public update(connectionRecord: ConnectionRecord) {
546+
return this.connectionRepository.update(connectionRecord)
547+
}
548+
532549
/**
533550
* Retrieve all connections records
534551
*

‎packages/core/src/modules/credentials/CredentialState.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,4 @@ export enum CredentialState {
1414
CredentialIssued = 'credential-issued',
1515
CredentialReceived = 'credential-received',
1616
Done = 'done',
17-
None = 'none',
1817
}

‎packages/core/src/modules/credentials/__tests__/CredentialService.test.ts

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ describe('CredentialService', () => {
981981
})
982982
})
983983

984-
test(`updates state to ${CredentialState.None} and returns credential record`, async () => {
984+
test(`updates problem report error message and returns credential record`, async () => {
985985
const repositoryUpdateSpy = jest.spyOn(credentialRepository, 'update')
986986

987987
// given
@@ -992,7 +992,7 @@ describe('CredentialService', () => {
992992

993993
// then
994994
const expectedCredentialRecord = {
995-
state: CredentialState.None,
995+
errorMessage: 'issuance-abandoned: Indy error',
996996
}
997997
expect(credentialRepository.getSingleByQuery).toHaveBeenNthCalledWith(1, {
998998
threadId: 'somethreadid',
@@ -1003,28 +1003,6 @@ describe('CredentialService', () => {
10031003
expect(updatedCredentialRecord).toMatchObject(expectedCredentialRecord)
10041004
expect(returnedCredentialRecord).toMatchObject(expectedCredentialRecord)
10051005
})
1006-
1007-
test(`emits stateChange event from ${CredentialState.OfferReceived} to ${CredentialState.None}`, async () => {
1008-
const eventListenerMock = jest.fn()
1009-
eventEmitter.on<CredentialStateChangedEvent>(CredentialEventTypes.CredentialStateChanged, eventListenerMock)
1010-
1011-
// given
1012-
mockFunction(credentialRepository.getSingleByQuery).mockReturnValue(Promise.resolve(credential))
1013-
1014-
// when
1015-
await credentialService.processProblemReport(messageContext)
1016-
1017-
// then
1018-
expect(eventListenerMock).toHaveBeenCalledWith({
1019-
type: 'CredentialStateChanged',
1020-
payload: {
1021-
previousState: CredentialState.OfferReceived,
1022-
credentialRecord: expect.objectContaining({
1023-
state: CredentialState.None,
1024-
}),
1025-
},
1026-
})
1027-
})
10281006
})
10291007

10301008
describe('repository methods', () => {

‎packages/core/src/modules/credentials/repository/CredentialRecord.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export interface CredentialRecordProps {
3333
credentialAttributes?: CredentialPreviewAttribute[]
3434
autoAcceptCredential?: AutoAcceptCredential
3535
linkedAttachments?: Attachment[]
36-
errorMsg?: string
36+
errorMessage?: string
3737
}
3838

3939
export type CustomCredentialTags = TagsBase
@@ -50,7 +50,7 @@ export class CredentialRecord extends BaseRecord<DefaultCredentialTags, CustomCr
5050
public credentialId?: string
5151
public state!: CredentialState
5252
public autoAcceptCredential?: AutoAcceptCredential
53-
public errorMsg?: string
53+
public errorMessage?: string
5454

5555
// message data
5656
@Type(() => ProposeCredentialMessage)
@@ -90,7 +90,7 @@ export class CredentialRecord extends BaseRecord<DefaultCredentialTags, CustomCr
9090
this.credentialAttributes = props.credentialAttributes
9191
this.autoAcceptCredential = props.autoAcceptCredential
9292
this.linkedAttachments = props.linkedAttachments
93-
this.errorMsg = props.errorMsg
93+
this.errorMessage = props.errorMessage
9494
}
9595
}
9696

‎packages/core/src/modules/credentials/services/CredentialService.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,9 @@ export class CredentialService {
733733
public async processProblemReport(
734734
messageContext: InboundMessageContext<CredentialProblemReportMessage>
735735
): Promise<CredentialRecord> {
736-
const { message: credentialProblemReportMessage, connection } = messageContext
736+
const { message: credentialProblemReportMessage } = messageContext
737+
738+
const connection = messageContext.assertReadyConnection()
737739

738740
this.logger.debug(`Processing problem report with id ${credentialProblemReportMessage.id}`)
739741

@@ -743,8 +745,8 @@ export class CredentialService {
743745
)
744746

745747
// Update record
746-
credentialRecord.errorMsg = `${credentialProblemReportMessage.description.code}: ${credentialProblemReportMessage.description.en}`
747-
await this.updateState(credentialRecord, CredentialState.None)
748+
credentialRecord.errorMessage = `${credentialProblemReportMessage.description.code}: ${credentialProblemReportMessage.description.en}`
749+
await this.update(credentialRecord)
748750
return credentialRecord
749751
}
750752

0 commit comments

Comments
 (0)