Skip to content

Commit

Permalink
Load FLS suggestions on-demand (elastic#98681)
Browse files Browse the repository at this point in the history 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@legrego @kibanamachine
legrego and kibanamachine committed May 5, 2021
1 parent dc0eb11 commit 46dc3ed
Show file tree
Hide file tree
Showing 9 changed files with 325 additions and 73 deletions.
130 changes: 120 additions & 10 deletions ...ns/security/public/management/roles/edit_role/privileges/es/index_privilege_form.test.tsx
View file
Expand Up @@ -5,11 +5,12 @@
* 2.0.
*/

import { EuiButtonIcon, EuiTextArea } from '@elastic/eui';
import { EuiButtonIcon, EuiComboBox, EuiTextArea } from '@elastic/eui';
import React from 'react';

import { mountWithIntl, shallowWithIntl } from '@kbn/test/jest';
import { findTestSubject, mountWithIntl, nextTick, shallowWithIntl } from '@kbn/test/jest';

import { indicesAPIClientMock } from '../../../index.mock';
import { RoleValidator } from '../../validate_role';
import { IndexPrivilegeForm } from './index_privilege_form';

Expand All @@ -25,7 +26,7 @@ test('it renders without crashing', () => {
},
formIndex: 0,
indexPatterns: [],
availableFields: [],
indicesAPIClient: indicesAPIClientMock.create(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
isRoleReadOnly: false,
allowDocumentLevelSecurity: true,
Expand All @@ -52,7 +53,7 @@ test('it allows for custom index privileges', () => {
},
formIndex: 0,
indexPatterns: [],
availableFields: [],
indicesAPIClient: indicesAPIClientMock.create(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
isRoleReadOnly: false,
allowDocumentLevelSecurity: true,
Expand Down Expand Up @@ -86,7 +87,7 @@ describe('delete button', () => {
},
formIndex: 0,
indexPatterns: [],
availableFields: [],
indicesAPIClient: indicesAPIClientMock.create(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
isRoleReadOnly: false,
allowDocumentLevelSecurity: true,
Expand Down Expand Up @@ -138,7 +139,7 @@ describe(`document level security`, () => {
},
formIndex: 0,
indexPatterns: [],
availableFields: [],
indicesAPIClient: indicesAPIClientMock.create(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
isRoleReadOnly: false,
allowDocumentLevelSecurity: true,
Expand Down Expand Up @@ -197,7 +198,7 @@ describe('field level security', () => {
},
formIndex: 0,
indexPatterns: [],
availableFields: [],
indicesAPIClient: indicesAPIClientMock.create(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
isRoleReadOnly: false,
allowDocumentLevelSecurity: true,
Expand All @@ -208,19 +209,21 @@ describe('field level security', () => {
intl: {} as any,
};

test(`inputs are hidden when FLS is not allowed`, () => {
test(`inputs are hidden when FLS is not allowed, and fields are not queried`, async () => {
const testProps = {
...props,
allowFieldLevelSecurity: false,
};

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('EuiSwitch[data-test-subj="restrictFieldsQuery0"]')).toHaveLength(0);
expect(wrapper.find('.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(0);
expect(wrapper.find('.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(0);
expect(testProps.indicesAPIClient.getFields).not.toHaveBeenCalled();
});

test('only the switch is shown when allowed, and FLS is empty', () => {
test('renders the FLS switch when available, but collapsed when no fields are selected', async () => {
const testProps = {
...props,
indexPrivilege: {
Expand All @@ -230,19 +233,126 @@ describe('field level security', () => {
};

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('EuiSwitch[data-test-subj="restrictFieldsQuery0"]')).toHaveLength(1);
expect(wrapper.find('.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(0);
expect(wrapper.find('.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(0);
expect(testProps.indicesAPIClient.getFields).not.toHaveBeenCalled();
});

test('inputs are shown when allowed', () => {
test('FLS inputs are shown when allowed', async () => {
const testProps = {
...props,
};

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('div.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(1);
expect(wrapper.find('div.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(1);
expect(testProps.indicesAPIClient.getFields).not.toHaveBeenCalled();
});

test('does not query for available fields when a request is already in flight', async () => {
jest.useFakeTimers();

const testProps = {
...props,
indexPrivilege: {
...props.indexPrivilege,
names: ['foo', 'bar-*'],
},
indicesAPIClient: indicesAPIClientMock.create(),
};

testProps.indicesAPIClient.getFields.mockImplementation(async () => {
return new Promise((resolve) =>
setTimeout(() => {
resolve(['foo']);
}, 5000)
);
});

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('div.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(1);
expect(wrapper.find('div.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(1);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(1);

findTestSubject(wrapper, 'fieldInput0').simulate('focus');
jest.advanceTimersByTime(2000);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(1);

findTestSubject(wrapper, 'fieldInput0').simulate('focus');
jest.advanceTimersByTime(4000);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(1);
});

test('queries for available fields when mounted, and FLS is available', async () => {
const testProps = {
...props,
indexPrivilege: {
...props.indexPrivilege,
names: ['foo', 'bar-*'],
},
indicesAPIClient: indicesAPIClientMock.create(),
};

testProps.indicesAPIClient.getFields.mockResolvedValue(['a', 'b', 'c']);

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('div.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(1);
expect(wrapper.find('div.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(1);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(1);
});

test('does not query for available fields when mounted, and FLS is unavailable', async () => {
const testProps = {
...props,
indexPrivilege: {
...props.indexPrivilege,
names: ['foo', 'bar-*'],
},
indicesAPIClient: indicesAPIClientMock.create(),
allowFieldLevelSecurity: false,
};

testProps.indicesAPIClient.getFields.mockResolvedValue(['a', 'b', 'c']);

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('div.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(0);
expect(wrapper.find('div.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(0);
expect(testProps.indicesAPIClient.getFields).not.toHaveBeenCalled();
});

test('queries for available fields when the set of index patterns change', async () => {
const testProps = {
...props,
indexPrivilege: {
...props.indexPrivilege,
names: ['foo', 'bar-*'],
},
indexPatterns: ['foo', 'bar-*', 'newPattern'],
indicesAPIClient: indicesAPIClientMock.create(),
};

testProps.indicesAPIClient.getFields.mockResolvedValue(['a', 'b', 'c']);

const wrapper = mountWithIntl(<IndexPrivilegeForm {...testProps} />);
await nextTick();
expect(wrapper.find('div.indexPrivilegeForm__grantedFieldsRow')).toHaveLength(1);
expect(wrapper.find('div.indexPrivilegeForm__deniedFieldsRow')).toHaveLength(1);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(1);

wrapper
.find(EuiComboBox)
.filterWhere((item) => item.props()['data-test-subj'] === 'indicesInput0')
.props().onChange!([{ label: 'newPattern', value: 'newPattern' }]);

await nextTick();
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledTimes(2);
expect(testProps.indicesAPIClient.getFields).toHaveBeenCalledWith('newPattern');
});

test('it displays a warning when no fields are granted', () => {
Expand Down
64 changes: 59 additions & 5 deletions ...plugins/security/public/management/roles/edit_role/privileges/es/index_privilege_form.tsx
View file
Expand Up @@ -23,8 +23,10 @@ import React, { Component, Fragment } from 'react';

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import type { PublicMethodsOf } from '@kbn/utility-types';

import type { RoleIndexPrivilege } from '../../../../../../common/model';
import type { IndicesAPIClient } from '../../../indices_api_client';
import type { RoleValidator } from '../../validate_role';

const fromOption = (option: any) => option.label;
Expand All @@ -35,7 +37,7 @@ interface Props {
indexPrivilege: RoleIndexPrivilege;
indexPatterns: string[];
availableIndexPrivileges: string[];
availableFields: string[];
indicesAPIClient: PublicMethodsOf<IndicesAPIClient>;
onChange: (indexPrivilege: RoleIndexPrivilege) => void;
onDelete: () => void;
isRoleReadOnly: boolean;
Expand All @@ -50,9 +52,16 @@ interface State {
grantedFields: string[];
exceptedFields: string[];
documentQuery?: string;
isFieldListLoading: boolean;
flsOptions: string[];
}

export class IndexPrivilegeForm extends Component<Props, State> {
// This is distinct from the field within `this.state`.
// We want to make sure that only one request for fields is in-flight at a time,
// and relying on state for this is error prone.
private isFieldListLoading: boolean = false;

constructor(props: Props) {
super(props);

Expand All @@ -64,9 +73,17 @@ export class IndexPrivilegeForm extends Component<Props, State> {
grantedFields: grant,
exceptedFields: except,
documentQuery: props.indexPrivilege.query,
isFieldListLoading: false,
flsOptions: [],
};
}

public componentDidMount() {
if (this.state.fieldSecurityExpanded && this.props.allowFieldLevelSecurity) {
this.loadFLSOptions(this.props.indexPrivilege.names);
}
}

public render() {
return (
<Fragment>
Expand Down Expand Up @@ -149,11 +166,30 @@ export class IndexPrivilegeForm extends Component<Props, State> {
);
};

private loadFLSOptions = (indexNames: string[], force = false) => {
if (!force && (this.isFieldListLoading || indexNames.length === 0)) return;

this.isFieldListLoading = true;
this.setState({
isFieldListLoading: true,
});

this.props.indicesAPIClient
.getFields(indexNames.join(','))
.then((fields) => {
this.isFieldListLoading = false;
this.setState({ flsOptions: fields, isFieldListLoading: false });
})
.catch(() => {
this.isFieldListLoading = false;
this.setState({ flsOptions: [], isFieldListLoading: false });
});
};

private getFieldLevelControls = () => {
const {
allowFieldLevelSecurity,
allowDocumentLevelSecurity,
availableFields,
indexPrivilege,
isRoleReadOnly,
} = this.props;
Expand Down Expand Up @@ -210,11 +246,13 @@ export class IndexPrivilegeForm extends Component<Props, State> {
<Fragment>
<EuiComboBox
data-test-subj={`fieldInput${this.props.formIndex}`}
options={availableFields ? availableFields.map(toOption) : []}
options={this.state.flsOptions.map(toOption)}
selectedOptions={grant.map(toOption)}
onCreateOption={this.onCreateGrantedField}
onChange={this.onGrantedFieldsChange}
isDisabled={this.props.isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</Fragment>
</EuiFormRow>
Expand All @@ -233,11 +271,13 @@ export class IndexPrivilegeForm extends Component<Props, State> {
<Fragment>
<EuiComboBox
data-test-subj={`deniedFieldInput${this.props.formIndex}`}
options={availableFields ? availableFields.map(toOption) : []}
options={this.state.flsOptions.map(toOption)}
selectedOptions={except.map(toOption)}
onCreateOption={this.onCreateDeniedField}
onChange={this.onDeniedFieldsChange}
isDisabled={isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</Fragment>
</EuiFormRow>
Expand Down Expand Up @@ -362,6 +402,11 @@ export class IndexPrivilegeForm extends Component<Props, State> {
const hasSavedFieldSecurity =
this.state.exceptedFields.length > 0 || this.state.grantedFields.length > 0;

// If turning on, then request available fields
if (willToggleOn) {
this.loadFLSOptions(this.props.indexPrivilege.names);
}

if (willToggleOn && !hasConfiguredFieldSecurity && hasSavedFieldSecurity) {
this.props.onChange({
...this.props.indexPrivilege,
Expand All @@ -380,13 +425,22 @@ export class IndexPrivilegeForm extends Component<Props, State> {
...this.props.indexPrivilege,
names: newIndexPatterns,
});
// If FLS controls are visible, then forcefully request a new set of options
if (this.state.fieldSecurityExpanded) {
this.loadFLSOptions(newIndexPatterns, true);
}
};

private onIndexPatternsChange = (newPatterns: EuiComboBoxOptionOption[]) => {
const names = newPatterns.map(fromOption);
this.props.onChange({
...this.props.indexPrivilege,
names: newPatterns.map(fromOption),
names,
});
// If FLS controls are visible, then forcefully request a new set of options
if (this.state.fieldSecurityExpanded) {
this.loadFLSOptions(names, true);
}
};

private onPrivilegeChange = (newPrivileges: EuiComboBoxOptionOption[]) => {
Expand Down
5 changes: 4 additions & 1 deletion ...lugins/security/public/management/roles/edit_role/privileges/es/index_privileges.test.tsx
View file
Expand Up @@ -56,6 +56,9 @@ test('it renders a IndexPrivilegeForm for each privilege on the role', async ()
allowRoleDocumentLevelSecurity: true,
} as any);

const indicesAPIClient = indicesAPIClientMock.create();
indicesAPIClient.getFields.mockResolvedValue(['foo']);

const props = {
role: {
name: '',
Expand All @@ -80,7 +83,7 @@ test('it renders a IndexPrivilegeForm for each privilege on the role', async ()
editable: true,
validator: new RoleValidator(),
availableIndexPrivileges: ['all', 'read', 'write', 'index'],
indicesAPIClient: indicesAPIClientMock.create(),
indicesAPIClient,
license,
};
const wrapper = mountWithIntl(<IndexPrivileges {...props} />);
Expand Down

0 comments on commit 46dc3ed

Please sign in to comment.