maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[APM] Ensure that /api/apm/security/indices_privileges doesn't fail when security is disabled (#64627)

Browse source 
* logging when security api is disable

* logging when security api is disable

* checking statuc code 400

* adding security plugin

* checking if security plugin is enabled before calling it

* fixing unit test

* show apm ui when index is empty

* addressing PR comments

* refactoring

* refactoring

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Cauê Marcondes 2020-05-04 20:22:06 +01:00 committed by GitHub
parent 99a5db6aab
commit 91b27570c1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 264 additions and 96 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
x-pack/plugins/apm/kibana.json
View file

@ -15,7 +15,8 @@
"usageCollection",
"taskManager",
"actions",
"alerting"
"alerting",
"security"
],
"server": true,
"ui": true,

110
x-pack/plugins/apm/public/components/app/APMIndicesPermission/__test__/APMIndicesPermission.test.tsx → x-pack/plugins/apm/public/components/app/APMIndicesPermission/index.test.tsx
View file

@ -4,16 +4,16 @@
* you may not use this file except in compliance with the Elastic License.
*/
import React from 'react';
import { render, fireEvent } from '@testing-library/react';
import { render, fireEvent, act } from '@testing-library/react';
import { shallow } from 'enzyme';
import { APMIndicesPermission } from '../';
import { APMIndicesPermission } from './';
import * as hooks from '../../../../hooks/useFetcher';
import * as hooks from '../../../hooks/useFetcher';
import {
expectTextsInDocument,
expectTextsNotInDocument
} from '../../../../utils/testHelpers';
import { MockApmPluginContextWrapper } from '../../../../context/ApmPluginContext/MockApmPluginContext';
} from '../../../utils/testHelpers';
import { MockApmPluginContextWrapper } from '../../../context/ApmPluginContext/MockApmPluginContext';
describe('APMIndicesPermission', () => {
it('returns empty component when api status is loading', () => {
@ -34,7 +34,10 @@ describe('APMIndicesPermission', () => {
spyOn(hooks, 'useFetcher').and.returnValue({
status: hooks.FETCH_STATUS.SUCCESS,
data: {
'apm-*': { read: false }
has_all_requested: false,
index: {
'apm-*': { read: false }
}
}
});
const component = render(
@ -48,39 +51,32 @@ describe('APMIndicesPermission', () => {
'apm-*'
]);
});
it('shows escape hatch button when at least one indice has read privileges', () => {
it('shows children component when no index is returned', () => {
spyOn(hooks, 'useFetcher').and.returnValue({
status: hooks.FETCH_STATUS.SUCCESS,
data: {
'apm-7.5.1-error-*': { read: false },
'apm-7.5.1-metric-*': { read: false },
'apm-7.5.1-transaction-*': { read: false },
'apm-7.5.1-span-*': { read: true }
has_all_requested: false,
index: {}
}
});
const component = render(
<MockApmPluginContextWrapper>
<APMIndicesPermission />
<APMIndicesPermission>
<p>My amazing component</p>
</APMIndicesPermission>
</MockApmPluginContextWrapper>
);
expectTextsInDocument(component, [
'Missing permissions to access APM',
'apm-7.5.1-error-*',
'apm-7.5.1-metric-*',
'apm-7.5.1-transaction-*',
'Dismiss'
]);
expectTextsNotInDocument(component, ['apm-7.5.1-span-*']);
expectTextsNotInDocument(component, ['Missing permissions to access APM']);
expectTextsInDocument(component, ['My amazing component']);
});
it('shows children component when indices have read privileges', () => {
spyOn(hooks, 'useFetcher').and.returnValue({
status: hooks.FETCH_STATUS.SUCCESS,
data: {
'apm-7.5.1-error-*': { read: true },
'apm-7.5.1-metric-*': { read: true },
'apm-7.5.1-transaction-*': { read: true },
'apm-7.5.1-span-*': { read: true }
has_all_requested: true,
index: {}
}
});
const component = render(
@ -90,6 +86,50 @@ describe('APMIndicesPermission', () => {
</APMIndicesPermission>
</MockApmPluginContextWrapper>
);
expectTextsNotInDocument(component, ['Missing permissions to access APM']);
expectTextsInDocument(component, ['My amazing component']);
});
it('dismesses the warning by clicking on the escape hatch', () => {
spyOn(hooks, 'useFetcher').and.returnValue({
status: hooks.FETCH_STATUS.SUCCESS,
data: {
has_all_requested: false,
index: {
'apm-error-*': { read: false },
'apm-trasanction-*': { read: false },
'apm-metrics-*': { read: true },
'apm-span-*': { read: true }
}
}
});
const component = render(
<MockApmPluginContextWrapper>
<APMIndicesPermission>
<p>My amazing component</p>
</APMIndicesPermission>
</MockApmPluginContextWrapper>
);
expectTextsInDocument(component, [
'Dismiss',
'apm-error-*',
'apm-trasanction-*'
]);
act(() => {
fireEvent.click(component.getByText('Dismiss'));
});
expectTextsInDocument(component, ['My amazing component']);
});
it("shows children component when api doesn't return value", () => {
spyOn(hooks, 'useFetcher').and.returnValue({});
const component = render(
<MockApmPluginContextWrapper>
<APMIndicesPermission>
<p>My amazing component</p>
</APMIndicesPermission>
</MockApmPluginContextWrapper>
);
expectTextsNotInDocument(component, [
'Missing permissions to access APM',
'apm-7.5.1-error-*',
@ -99,26 +139,4 @@ describe('APMIndicesPermission', () => {
]);
expectTextsInDocument(component, ['My amazing component']);
});
it('dismesses the warning by clicking on the escape hatch', () => {
spyOn(hooks, 'useFetcher').and.returnValue({
status: hooks.FETCH_STATUS.SUCCESS,
data: {
'apm-7.5.1-error-*': { read: false },
'apm-7.5.1-metric-*': { read: false },
'apm-7.5.1-transaction-*': { read: false },
'apm-7.5.1-span-*': { read: true }
}
});
const component = render(
<MockApmPluginContextWrapper>
<APMIndicesPermission>
<p>My amazing component</p>
</APMIndicesPermission>
</MockApmPluginContextWrapper>
);
expectTextsInDocument(component, ['Dismiss']);
fireEvent.click(component.getByText('Dismiss'));
expectTextsInDocument(component, ['My amazing component']);
});
});

20
x-pack/plugins/apm/public/components/app/APMIndicesPermission/index.tsx
View file

@ -15,9 +15,9 @@ import {
EuiTitle
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { isEmpty } from 'lodash';
import React, { useState } from 'react';
import styled from 'styled-components';
import { isEmpty } from 'lodash';
import { FETCH_STATUS, useFetcher } from '../../../hooks/useFetcher';
import { fontSize, pct, px, units } from '../../../style/variables';
import { ElasticDocsLink } from '../../shared/Links/ElasticDocsLink';
@ -29,7 +29,7 @@ export const APMIndicesPermission: React.FC = ({ children }) => {
setIsPermissionWarningDismissed
] = useState(false);
const { data: indicesPrivileges = {}, status } = useFetcher(callApmApi => {
const { data: indicesPrivileges, status } = useFetcher(callApmApi => {
return callApmApi({
pathname: '/api/apm/security/indices_privileges'
});
@ -40,13 +40,17 @@ export const APMIndicesPermission: React.FC = ({ children }) => {
return null;
}
const indicesWithoutPermission = Object.keys(indicesPrivileges).filter(
index => !indicesPrivileges[index].read
);
// Show permission warning when a user has at least one index without Read privilege,
// and he has not manually dismissed the warning
if (!isEmpty(indicesWithoutPermission) && !isPermissionWarningDismissed) {
// and they have not manually dismissed the warning
if (
indicesPrivileges &&
!indicesPrivileges.has_all_requested &&
!isEmpty(indicesPrivileges.index) &&
!isPermissionWarningDismissed
) {
const indicesWithoutPermission = Object.keys(
indicesPrivileges.index
).filter(index => !indicesPrivileges.index[index].read);
return (
<PermissionWarning
indicesWithoutPermission={indicesWithoutPermission}

3
x-pack/plugins/apm/server/lib/helpers/es_client.ts
View file

@ -27,9 +27,8 @@ import { getApmIndices } from '../settings/apm_indices/get_apm_indices';
// `type` was deprecated in 7.0
export type APMIndexDocumentParams<T> = Omit<IndexDocumentParams<T>, 'type'>;
interface IndexPrivileges {
export interface IndexPrivileges {
has_all_requested: boolean;
username: string;
index: Record<string, { read: boolean }>;
}

148
x-pack/plugins/apm/server/lib/security/get_indices_privileges.test.ts Normal file
View file

@ -0,0 +1,148 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { Setup } from '../helpers/setup_request';
import { getIndicesPrivileges } from './get_indices_privileges';
describe('getIndicesPrivileges', () => {
const indices = {
apm_oss: {
errorIndices: 'apm-*',
metricsIndices: 'apm-*',
transactionIndices: 'apm-*',
spanIndices: 'apm-*'
}
};
it('return that the user has privileges when security plugin is disabled', async () => {
const setup = ({
indices,
client: {
hasPrivileges: () => {
const error = {
message:
'no handler found for uri [/_security/user/_has_privileges]',
statusCode: 400
};
throw error;
}
}
} as unknown) as Setup;
const privileges = await getIndicesPrivileges({
setup,
isSecurityPluginEnabled: false
});
expect(privileges).toEqual({
has_all_requested: true,
index: {}
});
});
it('throws when an error happens while fetching indices privileges', async () => {
const setup = ({
indices,
client: {
hasPrivileges: () => {
throw new Error('unknow error');
}
}
} as unknown) as Setup;
await expect(
getIndicesPrivileges({ setup, isSecurityPluginEnabled: true })
).rejects.toThrowError('unknow error');
});
it("has privileges to read from 'apm-*'", async () => {
const setup = ({
indices,
client: {
hasPrivileges: () => {
return Promise.resolve({
has_all_requested: true,
index: { 'apm-*': { read: true } }
});
}
}
} as unknown) as Setup;
const privileges = await getIndicesPrivileges({
setup,
isSecurityPluginEnabled: true
});
expect(privileges).toEqual({
has_all_requested: true,
index: {
'apm-*': {
read: true
}
}
});
});
it("doesn't have privileges to read from 'apm-*'", async () => {
const setup = ({
indices,
client: {
hasPrivileges: () => {
return Promise.resolve({
has_all_requested: false,
index: { 'apm-*': { read: false } }
});
}
}
} as unknown) as Setup;
const privileges = await getIndicesPrivileges({
setup,
isSecurityPluginEnabled: true
});
expect(privileges).toEqual({
has_all_requested: false,
index: {
'apm-*': {
read: false
}
}
});
});
it("doesn't have privileges on multiple indices", async () => {
const setup = ({
indices: {
apm_oss: {
errorIndices: 'apm-error-*',
metricsIndices: 'apm-metrics-*',
transactionIndices: 'apm-trasanction-*',
spanIndices: 'apm-span-*'
}
},
client: {
hasPrivileges: () => {
return Promise.resolve({
has_all_requested: false,
index: {
'apm-error-*': { read: false },
'apm-trasanction-*': { read: false },
'apm-metrics-*': { read: true },
'apm-span-*': { read: true }
}
});
}
}
} as unknown) as Setup;
const privileges = await getIndicesPrivileges({
setup,
isSecurityPluginEnabled: true
});
expect(privileges).toEqual({
has_all_requested: false,
index: {
'apm-error-*': { read: false },
'apm-trasanction-*': { read: false },
'apm-metrics-*': { read: true },
'apm-span-*': { read: true }
}
});
});
});

16
x-pack/plugins/apm/server/lib/security/get_indices_privileges.ts
View file

@ -4,8 +4,20 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { Setup } from '../helpers/setup_request';
import { IndexPrivileges } from '../helpers/es_client';
export async function getIndicesPrivileges({
setup,
isSecurityPluginEnabled
}: {
setup: Setup;
isSecurityPluginEnabled: boolean;
}): Promise<IndexPrivileges> {
// When security plugin is not enabled, returns that the user has all requested privileges.
if (!isSecurityPluginEnabled) {
return { has_all_requested: true, index: {} };
}
export async function getIndicesPrivileges(setup: Setup) {
const { client, indices } = setup;
const response = await client.hasPrivileges({
index: [
@ -20,5 +32,5 @@ export async function getIndicesPrivileges(setup: Setup) {
}
]
});
return response.index;
return response;
}

32
x-pack/plugins/apm/server/lib/security/get_permissions.ts
View file

@ -1,32 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { Setup } from '../helpers/setup_request';
export async function getPermissions(setup: Setup) {
const { client, indices } = setup;
const params = {
index: Object.values(indices),
body: {
size: 0,
query: {
match_all: {}
}
}
};
try {
await client.search(params);
return { hasPermission: true };
} catch (e) {
// If 403, it means the user doesnt have permission.
if (e.status === 403) {
return { hasPermission: false };
}
// if any other error happens, throw it.
throw e;
}
}

7
x-pack/plugins/apm/server/plugin.ts
View file

@ -12,6 +12,7 @@ import {
} from 'src/core/server';
import { Observable, combineLatest } from 'rxjs';
import { map, take } from 'rxjs/operators';
import { SecurityPluginSetup } from '../../security/public';
import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/server';
import { TaskManagerSetupContract } from '../../task_manager/server';
import { AlertingPlugin } from '../../alerting/server';
@ -57,6 +58,7 @@ export class APMPlugin implements Plugin<APMPluginSetup> {
alerting?: AlertingPlugin['setup'];
actions?: ActionsPlugin['setup'];
features: FeaturesPluginSetup;
security?: SecurityPluginSetup;
}
) {
this.logger = this.initContext.logger.get();
@ -110,7 +112,10 @@ export class APMPlugin implements Plugin<APMPluginSetup> {
createApmApi().init(core, {
config$: mergedConfig$,
logger: this.logger!
logger: this.logger!,
plugins: {
security: plugins.security
}
});
return {

3
x-pack/plugins/apm/server/routes/create_api/index.test.ts
View file

@ -39,7 +39,8 @@ const getCoreMock = () => {
config$: new BehaviorSubject({} as APMConfig),
logger: ({
error: jest.fn()
} as unknown) as Logger
} as unknown) as Logger,
plugins: {}
}
};
};

5
x-pack/plugins/apm/server/routes/create_api/index.ts
View file

@ -30,7 +30,7 @@ export function createApi() {
factoryFns.push(fn);
return this as any;
},
init(core, { config$, logger }) {
init(core, { config$, logger, plugins }) {
const router = core.http.createRouter();
let config = {} as APMConfig;
@ -141,7 +141,8 @@ export function createApi() {
// it's not defined in the route.
params: pick(parsedParams, ...Object.keys(params), 'query'),
config,
logger
logger,
plugins
}
});

6
x-pack/plugins/apm/server/routes/security.ts
View file

@ -12,6 +12,10 @@ export const indicesPrivilegesRoute = createRoute(() => ({
path: '/api/apm/security/indices_privileges',
handler: async ({ context, request }) => {
const setup = await setupRequest(context, request);
return getIndicesPrivileges(setup);
return getIndicesPrivileges({
setup,
isSecurityPluginEnabled:
context.plugins.security?.license.isEnabled() ?? false
});
}
}));

7
x-pack/plugins/apm/server/routes/typings.ts
View file

@ -16,6 +16,7 @@ import { Observable } from 'rxjs';
import { Server } from 'hapi';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { FetchOptions } from '../../public/services/rest/callApi';
import { SecurityPluginSetup } from '../../../security/public';
import { APMConfig } from '..';
export interface Params {
@ -62,6 +63,9 @@ export type APMRequestHandlerContext<
params: { query: { _debug: boolean } } & TDecodedParams;
config: APMConfig;
logger: Logger;
plugins: {
security?: SecurityPluginSetup;
};
};
export type RouteFactoryFn<
@ -105,6 +109,9 @@ export interface ServerAPI<TRouteState extends RouteState> {
context: {
config$: Observable<APMConfig>;
logger: Logger;
plugins: {
security?: SecurityPluginSetup;
};
}
) => void;
}