Skip to content

Commit

Permalink
Disable plugin access if a normal user does not have access to App Se…
Browse files Browse the repository at this point in the history
…arch (#19)

* Set up new server security dependency and configs

* Set up access capabilities

* Set up checkAccess helper/caller

* Remove NoUserState component from the public UI

- Since this is now being handled by checkAccess / normal users should never see the plugin at all if they don't have an account/access, the component is no longer needed

* Update server routes to account for new changes

- Remove login redirect catch from routes, since the access helper should now handle that for most users by disabling the plugin (superusers will see a generic cannot connect/error screen)
- Refactor out new config values to a shared mock

* Refactor Enterprise Search http call to hit/return new internal API endpoint

+ pull out the http call to a separate library for upcoming public URL work (so that other files can call it directly as well)

* [Discussion] Increase timeout but add another warning timeout for slow servers

- per recommendation/convo with Brandon

* Register feature control

* Remove no_as_account from UI telemetry

- since we're no longer tracking that in the UI

* Address PR feedback - isSuperUser check
  • Loading branch information
Constance authored and cee-chen committed Jul 7, 2020
1 parent df577cf commit e6fc554
Show file tree
Hide file tree
Showing 24 changed files with 497 additions and 212 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/enterprise_search/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
"id": "enterpriseSearch",
"version": "1.0.0",
"kibanaVersion": "kibana",
"requiredPlugins": ["home", "licensing"],
"requiredPlugins": ["home", "features", "licensing"],
"configPath": ["enterpriseSearch"],
"optionalPlugins": ["usageCollection"],
"optionalPlugins": ["usageCollection", "security"],
"server": true,
"ui": true
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,15 @@ import '../../../__mocks__/shallow_usecontext.mock';

import React from 'react';
import { shallow } from 'enzyme';
import { EuiEmptyPrompt, EuiButton, EuiCode, EuiLoadingContent } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { shallowWithIntl } from '../../../__mocks__';

jest.mock('../../../shared/get_username', () => ({ getUserName: jest.fn() }));
import { getUserName } from '../../../shared/get_username';
import { EuiEmptyPrompt, EuiButton, EuiLoadingContent } from '@elastic/eui';

jest.mock('../../../shared/telemetry', () => ({
sendTelemetry: jest.fn(),
SendAppSearchTelemetry: jest.fn(),
}));
import { sendTelemetry } from '../../../shared/telemetry';

import { ErrorState, NoUserState, EmptyState, LoadingState } from './';
import { ErrorState, EmptyState, LoadingState } from './';

describe('ErrorState', () => {
it('renders', () => {
Expand All @@ -31,24 +26,6 @@ describe('ErrorState', () => {
});
});

describe('NoUserState', () => {
it('renders', () => {
const wrapper = shallow(<NoUserState />);

expect(wrapper.find(EuiEmptyPrompt)).toHaveLength(1);
});

it('renders with username', () => {
(getUserName as jest.Mock).mockImplementationOnce(() => 'dolores-abernathy');

const wrapper = shallowWithIntl(<NoUserState />);
const prompt = wrapper.find(EuiEmptyPrompt).dive();
const description1 = prompt.find(FormattedMessage).at(1).dive();

expect(description1.find(EuiCode).prop('children')).toContain('dolores-abernathy');
});
});

describe('EmptyState', () => {
it('renders', () => {
const wrapper = shallow(<EmptyState />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@

export { LoadingState } from './loading_state';
export { EmptyState } from './empty_state';
export { NoUserState } from './no_user_state';
export { ErrorState } from './error_state';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { KibanaContext } from '../../../';
import { LicenseContext } from '../../../shared/licensing';
import { mountWithContext, mockKibanaContext } from '../../../__mocks__';

import { EmptyState, ErrorState, NoUserState } from '../empty_states';
import { EmptyState, ErrorState } from '../empty_states';
import { EngineTable, IEngineTablePagination } from './engine_table';

import { EngineOverview } from './';
Expand Down Expand Up @@ -56,13 +56,6 @@ describe('EngineOverview', () => {
});
expect(wrapper.find(ErrorState)).toHaveLength(1);
});

it('hasNoAccount', async () => {
const wrapper = await mountWithApiMock({
get: () => Promise.reject({ body: { message: 'no-as-account' } }),
});
expect(wrapper.find(NoUserState)).toHaveLength(1);
});
});

describe('happy-path states', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { KibanaContext, IKibanaContext } from '../../../index';
import EnginesIcon from '../../assets/engine.svg';
import MetaEnginesIcon from '../../assets/meta_engine.svg';

import { LoadingState, EmptyState, NoUserState, ErrorState } from '../empty_states';
import { LoadingState, EmptyState, ErrorState } from '../empty_states';
import { EngineOverviewHeader } from '../engine_overview_header';
import { EngineTable } from './engine_table';

Expand All @@ -35,7 +35,6 @@ export const EngineOverview: React.FC = () => {
const { license } = useContext(LicenseContext) as ILicenseContext;

const [isLoading, setIsLoading] = useState(true);
const [hasNoAccount, setHasNoAccount] = useState(false);
const [hasErrorConnecting, setHasErrorConnecting] = useState(false);

const [engines, setEngines] = useState([]);
Expand All @@ -59,11 +58,7 @@ export const EngineOverview: React.FC = () => {

setIsLoading(false);
} catch (error) {
if (error?.body?.message === 'no-as-account') {
setHasNoAccount(true);
} else {
setHasErrorConnecting(true);
}
setHasErrorConnecting(true);
}
};

Expand All @@ -84,7 +79,6 @@ export const EngineOverview: React.FC = () => {
}, [license, metaEnginesPage]);

if (hasErrorConnecting) return <ErrorState />;
if (hasNoAccount) return <NoUserState />;
if (isLoading) return <LoadingState />;
if (!engines.length) return <EmptyState />;

Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('App Search Telemetry Usage Collector', () => {
'ui_viewed.setup_guide': 10,
'ui_viewed.engines_overview': 20,
'ui_error.cannot_connect': 3,
'ui_error.no_as_account': 4,
'ui_clicked.create_first_engine_button': 40,
'ui_clicked.header_launch_button': 50,
'ui_clicked.engine_table_link': 60,
Expand Down Expand Up @@ -64,7 +63,6 @@ describe('App Search Telemetry Usage Collector', () => {
},
ui_error: {
cannot_connect: 3,
no_as_account: 4,
},
ui_clicked: {
create_first_engine_button: 40,
Expand All @@ -87,7 +85,6 @@ describe('App Search Telemetry Usage Collector', () => {
},
ui_error: {
cannot_connect: 0,
no_as_account: 0,
},
ui_clicked: {
create_first_engine_button: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ interface ITelemetry {
};
ui_error: {
cannot_connect: number;
no_as_account: number;
};
ui_clicked: {
create_first_engine_button: number;
Expand Down Expand Up @@ -49,7 +48,6 @@ export const registerTelemetryUsageCollector = (
},
ui_error: {
cannot_connect: { type: 'long' },
no_as_account: { type: 'long' },
},
ui_clicked: {
create_first_engine_button: { type: 'long' },
Expand Down Expand Up @@ -78,7 +76,6 @@ const fetchTelemetryMetrics = async (savedObjects: SavedObjectsServiceStart) =>
},
ui_error: {
cannot_connect: 0,
no_as_account: 0,
},
ui_clicked: {
create_first_engine_button: 0,
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/enterprise_search/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export const plugin = (initializerContext: PluginInitializerContext) => {

export const configSchema = schema.object({
host: schema.maybe(schema.string()),
enabled: schema.boolean({ defaultValue: true }),
accessCheckTimeout: schema.number({ defaultValue: 5000 }),
accessCheckTimeoutWarning: schema.number({ defaultValue: 300 }),
});

type ConfigType = TypeOf<typeof configSchema>;
Expand Down
Loading

0 comments on commit e6fc554

Please sign in to comment.