Skip to content

Commit

Permalink
feat: add ability to exclude some operations from security-defined ru…
Browse files Browse the repository at this point in the history
…le (#1570)
  • Loading branch information
tatomyr committed May 29, 2024
1 parent a53bb35 commit f6cce63
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .changeset/dull-comics-taste.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 11 additions & 3 deletions docs/rules/security-defined.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
103 changes: 103 additions & 0 deletions packages/core/src/rules/common/__tests__/security-defined.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": [],
},
]
`);
});
});
39 changes: 31 additions & 8 deletions packages/core/src/rules/common/security-defined.ts
Original file line number Diff line number Diff line change
@@ -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,
{
Expand All @@ -15,6 +27,7 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => {

const operationsWithoutSecurity: Location[] = [];
let eachOperationHasSecurity: boolean = true;
let path: string | undefined;

return {
Root: {
Expand Down Expand Up @@ -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);
}
},
},
};
};

1 comment on commit f6cce63

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.26% 4485/5805
🟡 Branches 67.57% 2479/3669
🟡 Functions 70.94% 747/1053
🟡 Lines 77.45% 4218/5446

Test suite run success

738 tests passing in 102 suites.

Report generated by 🧪jest coverage report action from f6cce63

Please sign in to comment.