mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 17:59:23 -04:00
# Backport This will backport the following commits from `main` to `8.x`: - [[Authz] Fix description generation for Open API spec for an API (#198054)](https://github.com/elastic/kibana/pull/198054) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sid","email":"siddharthmantri1@gmail.com"},"sourceCommit":{"committedDate":"2024-11-04T15:57:45Z","message":"[Authz] Fix description generation for Open API spec for an API (#198054)\n\nCloses https://github.com/elastic/kibana/issues/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/elastic/kibana/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 <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","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] Fix description generation for Open API spec for an API","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 https://github.com/elastic/kibana/issues/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/elastic/kibana/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 <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","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] Fix description generation for Open API spec for an API (#198054)\n\nCloses https://github.com/elastic/kibana/issues/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/elastic/kibana/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 <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","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"}]}] BACKPORT--> Co-authored-by: Sid <siddharthmantri1@gmail.com>
This commit is contained in:
parent
8fdcef5aef
commit
312f642c4a
8 changed files with 52 additions and 19 deletions
|
@ -6934,7 +6934,7 @@
|
|||
},
|
||||
"/api/spaces/_copy_saved_objects": {
|
||||
"post": {
|
||||
"description": "It also allows you to automatically copy related objects, so when you copy a dashboard, this can automatically copy over the associated visualizations, data views, and saved searches, as required. You can request to overwrite any objects that already exist in the target space if they share an identifier or you can use the resolve copy saved objects conflicts API to do this on a per-object basis.",
|
||||
"description": "It also allows you to automatically copy related objects, so when you copy a dashboard, this can automatically copy over the associated visualizations, data views, and saved searches, as required. You can request to overwrite any objects that already exist in the target space if they share an identifier or you can use the resolve copy saved objects conflicts API to do this on a per-object basis.<br/><br/>[Required authorization] Route required privileges: ALL of [copySavedObjectsToSpaces].",
|
||||
"operationId": "post-spaces-copy-saved-objects",
|
||||
"parameters": [
|
||||
{
|
||||
|
@ -7177,7 +7177,7 @@
|
|||
},
|
||||
"/api/spaces/_resolve_copy_saved_objects_errors": {
|
||||
"post": {
|
||||
"description": "Overwrite saved objects that are returned as errors from the copy saved objects to space API.",
|
||||
"description": "Overwrite saved objects that are returned as errors from the copy saved objects to space API.<br/><br/>[Required authorization] Route required privileges: ALL of [copySavedObjectsToSpaces].",
|
||||
"operationId": "post-spaces-resolve-copy-saved-objects-errors",
|
||||
"parameters": [
|
||||
{
|
||||
|
|
|
@ -21350,7 +21350,9 @@ paths:
|
|||
visualizations, data views, and saved searches, as required. You can
|
||||
request to overwrite any objects that already exist in the target space
|
||||
if they share an identifier or you can use the resolve copy saved
|
||||
objects conflicts API to do this on a per-object basis.
|
||||
objects conflicts API to do this on a per-object
|
||||
basis.<br/><br/>[Required authorization] Route required privileges: ALL
|
||||
of [copySavedObjectsToSpaces].
|
||||
operationId: post-spaces-copy-saved-objects
|
||||
parameters:
|
||||
- description: The version of the API to use
|
||||
|
@ -21539,7 +21541,8 @@ paths:
|
|||
post:
|
||||
description: >-
|
||||
Overwrite saved objects that are returned as errors from the copy saved
|
||||
objects to space API.
|
||||
objects to space API.<br/><br/>[Required authorization] Route required
|
||||
privileges: ALL of [copySavedObjectsToSpaces].
|
||||
operationId: post-spaces-resolve-copy-saved-objects-errors
|
||||
parameters:
|
||||
- description: The version of the API to use
|
||||
|
|
|
@ -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', () => {
|
||||
|
@ -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 = {
|
||||
|
@ -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].'
|
||||
);
|
||||
}
|
||||
{
|
||||
|
@ -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].'
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
@ -56,5 +56,5 @@ export const extractAuthzDescription = (routeSecurity: InternalRouteSecurity | u
|
|||
return `Route required privileges: ${getPrivilegesDescription(allRequired, anyRequired)}.`;
|
||||
};
|
||||
|
||||
return `[Authz] ${getDescriptionForRoute()}`;
|
||||
return `[Required authorization] ${getDescriptionForRoute()}`;
|
||||
};
|
||||
|
|
|
@ -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;
|
||||
|
||||
|
@ -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',
|
||||
|
@ -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].'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
@ -64,11 +64,13 @@ 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 hasDeprecations = !!route.options.deprecated;
|
||||
|
@ -76,7 +78,6 @@ export const processRouter = (
|
|||
summary: route.options.summary ?? '',
|
||||
tags: route.options.tags ? extractTags(route.options.tags) : [],
|
||||
...(description ? { description } : {}),
|
||||
...(route.options.description ? { description: route.options.description } : {}),
|
||||
...(hasDeprecations ? { deprecated: true } : {}),
|
||||
...(route.options.discontinued ? { 'x-discontinued': route.options.discontinued } : {}),
|
||||
requestBody: !!validationSchemas?.body
|
||||
|
|
|
@ -157,7 +157,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].'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
@ -176,6 +176,7 @@ const createTestRoute: () => VersionedRouterRoute = () => ({
|
|||
requiredPrivileges: ['manage_spaces'],
|
||||
},
|
||||
},
|
||||
description: 'This is a test route description.',
|
||||
},
|
||||
handlers: [
|
||||
{
|
||||
|
|
|
@ -90,12 +90,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);
|
||||
|
@ -107,7 +108,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 } : {}),
|
||||
...(hasDeprecations ? { deprecated: true } : {}),
|
||||
...(route.options.discontinued ? { 'x-discontinued': route.options.discontinued } : {}),
|
||||
requestBody: hasBody
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue