Skip to content

Commit

Permalink
[Security Solution] UI Event Filters RBAC (#146111)
Browse files Browse the repository at this point in the history 
## Summary

Similarly to #145593, this PR
handles the _None_ and _Read_ privileges for the Event Filters
sub-feature. The _All_ privilege should not need any UI modification,
but will need API modification.
<img width="554" alt="image"
src="https://user-images.githubusercontent.com/39014407/203514418-b016a47b-819c-4057-a86e-d7b4a3d8e5c5.png">

The modification should:
- hide Event Filters from Manage navigation items if privilege is NONE,
~(note: it is still displayed for non-superusers, if the feature flag is
disabled)~ update: it is hidden for non-superusers if the feature flag
is disabled
- disable add/edit/delete for Event Filters if privilege is READ.



#### Checked:
- the Event Filters form still works from the "Hosts > Events" side of
the app ✅
<img width="1354" alt="image"
src="https://user-images.githubusercontent.com/39014407/204316619-85121106-9d28-4165-9675-522890e39dfe.png">
<img width="1323" alt="image"
src="https://user-images.githubusercontent.com/39014407/204326904-6917c8fe-a364-4a40-8bdc-e8240115fa1d.png">



### Checklist

Delete any items that are not applicable to this PR.

- [x] [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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@gergoabraham @kibanamachine
gergoabraham and kibanamachine authored Dec 1, 2022
1 parent bbd229f commit 5f3ac5d
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 53 deletions.
87 changes: 35 additions & 52 deletions x-pack/plugins/security_solution/public/management/links.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,10 @@ describe('links', () => {
} as unknown as StartPlugins);
});

it('should return all links without filtering when having isolate permission', async () => {
it('should return all links for user with all sub-feature privileges', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock());

const filteredLinks = await getManagementFilteredLinks(
coreMockStarted,
getPlugins(['superuser'])
);
const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));
expect(filteredLinks).toEqual(links);
});

Expand All @@ -87,13 +84,14 @@ describe('links', () => {
coreMockStarted,
getPlugins(['superuser'])
);
expect(filteredLinks).toEqual({
...links,
links: links.links?.filter((link) => link.id !== SecurityPageName.responseActionsHistory),
});
expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.responseActionsHistory));
});
});

// todo: these tests should be updated, because in the end, showing/hiding HIE depends on nothing
// else but the mock return of `calculateEndpointAuthz`.
// These tests should check what is the value of `hasHostIsolationExceptions` which is passed to
// `calculateEndpointAuthz`.
describe('Host Isolation Exception', () => {
it('should return all but HIE when NO isolation permission due to privilege', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue({
Expand All @@ -102,6 +100,8 @@ describe('links', () => {
canAccessEndpointManagement: true,
canReadActionsLogManagement: true,
canReadEndpointList: true,
canReadTrustedApplications: true,
canReadEventFilters: true,
});

const filteredLinks = await getManagementFilteredLinks(
Expand All @@ -118,6 +118,8 @@ describe('links', () => {
canAccessEndpointManagement: true,
canReadActionsLogManagement: true,
canReadEndpointList: true,
canReadTrustedApplications: true,
canReadEventFilters: true,
});
fakeHttpServices.get.mockResolvedValue({ total: 0 });

Expand All @@ -135,6 +137,8 @@ describe('links', () => {
canAccessEndpointManagement: false,
canReadActionsLogManagement: true,
canReadEndpointList: true,
canReadTrustedApplications: true,
canReadEventFilters: true,
});
fakeHttpServices.get.mockResolvedValue({ total: 1 });

Expand Down Expand Up @@ -166,6 +170,8 @@ describe('links', () => {
canUnIsolateHost: true,
canReadActionsLogManagement: true,
canReadEndpointList: true,
canReadTrustedApplications: true,
canReadEventFilters: true,
});
fakeHttpServices.get.mockRejectedValue(new Error());

Expand All @@ -182,6 +188,8 @@ describe('links', () => {
canUnIsolateHost: true,
canReadActionsLogManagement: false,
canReadEndpointList: true,
canReadTrustedApplications: true,
canReadEventFilters: true,
});
fakeHttpServices.get.mockRejectedValue(new Error());

Expand All @@ -198,52 +206,30 @@ describe('links', () => {
});
});

// this can be removed after removing endpointRbacEnabled feature flag
describe('without endpointRbacEnabled', () => {
beforeAll(() => {
ExperimentalFeaturesService.init({
experimentalFeatures: { ...allowedExperimentalValues, endpointRbacEnabled: false },
});
});

it('shows Trusted Applications for non-superuser, too', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock());
it('should hide Trusted Applications for user without privilege', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(
getEndpointAuthzInitialStateMock({
canReadTrustedApplications: false,
})
);

const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));
const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));

expect(filteredLinks).toEqual(links);
});
expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.trustedApps));
});

// this can be the default after removing endpointRbacEnabled feature flag
describe('with endpointRbacEnabled', () => {
beforeAll(() => {
ExperimentalFeaturesService.init({
experimentalFeatures: { ...allowedExperimentalValues, endpointRbacEnabled: true },
});
});

it('hides Trusted Applications for user without privilege', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(
getEndpointAuthzInitialStateMock({
canReadTrustedApplications: false,
canReadHostIsolationExceptions: true,
})
);

const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));

expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.trustedApps));
});

it('shows Trusted Applications for user with privilege', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock());
it('should hide Event Filters for user without privilege', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(
getEndpointAuthzInitialStateMock({
canReadEventFilters: false,
})
);

const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));
const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([]));

expect(filteredLinks).toEqual(links);
});
expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.eventFilters));
});

describe('Endpoint List', () => {
it('should return all but endpoints link when no Endpoint List READ access', async () => {
(calculateEndpointAuthz as jest.Mock).mockReturnValue(
Expand All @@ -255,10 +241,7 @@ describe('links', () => {
coreMockStarted,
getPlugins(['superuser'])
);
expect(filteredLinks).toEqual({
...links,
links: links.links?.filter((link) => link.id !== SecurityPageName.endpoints),
});
expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.endpoints));
});
});
});
7 changes: 6 additions & 1 deletion x-pack/plugins/security_solution/public/management/links.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ export const getManagementFilteredLinks = async (
canReadHostIsolationExceptions,
canReadEndpointList,
canReadTrustedApplications,
canReadEventFilters,
} = fleetAuthz
? calculateEndpointAuthz(
licenseService,
Expand All @@ -301,9 +302,13 @@ export const getManagementFilteredLinks = async (
linksToExclude.push(SecurityPageName.hostIsolationExceptions);
}

if (endpointRbacEnabled && !canReadTrustedApplications) {
if (!canReadTrustedApplications) {
linksToExclude.push(SecurityPageName.trustedApps);
}

if (!canReadEventFilters) {
linksToExclude.push(SecurityPageName.eventFilters);
}

return excludeLinks(linksToExclude);
};
5 changes: 5 additions & 0 deletions ...ugins/security_solution/public/management/pages/event_filters/view/event_filters_list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { FormattedMessage } from '@kbn/i18n-react';
import type { DocLinks } from '@kbn/doc-links';
import { EuiLink } from '@elastic/eui';

import { useUserPrivileges } from '../../../../common/components/user_privileges';
import { useHttp } from '../../../../common/lib/kibana';
import type { ArtifactListPageProps } from '../../../components/artifact_list_page';
import { ArtifactListPage } from '../../../components/artifact_list_page';
Expand Down Expand Up @@ -133,6 +134,7 @@ const EVENT_FILTERS_PAGE_LABELS: ArtifactListPageProps['labels'] = {
};

export const EventFiltersList = memo(() => {
const { canWriteEventFilters } = useUserPrivileges().endpointPrivileges;
const http = useHttp();
const eventFiltersApiClient = EventFiltersApiClient.getInstance(http);

Expand All @@ -144,6 +146,9 @@ export const EventFiltersList = memo(() => {
data-test-subj="EventFiltersListPage"
searchableFields={SEARCHABLE_FIELDS}
flyoutSize="l"
allowCardCreateAction={canWriteEventFilters}
allowCardEditAction={canWriteEventFilters}
allowCardDeleteAction={canWriteEventFilters}
/>
);
});
Expand Down
69 changes: 69 additions & 0 deletions .../public/management/pages/event_filters/view/integration_tests/event_filters_list.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ import { EventFiltersList } from '../event_filters_list';
import { exceptionsListAllHttpMocks } from '../../../../mocks/exceptions_list_http_mocks';
import { SEARCHABLE_FIELDS } from '../../constants';
import { parseQueryFilterToKQL } from '../../../../common/utils';
import type { EndpointPrivileges } from '../../../../../../common/endpoint/types';
import { useUserPrivileges } from '../../../../../common/components/user_privileges';

jest.mock('../../../../../common/components/user_privileges');
const mockUserPrivileges = useUserPrivileges as jest.Mock;

describe('When on the Event Filters list page', () => {
let render: () => ReturnType<AppContextTestRender['render']>;
let renderResult: ReturnType<typeof render>;
let history: AppContextTestRender['history'];
let mockedContext: AppContextTestRender;
let apiMocks: ReturnType<typeof exceptionsListAllHttpMocks>;
let mockedEndpointPrivileges: Partial<EndpointPrivileges>;

beforeEach(() => {
mockedContext = createAppRootMockRenderer();
Expand All @@ -31,6 +37,13 @@ describe('When on the Event Filters list page', () => {
act(() => {
history.push(EVENT_FILTERS_PATH);
});

mockedEndpointPrivileges = { canWriteTrustedApplications: true };
mockUserPrivileges.mockReturnValue({ endpointPrivileges: mockedEndpointPrivileges });
});

afterEach(() => {
mockUserPrivileges.mockReset();
});

it('should search using expected exception item fields', async () => {
Expand All @@ -55,4 +68,60 @@ describe('When on the Event Filters list page', () => {
})
);
});

describe('RBAC Event Filters', () => {
describe('ALL privilege', () => {
beforeEach(() => {
mockedEndpointPrivileges.canWriteEventFilters = true;
});

it('should enable adding entries', async () => {
render();

await waitFor(() =>
expect(renderResult.queryByTestId('EventFiltersListPage-pageAddButton')).toBeTruthy()
);
});

it('should enable modifying/deleting entries', async () => {
render();

const actionsButton = await waitFor(
() => renderResult.getAllByTestId('EventFiltersListPage-card-header-actions-button')[0]
);
userEvent.click(actionsButton);

expect(renderResult.getByTestId('EventFiltersListPage-card-cardEditAction')).toBeTruthy();
expect(renderResult.getByTestId('EventFiltersListPage-card-cardDeleteAction')).toBeTruthy();
});
});

describe('READ privilege', () => {
beforeEach(() => {
mockedEndpointPrivileges.canWriteEventFilters = false;
});

it('should disable adding entries', async () => {
render();

await waitFor(() =>
expect(renderResult.queryByTestId('EventFiltersListPage-container')).toBeTruthy()
);

expect(renderResult.queryByTestId('EventFiltersListPage-pageAddButton')).toBeNull();
});

it('should disable modifying/deleting entries', async () => {
render();

await waitFor(() =>
expect(renderResult.queryByTestId('EventFiltersListPage-container')).toBeTruthy()
);

expect(
renderResult.queryByTestId('EventFiltersListPage-card-header-actions-button')
).toBeNull();
});
});
});
});

0 comments on commit 5f3ac5d

Please sign in to comment.