Skip to content

Commit 68b1306

Browse files
b41exb41ex
committed
fix: collect only security and security schema changes relevant for the operation
1 parent fa25f51 commit 68b1306

File tree

51 files changed

+1587
-7
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+1587
-7
lines changed

‎src/apitypes/rest/rest.changes.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,12 @@ import { calculateObjectHash } from '../../utils/hashes'
5959
import { REST_API_TYPE } from './rest.consts'
6060
import { OpenAPIV3 } from 'openapi-types'
6161
import {
62+
extractOperationSecurityDiffs,
6263
extractPathParamRenameDiff,
6364
extractRootSecurityDiffs,
6465
extractRootServersDiffs,
66+
extractSecuritySchemesDiffs,
67+
extractSecuritySchemesNames,
6568
validateGroupPrefix,
6669
} from './rest.utils'
6770
import { createOperationChange, getOperationTags, OperationsMap } from '../../components'
@@ -166,12 +169,17 @@ export const compareDocuments = async (
166169

167170
let operationDiffs: Diff[] = []
168171
if (operationPotentiallyChanged) {
172+
const operationSecurityDiffs = extractOperationSecurityDiffs(methodData as OpenAPIV3.OperationObject)
173+
const shouldTakeRootSecurityDiffs = operationSecurityDiffs.length === 0 && !methodData?.security
174+
const relevantSecuritySchemesNames = shouldTakeRootSecurityDiffs ? extractSecuritySchemesNames(merged.security ?? []) : extractSecuritySchemesNames(methodData?.security ?? [])
169175
operationDiffs = [
170-
...(methodData as WithAggregatedDiffs<OpenAPIV3.OperationObject>)[DIFFS_AGGREGATED_META_KEY]??[],
176+
...(methodData as WithAggregatedDiffs<OpenAPIV3.OperationObject>)[DIFFS_AGGREGATED_META_KEY] ?? [],
171177
...extractRootServersDiffs(merged),
172-
...extractRootSecurityDiffs(merged),
178+
...shouldTakeRootSecurityDiffs ? extractRootSecurityDiffs(merged) : [],
179+
...extractSecuritySchemesDiffs(merged.components, relevantSecuritySchemesNames),
173180
...extractPathParamRenameDiff(merged, path),
174181
// parameters, servers, summary, description and extensionKeys are moved from path to method in pathItemsUnification during normalization in apiDiff, so no need to aggregate them here
182+
// note that operation security diffs are not aggregated here, because they are in aggregated diffs for operation object
175183
]
176184
}
177185
if (operationAddedOrRemoved) {

‎src/apitypes/rest/rest.utils.ts‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,55 @@ export const extractRootServersDiffs = (doc: OpenAPIV3.Document): Diff[] => {
8484
]
8585
}
8686

87-
export const extractRootSecurityDiffs = (doc: OpenAPIV3.Document): Diff[] => {
88-
const addOrRemoveSecurityDiff = (doc as WithDiffMetaRecord<OpenAPIV3.Document>)[DIFF_META_KEY]?.security
89-
const securityInternalDiffs = (doc.security as WithAggregatedDiffs<OpenAPIV3.SecurityRequirementObject[]> | undefined)?.[DIFFS_AGGREGATED_META_KEY] ?? []
90-
const componentsSecuritySchemesDiffs = (doc.components?.securitySchemes as WithAggregatedDiffs<Record<string, OpenAPIV3.SecuritySchemeObject>>)[DIFFS_AGGREGATED_META_KEY] ?? []
87+
const extractSecurityDiffs = (source: WithDiffMetaRecord<{ security?: OpenAPIV3.SecurityRequirementObject[] }>): Diff[] => {
88+
const addOrRemoveSecurityDiff = source[DIFF_META_KEY]?.security
89+
const securityInternalDiffs = (source.security as WithAggregatedDiffs<OpenAPIV3.SecurityRequirementObject[]> | undefined)?.[DIFFS_AGGREGATED_META_KEY] ?? []
9190
return [
9291
...(addOrRemoveSecurityDiff ? [addOrRemoveSecurityDiff] : []),
9392
...securityInternalDiffs,
94-
...componentsSecuritySchemesDiffs,
9593
]
9694
}
9795

96+
export const extractRootSecurityDiffs = (doc: OpenAPIV3.Document): Diff[] => {
97+
return extractSecurityDiffs(doc as WithDiffMetaRecord<OpenAPIV3.Document>)
98+
}
99+
100+
export const extractOperationSecurityDiffs = (operation: OpenAPIV3.OperationObject): Diff[] => {
101+
return extractSecurityDiffs(operation as WithDiffMetaRecord<OpenAPIV3.OperationObject>)
102+
}
103+
104+
export const extractSecuritySchemesNames = (security: OpenAPIV3.SecurityRequirementObject[]): Set<string> => {
105+
return new Set(security.flatMap(securityRequirement => Object.keys(securityRequirement)))
106+
}
107+
108+
export const extractSecuritySchemesDiffs = (components: OpenAPIV3.ComponentsObject | undefined, securitySchemesNames: Set<string>): Diff[] => {
109+
if (!components || !components.securitySchemes) {
110+
return []
111+
}
112+
const result: Diff[] = []
113+
114+
const addRemoveSecuritySchemesDiffs = (components.securitySchemes as WithDiffMetaRecord<Record<string, OpenAPIV3.SecuritySchemeObject>>)?.[DIFF_META_KEY]
115+
if (addRemoveSecuritySchemesDiffs) {
116+
for (const schemeName of securitySchemesNames) {
117+
const diff = addRemoveSecuritySchemesDiffs[schemeName]
118+
if (diff) {
119+
result.push(diff)
120+
}
121+
}
122+
}
123+
124+
for (const schemeName of securitySchemesNames) {
125+
const securityScheme = components.securitySchemes[schemeName]
126+
if (securityScheme) {
127+
const aggregatedDiffs = (securityScheme as WithAggregatedDiffs<OpenAPIV3.SecuritySchemeObject>)?.[DIFFS_AGGREGATED_META_KEY] ?? []
128+
result.push(...aggregatedDiffs)
129+
}
130+
}
131+
132+
133+
return result
134+
}
135+
98136
export function validateGroupPrefix(group: unknown, paramName: string): void {
99137
if (group === undefined) {
100138
return

‎test/changelog.security.test.ts‎

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
/**
2+
* Copyright 2024-2025 NetCracker Technology Corporation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { describe, test, expect } from '@jest/globals'
18+
import { buildChangelogPackage, changesSummaryMatcher, noChangesMatcher, numberOfImpactedOperationsMatcher, operationChangesMatcher } from './helpers'
19+
import { BREAKING_CHANGE_TYPE, NON_BREAKING_CHANGE_TYPE, UNCLASSIFIED_CHANGE_TYPE } from '../src/types/external/comparison'
20+
21+
describe('Security Diff Collection', () => {
22+
describe('Operation Explicit Security Precedence', () => {
23+
test('Operation with explicit security ignores global security changes', async () => {
24+
const result = await buildChangelogPackage('changelog/security/operation-security-precedence/global-changes-ignored')
25+
26+
expect(result).toEqual(noChangesMatcher())
27+
})
28+
29+
test('Operation security changes are reported for operation with explicit security', async () => {
30+
const result = await buildChangelogPackage('changelog/security/operation-security-precedence/operation-security-changes')
31+
32+
expect(result).toEqual(changesSummaryMatcher({
33+
[BREAKING_CHANGE_TYPE]: 1,
34+
[NON_BREAKING_CHANGE_TYPE]: 1,
35+
}))
36+
})
37+
38+
test('Operation security changes reported, global changes ignored', async () => {
39+
const result = await buildChangelogPackage('changelog/security/operation-security-precedence/both-change')
40+
41+
// Both global and operation security change
42+
// Expected: Only operation security diffs reported
43+
expect(result).toEqual(changesSummaryMatcher({
44+
[BREAKING_CHANGE_TYPE]: 1,
45+
[NON_BREAKING_CHANGE_TYPE]: 1,
46+
}))
47+
})
48+
49+
test('Add explicit operation security', async () => {
50+
const result = await buildChangelogPackage('changelog/security/operation-security-precedence/adds-explicit-security')
51+
52+
// Operation adds explicit security
53+
// Expected: Diffs for adding operation security
54+
expect(result).toEqual(changesSummaryMatcher({
55+
[BREAKING_CHANGE_TYPE]: 1,
56+
}))
57+
})
58+
})
59+
60+
describe('Global Security Fallback', () => {
61+
test('Global security changes affect operations without explicit security', async () => {
62+
const result = await buildChangelogPackage('changelog/security/global-security-fallback/global-security-changes')
63+
64+
// Global security changes, operation has no explicit security
65+
// Expected: Diffs from global security change
66+
// todo: fix the ER once api-diff classification is changed
67+
expect(result).toEqual(changesSummaryMatcher({
68+
[UNCLASSIFIED_CHANGE_TYPE]: 2,
69+
}))
70+
})
71+
72+
test('Adding global security affects operations without explicit security', async () => {
73+
const result = await buildChangelogPackage('changelog/security/global-security-fallback/add-global-security')
74+
75+
// Global security added
76+
// Expected: Diffs for adding global security
77+
expect(result).toEqual(changesSummaryMatcher({
78+
[BREAKING_CHANGE_TYPE]: 1,
79+
}))
80+
})
81+
82+
test('Removing global security affects operations without explicit security', async () => {
83+
const result = await buildChangelogPackage('changelog/security/global-security-fallback/remove-global-security')
84+
85+
// Global security removed
86+
// Expected: Diffs for removing global security
87+
expect(result).toEqual(changesSummaryMatcher({
88+
[NON_BREAKING_CHANGE_TYPE]: 1,
89+
}))
90+
})
91+
92+
test('Removing operation security shows change', async () => {
93+
const result = await buildChangelogPackage('changelog/security/global-security-fallback/removes-explicit-security')
94+
95+
// Operation removes explicit security, global exists
96+
// Expected: Diffs for removing operation security
97+
expect(result).toEqual(changesSummaryMatcher({
98+
[BREAKING_CHANGE_TYPE]: 1,
99+
}))
100+
})
101+
})
102+
103+
describe('Empty Security Array', () => {
104+
test('Operation with explicit empty array as security ignores global changes', async () => {
105+
const result = await buildChangelogPackage('changelog/security/empty-security-array/empty-array-global-changes')
106+
107+
expect(result).toEqual(noChangesMatcher())
108+
})
109+
110+
test('Adding empty security array to operation', async () => {
111+
const result = await buildChangelogPackage('changelog/security/empty-security-array/add-empty-array')
112+
113+
// Operation adds security: []
114+
// Expected: Diffs for adding empty security
115+
expect(result).toEqual(changesSummaryMatcher({
116+
[NON_BREAKING_CHANGE_TYPE]: 1,
117+
}))
118+
})
119+
120+
test('Removing empty security array from operation causes global security to be applied', async () => {
121+
const result = await buildChangelogPackage('changelog/security/empty-security-array/remove-empty-array')
122+
123+
// Operation removes security: []
124+
// Expected: Diffs for removing empty security
125+
expect(result).toEqual(changesSummaryMatcher({
126+
[BREAKING_CHANGE_TYPE]: 1,
127+
}))
128+
})
129+
130+
test('Changing from real security to empty array', async () => {
131+
const result = await buildChangelogPackage('changelog/security/empty-security-array/change-to-empty-array')
132+
133+
// Operation security changes to []
134+
// Expected: Diffs for changing operation security
135+
expect(result).toEqual(changesSummaryMatcher({
136+
[NON_BREAKING_CHANGE_TYPE]: 1,
137+
}))
138+
})
139+
})
140+
141+
describe('Security Scheme Relevance', () => {
142+
test('Security scheme used in operation security changes', async () => {
143+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/scheme-used-in-security-changes')
144+
145+
// Operation uses ApiKeyAuth, ApiKeyAuth scheme changes
146+
// Expected: Diffs for security scheme change
147+
expect(result).toEqual(changesSummaryMatcher({
148+
[BREAKING_CHANGE_TYPE]: 1,
149+
}))
150+
})
151+
152+
test('Unused scheme removed from components', async () => {
153+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/unused-scheme-removed')
154+
155+
expect(result).toEqual(noChangesMatcher())
156+
})
157+
158+
test('Unused scheme added to components', async () => {
159+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/unused-scheme-added')
160+
161+
expect(result).toEqual(noChangesMatcher())
162+
})
163+
164+
test('Unused security scheme changes are not reported', async () => {
165+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/unused-scheme-changes')
166+
167+
expect(result).toEqual(noChangesMatcher())
168+
})
169+
170+
test('Security scheme used in global security is not reported if operation has explicit security', async () => {
171+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/global-scheme-changes-ignored')
172+
173+
expect(result).toEqual(noChangesMatcher())
174+
})
175+
176+
test('Operation uses multiple schemes, several schemes changes', async () => {
177+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/multiple-schemes-changes')
178+
179+
// Operation uses ApiKeyAuth and OAuth2, ApiKeyAuth and OAuth2 changes
180+
// Expected: Diffs for ApiKeyAuth scheme change only
181+
expect(result).toEqual(changesSummaryMatcher({
182+
[BREAKING_CHANGE_TYPE]: 1,
183+
[UNCLASSIFIED_CHANGE_TYPE]: 1, // todo: fix the ER once api-diff classification is changed
184+
}))
185+
})
186+
187+
test('Security scheme used in global security changes are reported if operation has no explicit security', async () => {
188+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/used-global-scheme-changes')
189+
190+
expect(result).toEqual(changesSummaryMatcher({
191+
[BREAKING_CHANGE_TYPE]: 1,
192+
}))
193+
})
194+
195+
test('Scheme added to components and used by operation', async () => {
196+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/add-and-use-schema-in-operation')
197+
198+
// ApiKeyAuth scheme added, operation adds security using it
199+
// Expected: Operation has diffs for adding operation security and scheme
200+
// todo: fix the ER once api-diff classification is changed, adding schema in components should not be breaking
201+
expect(result).toEqual(changesSummaryMatcher({
202+
[BREAKING_CHANGE_TYPE]: 2,
203+
}))
204+
})
205+
206+
test('Scheme removed from components, still referenced', async () => {
207+
const result = await buildChangelogPackage('changelog/security/scheme-relevance/scheme-removed-still-referenced')
208+
209+
// ApiKeyAuth scheme removed, still referenced by operation
210+
// Expected: Operation has diffs for removing scheme
211+
// todo: fix the ER once api-diff classification is changed
212+
// removing schema from components should be annotation
213+
// security scheme for which there is not definition should be removed by api-unifier during validation?
214+
expect(result).toEqual(changesSummaryMatcher({
215+
[NON_BREAKING_CHANGE_TYPE]: 1,
216+
}))
217+
})
218+
})
219+
220+
describe('Multiple Operations - Different Security Configurations', () => {
221+
test('One operation has explicit security, another uses global, global changes', async () => {
222+
const result = await buildChangelogPackage('changelog/security/multiple-operations/op1-explicit-op2-global')
223+
224+
//todo: fix the ER once api-diff classification is changed
225+
// both changes to root security and security scheme definitions in components are reported
226+
expect(result).toEqual(changesSummaryMatcher({
227+
[BREAKING_CHANGE_TYPE]: 1,
228+
[NON_BREAKING_CHANGE_TYPE]: 1,
229+
[UNCLASSIFIED_CHANGE_TYPE]: 2,
230+
}))
231+
232+
// only operation without explicit security is impacted
233+
expect(result).toEqual(numberOfImpactedOperationsMatcher({
234+
[BREAKING_CHANGE_TYPE]: 1,
235+
[NON_BREAKING_CHANGE_TYPE]: 1,
236+
[UNCLASSIFIED_CHANGE_TYPE]: 1,
237+
}))
238+
expect(result).toEqual(operationChangesMatcher([
239+
expect.objectContaining({
240+
operationId: 'test2-get',
241+
previousOperationId: 'test2-get',
242+
}),
243+
]))
244+
})
245+
246+
test('Security scheme used by one operation only', async () => {
247+
const result = await buildChangelogPackage('changelog/security/multiple-operations/explicit-security-schema-used-by-one-operation-changes')
248+
249+
//todo: fix the ER once api-diff classification is changed
250+
expect(result).toEqual(changesSummaryMatcher({
251+
[UNCLASSIFIED_CHANGE_TYPE]: 1,
252+
}))
253+
254+
// only operation which uses changed security scheme is reported
255+
expect(result).toEqual(numberOfImpactedOperationsMatcher({
256+
[UNCLASSIFIED_CHANGE_TYPE]: 1,
257+
}))
258+
expect(result).toEqual(operationChangesMatcher([
259+
expect.objectContaining({
260+
operationId: 'test1-get',
261+
previousOperationId: 'test1-get',
262+
}),
263+
]))
264+
})
265+
266+
test('Security scheme used by global, affects only operations without explicit security', async () => {
267+
const result = await buildChangelogPackage('changelog/security/multiple-operations/scheme-global-affects-subset')
268+
269+
expect(result).toEqual(changesSummaryMatcher({
270+
[BREAKING_CHANGE_TYPE]: 1,
271+
}))
272+
273+
// only operation which does not have explicit security is affected by security scheme change used in global security
274+
expect(result).toEqual(numberOfImpactedOperationsMatcher({
275+
[BREAKING_CHANGE_TYPE]: 1,
276+
}))
277+
expect(result).toEqual(operationChangesMatcher([
278+
expect.objectContaining({
279+
operationId: 'test2-get',
280+
previousOperationId: 'test2-get',
281+
}),
282+
]))
283+
})
284+
})
285+
})
286+

0 commit comments

Comments
 (0)