Skip to content

Commit

Permalink
[Reporting/JobListing] fix user ID for non-security in queries
Browse files Browse the repository at this point in the history
  • Loading branch information
tsullivan committed Aug 18, 2020
1 parent 0707e29 commit 85abb52
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 108 deletions.
10 changes: 4 additions & 6 deletions x-pack/plugins/reporting/server/lib/enqueue_job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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) {
Expand All @@ -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}`);

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/reporting/server/lib/store/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 12 additions & 8 deletions x-pack/plugins/reporting/server/lib/store/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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]`);
});

Expand All @@ -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]`);
});

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/reporting/server/lib/store/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -151,7 +155,7 @@ export class ReportingStore {
_index: index,
payload,
jobtype: type,
created_by: username,
created_by: user ? user.username : false,
...this.jobSettings,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<P, Q, B> = RequestHandler<P, Q, B> extends (...a: infer U) => infer R
Expand All @@ -23,7 +22,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: ReportingUser = false;
if (setupDeps.security && setupDeps.security.license.isEnabled()) {
// find the authenticated user, or null if security is not enabled
user = getUser(req);
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/reporting/server/routes/lib/get_user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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 = {}
) {
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 85abb52

Please sign in to comment.