From 62de8aabc7d74ad5e4b936078a2e552e9af28dc6 Mon Sep 17 00:00:00 2001 From: Carlos Ravelo Date: Wed, 1 Sep 2021 17:16:03 -0400 Subject: [PATCH] Update path transform to make the security key optional (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update path transform to make the security key optional If the security key is present in a path swagger will take that definition over the global security one. By always adding this key we are disabling security globally. Modified the transform for a path to only set the key when values are found. * fix(linting): error not related to my change ???? forced by commit hook feat(security): Added the ability to disable security By adding an empty @security tag to the jsdoc block for a path * fix(test): test failed after making an unrelated change These tests broke. I'm not sure why only making a change in transforms/paths/index.js under the parsePath response triggers the failures. * fix(test): fixed tests and added a couple Co-authored-by: Kevin Julián Martínez Escobar --- package-lock.json | 2 +- test/e2e/configuration/configuration.test.js | 20 +++++++- test/e2e/parameters/parameters.test.js | 2 - test/transforms/paths/combineSchemas.test.js | 2 - test/transforms/paths/formParameters.test.js | 5 -- test/transforms/paths/index.test.js | 6 --- test/transforms/paths/parameters.test.js | 8 --- test/transforms/paths/requestBody.test.js | 10 ---- test/transforms/paths/responses.test.js | 13 ----- test/transforms/paths/security.test.js | 54 ++++++++++++++++++++ test/transforms/paths/tags.test.js | 3 -- transforms/paths/index.js | 12 ++++- transforms/paths/security.js | 30 ++++++----- 13 files changed, 102 insertions(+), 65 deletions(-) diff --git a/package-lock.json b/package-lock.json index 74d7a7a..cca656c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6858,7 +6858,7 @@ "resolved": "https://registry.npmjs.org/swagger-ui-express/-/swagger-ui-express-4.1.6.tgz", "integrity": "sha512-Xs2BGGudvDBtL7RXcYtNvHsFtP1DBFPMJFRxHe5ez/VG/rzVOEjazJOOSc/kSCyxreCTKfJrII6MJlL9a6t8vw==", "requires": { - "swagger-ui-dist": "^3.43.0" + "swagger-ui-dist": "^3.18.1" }, "dependencies": { "swagger-ui-dist": { diff --git a/test/e2e/configuration/configuration.test.js b/test/e2e/configuration/configuration.test.js index a2311c3..b378f18 100644 --- a/test/e2e/configuration/configuration.test.js +++ b/test/e2e/configuration/configuration.test.js @@ -38,6 +38,13 @@ const options = { }, }, ], + security: { + BearerAuth: { + type: 'http', + scheme: 'bearer', + bearerFormat: 'JWT', + }, + }, filesPattern: './main.js', baseDir: __dirname, }; @@ -82,11 +89,22 @@ test('should parse basic info', async () => { }, }, ], - security: undefined, + security: [ + { + BearerAuth: [], + }, + ], paths: {}, tags: [], components: { schemas: {}, + securitySchemes: { + BearerAuth: { + bearerFormat: 'JWT', + scheme: 'bearer', + type: 'http', + }, + }, }, }; const result = await processSwagger(options); diff --git a/test/e2e/parameters/parameters.test.js b/test/e2e/parameters/parameters.test.js index df6a3ef..db921f5 100644 --- a/test/e2e/parameters/parameters.test.js +++ b/test/e2e/parameters/parameters.test.js @@ -64,7 +64,6 @@ test('should parse parameters from jsdoc-example', async () => { }, }, ], - security: [], tags: [], }, }, @@ -110,7 +109,6 @@ test('should parse parameters from jsdoc-example', async () => { }, }, ], - security: [], tags: [], }, }, diff --git a/test/transforms/paths/combineSchemas.test.js b/test/transforms/paths/combineSchemas.test.js index 0b23bc0..68b1a8e 100644 --- a/test/transforms/paths/combineSchemas.test.js +++ b/test/transforms/paths/combineSchemas.test.js @@ -19,7 +19,6 @@ test('should parse jsdoc path response with oneOf keyword', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -130,7 +129,6 @@ test('should parse jsdoc path reference params with allOf keyword', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: false, description: 'name param description', diff --git a/test/transforms/paths/formParameters.test.js b/test/transforms/paths/formParameters.test.js index 91b6be0..72f0d82 100644 --- a/test/transforms/paths/formParameters.test.js +++ b/test/transforms/paths/formParameters.test.js @@ -18,7 +18,6 @@ describe('form requestBody tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { content: { @@ -66,7 +65,6 @@ describe('form requestBody tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [ { deprecated: false, @@ -126,7 +124,6 @@ describe('form requestBody tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -177,7 +174,6 @@ describe('form requestBody tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [ { deprecated: false, @@ -247,7 +243,6 @@ describe('form requestBody tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [ { deprecated: false, diff --git a/test/transforms/paths/index.test.js b/test/transforms/paths/index.test.js index e868756..5f73eab 100644 --- a/test/transforms/paths/index.test.js +++ b/test/transforms/paths/index.test.js @@ -40,7 +40,6 @@ describe('setPaths method', () => { description: 'This is the description of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -79,7 +78,6 @@ describe('setPaths method', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -125,7 +123,6 @@ describe('setPaths method', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -147,7 +144,6 @@ describe('setPaths method', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -193,7 +189,6 @@ describe('setPaths method', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -213,7 +208,6 @@ describe('setPaths method', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', diff --git a/test/transforms/paths/parameters.test.js b/test/transforms/paths/parameters.test.js index 04697d0..4e77891 100644 --- a/test/transforms/paths/parameters.test.js +++ b/test/transforms/paths/parameters.test.js @@ -19,7 +19,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], operationId: 'getInfo', parameters: [{ deprecated: false, @@ -56,7 +55,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], }, }, @@ -83,7 +81,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: true, description: 'name param description', @@ -123,7 +120,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: true, description: 'name param description', @@ -171,7 +167,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: false, description: 'name param description', @@ -207,7 +202,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: true, description: 'name param description', @@ -246,7 +240,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: false, description: 'name param description', @@ -286,7 +279,6 @@ describe('params tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [{ deprecated: false, description: 'name param description', diff --git a/test/transforms/paths/requestBody.test.js b/test/transforms/paths/requestBody.test.js index 0ca5a45..dfe8ebb 100644 --- a/test/transforms/paths/requestBody.test.js +++ b/test/transforms/paths/requestBody.test.js @@ -17,7 +17,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], }, }, @@ -44,7 +43,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -83,7 +81,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: '', @@ -124,7 +121,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -166,7 +162,6 @@ describe('request body tests', () => { summary: 'Delete messages listed under the specified tags', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'Tags of the messages to delete', @@ -208,7 +203,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -249,7 +243,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -287,7 +280,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -330,7 +322,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', @@ -383,7 +374,6 @@ describe('request body tests', () => { summary: '', responses: {}, tags: [], - security: [], parameters: [], requestBody: { description: 'name body description', diff --git a/test/transforms/paths/responses.test.js b/test/transforms/paths/responses.test.js index 674dadd..c1b61f1 100644 --- a/test/transforms/paths/responses.test.js +++ b/test/transforms/paths/responses.test.js @@ -21,7 +21,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -58,7 +57,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -109,7 +107,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -164,7 +161,6 @@ describe('response tests', () => { responses: {}, parameters: [], tags: [], - security: [], }, }, }, @@ -199,7 +195,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { default: { description: 'success response', @@ -243,7 +238,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -286,7 +280,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -326,7 +319,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -371,7 +363,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -436,7 +427,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -548,7 +538,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -618,7 +607,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', @@ -676,7 +664,6 @@ describe('response tests', () => { summary: 'This is the summary of the endpoint', parameters: [], tags: [], - security: [], responses: { 200: { description: 'success response', diff --git a/test/transforms/paths/security.test.js b/test/transforms/paths/security.test.js index a81b432..234ec65 100644 --- a/test/transforms/paths/security.test.js +++ b/test/transforms/paths/security.test.js @@ -138,4 +138,58 @@ describe('Paths - security', () => { const result = setPaths({}, parsedJSDocs); expect(result).toEqual(expected); }); + + it('Should parse jsdoc empty security tag into empty array', () => { + const jsodInput = [` + /** + * POST /api/v1/song + * @summary Create new song + * @security + */ + `]; + const expected = { + paths: { + '/api/v1/song': { + post: { + deprecated: false, + description: undefined, + summary: 'Create new song', + security: [], + tags: [], + responses: {}, + parameters: [], + }, + }, + }, + }; + const parsedJSDocs = jsdocInfo()(jsodInput); + const result = setPaths({}, parsedJSDocs); + expect(result).toEqual(expected); + }); + + it('Should parse jsdoc with no security tag and not include security in the swagger object', () => { + const jsodInput = [` + /** + * POST /api/v1/song + * @summary Create new song + */ + `]; + const expected = { + paths: { + '/api/v1/song': { + post: { + deprecated: false, + description: undefined, + summary: 'Create new song', + tags: [], + responses: {}, + parameters: [], + }, + }, + }, + }; + const parsedJSDocs = jsdocInfo()(jsodInput); + const result = setPaths({}, parsedJSDocs); + expect(result).toEqual(expected); + }); }); diff --git a/test/transforms/paths/tags.test.js b/test/transforms/paths/tags.test.js index bdb6fda..6360dce 100644 --- a/test/transforms/paths/tags.test.js +++ b/test/transforms/paths/tags.test.js @@ -16,7 +16,6 @@ describe('Paths - tags', () => { deprecated: false, description: undefined, summary: '', - security: [], tags: [ 'album', ], @@ -46,7 +45,6 @@ describe('Paths - tags', () => { deprecated: false, description: undefined, summary: '', - security: [], tags: [ 'album', 'Years', @@ -77,7 +75,6 @@ describe('Paths - tags', () => { deprecated: false, description: undefined, summary: '', - security: [], tags: [ 'album', 'Years', diff --git a/transforms/paths/index.js b/transforms/paths/index.js index 10229c5..4d19ef6 100644 --- a/transforms/paths/index.js +++ b/transforms/paths/index.js @@ -24,6 +24,16 @@ const setRequestBody = (lowerCaseMethod, bodyValues, requestExamples) => { return bodyMethods[lowerCaseMethod] && hasBodyValues ? { requestBody } : {}; }; +const setSecurity = securityValues => { + if (securityValues === null) { + // If securityValues are null then the tag was not set and we shouldn't override + return {}; + } + + // otherwise if securityValues is an array with values return that or return an empty + return { security: securityValues }; +}; + const bodyParams = ({ name }) => name.includes('request.body') || name.includes('.form'); const getOperationId = operationIdTag => { @@ -100,10 +110,10 @@ const parsePath = (path, state) => { deprecated: isDeprecated, summary: formatSummary(summary), description: formatDescription(description), - security: formatSecurity(securityValues), responses, parameters, tags: formatTags(tagsValues), + ...(setSecurity(formatSecurity(securityValues))), ...(setRequestBody(lowerCaseMethod, bodyValues, requestExamples)), ...(getOperationId(operationId)), }, diff --git a/transforms/paths/security.js b/transforms/paths/security.js index 1128cd7..41711ac 100644 --- a/transforms/paths/security.js +++ b/transforms/paths/security.js @@ -4,6 +4,8 @@ const AND_SEPARATOR = ' & '; const OR_SEPARATOR = ' | '; const formatOrValues = ({ description }) => { + if (!description) { return []; } + const securityNames = description.split(OR_SEPARATOR); return securityNames.map(names => ({ description: names, @@ -11,19 +13,21 @@ const formatOrValues = ({ description }) => { }; const formatSecurity = (securityValues = []) => ( - flatArray(securityValues - .map(formatOrValues)) - .map(({ description }) => { - const securityNames = description.split(AND_SEPARATOR); - return { - ...securityNames.reduce((acum, names) => ( - { - ...acum, - [names]: [], - } - ), {}), - }; - }) + securityValues.length + ? flatArray(securityValues + .map(formatOrValues)) + .map(({ description }) => { + const securityNames = description.split(AND_SEPARATOR); + return { + ...securityNames.reduce((acum, names) => ( + { + ...acum, + [names]: [], + } + ), {}), + }; + }) + : null ); module.exports = formatSecurity;