Skip to content

Commit

Permalink
[Profiling] Remove Security checks (elastic#168341)
Browse files Browse the repository at this point in the history
- Use Kibana internal user to fetch Profiling status.
- Check if current user has superadmin role to set up Profiling.
- Viewer users: disable set up button when Profiling is not yet
initialised.
- Remove Profiling role creation and check.

<img width="1187" alt="Screenshot 2023-10-06 at 10 50 44"
src="https://github.com/elastic/kibana/assets/55978943/468dd22e-e9db-4691-86b3-acb2428d4ad8">
<img width="1079" alt="Screenshot 2023-10-06 at 10 51 15"
src="https://github.com/elastic/kibana/assets/55978943/939ac662-5ccb-4210-a582-7ace77b6cf29">

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
cauemarcondes and kibanamachine authored Oct 27, 2023
1 parent dba5ab7 commit 1d54963
Show file tree
Hide file tree
Showing 21 changed files with 153 additions and 178 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/profiling/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
28 changes: 28 additions & 0 deletions x-pack/plugins/profiling/e2e/cypress/e2e/empty_state/home.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
1 change: 1 addition & 0 deletions x-pack/plugins/profiling/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"optionalPlugins": [
"spaces",
"usageCollection",
"security",
"cloud",
"fleet"
],
Expand Down
86 changes: 49 additions & 37 deletions x-pack/plugins/profiling/public/components/check_setup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EuiLink,
EuiLoadingSpinner,
EuiText,
EuiToolTip,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
Expand Down Expand Up @@ -91,6 +92,7 @@ export function CheckSetup({ children }: { children: React.ReactElement }) {
!!error;

if (displaySetupScreen) {
const isButtonDisabled = postSetupLoading || data?.has_required_role === false;
return (
<ProfilingAppPageTemplate
tabs={[]}
Expand Down Expand Up @@ -152,44 +154,54 @@ export function CheckSetup({ children }: { children: React.ReactElement }) {
event.preventDefault();
},
button: (
<EuiButton
data-test-subj="profilingCheckSetupButton"
disabled={postSetupLoading}
onClick={(event) => {
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}
<EuiToolTip
content={
data?.has_required_role === false
? i18n.translate('xpack.profiling.checkSetup.tooltip', {
defaultMessage: 'You must be logged in as a superuser',
})
: ''
}
>
{!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...',
})}
</EuiButton>
<EuiButton
data-test-subj="profilingCheckSetupButton"
disabled={isButtonDisabled}
onClick={(event) => {
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...',
})}
</EuiButton>
</EuiToolTip>
),
},
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/profiling/public/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 { INTEGRATIONS_PLUGIN_ID, PLUGIN_ID as FLEET_PLUGIN_ID } from '@kbn/fleet-plugin/common';
import { ProfilingPluginStartDeps } from '../../types';

export async function getHasSetupPrivileges({
securityPluginStart,
request,
}: {
securityPluginStart: NonNullable<ProfilingPluginStartDeps['security']>;
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({
elasticsearch: {
cluster: ['manage', 'monitor'],
index: {
'profiling-*': ['read'],
},
},
kibana: [
securityPluginStart.authz.actions.api.get(`${FLEET_PLUGIN_ID}-all`),
securityPluginStart.authz.actions.api.get(`${INTEGRATIONS_PLUGIN_ID}-all`),
],
});
return hasAllRequested;
}
29 changes: 0 additions & 29 deletions x-pack/plugins/profiling/server/lib/setup/security_role.ts

This file was deleted.

23 changes: 15 additions & 8 deletions x-pack/plugins/profiling/server/routes/setup/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@
* 2.0.
*/

import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common';
import { ProfilingSetupOptions } from '@kbn/profiling-data-access-plugin/common/setup';
import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common';
import { RouteRegisterParameters } from '..';
import { getRoutePaths } from '../../../common';
import { getCloudSetupInstructions } from './get_cloud_setup_instructions';
import { getHasSetupPrivileges } from '../../lib/setup/get_has_setup_privileges';
import { handleRouteHandlerError } from '../../utils/handle_route_error_handler';
import { getClient } from '../compat';
import { getCloudSetupInstructions } from './get_cloud_setup_instructions';
import { getSelfManagedInstructions } from './get_self_managed_instructions';
import { setupCloud } from './setup_cloud';
import { setupSelfManaged } from './setup_self_managed';
import { getSelfManagedInstructions } from './get_self_managed_instructions';

export function registerSetupRoute({
router,
Expand All @@ -23,7 +24,6 @@ export function registerSetupRoute({
dependencies,
}: RouteRegisterParameters) {
const paths = getRoutePaths();
// Check if Elasticsearch and Fleet are set up for Universal Profiling
router.get(
{
path: paths.HasSetupESResources,
Expand All @@ -32,16 +32,22 @@ export function registerSetupRoute({
},
async (context, request, response) => {
try {
const esClient = await getClient(context);
const hasRequiredRole = dependencies.start.security
? await getHasSetupPrivileges({
securityPluginStart: dependencies.start.security,
request,
})
: true;

const core = await context.core;

const profilingStatus = await dependencies.start.profilingDataAccess.services.getStatus({
esClient,
esClient: core.elasticsearch.client,
soClient: core.savedObjects.client,
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,
Expand Down Expand Up @@ -83,9 +89,10 @@ export function registerSetupRoute({
dependencies.setup.spaces?.spacesService?.getSpaceId(request) ?? DEFAULT_SPACE_ID,
};

const scopedESClient = (await context.core).elasticsearch.client;
const { type, setupState } =
await dependencies.start.profilingDataAccess.services.getSetupState({
esClient,
esClient: scopedESClient,
soClient: core.savedObjects.client,
spaceId:
dependencies.setup.spaces?.spacesService?.getSpaceId(request) ?? DEFAULT_SPACE_ID,
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/profiling/server/routes/setup/setup_cloud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
createSymbolizerPackagePolicy,
removeProfilingFromApmPackagePolicy,
} from '../../lib/setup/fleet_policies';
import { setSecurityRole } from '../../lib/setup/security_role';
import { ProfilingCloudSetupOptions } from '../../lib/setup/types';

export async function setupCloud({
Expand All @@ -24,7 +23,6 @@ export async function setupCloud({
}) {
const executeAdminFunctions = [
...(setupState.resource_management.enabled ? [] : [enableResourceManagement]),
...(setupState.permissions.configured ? [] : [setSecurityRole]),
...(setupState.settings.configured ? [] : [setMaximumBuckets]),
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import { ProfilingSetupOptions, SetupState } from '@kbn/profiling-data-access-plugin/common/setup';
import { enableResourceManagement, setMaximumBuckets } from '../../lib/setup/cluster_settings';
import { setSecurityRole } from '../../lib/setup/security_role';

export async function setupSelfManaged({
setupState,
Expand All @@ -18,7 +17,6 @@ export async function setupSelfManaged({
}) {
const executeFunctions = [
...(setupState.resource_management.enabled ? [] : [enableResourceManagement]),
...(setupState.permissions.configured ? [] : [setSecurityRole]),
...(setupState.settings.configured ? [] : [setMaximumBuckets]),
];

Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/profiling/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +26,7 @@ export interface ProfilingPluginSetupDeps {
spaces?: SpacesPluginSetup;
usageCollection?: UsageCollectionSetup;
profilingDataAccess: ProfilingDataAccessPluginSetup;
security?: SecurityPluginSetup;
}

export interface ProfilingPluginStartDeps {
Expand All @@ -34,6 +36,7 @@ export interface ProfilingPluginStartDeps {
fleet?: FleetStartContract;
spaces?: SpacesPluginStart;
profilingDataAccess: ProfilingDataAccessPluginStart;
security?: SecurityPluginStart;
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/profiling/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 1d54963

Please sign in to comment.