[7.x] [Reporting/JobListing] fix user ID for non-security in queries (#…

…75365) (#75496)

* [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
# Conflicts:
#	x-pack/scripts/functional_tests.js
#	x-pack/test/reporting_api_integration/reporting_and_security/bwc_existing_indexes.ts
#	x-pack/test/reporting_api_integration/reporting_and_security/bwc_generation_urls.ts

* fix bad merge

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

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<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}`);
 

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<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;

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,
@@ -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: {},

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<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,
     });
 

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,15 +58,15 @@ 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);
 
     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);

x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts

@@ -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);

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;
   };
 }

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;

x-pack/plugins/reporting/server/routes/lib/jobs_query.ts

@@ -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);
 

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,

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'];
 

x-pack/scripts/functional_tests.js

@@ -56,8 +56,9 @@ require('@kbn/test').runTestsCli([
   require.resolve('../test/licensing_plugin/config.public'),
   require.resolve('../test/licensing_plugin/config.legacy'),
   require.resolve('../test/endpoint_api_integration_no_ingest/config.ts'),
-  require.resolve('../test/reporting_api_integration/config.js'),
   require.resolve('../test/functional_embedded/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'),
   require.resolve('../test/functional_enterprise_search/without_host_configured.config.ts'),

x-pack/test/reporting_api_integration/config.js → 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,