Skip to content

Commit

Permalink
[8.x] Authorized route migration for routes owned by kibana-security (#…
Browse files Browse the repository at this point in the history
…198380) (#198657)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Authorized route migration for routes owned by kibana-security
(#198380)](#198380)

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

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

<!--BACKPORT [{"author":{"name":"Kibana
Machine","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-01T11:42:40Z","message":"Authorized
route migration for routes owned by kibana-security (#198380)\n\n###
Authz API migration for authorized routes\r\n\r\nThis PR migrates
`access:<privilege>` tags used in route definitions to\r\nnew security
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\nAccess control tags were defined in the
`options` object of the route:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n options: {\r\n tags: ['access:<privilege_1>',
'access:<privilege_2>'],\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### **After migration:**\r\nTags have been
replaced with the more robust\r\n`security.authz.requiredPrivileges`
field under `security`:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n security: {\r\n authz: {\r\n requiredPrivileges:
['<privilege_1>', '<privilege_2>'],\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have tests that rely on checking
`access` tags.\r\n - If you have snapshot tests that include the route
definition.\r\n- If you have FTR tests that rely on checking
unauthorized error\r\nmessage. The error message changed to also include
missing privileges.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Elena Shostak
<[email protected]>","sha":"20989b64db84aee2500f318bb3b84bf4afedf6b2","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","enhancement","release_note:skip","Feature:Security/Authorization","v9.0.0","backport:prev-minor","Authz:
API
migration"],"number":198380,"url":"https://github.com/elastic/kibana/pull/198380","mergeCommit":{"message":"Authorized
route migration for routes owned by kibana-security (#198380)\n\n###
Authz API migration for authorized routes\r\n\r\nThis PR migrates
`access:<privilege>` tags used in route definitions to\r\nnew security
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\nAccess control tags were defined in the
`options` object of the route:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n options: {\r\n tags: ['access:<privilege_1>',
'access:<privilege_2>'],\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### **After migration:**\r\nTags have been
replaced with the more robust\r\n`security.authz.requiredPrivileges`
field under `security`:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n security: {\r\n authz: {\r\n requiredPrivileges:
['<privilege_1>', '<privilege_2>'],\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have tests that rely on checking
`access` tags.\r\n - If you have snapshot tests that include the route
definition.\r\n- If you have FTR tests that rely on checking
unauthorized error\r\nmessage. The error message changed to also include
missing privileges.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Elena Shostak
<[email protected]>","sha":"20989b64db84aee2500f318bb3b84bf4afedf6b2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198380","number":198380,"mergeCommit":{"message":"Authorized
route migration for routes owned by kibana-security (#198380)\n\n###
Authz API migration for authorized routes\r\n\r\nThis PR migrates
`access:<privilege>` tags used in route definitions to\r\nnew security
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\nAccess control tags were defined in the
`options` object of the route:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n options: {\r\n tags: ['access:<privilege_1>',
'access:<privilege_2>'],\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### **After migration:**\r\nTags have been
replaced with the more robust\r\n`security.authz.requiredPrivileges`
field under `security`:\r\n\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n security: {\r\n authz: {\r\n requiredPrivileges:
['<privilege_1>', '<privilege_2>'],\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have tests that rely on checking
`access` tags.\r\n - If you have snapshot tests that include the route
definition.\r\n- If you have FTR tests that rely on checking
unauthorized error\r\nmessage. The error message changed to also include
missing privileges.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Elena Shostak
<[email protected]>","sha":"20989b64db84aee2500f318bb3b84bf4afedf6b2"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
elena-shostak and kibanamachine authored Nov 1, 2024
1 parent d10dcae commit 83ac80c
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe('GET all roles by space id', () => {

const paramsSchema = (config.validate as any).params;

expect(config.options).toEqual({ tags: ['access:manageSpaces'] });
expect(config.security?.authz).toEqual({ requiredPrivileges: ['manage_spaces'] });
expect(() => paramsSchema.validate({})).toThrowErrorMatchingInlineSnapshot(
`"[spaceId]: expected value of type [string] but got [undefined]"`
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ export function defineGetAllRolesBySpaceRoutes({
router.get(
{
path: '/internal/security/roles/{spaceId}',
options: {
tags: ['access:manageSpaces'],
security: {
authz: {
requiredPrivileges: ['manage_spaces'],
},
},
validate: {
params: schema.object({ spaceId: schema.string({ minLength: 1 }) }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ describe('Invalidate sessions routes', () => {
expect(routeConfig.options).toEqual({
access: 'public',
summary: 'Invalidate user sessions',
tags: ['access:sessionManagement'],
});

expect(routeConfig.security?.authz).toEqual({ requiredPrivileges: ['sessionManagement'] });

const bodySchema = (routeConfig.validate as any).body as ObjectType;
expect(() => bodySchema.validate({})).toThrowErrorMatchingInlineSnapshot(
`"[match]: expected at least one defined value but got [undefined]"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,19 @@ export function defineInvalidateSessionsRoutes({
),
}),
},
security: {
authz: {
requiredPrivileges: ['sessionManagement'],
},
},
options: {
// The invalidate session API was introduced to address situations where the session index
// could grow rapidly - when session timeouts are disabled, or with anonymous access.
// In the serverless environment, sessions timeouts are always be enabled, and there is no
// anonymous access. However, keeping this endpoint available internally in serverless would
// be useful in situations where we need to batch-invalidate user sessions.
access: buildFlavor === 'serverless' ? 'internal' : 'public',
tags: ['access:sessionManagement'],

summary: `Invalidate user sessions`,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('Bulk get profile routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({ tags: ['access:bulkGetUserProfiles'] });
expect(routeConfig.security?.authz).toEqual({ requiredPrivileges: ['bulkGetUserProfiles'] });

const bodySchema = (routeConfig.validate as any).body as ObjectType;
expect(() => bodySchema.validate(0)).toThrowErrorMatchingInlineSnapshot(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ export function defineBulkGetUserProfilesRoute({
dataPath: schema.maybe(schema.string()),
}),
},
options: { tags: ['access:bulkGetUserProfiles'] },
security: {
authz: {
requiredPrivileges: ['bulkGetUserProfiles'],
},
},
},
createLicensedRouteHandler(async (context, request, response) => {
const userProfileServiceInternal = getUserProfileService();
Expand Down
14 changes: 12 additions & 2 deletions x-pack/plugins/spaces/server/routes/api/external/copy_to_space.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ export function initCopyToSpacesApi(deps: ExternalRouteDeps) {
router.post(
{
path: '/api/spaces/_copy_saved_objects',
security: {
authz: {
requiredPrivileges: ['copySavedObjectsToSpaces'],
},
},
options: {
access: isServerless ? 'internal' : 'public',
tags: ['access:copySavedObjectsToSpaces', 'oas-tag:spaces'],
tags: ['oas-tag:spaces'],
summary: `Copy saved objects between spaces`,
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.',
Expand Down Expand Up @@ -188,9 +193,14 @@ export function initCopyToSpacesApi(deps: ExternalRouteDeps) {
router.post(
{
path: '/api/spaces/_resolve_copy_saved_objects_errors',
security: {
authz: {
requiredPrivileges: ['copySavedObjectsToSpaces'],
},
},
options: {
access: isServerless ? 'internal' : 'public',
tags: ['access:copySavedObjectsToSpaces'],

summary: `Resolve conflicts copying saved objects`,
description:
'Overwrite saved objects that are returned as errors from the copy saved objects to space API.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('GET /internal/spaces/{spaceId}/content_summary', () => {

const paramsSchema = (config.validate as any).params;

expect(config.options).toEqual({ tags: ['access:manageSpaces'] });
expect(config.security?.authz).toEqual({ requiredPrivileges: ['manage_spaces'] });
expect(() => paramsSchema.validate({})).toThrowErrorMatchingInlineSnapshot(
`"[spaceId]: expected value of type [string] but got [undefined]"`
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ export function initGetSpaceContentSummaryApi(deps: InternalRouteDeps) {
router.get(
{
path: '/internal/spaces/{spaceId}/content_summary',
options: {
tags: ['access:manageSpaces'],
security: {
authz: {
requiredPrivileges: ['manage_spaces'],
},
},
validate: {
params: schema.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ export function copyToSpaceTestSuiteFactory(context: FtrProviderContext) {
expect(resp.body).to.eql({
statusCode: 403,
error: 'Forbidden',
message: 'Forbidden',
message:
'API [POST /api/spaces/_copy_saved_objects] is unauthorized for user, this action is granted by the Kibana privileges [copySavedObjectsToSpaces]',
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ export function resolveCopyToSpaceConflictsSuite(context: FtrProviderContext) {
expect(resp.body).to.eql({
statusCode: 403,
error: 'Forbidden',
message: 'Forbidden',
message:
'API [POST /api/spaces/_resolve_copy_saved_objects_errors] is unauthorized for user, this action is granted by the Kibana privileges [copySavedObjectsToSpaces]',
});
};

Expand Down

0 comments on commit 83ac80c

Please sign in to comment.