Skip to content

Commit

Permalink
[Security solution][Endpoint] Remove unnecessary experimental feature…
Browse files Browse the repository at this point in the history
… flags from plugin config (#158969)

## Summary

- Changes the behaviour of the validation of
`xpack.securitySolution.enableExperimental` values (defined in the
`kibana.yml`) so that unknown/unsupported values will no longer prevent
the application from starting (will no longer `throw`)
    - This change is only done in the config schema validation
- Plugin's `setup` will now output a message (warning) to the log if it
finds experimental feature values that are not supported.
- Removes the following experimental feature flags (no longer needed):
    - `policyListEnabled`
    - `diableIsolationUIPendingStatuses`
    - `responseActionsConsoleEnabled`
    - `endpointRbacEnabled`
    - `endpointRbacV1Enabled`
    - `responseActionGetFileEnabled`
    - `responseActionExecuteEnabled`
    - `pendingActionResponsesWithAck`
    - `policyResponseInFleetEnabled`
    - `riskyUsersEnabled`
    - `riskyHostsEnabled`

--------

Message to kibana log when unsupported values are defined:

```
[2023-06-02T16:37:48.997-04:00][WARN ][plugins.securitySolution.config] Unsupported "xpack.securitySolution.enableExperimental" values detected.
The following configuration values are no longer supported and should be removed from the kibana configuration file:

    xpack.securitySolution.enableExperimental:
      - endpointRbacEnabled
      - responseActionGetFileEnabled
      - responseActionExecuteEnabled

```
  • Loading branch information
paul-tavares authored Jun 8, 2023
1 parent 9d15681 commit 3bb4edf
Show file tree
Hide file tree
Showing 40 changed files with 395 additions and 784 deletions.
59 changes: 17 additions & 42 deletions x-pack/plugins/security_solution/common/experimental_features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ export type ExperimentalFeatures = { [K in keyof typeof allowedExperimentalValue
export const allowedExperimentalValues = Object.freeze({
tGridEnabled: true,
tGridEventRenderedViewEnabled: true,

// FIXME:PT delete?
excludePoliciesInFilterEnabled: false,

kubernetesEnabled: true,
disableIsolationUIPendingStatuses: false,
pendingActionResponsesWithAck: true,
policyListEnabled: true,
policyResponseInFleetEnabled: true,
chartEmbeddablesEnabled: true,
donutChartEmbeddablesEnabled: false, // Depends on https://github.com/elastic/kibana/issues/136409 item 2 - 6
alertsPreviewChartEmbeddablesEnabled: false, // Depends on https://github.com/elastic/kibana/issues/136409 item 9
Expand All @@ -33,11 +32,6 @@ export const allowedExperimentalValues = Object.freeze({
*/
previewTelemetryUrlEnabled: false,

/**
* Enables the Endpoint response actions console in various areas of the app
*/
responseActionsConsoleEnabled: true,

/**
* Enables the insights module for related alerts by process ancestry
*/
Expand Down Expand Up @@ -73,33 +67,13 @@ export const allowedExperimentalValues = Object.freeze({
*/
endpointResponseActionsEnabled: false,

/**
* Enables endpoint package level rbac
*/
endpointRbacEnabled: true,

/**
* Enables endpoint package level rbac for response actions only.
* if endpointRbacEnabled is enabled, it will take precedence.
*/
endpointRbacV1Enabled: true,
/**
* Enables the alert details page currently only accessible via the alert details flyout and alert table context menu
*/
alertDetailsPageEnabled: false,

/**
* Enables the `get-file` endpoint response action
*/
responseActionGetFileEnabled: true,

/**
* Enables the `execute` endpoint response action
*/
responseActionExecuteEnabled: true,

/**
* Enables the `upload` endpoint response action
* Enables the `upload` endpoint response action (v8.9)
*/
responseActionUploadEnabled: false,

Expand Down Expand Up @@ -153,7 +127,6 @@ export const allowedExperimentalValues = Object.freeze({
type ExperimentalConfigKeys = Array<keyof ExperimentalFeatures>;
type Mutable<T> = { -readonly [P in keyof T]: T[P] };

const SecuritySolutionInvalidExperimentalValue = class extends Error {};
const allowedKeys = Object.keys(allowedExperimentalValues) as Readonly<ExperimentalConfigKeys>;

/**
Expand All @@ -163,25 +136,27 @@ const allowedKeys = Object.keys(allowedExperimentalValues) as Readonly<Experimen
* @param configValue
* @throws SecuritySolutionInvalidExperimentalValue
*/
export const parseExperimentalConfigValue = (configValue: string[]): ExperimentalFeatures => {
export const parseExperimentalConfigValue = (
configValue: string[]
): { features: ExperimentalFeatures; invalid: string[] } => {
const enabledFeatures: Mutable<Partial<ExperimentalFeatures>> = {};
const invalidKeys: string[] = [];

for (const value of configValue) {
if (!isValidExperimentalValue(value)) {
throw new SecuritySolutionInvalidExperimentalValue(`[${value}] is not valid.`);
if (!allowedKeys.includes(value as keyof ExperimentalFeatures)) {
invalidKeys.push(value);
} else {
enabledFeatures[value as keyof ExperimentalFeatures] = true;
}

enabledFeatures[value as keyof ExperimentalFeatures] = true;
}

return {
...allowedExperimentalValues,
...enabledFeatures,
features: {
...allowedExperimentalValues,
...enabledFeatures,
},
invalid: invalidKeys,
};
};

export const isValidExperimentalValue = (value: string): value is keyof ExperimentalFeatures => {
return allowedKeys.includes(value as keyof ExperimentalFeatures);
};

export const getExperimentalAllowedValues = (): string[] => [...allowedKeys];
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { HOST_STATUS_TO_BADGE_COLOR } from '../../../../management/pages/endpoin
import { getEmptyValue } from '../../empty_value';
import type { ResponseActionsApiCommandNames } from '../../../../../common/endpoint/service/response_actions/constants';
import { RESPONSE_ACTION_API_COMMANDS_TO_CONSOLE_COMMAND_MAP } from '../../../../../common/endpoint/service/response_actions/constants';
import { useIsExperimentalFeatureEnabled } from '../../../hooks/use_experimental_features';
import { useGetEndpointPendingActionsSummary } from '../../../../management/hooks/response_actions/use_get_endpoint_pending_actions_summary';
import { useTestIdGenerator } from '../../../../management/hooks/use_test_id_generator';
import type { HostInfo, EndpointPendingActions } from '../../../../../common/endpoint/types';
Expand Down Expand Up @@ -187,9 +186,6 @@ interface EndpointHostResponseActionsStatusProps {
const EndpointHostResponseActionsStatus = memo<EndpointHostResponseActionsStatusProps>(
({ pendingActions, isIsolated, 'data-test-subj': dataTestSubj }) => {
const getTestId = useTestIdGenerator(dataTestSubj);
const isPendingStatusDisabled = useIsExperimentalFeatureEnabled(
'disableIsolationUIPendingStatuses'
);

interface PendingActionsState {
actionList: Array<{ label: string; count: number }>;
Expand Down Expand Up @@ -269,15 +265,6 @@ const EndpointHostResponseActionsStatus = memo<EndpointHostResponseActionsStatus
);
}, [dataTestSubj]);

if (isPendingStatusDisabled) {
// If nothing is pending and host is not currently isolated, then render nothing
if (!isIsolated) {
return null;
}

return isolatedBadge;
}

// If nothing is pending
if (totalPending === 0) {
// and host is either releasing and or currently released, then render nothing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ jest.mock('../../../hooks/use_license', () => {
},
};
});
jest.mock('../../../hooks/use_experimental_features', () => ({
useIsExperimentalFeatureEnabled: jest.fn((feature: string) => feature === 'endpointRbacEnabled'),
}));

const useKibanaMock = useKibana as jest.Mocked<typeof useKibana>;
const useHttpMock = _useHttp as jest.Mock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
getEndpointAuthzInitialState,
} from '../../../../../common/endpoint/service/authz';
import { useSecuritySolutionStartDependencies } from './security_solution_start_dependencies';
import { useIsExperimentalFeatureEnabled } from '../../../hooks/use_experimental_features';

/**
* Retrieve the endpoint privileges for the current user.
Expand All @@ -49,9 +48,6 @@ export const useEndpointPrivileges = (): Immutable<EndpointPrivileges> => {
const [userRolesCheckDone, setUserRolesCheckDone] = useState<boolean>(false);
const [userRoles, setUserRoles] = useState<MaybeImmutable<string[]>>([]);

const isEndpointRbacEnabled = useIsExperimentalFeatureEnabled('endpointRbacEnabled');
const isEndpointRbacV1Enabled = useIsExperimentalFeatureEnabled('endpointRbacV1Enabled');

const [checkHostIsolationExceptionsDone, setCheckHostIsolationExceptionsDone] =
useState<boolean>(false);
const [hasHostIsolationExceptionsItems, setHasHostIsolationExceptionsItems] =
Expand All @@ -67,7 +63,7 @@ export const useEndpointPrivileges = (): Immutable<EndpointPrivileges> => {
licenseService,
fleetAuthz,
userRoles,
isEndpointRbacEnabled || isEndpointRbacV1Enabled,
true,
hasHostIsolationExceptionsItems
)
: getEndpointAuthzInitialState()),
Expand All @@ -81,8 +77,6 @@ export const useEndpointPrivileges = (): Immutable<EndpointPrivileges> => {
fleetAuthz,
licenseService,
userRoles,
isEndpointRbacEnabled,
isEndpointRbacV1Enabled,
hasHostIsolationExceptionsItems,
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('createInitialState', () => {
>;
const defaultState = {
defaultDataView: mockSourcererState.defaultDataView,
enableExperimental: parseExperimentalConfigValue([]),
enableExperimental: parseExperimentalConfigValue([]).features,
kibanaDataViews: [mockSourcererState.defaultDataView],
signalIndexName: 'siem-signals-default',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@ import {
isTimelineEventItemAnAlert,
} from '../../../common/utils/endpoint_alert_check';
import { ResponderContextMenuItem } from './responder_context_menu_item';
import { useIsExperimentalFeatureEnabled } from '../../../common/hooks/use_experimental_features';
import { getFieldValue } from '../host_isolation/helpers';

export const useResponderActionItem = (
eventDetailsData: TimelineEventsDetailsItem[] | null,
onClick: () => void
): JSX.Element[] => {
const isResponseActionsConsoleEnabled = useIsExperimentalFeatureEnabled(
'responseActionsConsoleEnabled'
);
const { loading: isAuthzLoading, canAccessResponseConsole } =
useUserPrivileges().endpointPrivileges;

Expand All @@ -42,7 +38,7 @@ export const useResponderActionItem = (
return useMemo(() => {
const actions: JSX.Element[] = [];

if (isResponseActionsConsoleEnabled && !isAuthzLoading && canAccessResponseConsole && isAlert) {
if (!isAuthzLoading && canAccessResponseConsole && isAlert) {
actions.push(
<ResponderContextMenuItem
key="endpointResponseActions-action-item"
Expand All @@ -53,13 +49,5 @@ export const useResponderActionItem = (
}

return actions;
}, [
canAccessResponseConsole,
endpointId,
isAlert,
isAuthzLoading,
isEndpointAlert,
isResponseActionsConsoleEnabled,
onClick,
]);
}, [canAccessResponseConsole, endpointId, isAlert, isAuthzLoading, isEndpointAlert, onClick]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { useKibana, useGetUserCasesPermissions, useHttp } from '../../../common/
import { mockCasesContract } from '@kbn/cases-plugin/public/mocks';
import { initialUserPrivilegesState as mockInitialUserPrivilegesState } from '../../../common/components/user_privileges/user_privileges_context';
import { useUserPrivileges } from '../../../common/components/user_privileges';
import { useIsExperimentalFeatureEnabled } from '../../../common/hooks/use_experimental_features';
import {
NOT_FROM_ENDPOINT_HOST_TOOLTIP,
HOST_ENDPOINT_UNENROLLED_TOOLTIP,
Expand Down Expand Up @@ -454,27 +453,6 @@ describe('take action dropdown', () => {
apiMocks = endpointMetadataHttpMocks(mockStartServicesMock.http as jest.Mocked<HttpSetup>);
});

describe('when the `responseActionsConsoleEnabled` feature flag is false', () => {
beforeAll(() => {
(useIsExperimentalFeatureEnabled as jest.Mock).mockImplementation((featureKey) => {
if (featureKey === 'responseActionsConsoleEnabled') {
return false;
}
return true;
});
});

afterAll(() => {
(useIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => true);
});

it('should hide the button if feature flag if off', async () => {
render();

expect(findLaunchResponderButton()).toHaveLength(0);
});
});

it('should not display the button if user is not allowed to write event filters', async () => {
(useUserPrivileges as jest.Mock).mockReturnValue({
...mockInitialUserPrivilegesState(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ export const getEndpointConsoleCommands = ({
}): CommandDefinition[] => {
const featureFlags = ExperimentalFeaturesService.get();

const isGetFileEnabled = featureFlags.responseActionGetFileEnabled;
const isExecuteEnabled = featureFlags.responseActionExecuteEnabled;
const isUploadEnabled = featureFlags.responseActionUploadEnabled;

const doesEndpointSupportCommand = (commandName: ConsoleResponseActionCommands) => {
Expand Down Expand Up @@ -379,11 +377,7 @@ export const getEndpointConsoleCommands = ({
helpDisabled: doesEndpointSupportCommand('processes') === false,
helpHidden: !getRbacControl({ commandName: 'processes', privileges: endpointPrivileges }),
},
];

// `get-file` is currently behind feature flag
if (isGetFileEnabled) {
consoleCommands.push({
{
name: 'get-file',
about: getCommandAboutInfo({
aboutInfo: i18n.translate('xpack.securitySolution.endpointConsoleCommands.getFile.about', {
Expand Down Expand Up @@ -429,13 +423,8 @@ export const getEndpointConsoleCommands = ({
commandName: 'get-file',
privileges: endpointPrivileges,
}),
});
}

// `execute` is currently behind feature flag
// planned for 8.8
if (isExecuteEnabled) {
consoleCommands.push({
},
{
name: 'execute',
about: getCommandAboutInfo({
aboutInfo: i18n.translate('xpack.securitySolution.endpointConsoleCommands.execute.about', {
Expand Down Expand Up @@ -487,8 +476,8 @@ export const getEndpointConsoleCommands = ({
commandName: 'execute',
privileges: endpointPrivileges,
}),
});
}
},
];

// `upload` command
// planned for 8.9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ describe('When displaying Endpoint Response Actions', () => {
beforeEach(() => {
const testSetup = getConsoleTestSetup();

testSetup.setExperimentalFlag({
responseActionGetFileEnabled: true,
responseActionExecuteEnabled: true,
});

const endpointMetadata = new EndpointMetadataGenerator().generate();
const commands = getEndpointConsoleCommands({
endpointAgentId: '123',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,6 @@ export const useActionsLogFilter = ({
: RESPONSE_ACTION_API_COMMANDS_NAMES.filter((commandName) => {
const featureFlags = ExperimentalFeaturesService.get();

// `get-file` is currently behind FF
if (commandName === 'get-file' && !featureFlags.responseActionGetFileEnabled) {
return false;
}

// TODO: remove this when `execute` is no longer behind FF
// planned for 8.8
if (commandName === 'execute' && !featureFlags.responseActionExecuteEnabled) {
return false;
}

// upload - v8.9
if (commandName === 'upload' && !featureFlags.responseActionUploadEnabled) {
return false;
Expand Down

This file was deleted.

Loading

0 comments on commit 3bb4edf

Please sign in to comment.