Skip to content

Commit

Permalink
[8.16] [Authz] Fix description generation for Open API spec for an API (
Browse files Browse the repository at this point in the history
#198054) (#198837)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Authz] Fix description generation for Open API spec for an API
(#198054)](#198054)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Sid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-04T15:57:45Z","message":"[Authz]
Fix description generation for Open API spec for an API
(#198054)\n\nCloses #198058.
\r\n\r\nAdds a fix for
https://github.com/elastic/kibana/pull/197001\r\n\r\n## Summary\r\nThere
was an error in how descriptions were added to the Open API spec\r\nfor
a given route - for the specific case when both a route
description\r\nand security authz required privileges were present. The
code with the\r\nerror
is:\r\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\r\n\r\nThis
PR fixes that error. \r\n\r\n\r\nAlso updated: Description field for
required privileges now includes a\r\nmore intuitive descriptor:
`Required authorization` as well as a line\r\nbreak.\r\n\r\n<img
width=\"838\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Security","release_note:skip","Feature:Security/Authorization","v9.0.0","backport:prev-major","v8.16.0","v8.17.0"],"number":198054,"url":"https://github.com/elastic/kibana/pull/198054","mergeCommit":{"message":"[Authz]
Fix description generation for Open API spec for an API
(#198054)\n\nCloses #198058.
\r\n\r\nAdds a fix for
https://github.com/elastic/kibana/pull/197001\r\n\r\n## Summary\r\nThere
was an error in how descriptions were added to the Open API spec\r\nfor
a given route - for the specific case when both a route
description\r\nand security authz required privileges were present. The
code with the\r\nerror
is:\r\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\r\n\r\nThis
PR fixes that error. \r\n\r\n\r\nAlso updated: Description field for
required privileges now includes a\r\nmore intuitive descriptor:
`Required authorization` as well as a line\r\nbreak.\r\n\r\n<img
width=\"838\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198054","number":198054,"mergeCommit":{"message":"[Authz]
Fix description generation for Open API spec for an API
(#198054)\n\nCloses #198058.
\r\n\r\nAdds a fix for
https://github.com/elastic/kibana/pull/197001\r\n\r\n## Summary\r\nThere
was an error in how descriptions were added to the Open API spec\r\nfor
a given route - for the specific case when both a route
description\r\nand security authz required privileges were present. The
code with the\r\nerror
is:\r\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\r\n\r\nThis
PR fixes that error. \r\n\r\n\r\nAlso updated: Description field for
required privileges now includes a\r\nmore intuitive descriptor:
`Required authorization` as well as a line\r\nbreak.\r\n\r\n<img
width=\"838\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/198814","number":198814,"state":"MERGED","mergeCommit":{"sha":"312f642c4a4451ff19dbb3a6dcf799996147c8f7","message":"[8.x]
[Authz] Fix description generation for Open API spec for an API
(#198054) (#198814)\n\n# Backport\n\nThis will backport the following
commits from `main` to `8.x`:\n- [[Authz] Fix description generation for
Open API spec for an
API\n(#198054)](https://github.com/elastic/kibana/pull/198054)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT\n[{\"author\":{\"name\":\"Sid\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2024-11-04T15:57:45Z\",\"message\":\"[Authz]\nFix
description generation for Open API spec for an
API\n(#198054)\\n\\nCloses
https://github.com/elastic/kibana/issues/198058.\n\\r\\n\\r\\nAdds a fix
for\nhttps://github.com//pull/197001\\r\\n\\r\\n##
Summary\\r\\nThere\nwas an error in how descriptions were added to the
Open API spec\\r\\nfor\na given route - for the specific case when both
a route\ndescription\\r\\nand security authz required privileges were
present. The\ncode with
the\\r\\nerror\nis:\\r\\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\\r\\n\\r\\nThis\nPR
fixes that error. \\r\\n\\r\\n\\r\\nAlso updated: Description field
for\nrequired privileges now includes a\\r\\nmore intuitive
descriptor:\n`Required authorization` as well as a
line\\r\\nbreak.\\r\\n\\r\\n<img\nwidth=\\\"838\\\"\nalt=\\\"image\\\"\\r\\nsrc=\\\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\\\">\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nElastic
Machine\n<[email protected]>\\r\\nCo-authored-by:\nkibanamachine\n<[email protected]>\",\"sha\":\"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72\",\"branchLabelMapping\":{\"^v9.0.0$\":\"main\",\"^v8.17.0$\":\"8.x\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"bug\",\"Team:Security\",\"release_note:skip\",\"Feature:Security/Authorization\",\"v9.0.0\",\"backport:prev-major\",\"v8.16.0\",\"v8.17.0\"],\"title\":\"[Authz]\nFix
description generation for Open API spec for
an\nAPI\",\"number\":198054,\"url\":\"https://github.com/elastic/kibana/pull/198054\",\"mergeCommit\":{\"message\":\"[Authz]\nFix
description generation for Open API spec for an
API\n(#198054)\\n\\nCloses
https://github.com/elastic/kibana/issues/198058.\n\\r\\n\\r\\nAdds a fix
for\nhttps://github.com//pull/197001\\r\\n\\r\\n##
Summary\\r\\nThere\nwas an error in how descriptions were added to the
Open API spec\\r\\nfor\na given route - for the specific case when both
a route\ndescription\\r\\nand security authz required privileges were
present. The\ncode with
the\\r\\nerror\nis:\\r\\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\\r\\n\\r\\nThis\nPR
fixes that error. \\r\\n\\r\\n\\r\\nAlso updated: Description field
for\nrequired privileges now includes a\\r\\nmore intuitive
descriptor:\n`Required authorization` as well as a
line\\r\\nbreak.\\r\\n\\r\\n<img\nwidth=\\\"838\\\"\nalt=\\\"image\\\"\\r\\nsrc=\\\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\\\">\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nElastic
Machine\n<[email protected]>\\r\\nCo-authored-by:\nkibanamachine\n<[email protected]>\",\"sha\":\"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[\"8.16\",\"8.x\"],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v9.0.0\",\"branchLabelMappingKey\":\"^v9.0.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/198054\",\"number\":198054,\"mergeCommit\":{\"message\":\"[Authz]\nFix
description generation for Open API spec for an
API\n(#198054)\\n\\nCloses
https://github.com/elastic/kibana/issues/198058.\n\\r\\n\\r\\nAdds a fix
for\nhttps://github.com//pull/197001\\r\\n\\r\\n##
Summary\\r\\nThere\nwas an error in how descriptions were added to the
Open API spec\\r\\nfor\na given route - for the specific case when both
a route\ndescription\\r\\nand security authz required privileges were
present. The\ncode with
the\\r\\nerror\nis:\\r\\nhttps://github.com//pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80\\r\\n\\r\\nThis\nPR
fixes that error. \\r\\n\\r\\n\\r\\nAlso updated: Description field
for\nrequired privileges now includes a\\r\\nmore intuitive
descriptor:\n`Required authorization` as well as a
line\\r\\nbreak.\\r\\n\\r\\n<img\nwidth=\\\"838\\\"\nalt=\\\"image\\\"\\r\\nsrc=\\\"https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b\\\">\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nElastic
Machine\n<[email protected]>\\r\\nCo-authored-by:\nkibanamachine\n<[email protected]>\",\"sha\":\"b12e7d0e79af8150ea9f2b5940a6ad1d428cff72\"}},{\"branch\":\"8.16\",\"label\":\"v8.16.0\",\"branchLabelMappingKey\":\"^v(\\\\d+).(\\\\d+).\\\\d+$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"},{\"branch\":\"8.x\",\"label\":\"v8.17.0\",\"branchLabelMappingKey\":\"^v8.17.0$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"}]}]\nBACKPORT-->\n\nCo-authored-by:
Sid <[email protected]>"}}]}] BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
SiddharthMantri and kibanamachine authored Nov 4, 2024
1 parent c8c8504 commit c450a85
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ describe('extractAuthzDescription', () => {
},
};
const description = extractAuthzDescription(routeSecurity);
expect(description).toBe('[Authz] Route required privileges: ALL of [manage_spaces].');
expect(description).toBe(
'[Required authorization] Route required privileges: ALL of [manage_spaces].'
);
});

it('should return route authz description for privilege groups', () => {
Expand All @@ -44,7 +46,9 @@ describe('extractAuthzDescription', () => {
},
};
const description = extractAuthzDescription(routeSecurity);
expect(description).toBe('[Authz] Route required privileges: ALL of [console].');
expect(description).toBe(
'[Required authorization] Route required privileges: ALL of [console].'
);
}
{
const routeSecurity: RouteSecurity = {
Expand All @@ -58,7 +62,7 @@ describe('extractAuthzDescription', () => {
};
const description = extractAuthzDescription(routeSecurity);
expect(description).toBe(
'[Authz] Route required privileges: ANY of [manage_spaces OR taskmanager].'
'[Required authorization] Route required privileges: ANY of [manage_spaces OR taskmanager].'
);
}
{
Expand All @@ -74,7 +78,7 @@ describe('extractAuthzDescription', () => {
};
const description = extractAuthzDescription(routeSecurity);
expect(description).toBe(
'[Authz] Route required privileges: ALL of [console, filesManagement] AND ANY of [manage_spaces OR taskmanager].'
'[Required authorization] Route required privileges: ALL of [console, filesManagement] AND ANY of [manage_spaces OR taskmanager].'
);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ export const extractAuthzDescription = (routeSecurity: InternalRouteSecurity | u
return `Route required privileges: ${getPrivilegesDescription(allRequired, anyRequired)}.`;
};

return `[Authz] ${getDescriptionForRoute()}`;
return `[Required authorization] ${getDescriptionForRoute()}`;
};
28 changes: 26 additions & 2 deletions packages/kbn-router-to-openapispec/src/process_router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,26 @@ describe('processRouter', () => {
},
},
},
{
path: '/quux',
method: 'post',
options: {
description: 'This a test route description.',
},
handler: jest.fn(),
validationSchemas: { request: { body: schema.object({}) } },
security: {
authz: {
requiredPrivileges: [
'manage_spaces',
{
allRequired: ['taskmanager'],
anyRequired: ['console'],
},
],
},
},
},
],
} as unknown as Router;

Expand All @@ -132,7 +152,7 @@ describe('processRouter', () => {
version: '2023-10-31',
});

expect(Object.keys(result1.paths!)).toHaveLength(4);
expect(Object.keys(result1.paths!)).toHaveLength(5);

const result2 = processRouter(testRouter, new OasConverter(), createOpIdGenerator(), {
version: '2024-10-31',
Expand All @@ -148,7 +168,11 @@ describe('processRouter', () => {
expect(result.paths['/qux']?.post).toBeDefined();

expect(result.paths['/qux']?.post?.description).toEqual(
'[Authz] Route required privileges: ALL of [manage_spaces, taskmanager] AND ANY of [console].'
'[Required authorization] Route required privileges: ALL of [manage_spaces, taskmanager] AND ANY of [console].'
);

expect(result.paths['/quux']?.post?.description).toEqual(
'This a test route description.<br/><br/>[Required authorization] Route required privileges: ALL of [manage_spaces, taskmanager] AND ANY of [console].'
);
});
});
7 changes: 4 additions & 3 deletions packages/kbn-router-to-openapispec/src/process_router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,19 @@ export const processRouter = (
parameters.push(...pathObjects, ...queryObjects);
}

let description = '';
let description = `${route.options.description ?? ''}`;
if (route.security) {
const authzDescription = extractAuthzDescription(route.security);

description = `${route.options.description ?? ''}${authzDescription ?? ''}`;
description += `${route.options.description && authzDescription ? `<br/><br/>` : ''}${
authzDescription ?? ''
}`;
}

const operation: CustomOperationObject = {
summary: route.options.summary ?? '',
tags: route.options.tags ? extractTags(route.options.tags) : [],
...(description ? { description } : {}),
...(route.options.description ? { description: route.options.description } : {}),
...(route.options.deprecated ? { deprecated: route.options.deprecated } : {}),
...(route.options.discontinued ? { 'x-discontinued': route.options.discontinued } : {}),
requestBody: !!validationSchemas?.body
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ describe('processVersionedRouter', () => {
expect(results.paths['/foo']!.get).toBeDefined();

expect(results.paths['/foo']!.get!.description).toBe(
'[Authz] Route required privileges: ALL of [manage_spaces].'
'This is a test route description.<br/><br/>[Required authorization] Route required privileges: ALL of [manage_spaces].'
);
});
});
Expand All @@ -175,6 +175,7 @@ const createTestRoute: () => any = () => ({
requiredPrivileges: ['manage_spaces'],
},
},
description: 'This is a test route description.',
},
handlers: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,13 @@ export const processVersionedRouter = (
...queryObjects,
];
}

let description = '';
let description = `${route.options.description ?? ''}`;
if (route.options.security) {
const authzDescription = extractAuthzDescription(route.options.security);

description = `${route.options.description ?? ''}${authzDescription ?? ''}`;
description += `${route.options.description && authzDescription ? '<br/><br/>' : ''}${
authzDescription ?? ''
}`;
}

const hasBody = Boolean(extractValidationSchemaFromVersionedHandler(handler)?.request?.body);
Expand All @@ -106,7 +107,6 @@ export const processVersionedRouter = (
summary: route.options.summary ?? '',
tags: route.options.options?.tags ? extractTags(route.options.options.tags) : [],
...(description ? { description } : {}),
...(route.options.description ? { description: route.options.description } : {}),
...(route.options.deprecated ? { deprecated: route.options.deprecated } : {}),
...(route.options.discontinued ? { 'x-discontinued': route.options.discontinued } : {}),
requestBody: hasBody
Expand Down

0 comments on commit c450a85

Please sign in to comment.