[Reporting/JobListing] fix user ID for non-security in queries (#75365)

* [Reporting/JobListing] fix user ID for non-security in queries

* fix tests

* add fn api test

* fix ci

* revert TS exploration
This commit is contained in:
Tim Sullivan 2020-08-19 13:52:43 -07:00 committed by GitHub
parent ad5c0f58fe
commit e48a5672c0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 242 additions and 49 deletions

View file

@ -6,15 +6,14 @@
import { KibanaRequest, RequestHandlerContext } from 'src/core/server';
import { ReportingCore } from '../';
import { AuthenticatedUser } from '../../../security/server';
import { CreateJobBaseParams, CreateJobFn } from '../types';
import { CreateJobBaseParams, CreateJobFn, ReportingUser } from '../types';
import { LevelLogger } from './';
import { Report } from './store';
export type EnqueueJobFn = (
exportTypeId: string,
jobParams: CreateJobBaseParams,
user: AuthenticatedUser | null,
user: ReportingUser,
context: RequestHandlerContext,
request: KibanaRequest
) => Promise<Report>;
@ -28,13 +27,12 @@ export function enqueueJobFactory(
return async function enqueueJob(
exportTypeId: string,
jobParams: CreateJobBaseParams,
user: AuthenticatedUser | null,
user: ReportingUser,
context: RequestHandlerContext,
request: KibanaRequest
) {
type ScheduleTaskFnType = CreateJobFn<CreateJobBaseParams>;
const username: string | null = user ? user.username : null;
const exportType = reporting.getExportTypesRegistry().getById(exportTypeId);
if (exportType == null) {
@ -50,7 +48,7 @@ export function enqueueJobFactory(
const payload = await scheduleTask(jobParams, context, request);
// store the pending report, puts it in the Reporting Management UI table
const report = await store.addReport(exportType.jobType, username, payload);
const report = await store.addReport(exportType.jobType, user, payload);
logger.info(`Scheduled ${exportType.name} report: ${report._id}`);

View file

@ -18,7 +18,7 @@ interface ReportingDocument {
_seq_no: unknown;
_primary_term: unknown;
jobtype: string;
created_by: string | null;
created_by: string | false;
payload: {
headers: string; // encrypted headers
objectType: string;
@ -53,7 +53,7 @@ export class Report implements Partial<ReportingDocument> {
public readonly jobtype: string;
public readonly created_at?: string;
public readonly created_by?: string | null;
public readonly created_by?: string | false;
public readonly payload: {
headers: string; // encrypted headers
objectType: string;

View file

@ -53,7 +53,9 @@ describe('ReportingStore', () => {
headers: 'rp_headers_1',
objectType: 'testOt',
};
await expect(store.addReport(reportType, 'username1', reportPayload)).resolves.toMatchObject({
await expect(
store.addReport(reportType, { username: 'username1' }, reportPayload)
).resolves.toMatchObject({
_primary_term: undefined,
_seq_no: undefined,
attempts: 0,
@ -84,9 +86,9 @@ describe('ReportingStore', () => {
headers: 'rp_headers_2',
objectType: 'testOt',
};
expect(store.addReport(reportType, 'user1', reportPayload)).rejects.toMatchInlineSnapshot(
`[Error: Invalid index interval: centurially]`
);
expect(
store.addReport(reportType, { username: 'user1' }, reportPayload)
).rejects.toMatchInlineSnapshot(`[Error: Invalid index interval: centurially]`);
});
it('handles error creating the index', async () => {
@ -102,7 +104,7 @@ describe('ReportingStore', () => {
objectType: 'testOt',
};
await expect(
store.addReport(reportType, 'user1', reportPayload)
store.addReport(reportType, { username: 'user1' }, reportPayload)
).rejects.toMatchInlineSnapshot(`[Error: horrible error]`);
});
@ -125,7 +127,7 @@ describe('ReportingStore', () => {
objectType: 'testOt',
};
await expect(
store.addReport(reportType, 'user1', reportPayload)
store.addReport(reportType, { username: 'user1' }, reportPayload)
).rejects.toMatchInlineSnapshot(`[Error: devastating error]`);
});
@ -143,7 +145,9 @@ describe('ReportingStore', () => {
headers: 'rp_headers_5',
objectType: 'testOt',
};
await expect(store.addReport(reportType, 'user1', reportPayload)).resolves.toMatchObject({
await expect(
store.addReport(reportType, { username: 'user1' }, reportPayload)
).resolves.toMatchObject({
_primary_term: undefined,
_seq_no: undefined,
attempts: 0,
@ -160,7 +164,7 @@ describe('ReportingStore', () => {
});
});
it('allows username string to be `null`', async () => {
it('allows username string to be `false`', async () => {
// setup
callClusterStub.withArgs('indices.exists').resolves(false);
callClusterStub
@ -174,13 +178,13 @@ describe('ReportingStore', () => {
headers: 'rp_test_headers',
objectType: 'testOt',
};
await expect(store.addReport(reportType, null, reportPayload)).resolves.toMatchObject({
await expect(store.addReport(reportType, false, reportPayload)).resolves.toMatchObject({
_primary_term: undefined,
_seq_no: undefined,
attempts: 0,
browser_type: undefined,
completed_at: undefined,
created_by: null,
created_by: false,
jobtype: 'unknowntype',
max_attempts: undefined,
payload: {},

View file

@ -7,7 +7,11 @@
import { ElasticsearchServiceSetup } from 'src/core/server';
import { LevelLogger, statuses } from '../';
import { ReportingCore } from '../../';
import { CreateJobBaseParams, CreateJobBaseParamsEncryptedFields } from '../../types';
import {
CreateJobBaseParams,
CreateJobBaseParamsEncryptedFields,
ReportingUser,
} from '../../types';
import { indexTimestamp } from './index_timestamp';
import { mapping } from './mapping';
import { Report } from './report';
@ -140,7 +144,7 @@ export class ReportingStore {
public async addReport(
type: string,
username: string | null,
user: ReportingUser,
payload: CreateJobBaseParams & CreateJobBaseParamsEncryptedFields
): Promise<Report> {
const timestamp = indexTimestamp(this.indexInterval);
@ -151,7 +155,7 @@ export class ReportingStore {
_index: index,
payload,
jobtype: type,
created_by: username,
created_by: user ? user.username : false,
...this.jobSettings,
});

View file

@ -46,7 +46,7 @@ describe('authorized_user_pre_routing', function () {
mockCore = await createMockReportingCore(mockReportingConfig);
});
it('should return from handler with a "null" user when security plugin is not found', async function () {
it('should return from handler with a "false" user when security plugin is not found', async function () {
mockCore.getPluginSetupDeps = () =>
(({
// @ts-ignore
@ -58,7 +58,7 @@ describe('authorized_user_pre_routing', function () {
let handlerCalled = false;
authorizedUserPreRouting((user: unknown) => {
expect(user).toBe(null); // verify the user is a null value
expect(user).toBe(false); // verify the user is a false value
handlerCalled = true;
return Promise.resolve({ status: 200, options: {} });
})(getMockContext(), getMockRequest(), mockResponseFactory);
@ -66,7 +66,7 @@ describe('authorized_user_pre_routing', function () {
expect(handlerCalled).toBe(true);
});
it('should return from handler with a "null" user when security is disabled', async function () {
it('should return from handler with a "false" user when security is disabled', async function () {
mockCore.getPluginSetupDeps = () =>
(({
// @ts-ignore
@ -82,7 +82,7 @@ describe('authorized_user_pre_routing', function () {
let handlerCalled = false;
authorizedUserPreRouting((user: unknown) => {
expect(user).toBe(null); // verify the user is a null value
expect(user).toBe(false); // verify the user is a false value
handlerCalled = true;
return Promise.resolve({ status: 200, options: {} });
})(getMockContext(), getMockRequest(), mockResponseFactory);

View file

@ -9,11 +9,11 @@ import { AuthenticatedUser } from '../../../../security/server';
import { ReportingCore } from '../../core';
import { getUserFactory } from './get_user';
type ReportingUser = AuthenticatedUser | null;
const superuserRole = 'superuser';
type ReportingRequestUser = AuthenticatedUser | false;
export type RequestHandlerUser<P, Q, B> = RequestHandler<P, Q, B> extends (...a: infer U) => infer R
? (user: ReportingUser, ...a: U) => R
? (user: ReportingRequestUser, ...a: U) => R
: never;
export const authorizedUserPreRoutingFactory = function authorizedUserPreRoutingFn(
@ -23,7 +23,7 @@ export const authorizedUserPreRoutingFactory = function authorizedUserPreRouting
const getUser = getUserFactory(setupDeps.security);
return <P, Q, B>(handler: RequestHandlerUser<P, Q, B>): RequestHandler<P, Q, B, RouteMethod> => {
return (context, req, res) => {
let user: ReportingUser = null;
let user: ReportingRequestUser = false;
if (setupDeps.security && setupDeps.security.license.isEnabled()) {
// find the authenticated user, or null if security is not enabled
user = getUser(req);

View file

@ -9,6 +9,6 @@ import { SecurityPluginSetup } from '../../../../security/server';
export function getUserFactory(security?: SecurityPluginSetup) {
return (request: KibanaRequest) => {
return security?.authc.getCurrentUser(request) ?? null;
return security?.authc.getCurrentUser(request) ?? false;
};
}

View file

@ -6,8 +6,8 @@
import { kibanaResponseFactory } from 'kibana/server';
import { ReportingCore } from '../../';
import { AuthenticatedUser } from '../../../../security/server';
import { WHITELISTED_JOB_CONTENT_TYPES } from '../../../common/constants';
import { ReportingUser } from '../../types';
import { getDocumentPayloadFactory } from './get_document_payload';
import { jobsQueryFactory } from './jobs_query';
@ -27,7 +27,7 @@ export function downloadJobResponseHandlerFactory(reporting: ReportingCore) {
return async function jobResponseHandler(
res: typeof kibanaResponseFactory,
validJobTypes: string[],
user: AuthenticatedUser | null,
user: ReportingUser,
params: JobResponseHandlerParams,
opts: JobResponseHandlerOpts = {}
) {
@ -71,7 +71,7 @@ export function deleteJobResponseHandlerFactory(reporting: ReportingCore) {
return async function deleteJobResponseHander(
res: typeof kibanaResponseFactory,
validJobTypes: string[],
user: AuthenticatedUser | null,
user: ReportingUser,
params: JobResponseHandlerParams
) {
const { docId } = params;

View file

@ -8,12 +8,12 @@ import { i18n } from '@kbn/i18n';
import { errors as elasticsearchErrors } from 'elasticsearch';
import { get } from 'lodash';
import { ReportingCore } from '../../';
import { AuthenticatedUser } from '../../../../security/server';
import { JobSource } from '../../types';
import { JobSource, ReportingUser } from '../../types';
const esErrors = elasticsearchErrors as Record<string, any>;
const defaultSize = 10;
// TODO: use SearchRequest from elasticsearch-client
interface QueryBody {
size?: number;
from?: number;
@ -35,11 +35,12 @@ interface GetOpts {
includeContent?: boolean;
}
// TODO: use SearchResult from elasticsearch-client
interface CountAggResult {
count: number;
}
const getUsername = (user: AuthenticatedUser | null) => (user ? user.username : false);
const getUsername = (user: ReportingUser) => (user ? user.username : false);
export function jobsQueryFactory(reportingCore: ReportingCore) {
const { elasticsearch } = reportingCore.getPluginSetupDeps();
@ -80,7 +81,7 @@ export function jobsQueryFactory(reportingCore: ReportingCore) {
return {
list(
jobTypes: string[],
user: AuthenticatedUser | null,
user: ReportingUser,
page = 0,
size = defaultSize,
jobIds: string[] | null
@ -109,7 +110,7 @@ export function jobsQueryFactory(reportingCore: ReportingCore) {
return getHits(execQuery('search', body));
},
count(jobTypes: string[], user: AuthenticatedUser | null) {
count(jobTypes: string[], user: ReportingUser) {
const username = getUsername(user);
const body: QueryBody = {
query: {
@ -129,11 +130,7 @@ export function jobsQueryFactory(reportingCore: ReportingCore) {
});
},
get(
user: AuthenticatedUser | null,
id: string,
opts: GetOpts = {}
): Promise<JobSource<unknown> | void> {
get(user: ReportingUser, id: string, opts: GetOpts = {}): Promise<JobSource<unknown> | void> {
if (!id) return Promise.resolve();
const username = getUsername(user);

View file

@ -5,11 +5,10 @@
*/
import { KibanaRequest, KibanaResponseFactory, RequestHandlerContext } from 'src/core/server';
import { AuthenticatedUser } from '../../../security/server';
import { CreateJobBaseParams, ScheduledTaskParams } from '../types';
import { CreateJobBaseParams, ReportingUser, ScheduledTaskParams } from '../types';
export type HandlerFunction = (
user: AuthenticatedUser | null,
user: ReportingUser,
exportType: string,
jobParams: CreateJobBaseParams,
context: RequestHandlerContext,

View file

@ -11,7 +11,7 @@ import { DataPluginStart } from 'src/plugins/data/server/plugin';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { CancellationToken } from '../../../plugins/reporting/common';
import { LicensingPluginSetup } from '../../licensing/server';
import { SecurityPluginSetup } from '../../security/server';
import { AuthenticatedUser, SecurityPluginSetup } from '../../security/server';
import { JobStatus } from '../common/types';
import { ReportingConfigType } from './config';
import { ReportingCore } from './core';
@ -164,6 +164,8 @@ export type ReportingSetup = object;
* Internal Types
*/
export type ReportingUser = { username: AuthenticatedUser['username'] } | false;
export type CaptureConfig = ReportingConfigType['capture'];
export type ScrollConfig = ReportingConfigType['csv']['scroll'];

View file

@ -58,7 +58,8 @@ const onlyNotInCoverageTests = [
require.resolve('../test/licensing_plugin/config.public.ts'),
require.resolve('../test/licensing_plugin/config.legacy.ts'),
require.resolve('../test/endpoint_api_integration_no_ingest/config.ts'),
require.resolve('../test/reporting_api_integration/config.js'),
require.resolve('../test/reporting_api_integration/reporting_and_security.config.ts'),
require.resolve('../test/reporting_api_integration/reporting_without_security.config.ts'),
require.resolve('../test/security_solution_endpoint_api_int/config.ts'),
require.resolve('../test/ingest_manager_api_integration/config.ts'),
];

View file

@ -5,10 +5,11 @@
*/
import { esTestConfig, kbnTestConfig, kibanaServerTestUser } from '@kbn/test';
import { FtrConfigProviderContext } from '@kbn/test/types/ftr';
import { format as formatUrl } from 'url';
import { ReportingAPIProvider } from './services';
export default async function ({ readConfigFile }) {
export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const apiConfig = await readConfigFile(require.resolve('../api_integration/config'));
const functionalConfig = await readConfigFile(require.resolve('../functional/config')); // Reporting API tests need a fully working UI
@ -23,7 +24,7 @@ export default async function ({ readConfigFile }) {
return {
servers: apiConfig.get('servers'),
junit: { reportName: 'X-Pack Reporting API Integration Tests' },
testFiles: [require.resolve('./reporting')],
testFiles: [require.resolve('./reporting_and_security')],
services: {
...apiConfig.get('services'),
reportingAPI: ReportingAPIProvider,

View file

@ -0,0 +1,45 @@
/*
* 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 { esTestConfig, kbnTestConfig } from '@kbn/test';
import { FtrConfigProviderContext } from '@kbn/test/types/ftr';
import { format as formatUrl } from 'url';
import { ReportingAPIProvider } from './services';
export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const apiConfig = await readConfigFile(require.resolve('../api_integration/config'));
return {
servers: apiConfig.get('servers'),
junit: { reportName: 'X-Pack Reporting Without Security API Integration Tests' },
testFiles: [require.resolve('./reporting_without_security')],
services: {
...apiConfig.get('services'),
reportingAPI: ReportingAPIProvider,
},
esArchiver: apiConfig.get('esArchiver'),
esTestCluster: {
...apiConfig.get('esTestCluster'),
serverArgs: [
...apiConfig.get('esTestCluster.serverArgs'),
'node.name=UnsecuredClusterNode01',
'xpack.security.enabled=false',
],
},
kbnTestServer: {
...apiConfig.get('kbnTestServer'),
serverArgs: [
`--elasticsearch.hosts=${formatUrl(esTestConfig.getUrlParts())}`,
`--logging.json=false`,
`--server.maxPayloadBytes=1679958`,
`--server.port=${kbnTestConfig.getPort()}`,
`--xpack.reporting.capture.maxAttempts=1`,
`--xpack.reporting.csv.maxSizeBytes=2850`,
`--xpack.security.enabled=false`,
],
},
};
}

View file

@ -0,0 +1,15 @@
/*
* 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 { FtrProviderContext } from '../ftr_provider_context';
// eslint-disable-next-line import/no-default-export
export default function ({ loadTestFile }: FtrProviderContext) {
describe('Reporting APIs', function () {
this.tags('ciGroup2');
loadTestFile(require.resolve('./job_apis'));
});
}

View file

@ -0,0 +1,126 @@
/*
* 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 expect from '@kbn/expect';
import { forOwn } from 'lodash';
import { JOB_PARAMS_RISON } from '../fixtures';
import { FtrProviderContext } from '../ftr_provider_context';
// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const supertestNoAuth = getService('supertestWithoutAuth');
const reportingAPI = getService('reportingAPI');
describe('Job Listing APIs, Without Security', () => {
before(async () => {
await esArchiver.load('reporting/logs');
await esArchiver.load('logstash_functional');
});
after(async () => {
await esArchiver.unload('reporting/logs');
await esArchiver.unload('logstash_functional');
});
afterEach(async () => {
await reportingAPI.deleteAllReports();
});
it('Posted CSV job is visible in the job count', async () => {
const { status: resStatus, text: resText } = await supertestNoAuth
.post(`/api/reporting/generate/csv`)
.set('kbn-xsrf', 'xxx')
.send({ jobParams: JOB_PARAMS_RISON });
expect(resStatus).to.be(200);
const { job: resJob } = JSON.parse(resText);
const expectedResJob: Record<string, any> = {
attempts: 0,
created_by: false,
jobtype: 'csv',
max_attempts: 1,
priority: 10,
status: 'pending',
timeout: 120000,
browser_type: 'chromium', // TODO: remove this field from the API response
// TODO: remove the payload field from the api respones
};
forOwn(expectedResJob, (value: any, key: string) => {
expect(resJob[key]).to.eql(value, key);
});
// call the job count api
const { text: countText } = await supertestNoAuth
.get(`/api/reporting/jobs/count`)
.set('kbn-xsrf', 'xxx');
const countResult = JSON.parse(countText);
expect(countResult).to.be(1);
});
it('Posted CSV job is visible in the status check', async () => {
const { status: resStatus, text: resText } = await supertestNoAuth
.post(`/api/reporting/generate/csv`)
.set('kbn-xsrf', 'xxx')
.send({ jobParams: JOB_PARAMS_RISON });
expect(resStatus).to.be(200);
const { job: resJob } = JSON.parse(resText);
// call the single job listing api (status check)
const { text: listText } = await supertestNoAuth
.get(`/api/reporting/jobs/list?page=0&ids=${resJob.id}`)
.set('kbn-xsrf', 'xxx');
const listingJobs = JSON.parse(listText);
const expectedListJob: Record<string, any> = {
attempts: 0,
created_by: false,
jobtype: 'csv',
timeout: 120000,
browser_type: 'chromium',
};
forOwn(expectedListJob, (value: any, key: string) => {
expect(listingJobs[0]._source[key]).to.eql(value, key);
});
expect(listingJobs.length).to.be(1);
expect(listingJobs[0]._id).to.be(resJob.id);
});
it('Posted CSV job is visible in the first page of jobs listing', async () => {
const { status: resStatus, text: resText } = await supertestNoAuth
.post(`/api/reporting/generate/csv`)
.set('kbn-xsrf', 'xxx')
.send({ jobParams: JOB_PARAMS_RISON });
expect(resStatus).to.be(200);
const { job: resJob } = JSON.parse(resText);
// call the ALL job listing api
const { text: listText } = await supertestNoAuth
.get(`/api/reporting/jobs/list?page=0`)
.set('kbn-xsrf', 'xxx');
const listingJobs = JSON.parse(listText);
const expectedListJob: Record<string, any> = {
attempts: 0,
created_by: false,
jobtype: 'csv',
timeout: 120000,
browser_type: 'chromium',
};
forOwn(expectedListJob, (value: any, key: string) => {
expect(listingJobs[0]._source[key]).to.eql(value, key);
});
expect(listingJobs.length).to.be(1);
expect(listingJobs[0]._id).to.be(resJob.id);
});
});
}

View file

@ -186,6 +186,7 @@ export function ReportingAPIProvider({ getService }: FtrProviderContext) {
export const services = {
...xpackServices,
supertestWithoutAuth: apiIntegrationServices.supertestWithoutAuth,
usageAPI: apiIntegrationServices.usageAPI,
reportingAPI: ReportingAPIProvider,
};