Skip to content

Commit

Permalink
[Authz] Migrate outstanding SharedUX routes with access tags (elastic…
Browse files Browse the repository at this point in the history
…#206260)

## Summary

Relates to elastic/kibana-team#1235, this PR
tackles routes could not have been migrated automatically by the
security team. Following the guidance by the security provided in the
aforementioned issue instances where the tag approach had been
previously used to configure access have been migrated to use the
`requiredPrivilege` property on `security.authz` for route definitions.

### Checklist
<!--
Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
-->
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.

<!--
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

-->

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
eokoneyo and elasticmachine authored Jan 13, 2025
1 parent 2a5af16 commit e6e4eda
Show file tree
Hide file tree
Showing 30 changed files with 202 additions and 155 deletions.
2 changes: 1 addition & 1 deletion examples/files_example/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const PLUGIN_ID = 'filesExample';
export const PLUGIN_NAME = 'Files example';

const httpTags = {
tags: [`access:${PLUGIN_ID}`],
requiredPrivileges: [PLUGIN_ID],
};

export const exampleFileKind: FileKind = {
Expand Down
2 changes: 1 addition & 1 deletion packages/content-management/content_insights/README.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if (plugins.usageCollection) {
{
domainId: 'dashboard',
// makes sure that only users with read/all access to dashboard app can access the routes
routeTags: ['access:dashboardUsageStats'],
routePrivileges: ['dashboardUsageStats'],
}
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export interface ContentInsightsConfig {
domainId: string;

/**
* Can control created routes access via access tags
* Can control created routes access via security access control
*/
routeTags?: string[];
routePrivileges?: string[];

/**
* Retention period in days for usage counter data
Expand Down Expand Up @@ -89,9 +89,15 @@ export const registerContentInsights = (
{
path: `/internal/content_management/insights/${config.domainId}/{id}/{eventType}`,
validate,
options: {
tags: config.routeTags,
},
...(config.routePrivileges
? {
security: {
authz: {
requiredPrivileges: config.routePrivileges,
},
},
}
: {}),
},
async (context, req, res) => {
const { id, eventType } = req.params;
Expand All @@ -108,9 +114,15 @@ export const registerContentInsights = (
{
path: `/internal/content_management/insights/${config.domainId}/{id}/{eventType}/stats`,
validate,
options: {
tags: config.routeTags,
},
...(config.routePrivileges
? {
security: {
authz: {
requiredPrivileges: config.routePrivileges,
},
},
}
: {}),
},
async (context, req, res) => {
const { id, eventType } = req.params;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/plugins/shared/dashboard/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class DashboardPlugin
{
domainId: 'dashboard',
// makes sure that only users with read/all access to dashboard app can access the routes
routeTags: ['access:dashboardUsageStats'],
routePrivileges: ['dashboardUsageStats'],
}
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { FileKindBase } from '@kbn/shared-ux-file-types';
export const id = 'defaultImage' as const;
export const tag = 'files:defaultImage' as const;
export const tags = [`access:${tag}`];
export const requiredPrivileges = [tag];
export const maxSize = 1024 * 1024 * 10;

export const kind: FileKindBase = {
Expand Down
4 changes: 2 additions & 2 deletions src/platform/plugins/shared/files/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ interface HttpEndpointDefinition {
*
* @example
* // This will enable access control to this endpoint for users that can access "myApp" only.
* { tags: ['access:myApp'] }
* { requiredPrivileges: ['myApp'] }
*
*/
tags: string[];
requiredPrivileges: string[];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/platform/plugins/shared/files/docs/tutorial.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import { FileKind } from '@kbn/files-plugin/common';
export const PLUGIN_ID = 'filesExample';

const httpTags = {
tags: [`access:${PLUGIN_ID}`], // ensure that only users with access to this plugin can files of this kind
requiredPrivileges: [PLUGIN_ID], // ensure that only users with the specified privilege can perform operations on files of this kind
};

export const exampleFileKind: FileKind = {
Expand Down
14 changes: 7 additions & 7 deletions src/platform/plugins/shared/files/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,13 @@ export class FilesPlugin
...DefaultImageKind.kind,
maxSizeBytes: DefaultImageKind.maxSize,
http: {
create: { tags: DefaultImageKind.tags },
delete: { tags: DefaultImageKind.tags },
download: { tags: DefaultImageKind.tags },
getById: { tags: DefaultImageKind.tags },
list: { tags: DefaultImageKind.tags },
share: { tags: DefaultImageKind.tags },
update: { tags: DefaultImageKind.tags },
create: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
delete: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
download: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
getById: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
list: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
share: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
update: { requiredPrivileges: DefaultImageKind.requiredPrivileges },
},
hashes: ['sha256'],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ export function register(router: FilesRouter) {
{
path: FILES_API_ROUTES.bulkDelete,
validate: { ...rt },
options: {
tags: [`access:${FILES_MANAGE_PRIVILEGE}`],
security: {
authz: {
requiredPrivileges: [FILES_MANAGE_PRIVILEGE],
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
validate: {
...rt,
},
options: {
tags: fileKind.http.create.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.create.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getDeleteRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.delete.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.delete.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getDownloadRoute(fileKind.id),
validate: { ...rt },
security: {
authz: {
requiredPrivileges: fileKind.http.download.requiredPrivileges,
},
},
options: {
tags: fileKind.http.download.tags,
access: 'public', // the endpoint is used by <img src=""/> and should work without any special headers
access: 'public', // The endpoint is used by <img src=""/> and should work without any special headers,
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getByIdRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.getById.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.getById.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getListRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.list.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.list.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getGetShareRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.share.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.share.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getListShareRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.share.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.share.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getShareRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.share.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.share.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getUnshareRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.share.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.share.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
{
path: FILES_API_ROUTES.fileKind.getUpdateRoute(fileKind.id),
validate: { ...rt },
options: {
tags: fileKind.http.update.tags,
security: {
authz: {
requiredPrivileges: fileKind.http.update.requiredPrivileges,
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
...rt,
},
options: {
tags: fileKind.http.create.tags,
body: {
output: 'stream',
parse: false,
Expand All @@ -111,6 +110,11 @@ export function register(fileKindRouter: FileKindRouter, fileKind: FileKind) {
maxBytes: 10 * 1024 * 1024 * 1024,
},
},
security: {
authz: {
requiredPrivileges: fileKind.http.create.requiredPrivileges,
},
},
},
handler
);
Expand Down
6 changes: 4 additions & 2 deletions src/platform/plugins/shared/files/server/routes/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ export function register(router: FilesRouter) {
{
path: FILES_API_ROUTES.find,
validate: { ...rt },
options: {
tags: [`access:${FILES_MANAGE_PRIVILEGE}`],
security: {
authz: {
requiredPrivileges: [FILES_MANAGE_PRIVILEGE],
},
},
},
handler
Expand Down
6 changes: 4 additions & 2 deletions src/platform/plugins/shared/files/server/routes/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ export function register(router: FilesRouter) {
{
path: FILES_API_ROUTES.metrics,
validate: {},
options: {
tags: [`access:${FILES_MANAGE_PRIVILEGE}`],
security: {
authz: {
requiredPrivileges: [FILES_MANAGE_PRIVILEGE],
},
},
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export async function setupIntegrationEnvironment() {
/**
* Register a test file type
*/
const testHttpConfig = { tags: ['access:myapp'] };
const testHttpConfig = { requiredPrivileges: ['myapp'] };
const myFileKind = {
id: fileKind,
blobStoreSettings: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import type { OWNERS } from './owners';

export enum HttpApiTagOperation {
export enum HttpApiPrivilegeOperation {
Read = 'Read',
Create = 'Create',
Delete = 'Delete',
Expand Down
14 changes: 7 additions & 7 deletions x-pack/platform/plugins/shared/cases/common/files/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
import {
CaseFileMetadataForDeletionRt,
constructFileKindIdByOwner,
constructFilesHttpOperationTag,
constructFilesHttpOperationPrivilege,
constructOwnerFromFileKind,
} from '.';
import { APP_ID, OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER } from '../constants';
import { HttpApiTagOperation } from '../constants/types';
import { HttpApiPrivilegeOperation } from '../constants/types';

describe('files index', () => {
describe('constructFilesHttpOperationTag', () => {
describe('constructFilesHttpOperationPrivilege', () => {
it.each([
[SECURITY_SOLUTION_OWNER, HttpApiTagOperation.Read, 'securitySolutionFilesCasesRead'],
[OBSERVABILITY_OWNER, HttpApiTagOperation.Create, 'observabilityFilesCasesCreate'],
[APP_ID, HttpApiTagOperation.Delete, 'casesFilesCasesDelete'],
[SECURITY_SOLUTION_OWNER, HttpApiPrivilegeOperation.Read, 'securitySolutionFilesCasesRead'],
[OBSERVABILITY_OWNER, HttpApiPrivilegeOperation.Create, 'observabilityFilesCasesCreate'],
[APP_ID, HttpApiPrivilegeOperation.Delete, 'casesFilesCasesDelete'],
])('builds the tag for owner: %p operation: %p tag: %p', (owner, operation, tag) => {
expect(constructFilesHttpOperationTag(owner, operation)).toEqual(tag);
expect(constructFilesHttpOperationPrivilege(owner, operation)).toEqual(tag);
});
});

Expand Down
7 changes: 5 additions & 2 deletions x-pack/platform/plugins/shared/cases/common/files/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import * as rt from 'io-ts';
import { isEmpty } from 'lodash';
import { OWNERS } from '../constants';
import type { HttpApiTagOperation, Owner } from '../constants/types';
import type { HttpApiPrivilegeOperation, Owner } from '../constants/types';

/**
* This type is only used to validate for deletion, it does not check all the fields that should exist in the file
Expand All @@ -22,7 +22,10 @@ export type CaseFileMetadataForDeletion = rt.TypeOf<typeof CaseFileMetadataForDe

const FILE_KIND_DELIMITER = 'FilesCases';

export const constructFilesHttpOperationTag = (owner: Owner, operation: HttpApiTagOperation) => {
export const constructFilesHttpOperationPrivilege = (
owner: Owner,
operation: HttpApiPrivilegeOperation
) => {
return `${owner}${FILE_KIND_DELIMITER}${operation}`;
};

Expand Down
Loading

0 comments on commit e6e4eda

Please sign in to comment.