github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 01:38:56 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Authz] Fix description generation for Open API spec for an API (#198054)

Browse source 
Closes https://github.com/elastic/kibana/issues/198058. 

Adds a fix for https://github.com/elastic/kibana/pull/197001

## Summary
There was an error in how descriptions were added to the Open API spec
for a given route - for the specific case when both a route description
and security authz required privileges were present. The code with the
error is:
https://github.com/elastic/kibana/pull/197001/files#diff-5942307fac5a7b321e7f317bacd2837a7f766f3e79d5aad285513b1f82951b46R79-R80

This PR fixes that error. 


Also updated: Description field for required privileges now includes a
more intuitive descriptor: `Required authorization` as well as a line
break.

<img width="838" alt="image"
src="https://github.com/user-attachments/assets/e6af0459-28e8-40e5-873d-924d1a49b01b">

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Sid 2024-11-04 16:57:45 +01:00 • committed by GitHub
parent 3aa60245c0
commit b12e7d0e79
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 52 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

4
oas_docs/bundle.json
View file

@ -40775,7 +40775,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": [
{
@ -41018,7 +41018,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": [
{

7
oas_docs/output/kibana.yaml
View file

@ -41050,7 +41050,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
@ -41239,7 +41241,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

12
packages/kbn-router-to-openapispec/src/extract_authz_description.test.ts
View file

@ -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].'
);
}
});

2
packages/kbn-router-to-openapispec/src/extract_authz_description.ts
View file

@ -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
packages/kbn-router-to-openapispec/src/process_router.test.ts
View file

@ -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].'
);
});
});

7
packages/kbn-router-to-openapispec/src/process_router.ts
View file

@ -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;
@ -77,7 +79,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

3
packages/kbn-router-to-openapispec/src/process_versioned_router.test.ts
View file

@ -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: [
{

8
packages/kbn-router-to-openapispec/src/process_versioned_router.ts
View file

@ -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