From f6cce633d0be7e3fbca0d433e66275f090becaca Mon Sep 17 00:00:00 2001 From: Andrew Tatomyr Date: Wed, 29 May 2024 09:13:24 +0300 Subject: [PATCH] feat: add ability to exclude some operations from security-defined rule (#1570) --- .changeset/dull-comics-taste.md | 6 + docs/rules/security-defined.md | 14 ++- .../common/__tests__/security-defined.test.ts | 103 ++++++++++++++++++ .../core/src/rules/common/security-defined.ts | 39 +++++-- 4 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 .changeset/dull-comics-taste.md diff --git a/.changeset/dull-comics-taste.md b/.changeset/dull-comics-taste.md new file mode 100644 index 000000000..efe0863a7 --- /dev/null +++ b/.changeset/dull-comics-taste.md @@ -0,0 +1,6 @@ +--- +"@redocly/openapi-core": minor +"@redocly/cli": minor +--- + +Added the ability to exclude some operations or entire paths from the `security-defined` rule. diff --git a/docs/rules/security-defined.md b/docs/rules/security-defined.md index b32ce4dd5..16ef1ab55 100644 --- a/docs/rules/security-defined.md +++ b/docs/rules/security-defined.md @@ -34,9 +34,17 @@ security: [] ## Configuration -| Option | Type | Description | -| -------- | ------ | ------------------------------------------------------------------------------------------ | -| severity | string | Possible values: `off`, `warn`, `error`. Default `error` (in `recommended` configuration). | +| Option | Type | Description | +| ---------- | --------------------------------------- | ------------------------------------------------------------------------------------------ | +| severity | string | Possible values: `off`, `warn`, `error`. Default `error` (in `recommended` configuration). | +| exceptions | [[Exception object](#exception-object)] | List of exceptions from the rule. | + +### Exception object + +| Option | Type | Description | +| ------- | -------- | ------------------------------------------------------------------------------------------- | +| path | string | **REQUIRED.** Excluded path. | +| methods | [string] | Optional list of operations to exclude. If not provided, the entire path is being excluded. | An example configuration: diff --git a/packages/core/src/rules/common/__tests__/security-defined.test.ts b/packages/core/src/rules/common/__tests__/security-defined.test.ts index f280dbe0d..53decfd3f 100644 --- a/packages/core/src/rules/common/__tests__/security-defined.test.ts +++ b/packages/core/src/rules/common/__tests__/security-defined.test.ts @@ -172,4 +172,107 @@ describe('Oas3 security-defined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); + + it('should not report if a pathItem is explicitly excluded in the option', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + paths: + /excluded: + get: + description: Should be skipped. + post: + description: Should be skipped.`, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await makeConfig({ + 'security-defined': { exceptions: [{ path: '/excluded' }] }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report only those operations without security defined that are not excluded in the options', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + paths: + /partially-excluded: + get: + description: Should be skipped. + post: + description: Has security. + security: [] + delete: + description: Should have security defined.`, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await makeConfig({ + 'security-defined': { exceptions: [{ path: '/partially-excluded', methods: ['GET'] }] }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/paths/~1partially-excluded/delete", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Every operation should have security defined on it or on the root level.", + "ruleId": "security-defined", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report operations from path items that are not excluded', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + paths: + /not-excluded: + get: + summary: Should have security defined.`, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await makeConfig({ 'security-defined': { exceptions: [{ path: '/excluded' }] } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/paths/~1not-excluded/get", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Every operation should have security defined on it or on the root level.", + "ruleId": "security-defined", + "severity": "error", + "suggest": [], + }, + ] + `); + }); }); diff --git a/packages/core/src/rules/common/security-defined.ts b/packages/core/src/rules/common/security-defined.ts index fd2ec55db..5f6012dc9 100644 --- a/packages/core/src/rules/common/security-defined.ts +++ b/packages/core/src/rules/common/security-defined.ts @@ -1,10 +1,22 @@ import { Oas3Rule, Oas2Rule } from '../../visitors'; import { Location } from '../../ref-utils'; import { UserContext } from '../../walk'; -import { Oas2Definition, Oas2Operation, Oas2SecurityScheme } from '../../typings/swagger'; -import { Oas3Definition, Oas3Operation, Oas3SecurityScheme } from '../../typings/openapi'; +import { + Oas2Definition, + Oas2Operation, + Oas2PathItem, + Oas2SecurityScheme, +} from '../../typings/swagger'; +import { + Oas3Definition, + Oas3Operation, + Oas3PathItem, + Oas3SecurityScheme, +} from '../../typings/openapi'; -export const SecurityDefined: Oas3Rule | Oas2Rule = () => { +export const SecurityDefined: Oas3Rule | Oas2Rule = (opts: { + exceptions?: { path: string; methods?: string[] }[]; +}) => { const referencedSchemes = new Map< string, { @@ -15,6 +27,7 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => { const operationsWithoutSecurity: Location[] = []; let eachOperationHasSecurity: boolean = true; + let path: string | undefined; return { Root: { @@ -55,11 +68,21 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => { } } }, - Operation(operation: Oas2Operation | Oas3Operation, { location }: UserContext) { - if (!operation?.security) { - eachOperationHasSecurity = false; - operationsWithoutSecurity.push(location); - } + PathItem: { + enter(pathItem: Oas2PathItem | Oas3PathItem, { key }: UserContext) { + path = key as string; + }, + Operation(operation: Oas2Operation | Oas3Operation, { location, key }: UserContext) { + const isException = opts.exceptions?.some( + (item) => + item.path === path && + (!item.methods || item.methods?.some((method) => method.toLowerCase() === key)) + ); + if (!operation?.security && !isException) { + eachOperationHasSecurity = false; + operationsWithoutSecurity.push(location); + } + }, }, }; };