Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Endpoint][Response Actions] Show Actions history on Endpoint Details for platinum users #145837

Merged
merged 4 commits into from
Nov 21, 2022
Merged

[Security Solution][Endpoint][Response Actions] Show Actions history on Endpoint Details for platinum users #145837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d119193
Show actions history on endpoint details for plat+
ashokaditya Nov 21, 2022
8e042a5
update test
ashokaditya Nov 21, 2022
bcbc279
update test
ashokaditya Nov 21, 2022
d043c42
update test that depends on license check
ashokaditya Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/service/authz/authz.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ describe('Endpoint Authz service', () => {
['canReadPolicyManagement', 'readPolicyManagement'],
['canWriteActionsLogManagement', 'writeActionsLogManagement'],
['canReadActionsLogManagement', 'readActionsLogManagement'],
['canAccessEndpointActionsLogManagement', 'readActionsLogManagement'],
['canIsolateHost', 'writeHostIsolation'],
['canUnIsolateHost', 'writeHostIsolation'],
['canKillProcess', 'writeProcessOperations'],
Expand All @@ -166,6 +167,10 @@ describe('Endpoint Authz service', () => {
['canReadPolicyManagement', ['writePolicyManagement', 'readPolicyManagement']],
['canWriteActionsLogManagement', ['writeActionsLogManagement']],
['canReadActionsLogManagement', ['writeActionsLogManagement', 'readActionsLogManagement']],
[
'canAccessEndpointActionsLogManagement',
['writeActionsLogManagement', 'readActionsLogManagement'],
],
['canIsolateHost', ['writeHostIsolation']],
['canUnIsolateHost', ['writeHostIsolation']],
['canKillProcess', ['writeProcessOperations']],
Expand Down Expand Up @@ -218,6 +223,7 @@ describe('Endpoint Authz service', () => {
canWriteSecuritySolution: false,
canReadSecuritySolution: false,
canAccessFleet: false,
canAccessEndpointActionsLogManagement: false,
canAccessEndpointManagement: false,
canCreateArtifactsByPolicy: false,
canDeleteHostIsolationExceptions: false,
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/service/authz/authz.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export function hasKibanaPrivilege(
* @param hasHostIsolationExceptionsItems if set to `true`, then Host Isolation Exceptions related authz properties
* may be adjusted to account for a license downgrade scenario
*/

// eslint-disable-next-line complexity
export const calculateEndpointAuthz = (
licenseService: LicenseService,
fleetAuthz: FleetAuthz,
Expand Down Expand Up @@ -223,6 +225,7 @@ export const calculateEndpointAuthz = (
canReadPolicyManagement,
canWriteActionsLogManagement,
canReadActionsLogManagement: canReadActionsLogManagement && isEnterpriseLicense,
canAccessEndpointActionsLogManagement: canReadActionsLogManagement && isPlatinumPlusLicense,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could name this canAccessEndpointListActionsLog? @paul-tavares.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok introducing the additional property, but can we name it consistency with how other are named? So in this case, it would be something like canReadSingleEndpointAcitonLog (notice I also renamed it a bit).

Also, looking at this change, are we saying that Action logs can be seen in Endpoint Details panel if license is Platinum++, but Action Log page is Enterprise?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually - never mind. I think the naming you created is good. I see there is precedence. Sorry for the noise.

// Response Actions
canIsolateHost: canIsolateHost && isPlatinumPlusLicense,
canUnIsolateHost: canIsolateHost,
Expand Down Expand Up @@ -250,6 +253,7 @@ export const getEndpointAuthzInitialState = (): EndpointAuthz => {
return {
...defaultEndpointPermissions(),
canAccessFleet: false,
canAccessEndpointActionsLogManagement: false,
canAccessEndpointManagement: false,
canCreateArtifactsByPolicy: false,
canWriteEndpointList: false,
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/types/authz.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export interface EndpointAuthz extends EndpointPermissions {
canAccessFleet: boolean;
/** If user has permissions to access Endpoint management (includes check to ensure they also have access to fleet) */
canAccessEndpointManagement: boolean;
/** If user has permissions to access Actions Log management and also has a platinum license (used for endpoint details flyout) */
canAccessEndpointActionsLogManagement: boolean;
/** if user has permissions to create Artifacts by Policy */
canCreateArtifactsByPolicy: boolean;
/** if user has write permissions to endpoint list */
Expand Down
1 change: 1 addition & 0 deletions ...olution/public/common/components/user_privileges/endpoint/use_endpoint_privileges.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ describe('When using useEndpointPrivileges hook', () => {
getEndpointPrivilegesInitialStateMock({
canCreateArtifactsByPolicy: false,
canIsolateHost: false,
canAccessEndpointActionsLogManagement: false,
canWriteHostIsolationExceptions: false,
canReadHostIsolationExceptions: hasHIE,
canDeleteHostIsolationExceptions: hasHIE,
Expand Down
5 changes: 3 additions & 2 deletions ...public/management/components/endpoint_response_actions_list/response_actions_log.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { RESPONSE_ACTION_API_COMMANDS_NAMES } from '../../../../common/endpoint/
import { useUserPrivileges as _useUserPrivileges } from '../../../common/components/user_privileges';
import { responseActionsHttpMocks } from '../../mocks/response_actions_http_mocks';
import { waitFor } from '@testing-library/react';
import { getUserPrivilegesMockDefaultValue } from '../../../common/components/user_privileges/__mocks__';

let mockUseGetEndpointActionList: {
isFetched?: boolean;
Expand Down Expand Up @@ -138,8 +139,7 @@ jest.mock('../../hooks/response_actions/use_get_file_info', () => {

const mockUseGetEndpointsList = useGetEndpointsList as jest.Mock;

// FLAKY https://github.com/elastic/kibana/issues/145635
describe.skip('Response actions history', () => {
describe('Response actions history', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming these tests (and the others below) are not flaky anymore because these changes.

const useUserPrivilegesMock = _useUserPrivileges as jest.Mock<
ReturnType<typeof _useUserPrivileges>
>;
Expand Down Expand Up @@ -195,6 +195,7 @@ describe.skip('Response actions history', () => {
...baseMockedActionList,
};
jest.clearAllMocks();
useUserPrivilegesMock.mockImplementation(getUserPrivilegesMockDefaultValue);
});

describe('When index does not exist yet', () => {
Expand Down
8 changes: 4 additions & 4 deletions ...ecurity_solution/public/management/pages/endpoint_hosts/view/details/endpoint_details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const EndpointDetails = memo(() => {
const policyInfo = useEndpointSelector(policyVersionInfo);
const hostStatus = useEndpointSelector(hostStatusInfo);
const show = useEndpointSelector(showView);
const { canReadActionsLogManagement } = useUserPrivileges().endpointPrivileges;
const { canAccessEndpointActionsLogManagement } = useUserPrivileges().endpointPrivileges;

const ContentLoadingMarkup = useMemo(
() => (
Expand Down Expand Up @@ -82,7 +82,7 @@ export const EndpointDetails = memo(() => {

// show the response actions history tab
// only when the user has the required permission
if (canReadActionsLogManagement) {
if (canAccessEndpointActionsLogManagement) {
tabs.push({
id: EndpointDetailsTabsTypes.activityLog,
name: i18.ACTIVITY_LOG.tabTitle,
Expand All @@ -97,7 +97,7 @@ export const EndpointDetails = memo(() => {
return tabs;
},
[
canReadActionsLogManagement,
canAccessEndpointActionsLogManagement,
ContentLoadingMarkup,
hostDetails,
policyInfo,
Expand Down Expand Up @@ -142,7 +142,7 @@ export const EndpointDetails = memo(() => {
hostname={hostDetails.host.hostname}
// show overview tab if forcing response actions history
// tab via URL without permission
show={!canReadActionsLogManagement ? 'details' : show}
show={!canAccessEndpointActionsLogManagement ? 'details' : show}
tabs={getTabs(hostDetails.agent.id)}
/>
)}
Expand Down
6 changes: 3 additions & 3 deletions ..._solution/public/management/pages/endpoint_hosts/view/hooks/use_endpoint_action_items.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const useEndpointActionItems = (
canAccessResponseConsole,
canIsolateHost,
canUnIsolateHost,
canReadActionsLogManagement,
canAccessEndpointActionsLogManagement,
} = useUserPrivileges().endpointPrivileges;

return useMemo<ContextMenuItemNavByRouterProps[]>(() => {
Expand Down Expand Up @@ -141,7 +141,7 @@ export const useEndpointActionItems = (
},
]
: []),
...(options?.isEndpointList && canReadActionsLogManagement
...(options?.isEndpointList && canAccessEndpointActionsLogManagement
? [
{
'data-test-subj': 'actionsLink',
Expand Down Expand Up @@ -253,7 +253,7 @@ export const useEndpointActionItems = (
}, [
allCurrentUrlParams,
canAccessResponseConsole,
canReadActionsLogManagement,
canAccessEndpointActionsLogManagement,
endpointMetadata,
fleetAgentPolicies,
getAppUrl,
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/security_solution/public/management/pages/integration_tests/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { AppContextTestRender } from '../../../common/mock/endpoint';
import { createAppRootMockRenderer } from '../../../common/mock/endpoint';
import { useUserPrivileges } from '../../../common/components/user_privileges';
import { endpointPageHttpMock } from '../endpoint_hosts/mocks';
import { getUserPrivilegesMockDefaultValue } from '../../../common/components/user_privileges/__mocks__';

jest.mock('../../../common/components/user_privileges');

Expand All @@ -29,7 +30,7 @@ describe('when in the Administration tab', () => {
});

afterEach(() => {
useUserPrivilegesMock.mockReset();
useUserPrivilegesMock.mockImplementation(getUserPrivilegesMockDefaultValue);
});

describe('when the user has no permissions', () => {
Expand Down Expand Up @@ -96,8 +97,7 @@ describe('when in the Administration tab', () => {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/145204
describe.skip('when the user has permissions', () => {
describe('when the user has permissions', () => {
it('should display the Management view if user has privileges', async () => {
useUserPrivilegesMock.mockReturnValue({
endpointPrivileges: { loading: false, canReadEndpointList: true },
Expand Down
31 changes: 26 additions & 5 deletions x-pack/plugins/security_solution/server/endpoint/routes/actions/list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ interface CallApiRouteInterface {
authz?: Partial<EndpointAuthz>;
}

const Platinum = licenseMock.createLicense({ license: { type: 'platinum', mode: 'platinum' } });
const Enterprise = licenseMock.createLicense({
license: { type: 'enterprise', mode: 'enterprise' },
});

const Platinum = licenseMock.createLicense({
license: { type: 'platinum', mode: 'platinum' },
});
const Gold = licenseMock.createLicense({ license: { type: 'gold', mode: 'gold' } });

describe('Action List Route', () => {
Expand Down Expand Up @@ -94,15 +100,16 @@ describe('Action List Route', () => {

const ctx = createRouteHandlerContext(mockScopedClient, mockSavedObjectClient);

const withLicense = license ? license : Platinum;
const withLicense = license ? license : Enterprise;
licenseEmitter.next(withLicense);

ctx.securitySolution.getEndpointAuthz.mockResolvedValue({
...getEndpointAuthzInitialStateMock({
// mimicking the behavior of the EndpointAuthz class
// just so we can test the license check here
// since getEndpointAuthzInitialStateMock sets all keys to true
canReadActionsLogManagement: licenseService.isPlatinumPlus(),
canReadActionsLogManagement: licenseService.isEnterprise(),
canAccessEndpointActionsLogManagement: licenseService.isPlatinumPlus(),
}),
...authz,
});
Expand Down Expand Up @@ -135,13 +142,27 @@ describe('Action List Route', () => {
expect(mockResponse.ok).toBeCalled();
});

it('does not allow user without `canReadActionsLogManagement` access for API requests', async () => {
it('allows user with `canAccessEndpointActionsLogManagement` access for API requests', async () => {
await callApiRoute(ENDPOINTS_ACTION_LIST_ROUTE, {
authz: { canReadActionsLogManagement: false },
authz: { canAccessEndpointActionsLogManagement: true },
});
expect(mockResponse.ok).toBeCalled();
});

it('does not allow user without `canReadActionsLogManagement` or `canAccessEndpointActionsLogManagement` access for API requests', async () => {
await callApiRoute(ENDPOINTS_ACTION_LIST_ROUTE, {
authz: { canReadActionsLogManagement: false, canAccessEndpointActionsLogManagement: false },
});
expect(mockResponse.forbidden).toBeCalled();
});

it('does allow user access to API requests if license is at least platinum', async () => {
await callApiRoute(ENDPOINTS_ACTION_LIST_ROUTE, {
license: Platinum,
});
expect(mockResponse.ok).toBeCalled();
});

it('does not allow user access to API requests if license is below platinum', async () => {
await callApiRoute(ENDPOINTS_ACTION_LIST_ROUTE, {
license: Gold,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/server/endpoint/routes/actions/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function registerActionListRoutes(
options: { authRequired: true, tags: ['access:securitySolution'] },
},
withEndpointAuthz(
{ all: ['canReadActionsLogManagement'] },
{ any: ['canReadActionsLogManagement', 'canAccessEndpointActionsLogManagement'] },
endpointContext.logFactory.get('endpointActionList'),
actionListHandler(endpointContext)
)
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/server/endpoint/routes/actions/list_handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jest.mock('../../services');
const mockGetActionList = getActionList as jest.Mock;
const mockGetActionListByStatus = getActionListByStatus as jest.Mock;

describe(' Action List Handler', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up typo here

describe('Action List Handler', () => {
let endpointAppContextService: EndpointAppContextService;
let mockResponse: jest.Mocked<KibanaResponseFactory>;

Expand Down