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

Jump to bottom

[Cases] RBAC on UI #99478

Merged
merged 18 commits into from
May 14, 2021
Merged

[Cases] RBAC on UI #99478

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8e33930
Pass onwer to create form
cnasikas May 6, 2021
cb62698
Pass owner to get cases
cnasikas May 11, 2021
242f608
Pass owner to tags and reporters
cnasikas May 12, 2021
3b93743
Pass owner to configuration
cnasikas May 12, 2021
240e17b
Use owner context
cnasikas May 12, 2021
9fc4772
Throw an error if owner context is not set
cnasikas May 13, 2021
5fd963f
Add owner provider to case view
cnasikas May 13, 2021
c680d71
Use APP_ID when attaching an alert to a case
cnasikas May 13, 2021
2439361
Clean up TODOs
cnasikas May 13, 2021
defa361
Merge branch 'cases-rbac-poc' into ui_rbac
kibanamachine May 13, 2021
29068cb
Use constants for owner
cnasikas May 13, 2021
12ac71d
Fix tests
cnasikas May 13, 2021
9973648
Move owner to child
cnasikas May 13, 2021
5acfe99
Fix type
cnasikas May 13, 2021
11f48b2
Move owner inside the hooks & components
cnasikas May 14, 2021
935e255
Fix bug
cnasikas May 14, 2021
3b4634e
Fix types
cnasikas May 14, 2021
11b01b0
Fix tests
cnasikas May 14, 2021
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
1 change: 1 addition & 0 deletions x-pack/plugins/cases/common/ui/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export interface FilterOptions {
status: CaseStatusWithAllStatus;
tags: string[];
reporters: User[];
owner: string[];
onlyCollectionType?: boolean;
}

Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/cases/public/common/mock/test_providers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { I18nProvider } from '@kbn/i18n/react';
import React from 'react';
import { BehaviorSubject } from 'rxjs';
import { ThemeProvider } from 'styled-components';
import { OwnerProvider } from '../../components/owner_context';
import {
createKibanaContextProviderMock,
createStartServicesMock,
Expand All @@ -29,7 +30,9 @@ const MockKibanaContextProvider = createKibanaContextProviderMock();
const TestProvidersComponent: React.FC<Props> = ({ children }) => (
<I18nProvider>
<MockKibanaContextProvider>
<ThemeProvider theme={() => ({ eui: euiDarkVars, darkMode: true })}>{children}</ThemeProvider>
<ThemeProvider theme={() => ({ eui: euiDarkVars, darkMode: true })}>
<OwnerProvider owner={['securitySolution']}>{children}</OwnerProvider>
cnasikas marked this conversation as resolved.
Show resolved Hide resolved
</ThemeProvider>
</MockKibanaContextProvider>
</I18nProvider>
);
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/cases/public/components/all_cases/all_cases_generic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { CasesTableFilters } from './table_filters';
import { EuiBasicTableOnChange } from './types';

import { CasesTable } from './table';
import { useOwnerContext } from '../owner_context/use_owner_context';

const ProgressLoader = styled(EuiProgress)`
${({ $isShow }: { $isShow: boolean }) =>
$isShow
Expand Down Expand Up @@ -79,6 +81,8 @@ export const AllCasesGeneric = React.memo<AllCasesGenericProps>(
userCanCrud,
}) => {
const { actionLicense } = useGetActionLicense();
const owner = useOwnerContext();

const {
data,
dispatchUpdateCaseProperty,
Expand All @@ -90,7 +94,7 @@ export const AllCasesGeneric = React.memo<AllCasesGenericProps>(
setFilters,
setQueryParams,
setSelectedCases,
} = useGetCases();
} = useGetCases({ initialFilterOptions: { owner } });
Copy link
Member Author

Choose a reason for hiding this comment

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

@jonathan-buttner @michaelolo24 Do you think is best for the useGetCases hook to use useOwnerContext instead of passing it as an argument?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you'll never call useGetCases without the context owner, then I say include it in the actual useGetCases call rather than passing it in, but if for some reason the owner can change from call to call, then pass it. There may be reasons I'm not thinking of, but that's how I work it out in my head


// Post Comment to Case
const { postComment, isLoading: isCommentUpdating } = usePostComment();
Expand Down Expand Up @@ -286,6 +290,7 @@ export const AllCasesGeneric = React.memo<AllCasesGenericProps>(
reporters: filterOptions.reporters,
tags: filterOptions.tags,
status: filterOptions.status,
owner: filterOptions.owner,
}}
setFilterRefetch={setFilterRefetch}
disabledStatuses={disabledStatuses}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/public/components/all_cases/selector_modal/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const defaultProps = {
createCaseNavigation,
onRowClick,
userCanCrud: true,
owner: ['securitySolution'],
};
const updateCase = jest.fn();

Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/cases/public/components/all_cases/table_filters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const defaultInitial = {
reporters: [],
status: StatusAll,
tags: [],
owner: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind of the same thing here. I figure when you're in a solution, everything listed here, but the owner will actually be modifiable by the user. You can just skip passing it through here and call it directly in the useGetTags and useGetReporters calls

};

const CasesTableFiltersComponent = ({
Expand All @@ -63,8 +64,8 @@ const CasesTableFiltersComponent = ({
);
const [search, setSearch] = useState(initial.search);
const [selectedTags, setSelectedTags] = useState(initial.tags);
const { tags, fetchTags } = useGetTags();
const { reporters, respReporters, fetchReporters } = useGetReporters();
const { tags, fetchTags } = useGetTags(initial.owner);
const { reporters, respReporters, fetchReporters } = useGetReporters(initial.owner);

const refetch = useCallback(() => {
fetchTags();
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/public/components/case_view/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ export const CaseComponent = React.memo<CaseComponentProps>(
tags={caseData.tags}
onSubmit={onSubmitTags}
isLoading={isLoading && updateKey === 'tags'}
owner={[caseData.owner]}
/>
<EditConnector
caseFields={caseData.connector.fields}
Expand Down
48 changes: 36 additions & 12 deletions x-pack/plugins/cases/public/components/configure_cases/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ describe('ConfigureCases', () => {
useConnectorsMock.mockImplementation(() => ({ ...useConnectorsResponse, connectors: [] }));
useGetUrlSearchMock.mockImplementation(() => searchURL);

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it renders the Connectors', () => {
Expand Down Expand Up @@ -155,7 +157,9 @@ describe('ConfigureCases', () => {
}));
useConnectorsMock.mockImplementation(() => ({ ...useConnectorsResponse, connectors: [] }));
useGetUrlSearchMock.mockImplementation(() => searchURL);
wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it shows the warning callout when configuration is invalid', () => {
Expand Down Expand Up @@ -200,7 +204,9 @@ describe('ConfigureCases', () => {
useConnectorsMock.mockImplementation(() => useConnectorsResponse);
useGetUrlSearchMock.mockImplementation(() => searchURL);

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it renders with correct props', () => {
Expand Down Expand Up @@ -282,7 +288,9 @@ describe('ConfigureCases', () => {
}));

useGetUrlSearchMock.mockImplementation(() => searchURL);
wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it disables correctly Connector when loading connectors', () => {
Expand Down Expand Up @@ -315,7 +323,9 @@ describe('ConfigureCases', () => {

useActionTypesMock.mockImplementation(() => ({ ...useActionTypesResponse, loading: true }));

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
expect(wrapper.find(Connectors).prop('isLoading')).toBe(true);
});
});
Expand All @@ -337,7 +347,9 @@ describe('ConfigureCases', () => {

useConnectorsMock.mockImplementation(() => useConnectorsResponse);
useGetUrlSearchMock.mockImplementation(() => searchURL);
wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it disables correctly Connector when saving configuration', () => {
Expand Down Expand Up @@ -378,7 +390,9 @@ describe('ConfigureCases', () => {
...useConnectorsResponse,
}));
useGetUrlSearchMock.mockImplementation(() => searchURL);
wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it hides the update connector button when loading the configuration', () => {
Expand Down Expand Up @@ -420,7 +434,9 @@ describe('ConfigureCases', () => {
useConnectorsMock.mockImplementation(() => useConnectorsResponse);
useGetUrlSearchMock.mockImplementation(() => searchURL);

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it submits the configuration correctly when changing connector', () => {
Expand Down Expand Up @@ -462,7 +478,9 @@ describe('ConfigureCases', () => {
},
}));

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});

wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
wrapper.update();
Expand Down Expand Up @@ -508,7 +526,9 @@ describe('closure options', () => {
useConnectorsMock.mockImplementation(() => useConnectorsResponse);
useGetUrlSearchMock.mockImplementation(() => searchURL);

wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
});

test('it submits the configuration correctly when changing closure type', () => {
Expand Down Expand Up @@ -555,7 +575,9 @@ describe('user interactions', () => {
});

test('it show the add flyout when pressing the add connector button', () => {
const wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
const wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
wrapper.find('button[data-test-subj="dropdown-connectors"]').simulate('click');
wrapper.update();
wrapper.find('button[data-test-subj="dropdown-connector-add-connector"]').simulate('click');
Expand All @@ -576,7 +598,9 @@ describe('user interactions', () => {
});

test('it show the edit flyout when pressing the update connector button', () => {
const wrapper = mount(<ConfigureCases userCanCrud />, { wrappingComponent: TestProviders });
const wrapper = mount(<ConfigureCases userCanCrud />, {
wrappingComponent: TestProviders,
});
wrapper
.find('button[data-test-subj="case-configure-update-selected-connector-button"]')
.simulate('click');
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/cases/public/components/configure_cases/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
normalizeCaseConnector,
} from './utils';
import * as i18n from './translations';
import { useOwnerContext } from '../owner_context/use_owner_context';

const FormWrapper = styled.div`
${({ theme }) => css`
Expand All @@ -56,6 +57,7 @@ export interface ConfigureCasesProps {

const ConfigureCasesComponent: React.FC<ConfigureCasesProps> = ({ userCanCrud }) => {
const { triggersActionsUi } = useKibana().services;
const owner = useOwnerContext();

const [connectorIsValid, setConnectorIsValid] = useState(true);
const [addFlyoutVisible, setAddFlyoutVisibility] = useState<boolean>(false);
Expand All @@ -74,7 +76,7 @@ const ConfigureCasesComponent: React.FC<ConfigureCasesProps> = ({ userCanCrud })
refetchCaseConfigure,
setConnector,
setClosureType,
} = useCaseConfigure();
} = useCaseConfigure(owner[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any chance owner could be empty? Or is it guaranteed to be populated with at least one value as part of the react context?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right about it. I had that debate with myself. After some thought, I decided to do it as 9fc4772 (#99478). If the array is empty when using the context an error will be thrown.

Copy link
Member Author

Choose a reason for hiding this comment

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

When cases have their own UI then we should reconsider what it means to have an empty owner.


const { loading: isLoadingConnectors, connectors, refetchConnectors } = useConnectors();
const { loading: isLoadingActionTypes, actionTypes, refetchActionTypes } = useActionTypes();
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/cases/public/components/connectors/case/alert_fields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,17 @@ const CaseParamsFields: React.FunctionComponent<ActionParamsProps<CaseActionPara
actionParams.subAction,
]);

/**
* TODO: When the case connector is enabled we need to figure out
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious. What's the issue here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Case as a connector isn't supported yet (the sub cases feature). I think this TODO is stating that we don't have a configurable way to pass owner to this component yet. Below we're hardcoding it to the security solution but if we support case as a connector in other plugins (observability) we'll need to allow the owner to be passed in like the other case components.

* how we can pass the owner to this component
*/
return (
<Container>
<ExistingCase onCaseChanged={onCaseChanged} selectedCase={selectedCase} />
<ExistingCase
onCaseChanged={onCaseChanged}
selectedCase={selectedCase}
owner="securitySolution"
cnasikas marked this conversation as resolved.
Show resolved Hide resolved
/>
<EuiSpacer size="m" />
<EuiCallOut size="s" title={i18n.CASE_CONNECTOR_CALL_OUT_TITLE} iconType="iInCircle">
<p>{i18n.CASE_CONNECTOR_CALL_OUT_MSG}</p>
Expand Down
22 changes: 18 additions & 4 deletions x-pack/plugins/cases/public/components/connectors/case/existing_case.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,25 @@ import { CasesDropdown, ADD_CASE_BUTTON_ID } from './cases_dropdown';
interface ExistingCaseProps {
selectedCase: string | null;
onCaseChanged: (id: string) => void;
/**
* We allow only one owner as this component queries
* for multiple cases and creates a single case.
*/
owner: string;
}

const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, selectedCase }) => {
const { data: cases, loading: isLoadingCases, refetchCases } = useGetCases(DEFAULT_QUERY_PARAMS, {
...DEFAULT_FILTER_OPTIONS,
onlyCollectionType: true,
const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({
onCaseChanged,
selectedCase,
owner,
}) => {
const { data: cases, loading: isLoadingCases, refetchCases } = useGetCases({
initialQueryParams: DEFAULT_QUERY_PARAMS,
initialFilterOptions: {
...DEFAULT_FILTER_OPTIONS,
onlyCollectionType: true,
owner: [owner],
},
});

const onCaseCreated = useCallback(
Expand All @@ -41,6 +54,7 @@ const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, sel
// We are making the assumption that this component is only used in rules creation
// that's why we want to hide ServiceNow SIR
hideConnectorServiceNowSir: true,
owner,
});

const onChange = useCallback(
Expand Down
Loading