Repository: stoplightio/spectral
Pull request #2046 (2022-02-03 06:35:45)

feat(rulesets): validate API security in oas-operation-security-defined
This PR adds checking in the function for oas[23]-operation-security-defined to also check the API-level security requirement to ensure it also only references defined security schemes.

I admit that it is stretching the intent of these rules a bit, to check the API-level definition in a rule that by its name would only work on the operation-level security requirement. But it seemed like overkill to define an entirely new rule to perform the same checking as this one, just at the global level.  And in some sense the API level security _is_ the operation-level security for any operation that does not override it, so it didn't seem too unreasonable to include these checks here.

UPDATE: I've updated this PR to add checking for _all_ the fields in a security requirements object -- not just the first one.

**Checklist**

- [x] Tests added / updated
- [ ] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [X] No



Patch:
diff --git a/packages/rulesets/src/oas/functions/oasOpSecurityDefined.ts b/packages/rulesets/src/oas/functions/oasOpSecurityDefined.ts
index 31f42c10..37edd2ec 100644
--- a/packages/rulesets/src/oas/functions/oasOpSecurityDefined.ts
+++ b/packages/rulesets/src/oas/functions/oasOpSecurityDefined.ts
@@ -20,7 +20,7 @@ type Options = {
   schemesPath: JsonPath;
 };
 
-export default createRulesetFunction<{ paths: Record<string, unknown> }, Options>(
+export default createRulesetFunction<{ paths: Record<string, unknown>; security: unknown[] }, Options>(
   {
     input: {
       type: 'object',
@@ -28,6 +28,9 @@ export default createRulesetFunction<{ paths: Record<string, unknown> }, Options
         paths: {
           type: 'object',
         },
+        security: {
+          type: 'array',
+        },
       },
     },
     options: {
@@ -50,6 +53,29 @@ export default createRulesetFunction<{ paths: Record<string, unknown> }, Options
     const schemes = _get(targetVal, schemesPath);
     const allDefs = isObject(schemes) ? Object.keys(schemes) : [];
 
+    // Check global security requirements
+
+    const { security } = targetVal;
+
+    if (Array.isArray(security)) {
+      for (const [index, value] of security.entries()) {
+        if (!isObject(value)) {
+          continue;
+        }
+
+        const securityKeys = Object.keys(value);
+
+        for (const securityKey of securityKeys) {
+          if (!allDefs.includes(securityKey)) {
+            results.push({
+              message: `API "security" values must match a scheme defined in the "${schemesPath.join('.')}" object.`,
+              path: ['security', index, securityKey],
+            });
+          }
+        }
+      }
+    }
+
     for (const { path, operation, value } of getAllOperations(paths)) {
       if (!isObject(value)) continue;
 
@@ -66,11 +92,15 @@ export default createRulesetFunction<{ paths: Record<string, unknown> }, Options
 
         const securityKeys = Object.keys(value);
 
-        if (securityKeys.length > 0 && !allDefs.includes(securityKeys[0])) {
-          results.push({
-            message: 'Operation must not reference an undefined security scheme.',
-            path: ['paths', path, operation, 'security', index],
-          });
+        for (const securityKey of securityKeys) {
+          if (!allDefs.includes(securityKey)) {
+            results.push({
+              message: `Operation "security" values must match a scheme defined in the "${schemesPath.join(
+                '.',
+              )}" object.`,
+              path: ['paths', path, operation, 'security', index, securityKey],
+            });
+          }
         }
       }
     }
diff --git a/packages/rulesets/src/oas/index.ts b/packages/rulesets/src/oas/index.ts
index 6e7a4d4f..8d7e817f 100644
--- a/packages/rulesets/src/oas/index.ts
+++ b/packages/rulesets/src/oas/index.ts
@@ -449,6 +449,7 @@ const ruleset = {
     },
     'oas2-operation-security-defined': {
       description: 'Operation "security" values must match a scheme defined in the "securityDefinitions" object.',
+      message: '{{error}}',
       recommended: true,
       formats: [oas2],
       type: 'validation',
@@ -590,6 +591,7 @@ const ruleset = {
     'oas3-operation-security-defined': {
       description:
         'Operation "security" values must match a scheme defined in the "components.securitySchemes" object.',
+      message: '{{error}}',
       recommended: true,
       formats: [oas3],
       type: 'validation',


Test patch:
diff --git a/packages/rulesets/src/oas/__tests__/oas2-operation-security-defined.test.ts b/packages/rulesets/src/oas/__tests__/oas2-operation-security-defined.test.ts
index df143124..92d714ac 100644
--- a/packages/rulesets/src/oas/__tests__/oas2-operation-security-defined.test.ts
+++ b/packages/rulesets/src/oas/__tests__/oas2-operation-security-defined.test.ts
@@ -5,6 +5,7 @@ testRule('oas2-operation-security-defined', [
   {
     name: 'a correct object (just in body)',
     document: {
+      swagger: '2.0',
       securityDefinitions: {
         apikey: {},
       },
@@ -23,6 +24,27 @@ testRule('oas2-operation-security-defined', [
     errors: [],
   },
 
+  {
+    name: 'a correct object (API-level security)',
+    document: {
+      swagger: '2.0',
+      securityDefinitions: {
+        apikey: {},
+      },
+      security: [
+        {
+          apikey: [],
+        },
+      ],
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [],
+  },
+
   {
     name: 'invalid object',
     document: {
@@ -43,7 +65,89 @@ testRule('oas2-operation-security-defined', [
     errors: [
       {
         message: 'Operation "security" values must match a scheme defined in the "securityDefinitions" object.',
-        path: ['paths', '/path', 'get', 'security', '0'],
+        path: ['paths', '/path', 'get', 'security', '0', 'apikey'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'invalid object (API-level security)',
+    document: {
+      swagger: '2.0',
+      securityDefinitions: {},
+      security: [
+        {
+          apikey: [],
+        },
+      ],
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'API "security" values must match a scheme defined in the "securityDefinitions" object.',
+        path: ['security', '0', 'apikey'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'valid and invalid object',
+    document: {
+      swagger: '2.0',
+      securityDefinitions: {
+        apikey: {},
+      },
+      paths: {
+        '/path': {
+          get: {
+            security: [
+              {
+                apikey: [],
+                basic: [],
+              },
+            ],
+          },
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'Operation "security" values must match a scheme defined in the "securityDefinitions" object.',
+        path: ['paths', '/path', 'get', 'security', '0', 'basic'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'valid and invalid object (API-level security)',
+    document: {
+      swagger: '2.0',
+      securityDefinitions: {
+        apikey: {},
+      },
+      security: [
+        {
+          apikey: [],
+          basic: [],
+        },
+      ],
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'API "security" values must match a scheme defined in the "securityDefinitions" object.',
+        path: ['security', '0', 'basic'],
         severity: DiagnosticSeverity.Warning,
       },
     ],
diff --git a/packages/rulesets/src/oas/__tests__/oas3-operation-security-defined.test.ts b/packages/rulesets/src/oas/__tests__/oas3-operation-security-defined.test.ts
index ea518c72..f3d7f521 100644
--- a/packages/rulesets/src/oas/__tests__/oas3-operation-security-defined.test.ts
+++ b/packages/rulesets/src/oas/__tests__/oas3-operation-security-defined.test.ts
@@ -25,6 +25,28 @@ testRule('oas3-operation-security-defined', [
     },
     errors: [],
   },
+  {
+    name: 'validate a correct object (API-level security)',
+    document: {
+      openapi: '3.0.2',
+      components: {
+        securitySchemes: {
+          apikey: {},
+        },
+        security: [
+          {
+            apikey: [],
+          },
+        ],
+      },
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [],
+  },
 
   {
     name: 'return errors on invalid object',
@@ -46,7 +68,93 @@ testRule('oas3-operation-security-defined', [
     errors: [
       {
         message: 'Operation "security" values must match a scheme defined in the "components.securitySchemes" object.',
-        path: ['paths', '/path', 'get', 'security', '0'],
+        path: ['paths', '/path', 'get', 'security', '0', 'apikey'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'return errors on invalid object (API-level)',
+    document: {
+      openapi: '3.0.2',
+      components: {},
+      security: [
+        {
+          apikey: [],
+        },
+      ],
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'API "security" values must match a scheme defined in the "components.securitySchemes" object.',
+        path: ['security', '0', 'apikey'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'return errors on valid and invalid object',
+    document: {
+      openapi: '3.0.2',
+      components: {
+        securitySchemes: {
+          apikey: {},
+        },
+      },
+      paths: {
+        '/path': {
+          get: {
+            security: [
+              {
+                apikey: [],
+                basic: [],
+              },
+            ],
+          },
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'Operation "security" values must match a scheme defined in the "components.securitySchemes" object.',
+        path: ['paths', '/path', 'get', 'security', '0', 'basic'],
+        severity: DiagnosticSeverity.Warning,
+      },
+    ],
+  },
+
+  {
+    name: 'valid and invalid object (API-level security)',
+    document: {
+      openapi: '3.0.2',
+      components: {
+        securitySchemes: {
+          apikey: {},
+        },
+      },
+      security: [
+        {
+          apikey: [],
+          basic: [],
+        },
+      ],
+      paths: {
+        '/path': {
+          get: {},
+        },
+      },
+    },
+    errors: [
+      {
+        message: 'API "security" values must match a scheme defined in the "components.securitySchemes" object.',
+        path: ['security', '0', 'basic'],
         severity: DiagnosticSeverity.Warning,
       },
     ],