fix(rest): make sure OpenAPI parameters with simple types are validat…

…ed by AJV Fixes #6247 Signed-off-by: Raymond Feng <[email protected]>
loopbackio · Sep 9, 2020 · 987c103 · 987c103
1 parent 5783f54
commit 987c103
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 29 deletions.
diff --git a/packages/rest/src/__tests__/unit/coercion/paramStringToInteger.unit.ts b/packages/rest/src/__tests__/unit/coercion/paramStringToInteger.unit.ts
@@ -34,7 +34,7 @@ const INTEGER_PARAM = {
 
 describe('coerce param from string to integer', () => {
   test(INT32_PARAM, '100', 100);
-  test(INT64_PARAM, '9223372036854775807', 9223372036854775807);
+  test(INT64_PARAM, '9007199254740991', 9007199254740991);
 });
 
 describe('coerce param from string to integer - required', function () {

diff --git a/packages/rest/src/__tests__/unit/coercion/paramStringToString.unit.ts b/packages/rest/src/__tests__/unit/coercion/paramStringToString.unit.ts
@@ -18,6 +18,12 @@ const REQUIRED_STRING_PARAM = {
   required: true,
 };
 
+const ENUM_STRING_PARAM: ParameterObject = {
+  in: 'query',
+  name: 'aparameter',
+  schema: {type: 'string', enum: ['A', 'B']},
+};
+
 describe('coerce param from string to string - required', () => {
   context('valid values', () => {
     test(REQUIRED_STRING_PARAM, 'text', 'text');
@@ -33,6 +39,29 @@ describe('coerce param from string to string - required', () => {
   });
 });
 
+describe('coerce param from string to string - enum', () => {
+  context('valid values', () => {
+    test(ENUM_STRING_PARAM, 'A', 'A');
+  });
+
+  context('invalid values trigger ERROR_BAD_REQUEST', () => {
+    const expectedError = RestHttpErrors.invalidData(
+      'C',
+      ENUM_STRING_PARAM.name,
+    );
+    expectedError.details = [
+      {
+        path: '',
+        code: 'enum',
+        message: 'should be equal to one of the allowed values',
+        info: {allowedValues: ['A', 'B']},
+      },
+    ];
+
+    test(ENUM_STRING_PARAM, 'C', expectedError);
+  });
+});
+
 describe('coerce param from string to string - optional', () => {
   context('valid values', () => {
     test(OPTIONAL_STRING_PARAM, 'text', 'text');

diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts
@@ -13,10 +13,10 @@ import debugModule from 'debug';
 import {
   RestHttpErrors,
   validateValueAgainstSchema,
-  ValidationOptions,
   ValueValidationOptions,
 } from '../';
 import {parseJson} from '../parse-json';
+import {ValidationOptions} from '../types';
 import {DEFAULT_AJV_VALIDATION_OPTIONS} from '../validation/ajv-factory.provider';
 import {
   DateCoercionOptions,
@@ -61,31 +61,51 @@ export async function coerceParameter(
   validator.validateParamBeforeCoercion(data);
   if (data === undefined) return data;
 
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  let result: any = data;
+
   switch (OAIType) {
     case 'byte':
-      return coerceBuffer(data, spec);
+      result = coerceBuffer(data, spec);
+      break;
     case 'date':
-      return coerceDatetime(data, spec, {dateOnly: true});
+      result = coerceDatetime(data, spec, {dateOnly: true});
+      break;
     case 'date-time':
-      return coerceDatetime(data, spec);
+      result = coerceDatetime(data, spec);
+      break;
     case 'float':
     case 'double':
     case 'number':
-      return coerceNumber(data, spec);
+      result = coerceNumber(data, spec);
+      break;
     case 'long':
-      return coerceInteger(data, spec, {isLong: true});
+      result = coerceInteger(data, spec, {isLong: true});
+      break;
     case 'integer':
-      return coerceInteger(data, spec);
+      result = coerceInteger(data, spec);
+      break;
     case 'boolean':
-      return coerceBoolean(data, spec);
+      result = coerceBoolean(data, spec);
+      break;
     case 'object':
-      return coerceObject(data, spec, options);
+      result = await coerceObject(data, spec);
+      break;
     case 'string':
     case 'password':
-      return coerceString(data, spec);
-    default:
-      return data;
+      result = coerceString(data, spec);
+      break;
+  }
+
+  if (result != null) {
+    // For date/date-time/byte, we need to pass the raw string value to AJV
+    if (OAIType === 'date' || OAIType === 'date-time' || OAIType === 'byte') {
+      await validateParam(spec, data, options);
+      return result;
+    }
+    result = await validateParam(spec, result, options);
   }
+  return result;
 }
 
 function coerceString(data: string | object, spec: ParameterObject) {
@@ -165,11 +185,7 @@ function coerceBoolean(data: string | object, spec: ParameterObject) {
   throw RestHttpErrors.invalidData(data, spec.name);
 }
 
-async function coerceObject(
-  input: string | object,
-  spec: ParameterObject,
-  options: ValidationOptions = DEFAULT_AJV_VALIDATION_OPTIONS,
-) {
+async function coerceObject(input: string | object, spec: ParameterObject) {
   const data = parseJsonIfNeeded(input, spec);
 
   if (data == null) {
@@ -180,17 +196,25 @@ async function coerceObject(
   if (typeof data !== 'object' || Array.isArray(data))
     throw RestHttpErrors.invalidData(input, spec.name);
 
+  return data;
+}
+
+function validateParam(
+  spec: ParameterObject,
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  data: any,
+  options: ValidationOptions = DEFAULT_AJV_VALIDATION_OPTIONS,
+) {
   const schema = extractSchemaFromSpec(spec);
   if (schema) {
     // Apply coercion based on properties defined by spec.schema
-    await validateValueAgainstSchema(
+    return validateValueAgainstSchema(
       data,
       schema,
       {},
       {...options, coerceTypes: true, source: 'parameter', name: spec.name},
     );
   }
-
   return data;
 }
 

diff --git a/packages/rest/src/validation/openapi-formats.ts b/packages/rest/src/validation/openapi-formats.ts
@@ -27,7 +27,7 @@ export const int64Format: AjvFormat = {
   type: 'number',
   validate: (value: number) => {
     const max = Number.MAX_SAFE_INTEGER; // 9007199254740991
-    const min = Number.MIN_SAFE_INTEGER; // 9007199254740991
+    const min = Number.MIN_SAFE_INTEGER; // -9007199254740991
     return Number.isInteger(value) && value >= min && value <= max;
   },
   async: false,

diff --git a/packages/rest/src/validation/request-body.validator.ts b/packages/rest/src/validation/request-body.validator.ts
@@ -118,11 +118,11 @@ function getKeyForOptions(
 }
 
 /**
- * Validate the request body data against JSON schema.
- * @param body - The request body data.
+ * Validate the value against JSON schema.
+ * @param value - The data value.
  * @param schema - The JSON schema used to perform the validation.
  * @param globalSchemas - Schema references.
- * @param options - Request body validation options.
+ * @param options - Value validation options.
  */
 export async function validateValueAgainstSchema(
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -155,11 +155,11 @@ export async function validateValueAgainstSchema(
   let validationErrors: ajv.ErrorObject[] = [];
   try {
     const validationResult = await validate(value);
-    // When value is optional & values is empty / null, ajv returns null
-    if (validationResult || validationResult === null) {
-      debug(`Value from ${options.source} passed AJV validation.`);
-      return;
-    }
+    debug(
+      `Value from ${options.source} passed AJV validation.`,
+      validationResult,
+    );
+    return validationResult;
   } catch (error) {
     validationErrors = error.errors;
   }