Skip to content

Commit

Permalink
[ResponseOps][Cases] Fix edit cases settings privilege (elastic#202053)
Browse files Browse the repository at this point in the history
## Summary

Fixes elastic#197650

Also fixes an issue where user has `cases: all ` and `edit case
settings: false`, user was able to edit settings.

Used `permissions.settings` instead of `permissions.update` and
`permissions.create` for custom fields and templates.

### How to test
- Verify by creating a user with different combinations of cases and
edit case settings privileges

### Checklist

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

(cherry picked from commit 8e8ba53)
  • Loading branch information
js-jankisalvi committed Dec 4, 2024
1 parent 0fc8148 commit 0d6986e
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 66 deletions.
1 change: 0 additions & 1 deletion x-pack/plugins/cases/common/utils/capabilities.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ describe('createUICapabilities', () => {
"update_cases",
"push_cases",
"cases_connectors",
"cases_settings",
],
"createComment": Array [
"create_comment",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/cases/common/utils/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const createUICapabilities = (): CasesUiCapabilities => ({
UPDATE_CASES_CAPABILITY,
PUSH_CASES_CAPABILITY,
CASES_CONNECTORS_CAPABILITY,
CASES_SETTINGS_CAPABILITY,
] as const,
read: [READ_CASES_CAPABILITY, CASES_CONNECTORS_CAPABILITY] as const,
delete: [DELETE_CASES_CAPABILITY] as const,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { waitFor, screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { ConfigureCases } from '.';
import { noUpdateCasesPermissions, TestProviders, createAppMockRenderer } from '../../common/mock';
import { noCasesSettingsPermission, TestProviders, createAppMockRenderer } from '../../common/mock';
import { customFieldsConfigurationMock, templatesConfigurationMock } from '../../containers/mock';
import type { AppMockRenderer } from '../../common/mock';
import { Connectors } from './connectors';
Expand Down Expand Up @@ -200,10 +200,10 @@ describe('ConfigureCases', () => {
expect(wrapper.find('[data-test-subj="edit-connector-flyout"]').exists()).toBe(false);
});

test('it disables correctly when the user cannot update', () => {
test('it disables correctly when the user does not have settings privilege', () => {
const newWrapper = mount(<ConfigureCases />, {
wrappingComponent: TestProviders as ComponentType<React.PropsWithChildren<{}>>,
wrappingComponentProps: { permissions: noUpdateCasesPermissions() },
wrappingComponentProps: { permissions: noCasesSettingsPermission() },
});

expect(newWrapper.find('button[data-test-subj="dropdown-connectors"]').prop('disabled')).toBe(
Expand Down
24 changes: 9 additions & 15 deletions x-pack/plugins/cases/public/components/configure_cases/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -480,12 +480,7 @@ export const ConfigureCases: React.FC = React.memo(() => {
flyOutVisibility?.type === 'customField' && flyOutVisibility?.visible ? (
<CommonFlyout<CustomFieldConfiguration>
isLoading={loadingCaseConfigure || isPersistingConfiguration}
disabled={
!permissions.create ||
!permissions.update ||
loadingCaseConfigure ||
isPersistingConfiguration
}
disabled={!permissions.settings || loadingCaseConfigure || isPersistingConfiguration}
onCloseFlyout={onCloseCustomFieldFlyout}
onSaveField={onCustomFieldSave}
renderHeader={() => (
Expand All @@ -502,12 +497,7 @@ export const ConfigureCases: React.FC = React.memo(() => {
flyOutVisibility?.type === 'template' && flyOutVisibility?.visible ? (
<CommonFlyout<TemplateFormProps, TemplateConfiguration>
isLoading={loadingCaseConfigure || isPersistingConfiguration}
disabled={
!permissions.create ||
!permissions.update ||
loadingCaseConfigure ||
isPersistingConfiguration
}
disabled={!permissions.settings || loadingCaseConfigure || isPersistingConfiguration}
onCloseFlyout={onCloseTemplateFlyout}
onSaveField={onTemplateSave}
renderHeader={() => (
Expand Down Expand Up @@ -565,7 +555,9 @@ export const ConfigureCases: React.FC = React.memo(() => {
<div css={sectionWrapperCss}>
<ClosureOptions
closureTypeSelected={closureType}
disabled={isPersistingConfiguration || isLoadingConnectors || !permissions.update}
disabled={
isPersistingConfiguration || isLoadingConnectors || !permissions.settings
}
onChangeClosureType={onChangeClosureType}
/>
</div>
Expand All @@ -574,13 +566,15 @@ export const ConfigureCases: React.FC = React.memo(() => {
<Connectors
actionTypes={actionTypes}
connectors={connectors ?? []}
disabled={isPersistingConfiguration || isLoadingConnectors || !permissions.update}
disabled={
isPersistingConfiguration || isLoadingConnectors || !permissions.settings
}
handleShowEditFlyout={onClickUpdateConnector}
isLoading={isLoadingAny}
mappings={mappings}
onChangeConnector={onChangeConnector}
selectedConnector={connector}
updateConnectorDisabled={updateConnectorDisabled || !permissions.update}
updateConnectorDisabled={updateConnectorDisabled || !permissions.settings}
onAddNewConnector={onAddNewConnector}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from '@elastic/eui';

import * as i18n from './translations';
import { useCasesContext } from '../cases_context/use_cases_context';
import type { CustomFieldsConfiguration } from '../../../common/types/domain';
import { MAX_CUSTOM_FIELDS_PER_CASE } from '../../../common/constants';
import { CustomFieldsList } from './custom_fields_list';
Expand All @@ -38,8 +37,6 @@ const CustomFieldsComponent: React.FC<Props> = ({
handleEditCustomField,
customFields,
}) => {
const { permissions } = useCasesContext();
const canAddCustomFields = permissions.create && permissions.update;
const [error, setError] = useState<boolean>(false);

const onAddCustomField = useCallback(() => {
Expand All @@ -64,7 +61,7 @@ const CustomFieldsComponent: React.FC<Props> = ({
setError(false);
}

return canAddCustomFields ? (
return (
<EuiDescribedFormGroup
fullWidth
title={<h3>{i18n.TITLE}</h3>}
Expand Down Expand Up @@ -113,10 +110,11 @@ const CustomFieldsComponent: React.FC<Props> = ({
)}
</EuiFlexItem>
</EuiFlexGroup>

<EuiSpacer size="s" />
</EuiPanel>
</EuiDescribedFormGroup>
) : null;
);
};
CustomFieldsComponent.displayName = 'CustomFields';

Expand Down
51 changes: 23 additions & 28 deletions x-pack/plugins/cases/public/components/templates/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from '@elastic/eui';
import { MAX_TEMPLATES_LENGTH } from '../../../common/constants';
import type { CasesConfigurationUITemplate } from '../../../common/ui';
import { useCasesContext } from '../cases_context/use_cases_context';
import { ExperimentalBadge } from '../experimental_badge/experimental_badge';
import * as i18n from './translations';
import { TemplatesList } from './templates_list';
Expand All @@ -39,8 +38,6 @@ const TemplatesComponent: React.FC<Props> = ({
onEditTemplate,
onDeleteTemplate,
}) => {
const { permissions } = useCasesContext();
const canAddTemplates = permissions.create && permissions.update;
const [error, setError] = useState<boolean>(false);

const handleAddTemplate = useCallback(() => {
Expand Down Expand Up @@ -103,31 +100,29 @@ const TemplatesComponent: React.FC<Props> = ({
</EuiFlexItem>
</EuiFlexGroup>
) : null}
{canAddTemplates ? (
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
{templates.length < MAX_TEMPLATES_LENGTH ? (
<EuiButtonEmpty
isLoading={isLoading}
isDisabled={disabled || error}
size="s"
onClick={handleAddTemplate}
iconType="plusInCircle"
data-test-subj="add-template"
>
{i18n.ADD_TEMPLATE}
</EuiButtonEmpty>
) : (
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
<EuiText>{i18n.MAX_TEMPLATE_LIMIT(MAX_TEMPLATES_LENGTH)}</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
)}
<EuiSpacer size="s" />
</EuiFlexItem>
</EuiFlexGroup>
) : null}
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
{templates.length < MAX_TEMPLATES_LENGTH ? (
<EuiButtonEmpty
isLoading={isLoading}
isDisabled={disabled || error}
size="s"
onClick={handleAddTemplate}
iconType="plusInCircle"
data-test-subj="add-template"
>
{i18n.ADD_TEMPLATE}
</EuiButtonEmpty>
) : (
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
<EuiText>{i18n.MAX_TEMPLATE_LIMIT(MAX_TEMPLATES_LENGTH)}</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
)}
<EuiSpacer size="s" />
</EuiFlexItem>
</EuiFlexGroup>
</EuiPanel>
</EuiDescribedFormGroup>
);
Expand Down
26 changes: 25 additions & 1 deletion x-pack/test/functional_with_es_ssl/apps/cases/common/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ export const casesNoDelete: Role = {
},
};

export const casesReadAndEditSettings: Role = {
name: 'cases_read_and_edit_settings',
privileges: {
elasticsearch: {
indices: [
{
names: ['*'],
privileges: ['all'],
},
],
},
kibana: [
{
feature: {
generalCasesV2: ['minimal_read', 'cases_settings'],
actions: ['all'],
actionsSimulators: ['all'],
},
spaces: ['*'],
},
],
},
};

export const casesAll: Role = {
name: 'cases_all_role',
privileges: {
Expand All @@ -83,4 +107,4 @@ export const casesAll: Role = {
},
};

export const roles = [casesReadDelete, casesNoDelete, casesAll];
export const roles = [casesReadDelete, casesNoDelete, casesAll, casesReadAndEditSettings];
28 changes: 20 additions & 8 deletions x-pack/test/functional_with_es_ssl/apps/cases/common/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { User } from '../../../../cases_api_integration/common/lib/authentication/types';
import { casesAll, casesNoDelete, casesReadDelete } from './roles';
import { casesAll, casesNoDelete, casesReadDelete, casesReadAndEditSettings } from './roles';

/**
* Users for Cases in the Stack
Expand All @@ -18,12 +18,6 @@ export const casesReadDeleteUser: User = {
roles: [casesReadDelete.name],
};

export const casesNoDeleteUser: User = {
username: 'cases_no_delete_user',
password: 'password',
roles: [casesNoDelete.name],
};

export const casesAllUser: User = {
username: 'cases_all_user',
password: 'password',
Expand All @@ -36,4 +30,22 @@ export const casesAllUser2: User = {
roles: [casesAll.name],
};

export const users = [casesReadDeleteUser, casesNoDeleteUser, casesAllUser, casesAllUser2];
export const casesReadAndEditSettingsUser: User = {
username: 'cases_read_and_edit_settings_user',
password: 'password',
roles: [casesReadAndEditSettings.name],
};

export const casesNoDeleteUser: User = {
username: 'cases_no_delete_user',
password: 'password',
roles: [casesNoDelete.name],
};

export const users = [
casesReadDeleteUser,
casesNoDeleteUser,
casesAllUser,
casesAllUser2,
casesReadAndEditSettingsUser,
];
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ export default ({ loadTestFile }: FtrProviderContext) => {
describe('Cases - group 1', function () {
loadTestFile(require.resolve('./create_case_form'));
loadTestFile(require.resolve('./view_case'));
loadTestFile(require.resolve('./deletion'));
loadTestFile(require.resolve('./sub_privileges'));
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,16 @@
* 2.0.
*/

import expect from '@kbn/expect';
import { FtrProviderContext } from '../../../ftr_provider_context';
import { users, roles, casesReadDeleteUser, casesAllUser, casesNoDeleteUser } from '../common';
import {
users,
roles,
casesReadDeleteUser,
casesAllUser,
casesNoDeleteUser,
casesReadAndEditSettingsUser,
} from '../common';
import {
createUsersAndRoles,
deleteUsersAndRoles,
Expand All @@ -16,8 +24,9 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
const PageObjects = getPageObjects(['security', 'header']);
const testSubjects = getService('testSubjects');
const cases = getService('cases');
const toasts = getService('toasts');

describe('cases deletion sub privilege', () => {
describe('cases sub privilege', () => {
before(async () => {
await createUsersAndRoles(getService, users, roles);
await PageObjects.security.forceLogout();
Expand All @@ -29,7 +38,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await PageObjects.security.forceLogout();
});

describe('create two cases', () => {
describe('cases_delete', () => {
beforeEach(async () => {
await cases.api.createNthRandomCases(2);
});
Expand Down Expand Up @@ -119,6 +128,56 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});
}
});

describe('cases_settings', () => {
afterEach(async () => {
await cases.api.deleteAllCases();
});

for (const user of [casesReadAndEditSettingsUser, casesAllUser]) {
describe(`logging in with user ${user.username}`, () => {
before(async () => {
await PageObjects.security.login(user.username, user.password);
});

after(async () => {
await PageObjects.security.forceLogout();
});

it(`User ${user.username} can navigate to settings`, async () => {
await cases.navigation.navigateToConfigurationPage();
});

it(`User ${user.username} can update settings`, async () => {
await cases.common.selectRadioGroupValue(
'closure-options-radio-group',
'close-by-pushing'
);
const toast = await toasts.getElementByIndex(1);
expect(await toast.getVisibleText()).to.be('Settings successfully updated');
await toasts.dismissAll();
});
});
}

// below users do not have access to settings
for (const user of [casesNoDeleteUser, casesReadDeleteUser]) {
describe(`cannot access settings page with user ${user.username}`, () => {
before(async () => {
await PageObjects.security.login(user.username, user.password);
});

after(async () => {
await PageObjects.security.forceLogout();
});

it(`User ${user.username} cannot navigate to settings`, async () => {
await cases.navigation.navigateToApp();
await testSubjects.missingOrFail('configure-case-button');
});
});
}
});
});
};

Expand Down

0 comments on commit 0d6986e

Please sign in to comment.