From 59f26b06ae39e635df21dfbd0f0bb549c9590135 Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Mon, 4 Feb 2019 13:02:29 -0600 Subject: [PATCH] fix: run security validations on security objects only --- .../2and3/semantic-validators/security-ibm.js | 83 +++- .../2and3/semantic-validators/security.js | 86 ---- test/plugins/validation/2and3/security-ibm.js | 314 +++++++++++++-- test/plugins/validation/2and3/security.js | 369 ------------------ 4 files changed, 358 insertions(+), 494 deletions(-) delete mode 100644 src/plugins/validation/2and3/semantic-validators/security.js delete mode 100644 test/plugins/validation/2and3/security.js diff --git a/src/plugins/validation/2and3/semantic-validators/security-ibm.js b/src/plugins/validation/2and3/semantic-validators/security-ibm.js index e6e7c39e7..0eab4af1d 100644 --- a/src/plugins/validation/2and3/semantic-validators/security-ibm.js +++ b/src/plugins/validation/2and3/semantic-validators/security-ibm.js @@ -7,6 +7,8 @@ // If the security scheme is of type "oauth2" or "openIdConnect", then the value is a list of scope // names required for the execution. For other security scheme types, the array MUST be empty. // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securityRequirementObject +// Assertation 2 +// Items in `security` must match a `securityDefinition`. const each = require('lodash/each'); @@ -49,16 +51,13 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) { ? jsSpec.components && jsSpec.components.securitySchemes : jsSpec.securityDefinitions; - // if there are no security definitions, don't bother running these checks - // the unmatched security requirements will throw errors in another part of - // the validator - if (securityObjects.length && securityDefinitions) { + if (securityObjects.length) { securityObjects.forEach(obj => { - checkForInvalidNonEmptyArrays(obj); + validateSecurityObject(obj); }); } - function checkForInvalidNonEmptyArrays({ security, path }) { + function validateSecurityObject({ security, path }) { security.forEach(schemeObject => { // each object in this array should only have one key - the name of the scheme const schemeNames = Object.keys(schemeObject); @@ -75,15 +74,27 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) { }); } - const isNonEmptyArray = schemeObject[schemeName].length > 0; - const schemeIsDefined = securityDefinitions[schemeName]; - const schemesWithNonEmptyArrays = isOAS3 - ? ['oauth2', 'openIdConnect'] - : ['oauth2']; + const schemeIsDefined = + securityDefinitions && securityDefinitions[schemeName]; - if (isNonEmptyArray && schemeIsDefined) { + // ensure the security scheme is defined + if (!schemeIsDefined) { + result.error.push({ + path: `${path}.${schemeName}`, + message: 'security requirements must match a security definition' + }); + } else { const schemeType = securityDefinitions[schemeName].type; - if (!schemesWithNonEmptyArrays.includes(schemeType)) { + const isNonEmptyArray = schemeObject[schemeName].length > 0; + const schemesWithNonEmptyArrays = isOAS3 + ? ['oauth2', 'openIdConnect'] + : ['oauth2']; + + const isSchemeWithNonEmptyArray = schemesWithNonEmptyArrays.includes( + schemeType + ); + + if (isNonEmptyArray && !isSchemeWithNonEmptyArray) { const checkStatus = config.invalid_non_empty_security_array; if (checkStatus !== 'off') { result[checkStatus].push({ @@ -94,9 +105,55 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) { }); } } + + if (isSchemeWithNonEmptyArray) { + // check for resolution of specific scopes + const scopes = schemeObject[schemeName]; + if (Array.isArray(scopes)) { + // Check for unknown scopes + const securityDefinition = securityDefinitions[schemeName]; + scopes.forEach((scope, i) => { + const scopeIsDefined = isOAS3 + ? checkOAS3Scopes(scope, securityDefinition) + : checkSwagger2Scopes(scope, securityDefinition); + if (!scopeIsDefined) { + result.error.push({ + message: `Definition could not be resolved for security scope: ${scope}`, + path: `${path}.${schemeName}.${i}` + }); + } + }); + } + } } }); } return { errors: result.error, warnings: result.warning }; }; + +// return true if scope is defined +function checkSwagger2Scopes(scope, definition) { + return Boolean(definition.scopes && definition.scopes[scope]); +} + +// return true if scope is defined +function checkOAS3Scopes(scope, definition) { + let scopeIsDefined = false; + if (definition.flows) { + Object.keys(definition.flows).forEach(flowType => { + if ( + definition.flows[flowType].scopes && + definition.flows[flowType].scopes[scope] + ) { + scopeIsDefined = true; + return; + } + }); + } + // scopes for openIdConnet are not definied in the document + if (definition.type && definition.type === 'openIdConnect') { + scopeIsDefined = true; + } + return scopeIsDefined; +} diff --git a/src/plugins/validation/2and3/semantic-validators/security.js b/src/plugins/validation/2and3/semantic-validators/security.js deleted file mode 100644 index 98b2e9266..000000000 --- a/src/plugins/validation/2and3/semantic-validators/security.js +++ /dev/null @@ -1,86 +0,0 @@ -// Assertation 1: -// Items in `security` must match a `securityDefinition`. - -module.exports.validate = function({ resolvedSpec, isOAS3 }) { - const errors = []; - const warnings = []; - - const securityDefinitions = isOAS3 - ? resolvedSpec.components && resolvedSpec.components.securitySchemes - : resolvedSpec.securityDefinitions; - - function walk(obj, path) { - if (typeof obj !== 'object' || obj === null) { - return; - } - - if (path[path.length - 2] === 'security') { - // Assertation 1 - Object.keys(obj).map(key => { - const securityDefinition = - securityDefinitions && securityDefinitions[key]; - if (!securityDefinition) { - errors.push({ - message: 'security requirements must match a security definition', - path: path - }); - } - - if (securityDefinition) { - const scopes = obj[key]; - if (Array.isArray(scopes)) { - // Check for unknown scopes - - scopes.forEach((scope, i) => { - const scopeIsDefined = isOAS3 - ? checkOAS3Scopes(scope, securityDefinition) - : checkSwagger2Scopes(scope, securityDefinition); - if (!scopeIsDefined) { - errors.push({ - message: `Definition could not be resolved for security scope: ${scope}`, - path: path.concat([i.toString()]) - }); - } - }); - } - } - }); - } - - if (Object.keys(obj).length) { - return Object.keys(obj).map(k => walk(obj[k], [...path, k])); - } else { - return null; - } - } - - walk(resolvedSpec, []); - - return { errors, warnings }; -}; - -// return true if scope is defined -function checkSwagger2Scopes(scope, definition) { - return Boolean(definition.scopes && definition.scopes[scope]); -} - -// return true if scope is defined -function checkOAS3Scopes(scope, definition) { - let scopeIsDefined = false; - if (definition.flows) { - Object.keys(definition.flows).forEach(flowType => { - if ( - definition.flows[flowType].scopes && - definition.flows[flowType].scopes[scope] - ) { - scopeIsDefined = true; - return; - } - }); - } - // scopes for openIdConnet are not definied in the document - if (definition.type && definition.type === 'openIdConnet') { - scopeIsDefined = true; - } - return scopeIsDefined; -} diff --git a/test/plugins/validation/2and3/security-ibm.js b/test/plugins/validation/2and3/security-ibm.js index 55f7385cc..8090e164e 100644 --- a/test/plugins/validation/2and3/security-ibm.js +++ b/test/plugins/validation/2and3/security-ibm.js @@ -3,15 +3,111 @@ const { validate } = require('../../../../src/plugins/validation/2and3/semantic-validators/security-ibm'); +const config = require('../../../../src/.defaultsForValidator').defaults.shared; + describe('validation plugin - semantic - security-ibm', function() { describe('Swagger 2', function() { - it('should error on a non-empty array for security object that is not of type oauth2', function() { - const config = { - security: { - invalid_non_empty_security_array: 'error' + it('should return an error when an operation references a non-existing security scope', () => { + const spec = { + securityDefinitions: { + api_key: { + type: 'oauth2', + name: 'apikey', + in: 'query', + scopes: { + asdf: 'blah blah' + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + api_key: ['write:pets'] + } + ] + } + } } }; + const res = validate({ jsSpec: spec }, config); + expect(res.errors.length).toEqual(1); + expect(res.errors[0].path).toEqual('paths./.get.security.api_key.0'); + expect(res.errors[0].message).toEqual( + 'Definition could not be resolved for security scope: write:pets' + ); + expect(res.warnings.length).toEqual(0); + }); + + it('should return an error when an operation references a non-existing security definition', () => { + const spec = { + securityDefinitions: { + api_key: { + type: 'apiKey', + name: 'apikey', + in: 'query' + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + fictional_security_definition: ['write:pets'] + } + ] + } + } + } + }; + + const res = validate({ jsSpec: spec }, config); + expect(res.errors.length).toEqual(1); + expect(res.errors[0].path).toEqual( + 'paths./.get.security.fictional_security_definition' + ); + expect(res.errors[0].message).toEqual( + 'security requirements must match a security definition' + ); + expect(res.warnings.length).toEqual(0); + }); + + it('should not return an error when an operation references an existing security scope', () => { + const spec = { + securityDefinitions: { + api_key: { + type: 'oauth2', + name: 'apikey', + in: 'query', + scopes: { + 'write:pets': 'write to pets' + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + api_key: ['write:pets'] + } + ] + } + } + } + }; + + const res = validate({ jsSpec: spec }, config); + expect(res.errors.length).toEqual(0); + expect(res.warnings.length).toEqual(0); + }); + + it('should error on a non-empty array for security object that is not of type oauth2', function() { const spec = { securityDefinitions: { basicAuth: { @@ -54,12 +150,6 @@ describe('validation plugin - semantic - security-ibm', function() { }); it('should not error on a non-empty array for security object of type oauth2', function() { - const config = { - security: { - invalid_non_empty_security_array: 'error' - } - }; - const spec = { securityDefinitions: { basicAuth: { @@ -99,13 +189,197 @@ describe('validation plugin - semantic - security-ibm', function() { }); describe('OpenAPI 3', function() { - it('should error on a non-empty array for security req that is not oauth2 or openIdConnect', function() { - const config = { - security: { - invalid_non_empty_security_array: 'error' + it('should return an error when an operation references a non-existing security scope', () => { + const spec = { + components: { + securitySchemes: { + TestAuth: { + type: 'oauth2', + description: 'just a test', + flows: { + implicit: { + authorizationUrl: 'https://example.com/api/oauth', + scopes: { + 'read:pets': "you can read but you can't write" + } + } + } + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + TestAuth: ['write:pets'] + } + ] + } + } } }; + const res = validate({ jsSpec: spec, isOAS3: true }, config); + expect(res.errors.length).toEqual(1); + expect(res.errors[0].path).toEqual('paths./.get.security.TestAuth.0'); + expect(res.errors[0].message).toEqual( + 'Definition could not be resolved for security scope: write:pets' + ); + expect(res.warnings.length).toEqual(0); + }); + + it('should return an error when one of a few scopes is undefined', () => { + const spec = { + components: { + securitySchemes: { + TestAuth: { + type: 'oauth2', + description: 'just a test', + flows: { + implicit: { + authorizationUrl: 'https://example.com/api/oauth', + scopes: { + 'read:pets': "you can read but you can't write", + 'read:houses': "you can read but you can't write", + 'write:houses': "you can write but you can't read" + } + } + } + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + TestAuth: [ + 'write:houses', + 'read:houses', + 'write:pets', + 'read:pets' + ] + } + ] + } + } + } + }; + + const res = validate({ jsSpec: spec, isOAS3: true }, config); + expect(res.errors.length).toEqual(1); + expect(res.errors[0].message).toEqual( + 'Definition could not be resolved for security scope: write:pets' + ); + expect(res.errors[0].path).toEqual('paths./.get.security.TestAuth.2'); + expect(res.warnings.length).toEqual(0); + }); + + it('should return an error when a security requirement is undefined', () => { + const spec = { + components: { + securitySchemes: { + TestAuth: { + type: 'oauth2', + description: 'just a test', + flows: { + implicit: { + authorizationUrl: 'https://example.com/api/oauth', + scopes: { + 'read:pets': "you can read but you can't write" + } + } + } + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + UndefinedAuth: ['read:pets'] + } + ] + } + } + } + }; + + const res = validate({ jsSpec: spec, isOAS3: true }, config); + expect(res.errors.length).toEqual(1); + expect(res.errors[0].message).toEqual( + 'security requirements must match a security definition' + ); + expect(res.errors[0].path).toEqual('paths./.get.security.UndefinedAuth'); + expect(res.warnings.length).toEqual(0); + }); + + it('should not return an error when a security requirement references an existing security scope', () => { + const spec = { + components: { + securitySchemes: { + TestAuth: { + type: 'oauth2', + description: 'just a test', + flows: { + implicit: { + authorizationUrl: 'https://example.com/api/oauth', + scopes: { + 'read:pets': "you can read but you can't write" + } + } + } + } + } + }, + security: [ + { + TestAuth: ['read:pets'] + } + ] + }; + + const res = validate({ jsSpec: spec, isOAS3: true }, config); + expect(res.errors.length).toEqual(0); + expect(res.warnings.length).toEqual(0); + }); + + it('should not return an error when referencing a non-existing scope for type openIdConnet', () => { + const spec = { + components: { + securitySchemes: { + TestAuth: { + type: 'openIdConnect', + description: 'just a test', + openIdConnectUrl: 'https://auth.com/openId' + } + } + }, + paths: { + '/': { + get: { + description: 'asdf', + security: [ + { + TestAuth: ['write:pets'] + } + ] + } + } + } + }; + + const res = validate({ jsSpec: spec, isOAS3: true }, config); + expect(res.errors.length).toEqual(0); + expect(res.warnings.length).toEqual(0); + }); + + it('should error on a non-empty array for security req that is not oauth2 or openIdConnect', function() { const spec = { components: { securitySchemes: { @@ -139,12 +413,6 @@ describe('validation plugin - semantic - security-ibm', function() { }); it('should not error on a non-empty array for security req that is of type oauth2', function() { - const config = { - security: { - invalid_non_empty_security_array: 'error' - } - }; - const spec = { components: { securitySchemes: { @@ -180,12 +448,6 @@ describe('validation plugin - semantic - security-ibm', function() { }); it('should not error on a non-empty array for security object that is of type openIdConnect', function() { - const config = { - security: { - invalid_non_empty_security_array: 'error' - } - }; - const spec = { components: { securitySchemes: { diff --git a/test/plugins/validation/2and3/security.js b/test/plugins/validation/2and3/security.js deleted file mode 100644 index f07b2c5dd..000000000 --- a/test/plugins/validation/2and3/security.js +++ /dev/null @@ -1,369 +0,0 @@ -const expect = require('expect'); -const { - validate -} = require('../../../../src/plugins/validation/2and3/semantic-validators/security'); - -describe('validation plugin - semantic - security', () => { - describe('Swagger 2', () => { - it('should return an error when an operation references a non-existing security scope', () => { - const spec = { - securityDefinitions: { - api_key: { - type: 'apiKey', - name: 'apikey', - in: 'query', - scopes: { - asdf: 'blah blah' - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - api_key: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0', - '0' - ]); - expect(res.errors[0].message).toEqual( - 'Definition could not be resolved for security scope: write:pets' - ); - expect(res.warnings.length).toEqual(0); - }); - it('should return an error when an operation references a security definition with no scopes', () => { - const spec = { - securityDefinitions: { - api_key: { - type: 'apiKey', - name: 'apikey', - in: 'query' - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - api_key: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0', - '0' - ]); - expect(res.errors[0].message).toEqual( - 'Definition could not be resolved for security scope: write:pets' - ); - expect(res.warnings.length).toEqual(0); - }); - - it('should return an error when an operation references a non-existing security definition', () => { - const spec = { - securityDefinitions: { - api_key: { - type: 'apiKey', - name: 'apikey', - in: 'query' - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - fictional_security_definition: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0' - ]); - expect(res.errors[0].message).toEqual( - 'security requirements must match a security definition' - ); - expect(res.warnings.length).toEqual(0); - }); - - it('should not return an error when an operation references an existing security scope', () => { - const spec = { - securityDefinitions: { - api_key: { - type: 'apiKey', - name: 'apikey', - in: 'query', - scopes: { - 'write:pets': 'write to pets' - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - api_key: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec }); - expect(res.errors.length).toEqual(0); - expect(res.warnings.length).toEqual(0); - }); - }); - - describe('OpenAPI 3', () => { - it('should return an error when an operation references a non-existing security scope', () => { - const spec = { - components: { - securitySchemes: { - TestAuth: { - type: 'oauth2', - description: 'just a test', - flows: { - implicit: { - authorizationUrl: 'https://example.com/api/oauth', - scopes: { - 'read:pets': "you can read but you can't write" - } - } - } - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - TestAuth: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec, isOAS3: true }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0', - '0' - ]); - expect(res.errors[0].message).toEqual( - 'Definition could not be resolved for security scope: write:pets' - ); - expect(res.warnings.length).toEqual(0); - }); - - it('should return an error when one of a few scopes is undefined', () => { - const spec = { - components: { - securitySchemes: { - TestAuth: { - type: 'oauth2', - description: 'just a test', - flows: { - implicit: { - authorizationUrl: 'https://example.com/api/oauth', - scopes: { - 'read:pets': "you can read but you can't write", - 'read:houses': "you can read but you can't write", - 'write:houses': "you can write but you can't read" - } - } - } - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - TestAuth: [ - 'write:houses', - 'read:houses', - 'write:pets', - 'read:pets' - ] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec, isOAS3: true }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].message).toEqual( - 'Definition could not be resolved for security scope: write:pets' - ); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0', - '2' - ]); - expect(res.warnings.length).toEqual(0); - }); - - it('should return an error when a security requirement is undefined', () => { - const spec = { - components: { - securitySchemes: { - TestAuth: { - type: 'oauth2', - description: 'just a test', - flows: { - implicit: { - authorizationUrl: 'https://example.com/api/oauth', - scopes: { - 'read:pets': "you can read but you can't write" - } - } - } - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - UndefinedAuth: ['read:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec, isOAS3: true }); - expect(res.errors.length).toEqual(1); - expect(res.errors[0].message).toEqual( - 'security requirements must match a security definition' - ); - expect(res.errors[0].path).toEqual([ - 'paths', - '/', - 'get', - 'security', - '0' - ]); - expect(res.warnings.length).toEqual(0); - }); - - it('should not return an error when a security requirement references an existing security scope', () => { - const spec = { - components: { - securitySchemes: { - TestAuth: { - type: 'oauth2', - description: 'just a test', - flows: { - implicit: { - authorizationUrl: 'https://example.com/api/oauth', - scopes: { - 'read:pets': "you can read but you can't write" - } - } - } - } - } - }, - security: [ - { - TestAuth: ['read:pets'] - } - ] - }; - - const res = validate({ resolvedSpec: spec, isOAS3: true }); - expect(res.errors.length).toEqual(0); - expect(res.warnings.length).toEqual(0); - }); - - it('should not return an error when referencing a non-existing scope for type openIdConnet', () => { - const spec = { - components: { - securitySchemes: { - TestAuth: { - type: 'openIdConnet', - description: 'just a test', - openIdConnectUrl: 'https://auth.com/openId' - } - } - }, - paths: { - '/': { - get: { - description: 'asdf', - security: [ - { - TestAuth: ['write:pets'] - } - ] - } - } - } - }; - - const res = validate({ resolvedSpec: spec, isOAS3: true }); - expect(res.errors.length).toEqual(0); - expect(res.warnings.length).toEqual(0); - }); - }); -});