feat: add validation to JSON transformer (#830)

Signed-off-by: Moriarty <[email protected]>

packages/core/src/agent/Agent.ts

@@ -277,7 +277,7 @@ export class Agent {
   }
 
   private async getMediationConnection(mediatorInvitationUrl: string) {
-    const outOfBandInvitation = await this.oob.parseInvitation(mediatorInvitationUrl)
+    const outOfBandInvitation = this.oob.parseInvitation(mediatorInvitationUrl)
     const outOfBandRecord = await this.oob.findByInvitationId(outOfBandInvitation.id)
     const [connection] = outOfBandRecord ? await this.connections.findAllByOutOfBandId(outOfBandRecord.id) : []
 

packages/core/src/agent/MessageReceiver.ts

@@ -13,7 +13,6 @@ import { ConnectionsModule } from '../modules/connections'
 import { ProblemReportError, ProblemReportMessage, ProblemReportReason } from '../modules/problem-reports'
 import { isValidJweStructure } from '../utils/JWE'
 import { JsonTransformer } from '../utils/JsonTransformer'
-import { MessageValidator } from '../utils/MessageValidator'
 import { canHandleMessageType, parseMessageType, replaceLegacyDidSovPrefixOnMessage } from '../utils/messageType'
 
 import { AgentConfig } from './AgentConfig'
@@ -168,7 +167,6 @@ export class MessageReceiver {
     let message: AgentMessage
     try {
       message = await this.transformMessage(plaintextMessage)
-      await this.validateMessage(message)
     } catch (error) {
       if (connection) await this.sendProblemReportMessage(error.message, connection, plaintextMessage)
       throw error
@@ -209,25 +207,19 @@ export class MessageReceiver {
     }
 
     // Cast the plain JSON object to specific instance of Message extended from AgentMessage
-    return JsonTransformer.fromJSON(message, MessageClass)
-  }
-
-  /**
-   * Validate an AgentMessage instance.
-   * @param message agent message to validate
-   */
-  private async validateMessage(message: AgentMessage) {
+    let messageTransformed: AgentMessage
     try {
-      await MessageValidator.validate(message)
+      messageTransformed = JsonTransformer.fromJSON(message, MessageClass)
     } catch (error) {
       this.logger.error(`Error validating message ${message.type}`, {
         errors: error,
-        message: message.toJSON(),
+        message: JSON.stringify(message),
       })
       throw new ProblemReportError(`Error validating message ${message.type}`, {
         problemCode: ProblemReportReason.MessageParseFailure,
       })
     }
+    return messageTransformed
   }
 
   /**

packages/core/src/agent/MessageSender.ts

@@ -313,7 +313,7 @@ export class MessageSender {
     }
 
     try {
-      await MessageValidator.validate(message)
+      MessageValidator.validateSync(message)
     } catch (error) {
       this.logger.error(
         `Aborting sending outbound message ${message.type} to ${service.serviceEndpoint}. Message validation failed`,

packages/core/src/agent/__tests__/AgentMessage.test.ts

@@ -1,6 +1,6 @@
 import { TestMessage } from '../../../tests/TestMessage'
+import { ClassValidationError } from '../../error/ClassValidationError'
 import { JsonTransformer } from '../../utils'
-import { MessageValidator } from '../../utils/MessageValidator'
 import { IsValidMessageType, parseMessageType } from '../../utils/messageType'
 import { AgentMessage } from '../AgentMessage'
 
@@ -32,7 +32,7 @@ describe('AgentMessage', () => {
 
       const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)
 
-      await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
+      expect(message).toBeInstanceOf(CustomProtocolMessage)
     })
 
     it('successfully validates if the message type minor version is lower than the supported message type', async () => {
@@ -43,37 +43,56 @@ describe('AgentMessage', () => {
 
       const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)
 
-      await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
+      expect(message).toBeInstanceOf(CustomProtocolMessage)
     })
 
-    it('successfully validates if the message type minor version is higher than the supported message type', async () => {
+    it('successfully validates if the message type minor version is higher than the supported message type', () => {
       const json = {
         '@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
         '@type': 'https://didcomm.org/fake-protocol/1.8/message',
       }
 
       const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)
 
-      await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
+      expect(message).toBeInstanceOf(CustomProtocolMessage)
     })
 
-    it('throws a validation error if the message type major version differs from the supported message type', async () => {
-      expect.assertions(1)
-
+    it('throws a validation error if the message type major version differs from the supported message type', () => {
       const json = {
         '@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
         '@type': 'https://didcomm.org/fake-protocol/2.0/message',
       }
 
-      const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)
-
-      await expect(MessageValidator.validate(message)).rejects.toMatchObject([
-        {
-          constraints: {
-            isValidMessageType: 'type does not match the expected message type (only minor version may be lower)',
+      expect(() => JsonTransformer.fromJSON(json, CustomProtocolMessage)).toThrowError(ClassValidationError)
+      try {
+        JsonTransformer.fromJSON(json, CustomProtocolMessage)
+      } catch (error) {
+        const thrownError = error as ClassValidationError
+        expect(thrownError.message).toEqual(
+          'CustomProtocolMessage: Failed to validate class.\nAn instance of CustomProtocolMessage has failed the validation:\n - property type has failed the following constraints: isValidMessageType \n'
+        )
+        expect(thrownError.validationErrors).toMatchObject([
+          {
+            target: {
+              appendedAttachments: undefined,
+              id: 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
+              l10n: undefined,
+              pleaseAck: undefined,
+              service: undefined,
+              thread: undefined,
+              timing: undefined,
+              transport: undefined,
+              type: 'https://didcomm.org/fake-protocol/2.0/message',
+            },
+            value: 'https://didcomm.org/fake-protocol/2.0/message',
+            property: 'type',
+            children: [],
+            constraints: {
+              isValidMessageType: 'type does not match the expected message type (only minor version may be lower)',
+            },
           },
-        },
-      ])
+        ])
+      }
     })
   })
 })

packages/core/src/decorators/ack/AckDecorator.test.ts

@@ -1,4 +1,5 @@
 import { BaseMessage } from '../../agent/BaseMessage'
+import { ClassValidationError } from '../../error/ClassValidationError'
 import { JsonTransformer } from '../../utils/JsonTransformer'
 import { MessageValidator } from '../../utils/MessageValidator'
 import { Compose } from '../../utils/mixins'
@@ -25,22 +26,9 @@ describe('Decorators | AckDecoratorExtension', () => {
   })
 
   test('transforms Json to AckDecorator class', () => {
-    const transformed = JsonTransformer.fromJSON({ '~please_ack': {} }, TestMessage)
-
-    expect(transformed).toEqual({
-      pleaseAck: {
-        on: ['RECEIPT'],
-      },
-    })
-    expect(transformed).toBeInstanceOf(TestMessage)
-  })
-
-  test('successfully transforms ack decorator with on field present', () => {
     const transformed = JsonTransformer.fromJSON(
       {
-        '~please_ack': {
-          on: ['RECEIPT'],
-        },
+        '~please_ack': {},
         '@id': '7517433f-1150-46f2-8495-723da61b872a',
         '@type': 'https://didcomm.org/test-protocol/1.0/test-message',
       },
@@ -88,45 +76,56 @@ describe('Decorators | AckDecoratorExtension', () => {
       TestMessage
     )
 
-    await expect(MessageValidator.validate(transformedWithDefault)).resolves.toBeUndefined()
+    expect(MessageValidator.validateSync(transformedWithDefault)).toBeUndefined()
+  })
 
-    const transformedWithoutDefault = JsonTransformer.fromJSON(
-      {
-        '~please_ack': {
-          on: ['OUTCOME'],
+  test('transforms Json to AckDecorator class', () => {
+    const transformed = () =>
+      JsonTransformer.fromJSON(
+        {
+          '~please_ack': {},
+          '@id': undefined,
+          '@type': undefined,
         },
-        '@id': '7517433f-1150-46f2-8495-723da61b872a',
-        '@type': 'https://didcomm.org/test-protocol/1.0/test-message',
-      },
-      TestMessage
-    )
-
-    await expect(MessageValidator.validate(transformedWithoutDefault)).resolves.toBeUndefined()
+        TestMessage
+      )
 
-    const transformedWithIncorrectValue = JsonTransformer.fromJSON(
-      {
-        '~please_ack': {
-          on: ['NOT_A_VALID_VALUE'],
+    expect(() => transformed()).toThrow(ClassValidationError)
+    try {
+      transformed()
+    } catch (e) {
+      const caughtError = e as ClassValidationError
+      expect(caughtError.message).toEqual(
+        'TestMessage: Failed to validate class.\nAn instance of TestMessage has failed the validation:\n - property id has failed the following constraints: matches \n\nAn instance of TestMessage has failed the validation:\n - property type has failed the following constraints: matches \n'
+      )
+      expect(caughtError.validationErrors).toMatchObject([
+        {
+          children: [],
+          constraints: {
+            matches: 'id must match /[-_./a-zA-Z0-9]{8,64}/ regular expression',
+          },
+          property: 'id',
+          target: {
+            pleaseAck: {
+              on: ['RECEIPT'],
+            },
+          },
+          value: undefined,
         },
-        '@id': '7517433f-1150-46f2-8495-723da61b872a',
-        '@type': 'https://didcomm.org/test-protocol/1.0/test-message',
-      },
-      TestMessage
-    )
-
-    await expect(MessageValidator.validate(transformedWithIncorrectValue)).rejects.toMatchObject([
-      {
-        children: [
-          {
-            children: [],
-            constraints: { isEnum: 'each value in on must be a valid enum value' },
-            property: 'on',
-            target: { on: ['NOT_A_VALID_VALUE'] },
-            value: ['NOT_A_VALID_VALUE'],
+        {
+          children: [],
+          constraints: {
+            matches: 'type must match /(.*?)([a-zA-Z0-9._-]+)\\/(\\d[^/]*)\\/([a-zA-Z0-9._-]+)$/ regular expression',
           },
-        ],
-        property: 'pleaseAck',
-      },
-    ])
+          property: 'type',
+          target: {
+            pleaseAck: {
+              on: ['RECEIPT'],
+            },
+          },
+          value: undefined,
+        },
+      ])
+    }
   })
 })

packages/core/src/decorators/service/ServiceDecorator.test.ts

@@ -25,7 +25,10 @@ describe('Decorators | ServiceDecoratorExtension', () => {
   })
 
   test('transforms Json to ServiceDecorator class', () => {
-    const transformed = JsonTransformer.fromJSON({ '~service': service }, TestMessage)
+    const transformed = JsonTransformer.fromJSON(
+      { '@id': 'randomID', '@type': 'https://didcomm.org/fake-protocol/1.5/message', '~service': service },
+      TestMessage
+    )
 
     expect(transformed.service).toEqual(service)
     expect(transformed).toBeInstanceOf(TestMessage)

packages/core/src/decorators/transport/TransportDecorator.test.ts

@@ -1,14 +1,14 @@
+import { ClassValidationError } from '../../error/ClassValidationError'
 import { JsonTransformer } from '../../utils/JsonTransformer'
 import { MessageValidator } from '../../utils/MessageValidator'
 
 import { TransportDecorator, ReturnRouteTypes } from './TransportDecorator'
 
 const validTransport = (transportJson: Record<string, unknown>) =>
-  MessageValidator.validate(JsonTransformer.fromJSON(transportJson, TransportDecorator))
-const expectValid = (transportJson: Record<string, unknown>) =>
-  expect(validTransport(transportJson)).resolves.toBeUndefined()
+  MessageValidator.validateSync(JsonTransformer.fromJSON(transportJson, TransportDecorator))
+const expectValid = (transportJson: Record<string, unknown>) => expect(validTransport(transportJson)).toBeUndefined()
 const expectInvalid = (transportJson: Record<string, unknown>) =>
-  expect(validTransport(transportJson)).rejects.not.toBeNull()
+  expect(() => validTransport(transportJson)).toThrowError(ClassValidationError)
 
 const valid = {
   all: {
@@ -60,18 +60,18 @@ describe('Decorators | TransportDecorator', () => {
     expect(json).toEqual(transformed)
   })
 
-  it('should only allow correct return_route values', async () => {
+  it('should only allow correct return_route values', () => {
     expect.assertions(4)
-    await expectValid(valid.all)
-    await expectValid(valid.none)
-    await expectValid(valid.thread)
-    await expectInvalid(invalid.random)
+    expectValid(valid.all)
+    expectValid(valid.none)
+    expectValid(valid.thread)
+    expectInvalid(invalid.random)
   })
 
   it('should require return_route_thread when return_route is thread', async () => {
     expect.assertions(3)
-    await expectValid(valid.thread)
-    await expectInvalid(invalid.invalidThreadId)
-    await expectInvalid(invalid.missingThreadId)
+    expectValid(valid.thread)
+    expectInvalid(invalid.invalidThreadId)
+    expectInvalid(invalid.missingThreadId)
   })
 })

packages/core/src/error/ClassValidationError.ts

@@ -0,0 +1,24 @@
+import type { ValidationError } from 'class-validator'
+
+import { AriesFrameworkError } from './AriesFrameworkError'
+
+export class ClassValidationError extends AriesFrameworkError {
+  public validationErrors: ValidationError[]
+
+  public validationErrorsToString() {
+    return this.validationErrors?.map((error) => error.toString(true)).join('\n') ?? ''
+  }
+
+  public constructor(
+    message: string,
+    { classType, cause, validationErrors }: { classType: string; cause?: Error; validationErrors?: ValidationError[] }
+  ) {
+    const validationErrorsStringified = validationErrors?.map((error) => error.toString()).join('\n')
+    super(
+      `${classType}: ${message}
+${validationErrorsStringified}`,
+      { cause }
+    )
+    this.validationErrors = validationErrors ?? []
+  }
+}

packages/core/src/error/ValidationErrorUtils.ts

@@ -0,0 +1,9 @@
+import { ValidationError } from 'class-validator'
+
+export function isValidationErrorArray(e: ValidationError[] | unknown): boolean {
+  if (Array.isArray(e)) {
+    const isErrorArray = e.length > 0 && e.every((err) => err instanceof ValidationError)
+    return isErrorArray
+  }
+  return false
+}

packages/core/src/error/__tests__/ValidationErrorUtils.test.ts

@@ -0,0 +1,24 @@
+import { ValidationError } from 'class-validator'
+
+import { isValidationErrorArray } from '../ValidationErrorUtils'
+
+describe('ValidationErrorUtils', () => {
+  test('returns true for an array of ValidationErrors', () => {
+    const error = new ValidationError()
+    const errorArray = [error, error]
+    const isErrorArray = isValidationErrorArray(errorArray)
+    expect(isErrorArray).toBeTruthy
+  })
+
+  test('returns false for an array of strings', () => {
+    const errorArray = ['hello', 'world']
+    const isErrorArray = isValidationErrorArray(errorArray)
+    expect(isErrorArray).toBeFalsy
+  })
+
+  test('returns false for a non array', () => {
+    const error = new ValidationError()
+    const isErrorArray = isValidationErrorArray(error)
+    expect(isErrorArray).toBeFalsy
+  })
+})

packages/core/src/error/index.ts

@@ -2,3 +2,4 @@ export * from './AriesFrameworkError'
 export * from './RecordNotFoundError'
 export * from './RecordDuplicateError'
 export * from './IndySdkError'
+export * from './ClassValidationError'