Skip to content

Commit

Permalink
[Reporting] Make CSV authentication failure a warning (#126358)
Browse files Browse the repository at this point in the history
* added ability for CSV to fail due to auth or unknown error and NOT throw

* wording

* added warning toast when report finishes, but with warnings

* added tests for CSV generator

* moved i18n to external file

* remove extra i18n for now

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jloleysens and kibanamachine authored Mar 1, 2022
1 parent 6e11bea commit 3285eb1
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 32 deletions.
11 changes: 10 additions & 1 deletion x-pack/plugins/reporting/common/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export interface ReportDocumentHead {

export interface ReportOutput extends TaskRunResult {
content: string | null;
error_code?: string;
size: number;
}

Expand Down Expand Up @@ -63,6 +62,16 @@ export interface TaskRunResult {
max_size_reached?: boolean;
warnings?: string[];
metrics?: TaskRunMetrics;

/**
* When running a report task we may finish with warnings that were triggered
* by an error. We can pass the error code via the task run result to the
* task runner so that it can be recorded for telemetry.
*
* Alternatively, this field can be populated in the event that the task does
* not complete in the task runner's error handler.
*/
error_code?: string;
}

export interface ReportSource {
Expand Down
14 changes: 12 additions & 2 deletions x-pack/plugins/reporting/public/lib/stream_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import { JOB_COMPLETION_NOTIFICATIONS_SESSION_KEY, JOB_STATUSES } from '../../co
import { JobId, JobSummary, JobSummarySet } from '../../common/types';
import {
getFailureToast,
getGeneralErrorToast,
getWarningToast,
getSuccessToast,
getWarningFormulasToast,
getGeneralErrorToast,
getWarningMaxSizeToast,
getWarningFormulasToast,
} from '../notifier';
import { Job } from './job';
import { ReportingAPIClient } from './reporting_api_client';
Expand Down Expand Up @@ -71,6 +72,15 @@ export class ReportingNotifierStreamHandler {
this.theme
)
);
} else if (job.status === JOB_STATUSES.WARNINGS) {
this.notifications.toasts.addWarning(
getWarningToast(
job,
this.apiClient.getManagementLink,
this.apiClient.getDownloadLink,
this.theme
)
);
} else {
this.notifications.toasts.addSuccess(
getSuccessToast(
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/reporting/public/notifier/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ export { getGeneralErrorToast } from './general_error';
export { getSuccessToast } from './job_success';
export { getWarningFormulasToast } from './job_warning_formulas';
export { getWarningMaxSizeToast } from './job_warning_max_size';
export { getWarningToast } from './job_warning';
46 changes: 46 additions & 0 deletions x-pack/plugins/reporting/public/notifier/job_warning.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { FormattedMessage } from '@kbn/i18n-react';
import React, { Fragment } from 'react';
import { ThemeServiceStart, ToastInput } from 'src/core/public';
import { toMountPoint } from '../../../../../src/plugins/kibana_react/public';
import { JobId, JobSummary } from '../../common/types';
import { DownloadButton } from './job_download_button';
import { ReportLink } from './report_link';

export const getWarningToast = (
job: JobSummary,
getReportLink: () => string,
getDownloadLink: (jobId: JobId) => string,
theme: ThemeServiceStart
): ToastInput => ({
title: toMountPoint(
<FormattedMessage
id="xpack.reporting.publicNotifier.warning.title"
defaultMessage="{reportType} completed with issues"
values={{ reportType: job.jobtype }}
/>,
{ theme$: theme.theme$ }
),
text: toMountPoint(
<Fragment>
<p>
<FormattedMessage
id="xpack.reporting.publicNotifier.warning.description"
defaultMessage="The report completed with issues."
/>
</p>
<p>
<ReportLink getUrl={getReportLink} />
</p>
<DownloadButton getUrl={getDownloadLink} job={job} />
</Fragment>,
{ theme$: theme.theme$ }
),
'data-test-subj': 'completeReportWarning',
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
UI_SETTINGS_CSV_SEPARATOR,
UI_SETTINGS_DATEFORMAT_TZ,
} from '../../../../common/constants';
import { AuthenticationExpiredError } from '../../../../common/errors';
import { UnknownError } from '../../../../common/errors';
import {
createMockConfig,
createMockConfigSchema,
Expand Down Expand Up @@ -808,25 +808,76 @@ it('can override ignoring frozen indices', async () => {
);
});

it('throws an AuthenticationExpiredError when ES does not accept credentials', async () => {
mockDataClient.search = jest.fn().mockImplementation(() => {
throw new esErrors.ResponseError({ statusCode: 403, meta: {} as any, warnings: [] });
describe('error codes', () => {
it('returns the expected error code when authentication expires', async () => {
mockDataClient.search = jest.fn().mockImplementation(() =>
Rx.of({
rawResponse: {
_scroll_id: 'test',
hits: {
hits: range(0, 5).map(() => ({
fields: {
date: ['2020-12-31T00:14:28.000Z'],
ip: ['110.135.176.89'],
message: ['super cali fragile istic XPLA docious'],
},
})),
total: 10,
},
},
})
);

mockEsClient.asCurrentUser.scroll = jest.fn().mockImplementation(() => {
throw new esErrors.ResponseError({ statusCode: 403, meta: {} as any, warnings: [] });
});

const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
{
es: mockEsClient,
data: mockDataClient,
uiSettings: uiSettingsClient,
},
{
searchSourceStart: mockSearchSourceService,
fieldFormatsRegistry: mockFieldFormatsRegistry,
},
new CancellationToken(),
logger,
stream
);

const { error_code: errorCode, warnings } = await generateCsv.generateData();
expect(errorCode).toBe('authentication_expired');
expect(warnings).toMatchInlineSnapshot(`
Array [
"This report contains partial CSV results because authentication expired before it could finish. Try exporting a smaller amount of data or increase your authentication timeout.",
]
`);
});

it('throws for unknown errors', async () => {
mockDataClient.search = jest.fn().mockImplementation(() => {
throw new esErrors.ResponseError({ statusCode: 500, meta: {} as any, warnings: [] });
});
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
{
es: mockEsClient,
data: mockDataClient,
uiSettings: uiSettingsClient,
},
{
searchSourceStart: mockSearchSourceService,
fieldFormatsRegistry: mockFieldFormatsRegistry,
},
new CancellationToken(),
logger,
stream
);
await expect(generateCsv.generateData()).rejects.toBeInstanceOf(UnknownError);
});
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
{
es: mockEsClient,
data: mockDataClient,
uiSettings: uiSettingsClient,
},
{
searchSourceStart: mockSearchSourceService,
fieldFormatsRegistry: mockFieldFormatsRegistry,
},
new CancellationToken(),
logger,
stream
);
await expect(generateCsv.generateData()).rejects.toEqual(new AuthenticationExpiredError());
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { errors as esErrors } from '@elastic/elasticsearch';
import { i18n } from '@kbn/i18n';
import type { IScopedClusterClient, IUiSettingsClient } from 'src/core/server';
import type { IScopedSearchClient } from 'src/plugins/data/server';
import type { Datatable } from 'src/plugins/expressions/server';
Expand All @@ -31,13 +30,18 @@ import type {
import { KbnServerError } from '../../../../../../../src/plugins/kibana_utils/server';
import type { CancellationToken } from '../../../../common/cancellation_token';
import { CONTENT_TYPE_CSV } from '../../../../common/constants';
import { AuthenticationExpiredError } from '../../../../common/errors';
import {
AuthenticationExpiredError,
UnknownError,
ReportingError,
} from '../../../../common/errors';
import { byteSizeValueToNumber } from '../../../../common/schema_utils';
import type { LevelLogger } from '../../../lib';
import type { TaskRunResult } from '../../../lib/tasks';
import type { JobParamsCSV } from '../types';
import { CsvExportSettings, getExportSettings } from './get_export_settings';
import { MaxSizeStringBuilder } from './max_size_string_builder';
import { i18nTexts } from './i18n_texts';

interface Clients {
es: IScopedClusterClient;
Expand Down Expand Up @@ -257,6 +261,7 @@ export class CsvGenerator {
),
this.dependencies.searchSourceStart.create(this.job.searchSource),
]);
let reportingError: undefined | ReportingError;

const index = searchSource.getField('index');

Expand Down Expand Up @@ -360,19 +365,19 @@ export class CsvGenerator {

// Add warnings to be logged
if (this.csvContainsFormulas && escapeFormulaValues) {
warnings.push(
i18n.translate('xpack.reporting.exportTypes.csv.generateCsv.escapedFormulaValues', {
defaultMessage: 'CSV may contain formulas whose values have been escaped',
})
);
warnings.push(i18nTexts.escapedFormulaValuesMessage);
}
} catch (err) {
this.logger.error(err);
if (err instanceof KbnServerError && err.errBody) {
throw JSON.stringify(err.errBody.error);
}

if (err instanceof esErrors.ResponseError && [401, 403].includes(err.statusCode ?? 0)) {
throw new AuthenticationExpiredError();
reportingError = new AuthenticationExpiredError();
warnings.push(i18nTexts.authenticationError.partialResultsMessage);
} else {
throw new UnknownError(err.message);
}
} finally {
// clear scrollID
Expand Down Expand Up @@ -405,6 +410,7 @@ export class CsvGenerator {
csv: { rows: this.csvRowCount },
},
warnings,
error_code: reportingError?.code,
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { i18n } from '@kbn/i18n';

export const i18nTexts = {
escapedFormulaValuesMessage: i18n.translate(
'xpack.reporting.exportTypes.csv.generateCsv.escapedFormulaValues',
{
defaultMessage: 'CSV may contain formulas whose values have been escaped',
}
),
authenticationError: {
partialResultsMessage: i18n.translate(
'xpack.reporting.exportTypes.csv.generateCsv.authenticationExpired.partialResultsMessage',
{
defaultMessage:
'This report contains partial CSV results because authentication expired before it could finish. Try exporting a smaller amount of data or increase your authentication timeout.',
}
),
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ export class ExecuteReportTask implements ReportingTask {
docOutput.size = output.size;
docOutput.warnings =
output.warnings && output.warnings.length > 0 ? output.warnings : undefined;
docOutput.error_code = output.error_code;
} else {
const defaultOutput = null;
docOutput.content = output.toString() || defaultOutput;
Expand Down

0 comments on commit 3285eb1

Please sign in to comment.