From e44e60d50eba2bec78317c73cc36011e1825a8cd Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Mon, 9 Oct 2023 12:12:20 +0100 Subject: [PATCH 01/11] [Profiling] Remove Security checks --- .../e2e/cypress/e2e/empty_state/home.cy.ts | 28 +++++++++++++++++ x-pack/plugins/profiling/kibana.jsonc | 3 +- .../public/components/check_setup.tsx | 3 +- x-pack/plugins/profiling/public/services.ts | 1 + .../server/lib/setup/is_super_user.ts | 20 +++++++++++++ .../server/lib/setup/security_role.ts | 29 ------------------ .../plugins/profiling/server/routes/setup.ts | 30 +++++++++++++++++-- x-pack/plugins/profiling/server/types.ts | 3 ++ .../profiling_data_access/common/index.ts | 1 - .../common/security_role.ts | 24 --------------- .../common/setup.test.ts | 22 -------------- .../profiling_data_access/common/setup.ts | 7 ----- .../server/services/get_setup_state/index.ts | 2 -- 13 files changed, 83 insertions(+), 90 deletions(-) create mode 100644 x-pack/plugins/profiling/server/lib/setup/is_super_user.ts delete mode 100644 x-pack/plugins/profiling/server/lib/setup/security_role.ts delete mode 100644 x-pack/plugins/profiling_data_access/common/security_role.ts diff --git a/x-pack/plugins/profiling/e2e/cypress/e2e/empty_state/home.cy.ts b/x-pack/plugins/profiling/e2e/cypress/e2e/empty_state/home.cy.ts index bba7a3c014c41..1a83949917690 100644 --- a/x-pack/plugins/profiling/e2e/cypress/e2e/empty_state/home.cy.ts +++ b/x-pack/plugins/profiling/e2e/cypress/e2e/empty_state/home.cy.ts @@ -83,4 +83,32 @@ describe('Home page with empty state', () => { cy.contains('Delete existing profiling data'); }); }); + + it('shows disabled button for users without privileges', () => { + cy.intercept('GET', '/internal/profiling/setup/es_resources', { + body: { + has_setup: false, + has_data: false, + pre_8_9_1_data: false, + has_required_role: false, + }, + }).as('getEsResources'); + cy.visitKibana('/app/profiling'); + cy.wait('@getEsResources'); + cy.contains('Set up Universal Profiling').should('be.disabled'); + }); + + it('shows emabled button for users without privileges', () => { + cy.intercept('GET', '/internal/profiling/setup/es_resources', { + body: { + has_setup: false, + has_data: false, + pre_8_9_1_data: false, + has_required_role: true, + }, + }).as('getEsResources'); + cy.visitKibana('/app/profiling'); + cy.wait('@getEsResources'); + cy.contains('Set up Universal Profiling').should('not.be.disabled'); + }); }); diff --git a/x-pack/plugins/profiling/kibana.jsonc b/x-pack/plugins/profiling/kibana.jsonc index 1eae495f8c85e..51e1ac29d6ade 100644 --- a/x-pack/plugins/profiling/kibana.jsonc +++ b/x-pack/plugins/profiling/kibana.jsonc @@ -9,7 +9,8 @@ "configPath": ["xpack", "profiling"], "optionalPlugins": [ "spaces", - "usageCollection" + "usageCollection", + "security" ], "requiredPlugins": [ "charts", diff --git a/x-pack/plugins/profiling/public/components/check_setup.tsx b/x-pack/plugins/profiling/public/components/check_setup.tsx index 3bfb920f25eb1..7c29832cbe99d 100644 --- a/x-pack/plugins/profiling/public/components/check_setup.tsx +++ b/x-pack/plugins/profiling/public/components/check_setup.tsx @@ -91,6 +91,7 @@ export function CheckSetup({ children }: { children: React.ReactElement }) { !!error; if (displaySetupScreen) { + const isButtonDisabled = postSetupLoading || data?.has_required_role === false; return ( { event.preventDefault(); diff --git a/x-pack/plugins/profiling/public/services.ts b/x-pack/plugins/profiling/public/services.ts index 785179b5bc65c..02993af72bd31 100644 --- a/x-pack/plugins/profiling/public/services.ts +++ b/x-pack/plugins/profiling/public/services.ts @@ -26,6 +26,7 @@ export interface ProfilingSetupStatus { has_setup: boolean; has_data: boolean; pre_8_9_1_data: boolean; + has_required_role: boolean; unauthorized?: boolean; } diff --git a/x-pack/plugins/profiling/server/lib/setup/is_super_user.ts b/x-pack/plugins/profiling/server/lib/setup/is_super_user.ts new file mode 100644 index 0000000000000..7cdbab1b6a4d8 --- /dev/null +++ b/x-pack/plugins/profiling/server/lib/setup/is_super_user.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { KibanaRequest } from '@kbn/core/server'; +import { ProfilingPluginStartDeps } from '../../types'; + +export function isSuperuser({ + securityPluginStart, + request, +}: { + securityPluginStart: NonNullable; + request: KibanaRequest; +}) { + const user = securityPluginStart.authc.getCurrentUser(request); + return user?.roles.includes('superuser'); +} diff --git a/x-pack/plugins/profiling/server/lib/setup/security_role.ts b/x-pack/plugins/profiling/server/lib/setup/security_role.ts deleted file mode 100644 index b578c2ef2cff6..0000000000000 --- a/x-pack/plugins/profiling/server/lib/setup/security_role.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { - METADATA_VERSION, - PROFILING_READER_ROLE_NAME, -} from '@kbn/profiling-data-access-plugin/common'; -import { ProfilingSetupOptions } from './types'; - -export async function setSecurityRole({ client }: ProfilingSetupOptions) { - const esClient = client.getEsClient(); - await esClient.security.putRole({ - name: PROFILING_READER_ROLE_NAME, - indices: [ - { - names: ['profiling-*', '.profiling-*'], - privileges: ['read', 'view_index_metadata'], - }, - ], - cluster: ['monitor'], - metadata: { - version: METADATA_VERSION, - }, - }); -} diff --git a/x-pack/plugins/profiling/server/routes/setup.ts b/x-pack/plugins/profiling/server/routes/setup.ts index 45a5f9d691307..bb5c4faf96e7c 100644 --- a/x-pack/plugins/profiling/server/routes/setup.ts +++ b/x-pack/plugins/profiling/server/routes/setup.ts @@ -15,9 +15,9 @@ import { removeProfilingFromApmPackagePolicy, } from '../lib/setup/fleet_policies'; import { getSetupInstructions } from '../lib/setup/get_setup_instructions'; -import { setSecurityRole } from '../lib/setup/security_role'; import { handleRouteHandlerError } from '../utils/handle_route_error_handler'; import { getClient } from './compat'; +import { isSuperuser } from '../lib/setup/is_super_user'; export function registerSetupRoute({ router, @@ -35,6 +35,13 @@ export function registerSetupRoute({ }, async (context, request, response) => { try { + const hasRequiredRole = dependencies.start.security + ? isSuperuser({ + securityPluginStart: dependencies.start.security, + request, + }) + : true; + const esClient = await getClient(context); const core = await context.core; @@ -44,7 +51,7 @@ export function registerSetupRoute({ spaceId: dependencies.setup.spaces?.spacesService?.getSpaceId(request), }); - return response.ok({ body: profilingStatus }); + return response.ok({ body: { ...profilingStatus, has_required_role: hasRequiredRole } }); } catch (error) { return handleRouteHandlerError({ error, @@ -77,6 +84,24 @@ export function registerSetupRoute({ }); } + // const hasRequiredRole = dependencies.start.security + // ? isSuperuser({ + // securityPluginStart: dependencies.start.security, + // request, + // }) + // : true; + + // if (hasRequiredRole === false) { + // const msg = `Operation only permitted by users with the superuser role`; + // logger.error(msg); + // return response.custom({ + // statusCode: 403, + // body: { + // message: msg, + // }, + // }); + // } + const esClient = await getClient(context); const core = await context.core; const clientWithDefaultAuth = createProfilingEsClient({ @@ -107,7 +132,6 @@ export function registerSetupRoute({ const executeAdminFunctions = [ ...(setupState.resource_management.enabled ? [] : [enableResourceManagement]), - ...(setupState.permissions.configured ? [] : [setSecurityRole]), ...(setupState.settings.configured ? [] : [setMaximumBuckets]), ]; diff --git a/x-pack/plugins/profiling/server/types.ts b/x-pack/plugins/profiling/server/types.ts index 6ee94e238effa..57e9303daf2d0 100644 --- a/x-pack/plugins/profiling/server/types.ts +++ b/x-pack/plugins/profiling/server/types.ts @@ -16,6 +16,7 @@ import { ProfilingDataAccessPluginSetup, ProfilingDataAccessPluginStart, } from '@kbn/profiling-data-access-plugin/server'; +import { SecurityPluginSetup, SecurityPluginStart } from '@kbn/security-plugin/server'; export interface ProfilingPluginSetupDeps { observability: ObservabilityPluginSetup; @@ -25,6 +26,7 @@ export interface ProfilingPluginSetupDeps { spaces?: SpacesPluginSetup; usageCollection?: UsageCollectionSetup; profilingDataAccess: ProfilingDataAccessPluginSetup; + security?: SecurityPluginSetup; } export interface ProfilingPluginStartDeps { @@ -34,6 +36,7 @@ export interface ProfilingPluginStartDeps { fleet: FleetStartContract; spaces?: SpacesPluginStart; profilingDataAccess: ProfilingDataAccessPluginStart; + security?: SecurityPluginStart; } // eslint-disable-next-line @typescript-eslint/no-empty-interface diff --git a/x-pack/plugins/profiling_data_access/common/index.ts b/x-pack/plugins/profiling_data_access/common/index.ts index 8e654f7a85144..e15a61f189e99 100644 --- a/x-pack/plugins/profiling_data_access/common/index.ts +++ b/x-pack/plugins/profiling_data_access/common/index.ts @@ -7,7 +7,6 @@ export { getApmPolicy, ELASTIC_CLOUD_APM_POLICY } from './get_apm_policy'; export { MAX_BUCKETS } from './cluster_settings'; -export { METADATA_VERSION, PROFILING_READER_ROLE_NAME } from './security_role'; export { getCollectorPolicy, getSymbolizerPolicy, diff --git a/x-pack/plugins/profiling_data_access/common/security_role.ts b/x-pack/plugins/profiling_data_access/common/security_role.ts deleted file mode 100644 index ed6cf1dbd4e62..0000000000000 --- a/x-pack/plugins/profiling_data_access/common/security_role.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { PartialSetupState, ProfilingSetupOptions } from './setup'; - -export const PROFILING_READER_ROLE_NAME = 'profiling-reader'; -export const METADATA_VERSION = 1; - -export async function validateSecurityRole({ - client, -}: ProfilingSetupOptions): Promise { - const esClient = client.getEsClient(); - const roles = await esClient.security.getRole(); - const profilingRole = roles[PROFILING_READER_ROLE_NAME]; - return { - permissions: { - configured: !!profilingRole && profilingRole.metadata.version === METADATA_VERSION, - }, - }; -} diff --git a/x-pack/plugins/profiling_data_access/common/setup.test.ts b/x-pack/plugins/profiling_data_access/common/setup.test.ts index 48b8136e39020..327f63aa2d34a 100644 --- a/x-pack/plugins/profiling_data_access/common/setup.test.ts +++ b/x-pack/plugins/profiling_data_access/common/setup.test.ts @@ -14,9 +14,6 @@ import { const createCloudState = (available: boolean): PartialSetupState => ({ cloud: { available } }); const createDataState = (available: boolean): PartialSetupState => ({ data: { available } }); -const createPermissionState = (configured: boolean): PartialSetupState => ({ - permissions: { configured }, -}); const createCollectorPolicyState = (installed: boolean): PartialSetupState => ({ policies: { collector: { installed } }, }); @@ -75,18 +72,6 @@ describe('Merging partial state operations', () => { expect(mergedState.policies.collector.installed).toEqual(true); expect(mergedState.policies.symbolizer.installed).toEqual(true); }); - it('returns false when permission is not configured', () => { - const mergedState = mergePartialSetupStates(defaultSetupState, [ - createCollectorPolicyState(true), - createSymbolizerPolicyState(true), - createProfilingInApmPolicyState(true), - createResourceState({ enabled: true, created: true }), - createSettingsState(true), - createPermissionState(false), - ]); - - expect(areResourcesSetup(mergedState)).toBeFalsy(); - }); it('returns false when resource management is not enabled', () => { const mergedState = mergePartialSetupStates(defaultSetupState, [ @@ -95,7 +80,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: false, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); @@ -108,7 +92,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: false }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); @@ -121,7 +104,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: true }), createSettingsState(false), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); @@ -134,7 +116,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeTruthy(); @@ -147,7 +128,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); @@ -160,7 +140,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); @@ -173,7 +152,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areResourcesSetup(mergedState)).toBeFalsy(); diff --git a/x-pack/plugins/profiling_data_access/common/setup.ts b/x-pack/plugins/profiling_data_access/common/setup.ts index d3411ea9ee020..236197125c2c9 100644 --- a/x-pack/plugins/profiling_data_access/common/setup.ts +++ b/x-pack/plugins/profiling_data_access/common/setup.ts @@ -27,9 +27,6 @@ export interface SetupState { data: { available: boolean; }; - permissions: { - configured: boolean; - }; policies: { collector: { installed: boolean; @@ -64,9 +61,6 @@ export function createDefaultSetupState(): SetupState { data: { available: false, }, - permissions: { - configured: false, - }, policies: { collector: { installed: false, @@ -98,7 +92,6 @@ export function areResourcesSetup(state: SetupState): boolean { !state.policies.apm.profilingEnabled && state.resource_management.enabled && state.resources.created && - state.permissions.configured && state.settings.configured ); } diff --git a/x-pack/plugins/profiling_data_access/server/services/get_setup_state/index.ts b/x-pack/plugins/profiling_data_access/server/services/get_setup_state/index.ts index 9110883900c27..e26ed87a72932 100644 --- a/x-pack/plugins/profiling_data_access/server/services/get_setup_state/index.ts +++ b/x-pack/plugins/profiling_data_access/server/services/get_setup_state/index.ts @@ -16,7 +16,6 @@ import { } from '../../../common/fleet_policies'; import { hasProfilingData } from '../../../common/has_profiling_data'; import { ProfilingESClient } from '../../../common/profiling_es_client'; -import { validateSecurityRole } from '../../../common/security_role'; import { ProfilingSetupOptions, createDefaultSetupState, @@ -34,7 +33,6 @@ export async function getSetupState( const verifyFunctions = [ validateMaximumBuckets, validateResourceManagement, - validateSecurityRole, validateCollectorPackagePolicy, validateSymbolizerPackagePolicy, validateProfilingInApmPackagePolicy, From 108fb6eae2bbe16e9aa7055c7a88a236aa008c3a Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 9 Oct 2023 11:22:12 +0000 Subject: [PATCH 02/11] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/profiling/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/profiling/tsconfig.json b/x-pack/plugins/profiling/tsconfig.json index af7971b5115d5..7705c70d0d1b4 100644 --- a/x-pack/plugins/profiling/tsconfig.json +++ b/x-pack/plugins/profiling/tsconfig.json @@ -50,7 +50,8 @@ "@kbn/profiling-data-access-plugin", "@kbn/embeddable-plugin", "@kbn/profiling-utils", - "@kbn/advanced-settings-plugin" + "@kbn/advanced-settings-plugin", + "@kbn/security-plugin" // add references to other TypeScript projects the plugin depends on // requiredPlugins from ./kibana.json From 6ce8de986709375bb68692fb44a8013db0ddabb3 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Tue, 10 Oct 2023 11:03:17 +0100 Subject: [PATCH 03/11] using kibana internal user --- .../apm/server/routes/profiling/route.ts | 2 +- .../public/components/check_setup.tsx | 85 +++++++++++-------- .../plugins/profiling/server/routes/setup.ts | 2 +- .../server/services/status/index.ts | 17 ++-- 4 files changed, 59 insertions(+), 47 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/profiling/route.ts b/x-pack/plugins/apm/server/routes/profiling/route.ts index 9f008996bcf92..9d5853c288336 100644 --- a/x-pack/plugins/apm/server/routes/profiling/route.ts +++ b/x-pack/plugins/apm/server/routes/profiling/route.ts @@ -157,7 +157,7 @@ const profilingStatusRoute = createApmServerRoute({ if (profilingDataAccessStart) { try { const response = await profilingDataAccessStart?.services.getStatus({ - esClient: esClient.asCurrentUser, + esClient, soClient: (await context.core).savedObjects.client, spaceId: ( await plugins.spaces?.start() diff --git a/x-pack/plugins/profiling/public/components/check_setup.tsx b/x-pack/plugins/profiling/public/components/check_setup.tsx index 7c29832cbe99d..72d2985cb90e6 100644 --- a/x-pack/plugins/profiling/public/components/check_setup.tsx +++ b/x-pack/plugins/profiling/public/components/check_setup.tsx @@ -13,6 +13,7 @@ import { EuiLink, EuiLoadingSpinner, EuiText, + EuiToolTip, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; @@ -153,44 +154,54 @@ export function CheckSetup({ children }: { children: React.ReactElement }) { event.preventDefault(); }, button: ( - { - event.preventDefault(); - - setPostSetupLoading(true); - - postSetupResources({ http }) - .then(() => refresh()) - .catch((err) => { - const message = err?.body?.message ?? err.message ?? String(err); - - notifications.toasts.addError(err, { - title: i18n.translate( - 'xpack.profiling.checkSetup.setupFailureToastTitle', - { - defaultMessage: 'Failed to complete setup', - } - ), - toastMessage: message, - }); - }) - .finally(() => { - setPostSetupLoading(false); - }); - }} - fill - isLoading={postSetupLoading} + - {!postSetupLoading - ? i18n.translate('xpack.profiling.noDataConfig.action.buttonLabel', { - defaultMessage: 'Set up Universal Profiling', - }) - : i18n.translate('xpack.profiling.noDataConfig.action.buttonLoadingLabel', { - defaultMessage: 'Setting up Universal Profiling...', - })} - + { + event.preventDefault(); + + setPostSetupLoading(true); + + postSetupResources({ http }) + .then(() => refresh()) + .catch((err) => { + const message = err?.body?.message ?? err.message ?? String(err); + + notifications.toasts.addError(err, { + title: i18n.translate( + 'xpack.profiling.checkSetup.setupFailureToastTitle', + { + defaultMessage: 'Failed to complete setup', + } + ), + toastMessage: message, + }); + }) + .finally(() => { + setPostSetupLoading(false); + }); + }} + fill + isLoading={postSetupLoading} + > + {!postSetupLoading + ? i18n.translate('xpack.profiling.noDataConfig.action.buttonLabel', { + defaultMessage: 'Set up Universal Profiling', + }) + : i18n.translate('xpack.profiling.noDataConfig.action.buttonLoadingLabel', { + defaultMessage: 'Setting up Universal Profiling...', + })} + + ), }, }, diff --git a/x-pack/plugins/profiling/server/routes/setup.ts b/x-pack/plugins/profiling/server/routes/setup.ts index bb5c4faf96e7c..e5e9ad72c0862 100644 --- a/x-pack/plugins/profiling/server/routes/setup.ts +++ b/x-pack/plugins/profiling/server/routes/setup.ts @@ -42,7 +42,7 @@ export function registerSetupRoute({ }) : true; - const esClient = await getClient(context); + const esClient = (await context.core).elasticsearch.client; const core = await context.core; const profilingStatus = await dependencies.start.profilingDataAccess.services.getStatus({ diff --git a/x-pack/plugins/profiling_data_access/server/services/status/index.ts b/x-pack/plugins/profiling_data_access/server/services/status/index.ts index 31581140b12e0..c42f53a1cf86c 100644 --- a/x-pack/plugins/profiling_data_access/server/services/status/index.ts +++ b/x-pack/plugins/profiling_data_access/server/services/status/index.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server'; +import { IScopedClusterClient, SavedObjectsClientContract } from '@kbn/core/server'; import { ProfilingStatus } from '@kbn/profiling-utils'; import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; import { getSetupState } from '../get_setup_state'; @@ -14,7 +14,7 @@ import { ProfilingSetupOptions, areResourcesSetup } from '../../../common/setup' export interface HasSetupParams { soClient: SavedObjectsClientContract; - esClient: ElasticsearchClient; + esClient: IScopedClusterClient; spaceId?: string; } @@ -35,17 +35,18 @@ export function createGetStatusService({ }; } - const clientWithDefaultAuth = createProfilingEsClient({ - esClient, + const kibanaInternalProfilingESClient = createProfilingEsClient({ + esClient: esClient.asInternalUser, useDefaultAuth: true, }); - const clientWithProfilingAuth = createProfilingEsClient({ - esClient, + + const profilingESClient = createProfilingEsClient({ + esClient: esClient.asCurrentUser, useDefaultAuth: false, }); const setupOptions: ProfilingSetupOptions = { - client: clientWithDefaultAuth, + client: kibanaInternalProfilingESClient, logger, packagePolicyClient: deps.fleet.packagePolicyService, soClient, @@ -53,7 +54,7 @@ export function createGetStatusService({ isCloudEnabled, }; - const setupState = await getSetupState(setupOptions, clientWithProfilingAuth); + const setupState = await getSetupState(setupOptions, profilingESClient); return { has_setup: areResourcesSetup(setupState), From 09fd0ff9450f5a6bd5a00164e5ef8e3636486d19 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Tue, 10 Oct 2023 13:26:45 +0100 Subject: [PATCH 04/11] removing comments --- .../plugins/profiling/server/routes/setup.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/x-pack/plugins/profiling/server/routes/setup.ts b/x-pack/plugins/profiling/server/routes/setup.ts index e5e9ad72c0862..548a7cccac74a 100644 --- a/x-pack/plugins/profiling/server/routes/setup.ts +++ b/x-pack/plugins/profiling/server/routes/setup.ts @@ -84,24 +84,6 @@ export function registerSetupRoute({ }); } - // const hasRequiredRole = dependencies.start.security - // ? isSuperuser({ - // securityPluginStart: dependencies.start.security, - // request, - // }) - // : true; - - // if (hasRequiredRole === false) { - // const msg = `Operation only permitted by users with the superuser role`; - // logger.error(msg); - // return response.custom({ - // statusCode: 403, - // body: { - // message: msg, - // }, - // }); - // } - const esClient = await getClient(context); const core = await context.core; const clientWithDefaultAuth = createProfilingEsClient({ From 3ef30e17671d57dfb6e9f3500eeca75421d596f0 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Mon, 16 Oct 2023 09:42:25 +0100 Subject: [PATCH 05/11] removing isSuperUser to test --- x-pack/plugins/profiling/server/routes/setup.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/profiling/server/routes/setup.ts b/x-pack/plugins/profiling/server/routes/setup.ts index 548a7cccac74a..96736e954a5e6 100644 --- a/x-pack/plugins/profiling/server/routes/setup.ts +++ b/x-pack/plugins/profiling/server/routes/setup.ts @@ -17,7 +17,6 @@ import { import { getSetupInstructions } from '../lib/setup/get_setup_instructions'; import { handleRouteHandlerError } from '../utils/handle_route_error_handler'; import { getClient } from './compat'; -import { isSuperuser } from '../lib/setup/is_super_user'; export function registerSetupRoute({ router, @@ -35,12 +34,13 @@ export function registerSetupRoute({ }, async (context, request, response) => { try { - const hasRequiredRole = dependencies.start.security - ? isSuperuser({ - securityPluginStart: dependencies.start.security, - request, - }) - : true; + const hasRequiredRole = true; + // dependencies.start.security + // ? isSuperuser({ + // securityPluginStart: dependencies.start.security, + // request, + // }) + // : true; const esClient = (await context.core).elasticsearch.client; const core = await context.core; From 8d44e8953bddb4a0c2467f226aeebd596b23c7c5 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Thu, 19 Oct 2023 13:44:23 +0100 Subject: [PATCH 06/11] changing privileges check --- ..._super_user.ts => get_has_setup_privileges.ts} | 15 ++++++++++++--- x-pack/plugins/profiling/server/routes/setup.ts | 14 +++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) rename x-pack/plugins/profiling/server/lib/setup/{is_super_user.ts => get_has_setup_privileges.ts} (60%) diff --git a/x-pack/plugins/profiling/server/lib/setup/is_super_user.ts b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts similarity index 60% rename from x-pack/plugins/profiling/server/lib/setup/is_super_user.ts rename to x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts index 7cdbab1b6a4d8..db38056d4537d 100644 --- a/x-pack/plugins/profiling/server/lib/setup/is_super_user.ts +++ b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts @@ -8,13 +8,22 @@ import { KibanaRequest } from '@kbn/core/server'; import { ProfilingPluginStartDeps } from '../../types'; -export function isSuperuser({ +export async function getHasSetupPrivileges({ securityPluginStart, request, }: { securityPluginStart: NonNullable; request: KibanaRequest; }) { - const user = securityPluginStart.authc.getCurrentUser(request); - return user?.roles.includes('superuser'); + const { hasAllRequested } = await securityPluginStart.authz + .checkPrivilegesWithRequest(request) + .globally({ + elasticsearch: { + cluster: ['manage', 'monitor'], + index: { + 'profiling-*': ['read'], + }, + }, + }); + return hasAllRequested; } diff --git a/x-pack/plugins/profiling/server/routes/setup.ts b/x-pack/plugins/profiling/server/routes/setup.ts index 96736e954a5e6..1c0343e5b18ef 100644 --- a/x-pack/plugins/profiling/server/routes/setup.ts +++ b/x-pack/plugins/profiling/server/routes/setup.ts @@ -14,6 +14,7 @@ import { createSymbolizerPackagePolicy, removeProfilingFromApmPackagePolicy, } from '../lib/setup/fleet_policies'; +import { getHasSetupPrivileges } from '../lib/setup/get_has_setup_privileges'; import { getSetupInstructions } from '../lib/setup/get_setup_instructions'; import { handleRouteHandlerError } from '../utils/handle_route_error_handler'; import { getClient } from './compat'; @@ -34,13 +35,12 @@ export function registerSetupRoute({ }, async (context, request, response) => { try { - const hasRequiredRole = true; - // dependencies.start.security - // ? isSuperuser({ - // securityPluginStart: dependencies.start.security, - // request, - // }) - // : true; + const hasRequiredRole = dependencies.start.security + ? await getHasSetupPrivileges({ + securityPluginStart: dependencies.start.security, + request, + }) + : true; const esClient = (await context.core).elasticsearch.client; const core = await context.core; From cb171ce01938680aad064682bfc3fe0d071a5d69 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Thu, 19 Oct 2023 14:42:55 +0100 Subject: [PATCH 07/11] adding kibana priv --- .../profiling/server/lib/setup/get_has_setup_privileges.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts index db38056d4537d..c496fc2137c4a 100644 --- a/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts +++ b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts @@ -6,6 +6,7 @@ */ import { KibanaRequest } from '@kbn/core/server'; +import { INTEGRATIONS_PLUGIN_ID, PLUGIN_ID as FLEET_PLUGIN_ID } from '@kbn/fleet-plugin/common'; import { ProfilingPluginStartDeps } from '../../types'; export async function getHasSetupPrivileges({ @@ -24,6 +25,10 @@ export async function getHasSetupPrivileges({ 'profiling-*': ['read'], }, }, + kibana: [ + securityPluginStart.authz.actions.api.get(`${FLEET_PLUGIN_ID}-all`), + securityPluginStart.authz.actions.api.get(`${INTEGRATIONS_PLUGIN_ID}-all`), + ], }); return hasAllRequested; } From 0984ebe3992605dd5065b35371d5568b652fe2ba Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Tue, 24 Oct 2023 14:33:33 +0100 Subject: [PATCH 08/11] addressing pr comments --- .../profiling/server/lib/setup/get_has_setup_privileges.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts index c496fc2137c4a..83bd21b1740b8 100644 --- a/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts +++ b/x-pack/plugins/profiling/server/lib/setup/get_has_setup_privileges.ts @@ -16,6 +16,11 @@ export async function getHasSetupPrivileges({ securityPluginStart: NonNullable; request: KibanaRequest; }) { + // If we have a license which doesn't enable security, or we're a legacy user we shouldn't disable any ui capabilities + if (!securityPluginStart.authz.mode.useRbacForRequest(request)) { + return true; + } + const { hasAllRequested } = await securityPluginStart.authz .checkPrivilegesWithRequest(request) .globally({ From 61ed31048bae86fbc7523c31f0ddf0f531c0a7c2 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Fri, 27 Oct 2023 11:43:54 +0100 Subject: [PATCH 09/11] removing sec --- .../services/setup_state/self_managed_setup_state.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/x-pack/plugins/profiling_data_access/server/services/setup_state/self_managed_setup_state.ts b/x-pack/plugins/profiling_data_access/server/services/setup_state/self_managed_setup_state.ts index 062a75f0f1f02..ac7ff7ae7459e 100644 --- a/x-pack/plugins/profiling_data_access/server/services/setup_state/self_managed_setup_state.ts +++ b/x-pack/plugins/profiling_data_access/server/services/setup_state/self_managed_setup_state.ts @@ -10,7 +10,6 @@ import { validateResourceManagement, } from '../../../common/cluster_settings'; import { hasProfilingData } from '../../../common/has_profiling_data'; -import { validateSecurityRole } from '../../../common/security_role'; import { createDefaultSetupState, mergePartialSetupStates, @@ -21,12 +20,7 @@ import { export async function selfManagedSetupState(params: ProfilingSetupOptions): Promise { const state = createDefaultSetupState(); - const verifyFunctions = [ - validateMaximumBuckets, - validateResourceManagement, - validateSecurityRole, - hasProfilingData, - ]; + const verifyFunctions = [validateMaximumBuckets, validateResourceManagement, hasProfilingData]; const partialStates = await Promise.all(verifyFunctions.map((fn) => fn(params))); From c34d40f29757f311ebf57176e2105e341d58aec9 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Fri, 27 Oct 2023 12:57:17 +0100 Subject: [PATCH 10/11] fix ci --- .../common/cloud_setup.test.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/x-pack/plugins/profiling_data_access/common/cloud_setup.test.ts b/x-pack/plugins/profiling_data_access/common/cloud_setup.test.ts index 1d99c6346c4c6..3071177cab26e 100644 --- a/x-pack/plugins/profiling_data_access/common/cloud_setup.test.ts +++ b/x-pack/plugins/profiling_data_access/common/cloud_setup.test.ts @@ -14,9 +14,6 @@ import { mergePartialSetupStates } from './setup'; const createCloudState = (available: boolean): PartialCloudSetupState => ({ cloud: { available } }); const createDataState = (available: boolean): PartialCloudSetupState => ({ data: { available } }); -const createPermissionState = (configured: boolean): PartialCloudSetupState => ({ - permissions: { configured }, -}); const createCollectorPolicyState = (installed: boolean): PartialCloudSetupState => ({ policies: { collector: { installed } }, }); @@ -75,18 +72,6 @@ describe('Merging partial state operations', () => { expect(mergedState.policies.collector.installed).toEqual(true); expect(mergedState.policies.symbolizer.installed).toEqual(true); }); - it('returns false when permission is not configured', () => { - const mergedState = mergePartialSetupStates(defaultSetupState, [ - createCollectorPolicyState(true), - createSymbolizerPolicyState(true), - createProfilingInApmPolicyState(true), - createResourceState({ enabled: true, created: true }), - createSettingsState(true), - createPermissionState(false), - ]); - - expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); - }); it('returns false when resource management is not enabled', () => { const mergedState = mergePartialSetupStates(defaultSetupState, [ @@ -95,7 +80,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: false, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); @@ -108,7 +92,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: false }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); @@ -121,7 +104,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: true }), createSettingsState(false), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); @@ -134,7 +116,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeTruthy(); @@ -147,7 +128,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); @@ -160,7 +140,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(false), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); @@ -173,7 +152,6 @@ describe('Merging partial state operations', () => { createProfilingInApmPolicyState(true), createResourceState({ enabled: true, created: true }), createSettingsState(true), - createPermissionState(true), ]); expect(areCloudResourcesSetup(mergedState)).toBeFalsy(); From 6918332b3a4f7db4b3a48de340a1d5260aae9b94 Mon Sep 17 00:00:00 2001 From: Caue Marcondes Date: Fri, 27 Oct 2023 13:23:47 +0100 Subject: [PATCH 11/11] fixing viewer --- .../profiling_data_access/server/services/setup_state/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/profiling_data_access/server/services/setup_state/index.ts b/x-pack/plugins/profiling_data_access/server/services/setup_state/index.ts index 91b2e2b5937be..d11668e1af6e9 100644 --- a/x-pack/plugins/profiling_data_access/server/services/setup_state/index.ts +++ b/x-pack/plugins/profiling_data_access/server/services/setup_state/index.ts @@ -58,7 +58,7 @@ export async function getSetupState({ } const setupState = await selfManagedSetupState({ - client: profilingESClient, + client: kibanaInternalProfilingESClient, clientWithProfilingAuth: profilingESClient, logger, soClient,