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.

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

[APM] Run API tests as restricted user #70050

Merged
merged 6 commits into from
Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,20 @@ import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function agentConfigurationTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const supertestRead = getService('supertestAsApmReadUser');
const supertestWrite = getService('supertestAsApmWriteUser');
const log = getService('log');

function searchConfigurations(configuration: any) {
return supertest
return supertestRead
.post(`/api/apm/settings/agent-configuration/search`)
.send(configuration)
.set('kbn-xsrf', 'foo');
}

async function createConfiguration(config: AgentConfigurationIntake) {
log.debug('creating configuration', config.service);
const res = await supertest
const res = await supertestWrite
.put(`/api/apm/settings/agent-configuration`)
.send(config)
.set('kbn-xsrf', 'foo');
Expand All @@ -34,7 +35,7 @@ export default function agentConfigurationTests({ getService }: FtrProviderConte

async function updateConfiguration(config: AgentConfigurationIntake) {
log.debug('updating configuration', config.service);
const res = await supertest
const res = await supertestWrite
.put(`/api/apm/settings/agent-configuration?overwrite=true`)
.send(config)
.set('kbn-xsrf', 'foo');
Expand All @@ -46,7 +47,7 @@ export default function agentConfigurationTests({ getService }: FtrProviderConte

async function deleteConfiguration({ service }: AgentConfigurationIntake) {
log.debug('deleting configuration', service);
const res = await supertest
const res = await supertestWrite
.delete(`/api/apm/settings/agent-configuration`)
.send({ service })
.set('kbn-xsrf', 'foo');
Expand Down
6 changes: 3 additions & 3 deletions x-pack/test/apm_api_integration/basic/tests/annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function annotationApiTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const supertestWrite = getService('supertestAsApmWriteUser');

function request({ method, url, data }: { method: string; url: string; data?: JsonObject }) {
switch (method.toLowerCase()) {
case 'post':
return supertest.post(url).send(data).set('kbn-xsrf', 'foo');
return supertestWrite.post(url).send(data).set('kbn-xsrf', 'foo');

default:
throw new Error(`Unsupported methoed ${method}`);
throw new Error(`Unsupported method ${method}`);
}
}

Expand Down
11 changes: 6 additions & 5 deletions x-pack/test/apm_api_integration/basic/tests/custom_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@ import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function customLinksTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const supertestRead = getService('supertestAsApmReadUser');
const supertestWrite = getService('supertestAsApmWriteUser');
const log = getService('log');

function searchCustomLinks(filters?: any) {
const path = URL.format({
pathname: `/api/apm/settings/custom_links`,
query: filters,
});
return supertest.get(path).set('kbn-xsrf', 'foo');
return supertestRead.get(path).set('kbn-xsrf', 'foo');
}

async function createCustomLink(customLink: CustomLink) {
log.debug('creating configuration', customLink);
const res = await supertest
const res = await supertestWrite
.post(`/api/apm/settings/custom_links`)
.send(customLink)
.set('kbn-xsrf', 'foo');
Expand All @@ -35,7 +36,7 @@ export default function customLinksTests({ getService }: FtrProviderContext) {

async function updateCustomLink(id: string, customLink: CustomLink) {
log.debug('updating configuration', id, customLink);
const res = await supertest
const res = await supertestWrite
.put(`/api/apm/settings/custom_links/${id}`)
.send(customLink)
.set('kbn-xsrf', 'foo');
Expand All @@ -47,7 +48,7 @@ export default function customLinksTests({ getService }: FtrProviderContext) {

async function deleteCustomLink(id: string) {
log.debug('deleting configuration', id);
const res = await supertest
const res = await supertestWrite
.delete(`/api/apm/settings/custom_links/${id}`)
.set('kbn-xsrf', 'foo');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { FtrProviderContext } from '../../common/ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function featureControlsTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const supertest = getService('supertestAsApmWriteUser');
const supertestWithoutAuth = getService('supertestWithoutAuth');
const security = getService('security');
const spaces = getService('spaces');
Expand Down
83 changes: 83 additions & 0 deletions x-pack/test/apm_api_integration/common/authentication.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* 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 { PromiseReturnType } from '../../../plugins/apm/typings/common';
import { SecurityServiceProvider } from '../../../../test/common/services/security';

type SecurityService = PromiseReturnType<typeof SecurityServiceProvider>;

export enum ApmUser {
APM_READ_USER = 'apm_read_user',
APM_WRITE_USER = 'apm_write_user',
}

export async function createApmUser(security: SecurityService, apmUser: ApmUser) {
switch (apmUser) {
case ApmUser.APM_READ_USER:
await security.role.create(ApmUser.APM_READ_USER, {
elasticsearch: {
cluster: [],
indices: [
{ names: ['observability-annotations'], privileges: ['read', 'view_index_metadata'] },
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't the plan to make this part of the apm_user?

Copy link
Member Author

@dgieselaar dgieselaar Jun 29, 2020

Choose a reason for hiding this comment

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

Yes, but didn't want to block on the ES PR. If the ES PR gets merged first, I will update this before merging.

Copy link
Member

Choose a reason for hiding this comment

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

👍

],
},
kibana: [
{
base: [],
feature: {
apm: ['read'],
},
spaces: ['*'],
},
],
});
await security.user.create(ApmUser.APM_READ_USER, {
full_name: ApmUser.APM_READ_USER,
password: APM_TEST_PASSWORD,
roles: ['apm_user', ApmUser.APM_READ_USER],
});
break;

case ApmUser.APM_WRITE_USER:
await security.role.create(ApmUser.APM_WRITE_USER, {
elasticsearch: {
cluster: [],
indices: [
{
names: ['observability-annotations'],
privileges: [
'read',
'view_index_metadata',
'index',
'manage',
'create_index',
'create_doc',
],
},
],
},
kibana: [
{
base: [],
feature: {
apm: ['all'],
},
spaces: ['*'],
},
],
});

await security.user.create(ApmUser.APM_WRITE_USER, {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it could be confusing with identical names for the user and role?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I don't think so. Do you see a scenario in our test where they could be confused?

Copy link
Member

Choose a reason for hiding this comment

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

No, probably fine 👍

full_name: ApmUser.APM_WRITE_USER,
password: APM_TEST_PASSWORD,
roles: ['apm_user', ApmUser.APM_WRITE_USER],
});
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be more clear to have separate functions: createApmReadUser, createApmReadWriteUser.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will make it more declarative. I need to add another user, one that is permitted to index annotations.


break;
}
}

export const APM_TEST_PASSWORD = 'changeme';
38 changes: 35 additions & 3 deletions x-pack/test/apm_api_integration/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,34 @@
*/

import { FtrConfigProviderContext } from '@kbn/test/types/ftr';
import supertestAsPromised from 'supertest-as-promised';
import { format, UrlObject } from 'url';
import { InheritedFtrProviderContext, InheritedServices } from './ftr_provider_context';
import { PromiseReturnType } from '../../../plugins/apm/typings/common';
import { createApmUser, APM_TEST_PASSWORD, ApmUser } from './authentication';

interface Settings {
license: 'basic' | 'trial';
testFiles: string[];
name: string;
}

const supertestAsApmUser = (kibanaServer: UrlObject, apmUser: ApmUser) => async (
context: InheritedFtrProviderContext
) => {
const security = context.getService('security');
await security.init();

await createApmUser(security, apmUser);

const url = format({
...kibanaServer,
auth: `${apmUser}:${APM_TEST_PASSWORD}`,
});

return supertestAsPromised(url);
};

export function createTestConfig(settings: Settings) {
const { testFiles, license, name } = settings;

Expand All @@ -20,14 +41,23 @@ export function createTestConfig(settings: Settings) {
require.resolve('../../api_integration/config.ts')
);

const services = xPackAPITestsConfig.get('services') as InheritedServices;
const servers = xPackAPITestsConfig.get('servers');

const supertestAsApmReadUser = supertestAsApmUser(servers.kibana, ApmUser.APM_READ_USER);

return {
testFiles,
servers: xPackAPITestsConfig.get('servers'),
services: xPackAPITestsConfig.get('services'),
servers,
services: {
...services,
supertest: supertestAsApmReadUser,
supertestAsApmReadUser,
supertestAsApmWriteUser: supertestAsApmUser(servers.kibana, ApmUser.APM_WRITE_USER),
},
junit: {
reportName: name,
},

esTestCluster: {
...xPackAPITestsConfig.get('esTestCluster'),
license,
Expand All @@ -36,3 +66,5 @@ export function createTestConfig(settings: Settings) {
};
};
}

export type ApmServices = PromiseReturnType<ReturnType<typeof createTestConfig>>['services'];
14 changes: 13 additions & 1 deletion x-pack/test/apm_api_integration/common/ftr_provider_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,16 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { FtrProviderContext } from '../../api_integration/ftr_provider_context';
import { GenericFtrProviderContext } from '@kbn/test/types/ftr';
import { FtrProviderContext as InheritedFtrProviderContext } from '../../api_integration/ftr_provider_context';
import { ApmServices } from './config';

export type InheritedServices = InheritedFtrProviderContext extends GenericFtrProviderContext<
infer TServices,
{}
>
? TServices
: {};

export { InheritedFtrProviderContext };
export type FtrProviderContext = GenericFtrProviderContext<ApmServices, {}>;
9 changes: 5 additions & 4 deletions x-pack/test/apm_api_integration/trial/tests/annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const DEFAULT_INDEX_NAME = 'observability-annotations';

// eslint-disable-next-line import/no-default-export
export default function annotationApiTests({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const supertestRead = getService('supertestAsApmReadUser');
const supertestWrite = getService('supertestAsApmWriteUser');
const es = getService('es');

function expectContainsObj(source: JsonObject, expected: JsonObject) {
Expand All @@ -30,13 +31,13 @@ export default function annotationApiTests({ getService }: FtrProviderContext) {
function request({ method, url, data }: { method: string; url: string; data?: JsonObject }) {
switch (method.toLowerCase()) {
case 'get':
return supertest.get(url).set('kbn-xsrf', 'foo');
return supertestRead.get(url).set('kbn-xsrf', 'foo');

case 'post':
return supertest.post(url).send(data).set('kbn-xsrf', 'foo');
return supertestWrite.post(url).send(data).set('kbn-xsrf', 'foo');

default:
throw new Error(`Unsupported methoed ${method}`);
throw new Error(`Unsupported method ${method}`);
}
}

Expand Down