From 85abb52cf2d9cad314b90964cbf8fe25f2c9cb6c Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 18 Aug 2020 12:09:54 -0700 Subject: [PATCH 1/5] [Reporting/JobListing] fix user ID for non-security in queries --- .../reporting/server/lib/enqueue_job.ts | 10 +- .../reporting/server/lib/store/report.ts | 4 +- .../reporting/server/lib/store/store.test.ts | 20 ++- .../reporting/server/lib/store/store.ts | 10 +- .../routes/lib/authorized_user_pre_routing.ts | 5 +- .../reporting/server/routes/lib/get_user.ts | 2 +- .../server/routes/lib/job_response_handler.ts | 6 +- .../reporting/server/routes/lib/jobs_query.ts | 157 +++++++++--------- .../reporting/server/routes/types.d.ts | 5 +- x-pack/plugins/reporting/server/types.ts | 4 +- 10 files changed, 115 insertions(+), 108 deletions(-) diff --git a/x-pack/plugins/reporting/server/lib/enqueue_job.ts b/x-pack/plugins/reporting/server/lib/enqueue_job.ts index 31960c782b7b9..3e3aef92c55a3 100644 --- a/x-pack/plugins/reporting/server/lib/enqueue_job.ts +++ b/x-pack/plugins/reporting/server/lib/enqueue_job.ts @@ -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; @@ -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; - 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}`); diff --git a/x-pack/plugins/reporting/server/lib/store/report.ts b/x-pack/plugins/reporting/server/lib/store/report.ts index 5ff71ae7a7182..5c9b9ced7cce7 100644 --- a/x-pack/plugins/reporting/server/lib/store/report.ts +++ b/x-pack/plugins/reporting/server/lib/store/report.ts @@ -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 { 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; diff --git a/x-pack/plugins/reporting/server/lib/store/store.test.ts b/x-pack/plugins/reporting/server/lib/store/store.test.ts index c66e2dd7742c4..d3106acc54657 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.test.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.test.ts @@ -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, @@ -174,7 +178,7 @@ 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, diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts index 12cff0e973ed6..88f6fa418a1b3 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.ts @@ -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 { 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, }); diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts index e2f393aad57d2..341cddb98d08d 100644 --- a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts +++ b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts @@ -5,11 +5,10 @@ */ import { RequestHandler, RouteMethod } from 'src/core/server'; -import { AuthenticatedUser } from '../../../../security/server'; import { ReportingCore } from '../../core'; +import { ReportingUser } from '../../types'; import { getUserFactory } from './get_user'; -type ReportingUser = AuthenticatedUser | null; const superuserRole = 'superuser'; export type RequestHandlerUser = RequestHandler extends (...a: infer U) => infer R @@ -23,7 +22,7 @@ export const authorizedUserPreRoutingFactory = function authorizedUserPreRouting const getUser = getUserFactory(setupDeps.security); return (handler: RequestHandlerUser): RequestHandler => { return (context, req, res) => { - let user: ReportingUser = null; + let user: ReportingUser = false; if (setupDeps.security && setupDeps.security.license.isEnabled()) { // find the authenticated user, or null if security is not enabled user = getUser(req); diff --git a/x-pack/plugins/reporting/server/routes/lib/get_user.ts b/x-pack/plugins/reporting/server/routes/lib/get_user.ts index fd56e8cfc28c7..ee6ad6ad7d671 100644 --- a/x-pack/plugins/reporting/server/routes/lib/get_user.ts +++ b/x-pack/plugins/reporting/server/routes/lib/get_user.ts @@ -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; }; } diff --git a/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts b/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts index df346c8b9b832..aec1eee0eee14 100644 --- a/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts +++ b/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts @@ -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; diff --git a/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts b/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts index f3955b4871b31..e2bc8486e8cc4 100644 --- a/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts +++ b/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts @@ -5,65 +5,60 @@ */ import { i18n } from '@kbn/i18n'; -import { errors as elasticsearchErrors } from 'elasticsearch'; +import { + CountParams, + CountResponse, + errors as elasticsearchErrors, + SearchParams, + SearchResponse, +} 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; -const defaultSize = 10; - -interface QueryBody { - size?: number; - from?: number; - _source?: { - excludes: string[]; - }; - query: { - constant_score: { - filter: { - bool: { - must: Array>; - }; - }; - }; - }; +// FIXME: typescript definition does not have the proper search API field name! +declare module 'elasticsearch' { + interface SearchParams { + _source_excludes?: SearchParams['_sourceExclude']; + } } -interface GetOpts { - includeContent?: boolean; -} +const esErrors = elasticsearchErrors as Record; +const defaultSize = 10; -interface CountAggResult { - count: number; -} +type QueryType = 'search' | 'count'; -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(); const { callAsInternalUser } = elasticsearch.legacy.client; - function execQuery(queryType: string, body: QueryBody) { - const defaultBody: Record = { - search: { - _source: { - excludes: ['output.content'], - }, - sort: [{ created_at: { order: 'desc' } }], - size: defaultSize, - }, - }; + function execQuery( + queryType: QueryType, + params: ParamType + ) { + const { body: paramsBody, ...restParams } = params; const config = reportingCore.getConfig(); const index = config.get('index'); - const query = { - index: `${index}-*`, - body: Object.assign(defaultBody[queryType] || {}, body), - }; + let query: SearchParams | CountParams; + if (queryType === 'count') { + query = { + body: paramsBody, + index: `${index}-*`, + }; + } else { + query = { + body: paramsBody, + index: `${index}-*`, + size: defaultSize, + sort: ['created_at'], + ...restParams, + }; + } - return callAsInternalUser(queryType, query).catch((err) => { + return callAsInternalUser(queryType, query).catch((err) => { if (err instanceof esErrors['401']) return; if (err instanceof esErrors['403']) return; if (err instanceof esErrors['404']) return; @@ -71,29 +66,30 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { }); } - type Result = number; - - function getHits(query: Promise) { + function getHits(query: Promise) { return query.then((res) => get(res, 'hits.hits', [])); } return { list( jobTypes: string[], - user: AuthenticatedUser | null, + user: ReportingUser, page = 0, size = defaultSize, jobIds: string[] | null ) { const username = getUsername(user); - const body: QueryBody = { + const params: SearchParams = { size, from: size * page, - query: { - constant_score: { - filter: { - bool: { - must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], + _source_excludes: ['output.content'], + body: { + query: { + constant_score: { + filter: { + bool: { + must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], + }, }, }, }, @@ -101,65 +97,70 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { }; if (jobIds) { - body.query.constant_score.filter.bool.must.push({ + params.body.query.constant_score.filter.bool.must.push({ ids: { values: jobIds }, }); } - return getHits(execQuery('search', body)); + return getHits(execQuery>('search', params)); }, - count(jobTypes: string[], user: AuthenticatedUser | null) { + count(jobTypes: string[], user: ReportingUser) { const username = getUsername(user); - const body: QueryBody = { - query: { - constant_score: { - filter: { - bool: { - must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], + const params: CountParams = { + body: { + query: { + constant_score: { + filter: { + bool: { + must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], + }, }, }, }, }, }; - return execQuery('count', body).then((doc: CountAggResult) => { + return execQuery('count', params).then((doc) => { if (!doc) return 0; return doc.count; }); }, get( - user: AuthenticatedUser | null, + user: ReportingUser, id: string, - opts: GetOpts = {} + opts: { includeContent?: boolean } = {} ): Promise | void> { if (!id) return Promise.resolve(); const username = getUsername(user); - const body: QueryBody = { - query: { - constant_score: { - filter: { - bool: { - must: [{ term: { _id: id } }, { term: { created_by: username } }], + const params: SearchParams = { + body: { + query: { + constant_score: { + filter: { + bool: { + must: [{ term: { _id: id } }, { term: { created_by: username } }], + }, }, }, }, }, + _source_excludes: ['output.content'], size: 1, }; if (opts.includeContent) { - body._source = { - excludes: [], - }; + params._source_excludes = []; } - return getHits(execQuery('search', body)).then((hits) => { - if (hits.length !== 1) return; - return hits[0]; - }); + return getHits(execQuery>('search', params)).then( + (hits) => { + if (hits.length !== 1) return; + return hits[0]; + } + ); }, async delete(deleteIndex: string, id: string) { diff --git a/x-pack/plugins/reporting/server/routes/types.d.ts b/x-pack/plugins/reporting/server/routes/types.d.ts index c92c9fb7eef74..ab1dd841bbbc2 100644 --- a/x-pack/plugins/reporting/server/routes/types.d.ts +++ b/x-pack/plugins/reporting/server/routes/types.d.ts @@ -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, diff --git a/x-pack/plugins/reporting/server/types.ts b/x-pack/plugins/reporting/server/types.ts index c9649cb6e558b..542c9cdb3f677 100644 --- a/x-pack/plugins/reporting/server/types.ts +++ b/x-pack/plugins/reporting/server/types.ts @@ -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']; From c05575fea7717b2eebbcaa826047786715247fd0 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 18 Aug 2020 13:17:19 -0700 Subject: [PATCH 2/5] fix tests --- x-pack/plugins/reporting/server/lib/store/store.test.ts | 4 ++-- .../server/routes/lib/authorized_user_pre_routing.test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/reporting/server/lib/store/store.test.ts b/x-pack/plugins/reporting/server/lib/store/store.test.ts index d3106acc54657..e6c4eb7346460 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.test.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.test.ts @@ -164,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 @@ -184,7 +184,7 @@ describe('ReportingStore', () => { attempts: 0, browser_type: undefined, completed_at: undefined, - created_by: null, + created_by: false, jobtype: 'unknowntype', max_attempts: undefined, payload: {}, diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts index f4591f8436bd2..50780a577af02 100644 --- a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts +++ b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts @@ -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); From 1e0a49fafcfeb09f7d706217ba88d674d92a5b07 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 18 Aug 2020 14:18:33 -0700 Subject: [PATCH 3/5] add fn api test --- .../routes/lib/authorized_user_pre_routing.ts | 7 +- x-pack/scripts/functional_tests.js | 3 +- ...ig.js => reporting_and_security.config.ts} | 5 +- .../constants.ts | 0 .../csv_job_params.ts | 0 .../csv_saved_search.ts | 0 .../index.ts | 0 .../network_policy.ts | 0 .../usage.ts | 0 .../reporting_without_security.config.ts | 45 +++++++ .../reporting_without_security/index.ts | 15 +++ .../reporting_without_security/job_apis.ts | 126 ++++++++++++++++++ .../reporting_api_integration/services.ts | 1 + 13 files changed, 196 insertions(+), 6 deletions(-) rename x-pack/test/reporting_api_integration/{config.js => reporting_and_security.config.ts} (90%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/constants.ts (100%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/csv_job_params.ts (100%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/csv_saved_search.ts (100%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/index.ts (100%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/network_policy.ts (100%) rename x-pack/test/reporting_api_integration/{reporting => reporting_and_security}/usage.ts (100%) create mode 100644 x-pack/test/reporting_api_integration/reporting_without_security.config.ts create mode 100644 x-pack/test/reporting_api_integration/reporting_without_security/index.ts create mode 100644 x-pack/test/reporting_api_integration/reporting_without_security/job_apis.ts diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts index 341cddb98d08d..4d57bc154444e 100644 --- a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts +++ b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts @@ -5,14 +5,15 @@ */ import { RequestHandler, RouteMethod } from 'src/core/server'; +import { AuthenticatedUser } from '../../../../security/server'; import { ReportingCore } from '../../core'; -import { ReportingUser } from '../../types'; import { getUserFactory } from './get_user'; const superuserRole = 'superuser'; +type ReportingRequestUser = AuthenticatedUser | false; export type RequestHandlerUser = RequestHandler extends (...a: infer U) => infer R - ? (user: ReportingUser, ...a: U) => R + ? (user: ReportingRequestUser, ...a: U) => R : never; export const authorizedUserPreRoutingFactory = function authorizedUserPreRoutingFn( @@ -22,7 +23,7 @@ export const authorizedUserPreRoutingFactory = function authorizedUserPreRouting const getUser = getUserFactory(setupDeps.security); return (handler: RequestHandlerUser): RequestHandler => { return (context, req, res) => { - let user: ReportingUser = false; + 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); diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 205ff500a36ec..94a5fd4ed6784 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -56,7 +56,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'), ]; diff --git a/x-pack/test/reporting_api_integration/config.js b/x-pack/test/reporting_api_integration/reporting_and_security.config.ts similarity index 90% rename from x-pack/test/reporting_api_integration/config.js rename to x-pack/test/reporting_api_integration/reporting_and_security.config.ts index 930c90da5637c..dc5aaba69604f 100644 --- a/x-pack/test/reporting_api_integration/config.js +++ b/x-pack/test/reporting_api_integration/reporting_and_security.config.ts @@ -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, diff --git a/x-pack/test/reporting_api_integration/reporting/constants.ts b/x-pack/test/reporting_api_integration/reporting_and_security/constants.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/constants.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/constants.ts diff --git a/x-pack/test/reporting_api_integration/reporting/csv_job_params.ts b/x-pack/test/reporting_api_integration/reporting_and_security/csv_job_params.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/csv_job_params.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/csv_job_params.ts diff --git a/x-pack/test/reporting_api_integration/reporting/csv_saved_search.ts b/x-pack/test/reporting_api_integration/reporting_and_security/csv_saved_search.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/csv_saved_search.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/csv_saved_search.ts diff --git a/x-pack/test/reporting_api_integration/reporting/index.ts b/x-pack/test/reporting_api_integration/reporting_and_security/index.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/index.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/index.ts diff --git a/x-pack/test/reporting_api_integration/reporting/network_policy.ts b/x-pack/test/reporting_api_integration/reporting_and_security/network_policy.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/network_policy.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/network_policy.ts diff --git a/x-pack/test/reporting_api_integration/reporting/usage.ts b/x-pack/test/reporting_api_integration/reporting_and_security/usage.ts similarity index 100% rename from x-pack/test/reporting_api_integration/reporting/usage.ts rename to x-pack/test/reporting_api_integration/reporting_and_security/usage.ts diff --git a/x-pack/test/reporting_api_integration/reporting_without_security.config.ts b/x-pack/test/reporting_api_integration/reporting_without_security.config.ts new file mode 100644 index 0000000000000..b040040fc5114 --- /dev/null +++ b/x-pack/test/reporting_api_integration/reporting_without_security.config.ts @@ -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`, + ], + }, + }; +} diff --git a/x-pack/test/reporting_api_integration/reporting_without_security/index.ts b/x-pack/test/reporting_api_integration/reporting_without_security/index.ts new file mode 100644 index 0000000000000..09351a2c9907a --- /dev/null +++ b/x-pack/test/reporting_api_integration/reporting_without_security/index.ts @@ -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')); + }); +} diff --git a/x-pack/test/reporting_api_integration/reporting_without_security/job_apis.ts b/x-pack/test/reporting_api_integration/reporting_without_security/job_apis.ts new file mode 100644 index 0000000000000..ab26f27076ebb --- /dev/null +++ b/x-pack/test/reporting_api_integration/reporting_without_security/job_apis.ts @@ -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 = { + 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 = { + 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 = { + 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); + }); + }); +} diff --git a/x-pack/test/reporting_api_integration/services.ts b/x-pack/test/reporting_api_integration/services.ts index f98d8246fc01e..e61e6483855af 100644 --- a/x-pack/test/reporting_api_integration/services.ts +++ b/x-pack/test/reporting_api_integration/services.ts @@ -186,6 +186,7 @@ export function ReportingAPIProvider({ getService }: FtrProviderContext) { export const services = { ...xpackServices, + supertestWithoutAuth: apiIntegrationServices.supertestWithoutAuth, usageAPI: apiIntegrationServices.usageAPI, reportingAPI: ReportingAPIProvider, }; From 5a719bf32c61e2be32b6edbdaf2c4dbf219a0086 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 18 Aug 2020 20:31:38 -0700 Subject: [PATCH 4/5] fix ci --- x-pack/scripts/functional_tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 94a5fd4ed6784..3c6135c2a0844 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -56,8 +56,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/reporting_and_security/config.ts'), - require.resolve('../test/reporting_api_integration/reporting_without_security/config.ts'), + 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'), ]; From eea31531cc9497d345fe35283df42e6b5083ce5c Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 19 Aug 2020 09:45:10 -0700 Subject: [PATCH 5/5] revert TS exploration --- .../reporting/server/routes/lib/jobs_query.ts | 152 +++++++++--------- 1 file changed, 74 insertions(+), 78 deletions(-) diff --git a/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts b/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts index e2bc8486e8cc4..b01c880abe820 100644 --- a/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts +++ b/x-pack/plugins/reporting/server/routes/lib/jobs_query.ts @@ -5,28 +5,40 @@ */ import { i18n } from '@kbn/i18n'; -import { - CountParams, - CountResponse, - errors as elasticsearchErrors, - SearchParams, - SearchResponse, -} from 'elasticsearch'; +import { errors as elasticsearchErrors } from 'elasticsearch'; import { get } from 'lodash'; import { ReportingCore } from '../../'; import { JobSource, ReportingUser } from '../../types'; -// FIXME: typescript definition does not have the proper search API field name! -declare module 'elasticsearch' { - interface SearchParams { - _source_excludes?: SearchParams['_sourceExclude']; - } -} - const esErrors = elasticsearchErrors as Record; const defaultSize = 10; -type QueryType = 'search' | 'count'; +// TODO: use SearchRequest from elasticsearch-client +interface QueryBody { + size?: number; + from?: number; + _source?: { + excludes: string[]; + }; + query: { + constant_score: { + filter: { + bool: { + must: Array>; + }; + }; + }; + }; +} + +interface GetOpts { + includeContent?: boolean; +} + +// TODO: use SearchResult from elasticsearch-client +interface CountAggResult { + count: number; +} const getUsername = (user: ReportingUser) => (user ? user.username : false); @@ -34,31 +46,25 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { const { elasticsearch } = reportingCore.getPluginSetupDeps(); const { callAsInternalUser } = elasticsearch.legacy.client; - function execQuery( - queryType: QueryType, - params: ParamType - ) { - const { body: paramsBody, ...restParams } = params; + function execQuery(queryType: string, body: QueryBody) { + const defaultBody: Record = { + search: { + _source: { + excludes: ['output.content'], + }, + sort: [{ created_at: { order: 'desc' } }], + size: defaultSize, + }, + }; const config = reportingCore.getConfig(); const index = config.get('index'); - let query: SearchParams | CountParams; - if (queryType === 'count') { - query = { - body: paramsBody, - index: `${index}-*`, - }; - } else { - query = { - body: paramsBody, - index: `${index}-*`, - size: defaultSize, - sort: ['created_at'], - ...restParams, - }; - } + const query = { + index: `${index}-*`, + body: Object.assign(defaultBody[queryType] || {}, body), + }; - return callAsInternalUser(queryType, query).catch((err) => { + return callAsInternalUser(queryType, query).catch((err) => { if (err instanceof esErrors['401']) return; if (err instanceof esErrors['403']) return; if (err instanceof esErrors['404']) return; @@ -66,7 +72,9 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { }); } - function getHits(query: Promise) { + type Result = number; + + function getHits(query: Promise) { return query.then((res) => get(res, 'hits.hits', [])); } @@ -79,17 +87,14 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { jobIds: string[] | null ) { const username = getUsername(user); - const params: SearchParams = { + const body: QueryBody = { size, from: size * page, - _source_excludes: ['output.content'], - body: { - query: { - constant_score: { - filter: { - bool: { - must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], - }, + query: { + constant_score: { + filter: { + bool: { + must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], }, }, }, @@ -97,70 +102,61 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { }; if (jobIds) { - params.body.query.constant_score.filter.bool.must.push({ + body.query.constant_score.filter.bool.must.push({ ids: { values: jobIds }, }); } - return getHits(execQuery>('search', params)); + return getHits(execQuery('search', body)); }, count(jobTypes: string[], user: ReportingUser) { const username = getUsername(user); - const params: CountParams = { - body: { - query: { - constant_score: { - filter: { - bool: { - must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], - }, + const body: QueryBody = { + query: { + constant_score: { + filter: { + bool: { + must: [{ terms: { jobtype: jobTypes } }, { term: { created_by: username } }], }, }, }, }, }; - return execQuery('count', params).then((doc) => { + return execQuery('count', body).then((doc: CountAggResult) => { if (!doc) return 0; return doc.count; }); }, - get( - user: ReportingUser, - id: string, - opts: { includeContent?: boolean } = {} - ): Promise | void> { + get(user: ReportingUser, id: string, opts: GetOpts = {}): Promise | void> { if (!id) return Promise.resolve(); const username = getUsername(user); - const params: SearchParams = { - body: { - query: { - constant_score: { - filter: { - bool: { - must: [{ term: { _id: id } }, { term: { created_by: username } }], - }, + const body: QueryBody = { + query: { + constant_score: { + filter: { + bool: { + must: [{ term: { _id: id } }, { term: { created_by: username } }], }, }, }, }, - _source_excludes: ['output.content'], size: 1, }; if (opts.includeContent) { - params._source_excludes = []; + body._source = { + excludes: [], + }; } - return getHits(execQuery>('search', params)).then( - (hits) => { - if (hits.length !== 1) return; - return hits[0]; - } - ); + return getHits(execQuery('search', body)).then((hits) => { + if (hits.length !== 1) return; + return hits[0]; + }); }, async delete(deleteIndex: string, id: string) {