Skip to content

Commit 6030bf0

Browse files
ymao1ymao1
committed
PR feedback
1 parent c1a94bd commit 6030bf0

File tree

5 files changed

+30
-12
lines changed

5 files changed

+30
-12
lines changed

‎x-pack/platform/plugins/private/reporting/server/routes/common/scheduled/scheduled_query.test.ts‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ const lastRunResponse: CreatedAtSearchResponse = {
144144
created_at: '2025-05-06T21:12:07.198Z',
145145
},
146146
fields: {
147-
'scheduled_report_id.keyword': ['2da1cb75-04c7-4202-a9f0-f8bcce63b0f4'],
147+
scheduled_report_id: ['2da1cb75-04c7-4202-a9f0-f8bcce63b0f4'],
148148
},
149149
sort: [1746565930198],
150150
},
@@ -156,7 +156,7 @@ const lastRunResponse: CreatedAtSearchResponse = {
156156
created_at: '2025-05-06T12:00:00.500Z',
157157
},
158158
fields: {
159-
'scheduled_report_id.keyword': ['aa8b6fb3-cf61-4903-bce3-eec9ddc823ca'],
159+
scheduled_report_id: ['aa8b6fb3-cf61-4903-bce3-eec9ddc823ca'],
160160
},
161161
sort: [1746565930198],
162162
},
@@ -227,14 +227,14 @@ describe('scheduledQueryFactory', () => {
227227
expect(client.search).toHaveBeenCalledTimes(1);
228228
expect(client.search).toHaveBeenCalledWith({
229229
_source: ['created_at'],
230-
collapse: { field: 'scheduled_report_id.keyword' },
230+
collapse: { field: 'scheduled_report_id' },
231231
index: '.reporting-*,.kibana-reporting*',
232232
query: {
233233
bool: {
234234
filter: [
235235
{
236236
terms: {
237-
'scheduled_report_id.keyword': [
237+
scheduled_report_id: [
238238
'aa8b6fb3-cf61-4903-bce3-eec9ddc823ca',
239239
'2da1cb75-04c7-4202-a9f0-f8bcce63b0f4',
240240
],
@@ -258,6 +258,7 @@ describe('scheduledQueryFactory', () => {
258258
created_by: 'elastic',
259259
enabled: true,
260260
jobtype: 'printable_pdf_v2',
261+
object_type: 'dashboard',
261262
last_run: '2025-05-06T12:00:00.500Z',
262263
next_run: expect.any(String),
263264
schedule: {
@@ -277,6 +278,7 @@ describe('scheduledQueryFactory', () => {
277278
created_by: 'not-elastic',
278279
enabled: true,
279280
jobtype: 'PNGV2',
281+
object_type: 'dashboard',
280282
last_run: '2025-05-06T21:12:07.198Z',
281283
next_run: expect.any(String),
282284
notification: {
@@ -317,14 +319,14 @@ describe('scheduledQueryFactory', () => {
317319
expect(client.search).toHaveBeenCalledTimes(1);
318320
expect(client.search).toHaveBeenCalledWith({
319321
_source: ['created_at'],
320-
collapse: { field: 'scheduled_report_id.keyword' },
322+
collapse: { field: 'scheduled_report_id' },
321323
index: '.reporting-*,.kibana-reporting*',
322324
query: {
323325
bool: {
324326
filter: [
325327
{
326328
terms: {
327-
'scheduled_report_id.keyword': [
329+
scheduled_report_id: [
328330
'aa8b6fb3-cf61-4903-bce3-eec9ddc823ca',
329331
'2da1cb75-04c7-4202-a9f0-f8bcce63b0f4',
330332
],
@@ -401,6 +403,7 @@ describe('scheduledQueryFactory', () => {
401403
created_by: 'elastic',
402404
enabled: true,
403405
jobtype: 'printable_pdf_v2',
406+
object_type: 'dashboard',
404407
next_run: expect.any(String),
405408
schedule: {
406409
rrule: {
@@ -419,6 +422,7 @@ describe('scheduledQueryFactory', () => {
419422
created_by: 'not-elastic',
420423
enabled: true,
421424
jobtype: 'PNGV2',
425+
object_type: 'dashboard',
422426
next_run: expect.any(String),
423427
notification: {
424428
email: {
@@ -531,11 +535,14 @@ describe('scheduledQueryFactory', () => {
531535
id: '2da1cb75-04c7-4202-a9f0-f8bcce63b0f4',
532536
message:
533537
'Insufficient privileges to disable scheduled report "2da1cb75-04c7-4202-a9f0-f8bcce63b0f4".',
534-
status: 403,
538+
status: 404,
535539
},
536540
],
537541
total: 2,
538542
});
543+
expect(mockLogger.warn).toHaveBeenCalledWith(
544+
`User "elastic" attempted to disable scheduled report "2da1cb75-04c7-4202-a9f0-f8bcce63b0f4" created by "not-elastic" without sufficient privileges.`
545+
);
539546
});
540547

541548
it('should handle errors in bulk get', async () => {
@@ -882,6 +889,7 @@ describe('transformResponse', () => {
882889
created_by: 'elastic',
883890
enabled: true,
884891
jobtype: 'printable_pdf_v2',
892+
object_type: 'dashboard',
885893
last_run: '2025-05-06T12:00:00.500Z',
886894
next_run: expect.any(String),
887895
schedule: {
@@ -901,6 +909,7 @@ describe('transformResponse', () => {
901909
created_by: 'not-elastic',
902910
enabled: true,
903911
jobtype: 'PNGV2',
912+
object_type: 'dashboard',
904913
last_run: '2025-05-06T21:12:07.198Z',
905914
next_run: expect.any(String),
906915
notification: {
@@ -941,6 +950,7 @@ describe('transformResponse', () => {
941950
created_by: 'elastic',
942951
enabled: true,
943952
jobtype: 'printable_pdf_v2',
953+
object_type: 'dashboard',
944954
last_run: undefined,
945955
next_run: expect.any(String),
946956
schedule: {
@@ -960,6 +970,7 @@ describe('transformResponse', () => {
960970
created_by: 'not-elastic',
961971
enabled: true,
962972
jobtype: 'PNGV2',
973+
object_type: 'dashboard',
963974
last_run: '2025-05-06T21:12:07.198Z',
964975
next_run: expect.any(String),
965976
notification: {
@@ -992,6 +1003,7 @@ describe('transformResponse', () => {
9921003
created_by: 'elastic',
9931004
enabled: true,
9941005
jobtype: 'printable_pdf_v2',
1006+
object_type: 'dashboard',
9951007
last_run: undefined,
9961008
next_run: expect.any(String),
9971009
schedule: {
@@ -1011,6 +1023,7 @@ describe('transformResponse', () => {
10111023
created_by: 'not-elastic',
10121024
enabled: true,
10131025
jobtype: 'PNGV2',
1026+
object_type: 'dashboard',
10141027
last_run: undefined,
10151028
next_run: expect.any(String),
10161029
notification: {

‎x-pack/platform/plugins/private/reporting/server/routes/common/scheduled/scheduled_query.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ import { SCHEDULED_REPORT_SAVED_OBJECT_TYPE } from '../../../saved_objects';
2626
export const MAX_SCHEDULED_REPORT_LIST_SIZE = 100;
2727
export const DEFAULT_SCHEDULED_REPORT_LIST_SIZE = 10;
2828

29-
// TODO - remove .keyword when mapping is fixed
30-
const SCHEDULED_REPORT_ID_FIELD = 'scheduled_report_id.keyword';
29+
const SCHEDULED_REPORT_ID_FIELD = 'scheduled_report_id';
3130
const CREATED_AT_FIELD = 'created_at';
3231
const getUsername = (user: ReportingUser) => (user ? user.username : false);
3332

@@ -85,6 +84,7 @@ export function transformResponse(
8584
created_by: so.attributes.createdBy,
8685
enabled: so.attributes.enabled,
8786
jobtype: so.attributes.jobType,
87+
object_type: so.attributes.meta.objectType,
8888
last_run: lastRunForId?._source?.[CREATED_AT_FIELD],
8989
next_run: _rrule.after(new Date())?.toISOString(),
9090
notification: so.attributes.notification,
@@ -210,9 +210,12 @@ export function scheduledQueryFactory(reportingCore: ReportingCore): ScheduledQu
210210
if (so.attributes.createdBy !== username && !canManageReporting) {
211211
bulkErrors.push({
212212
message: `Insufficient privileges to disable scheduled report "${so.id}".`,
213-
status: 403,
213+
status: 404,
214214
id: so.id,
215215
});
216+
logger.warn(
217+
`User "${username}" attempted to disable scheduled report "${so.id}" created by "${so.attributes.createdBy}" without sufficient privileges.`
218+
);
216219
} else if (so.attributes.enabled === false) {
217220
logger.debug(`Scheduled report ${so.id} is already disabled`);
218221
disabledScheduledReportIds.add(so.id);

‎x-pack/platform/plugins/private/reporting/server/routes/internal/management/scheduled.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ export function registerScheduledRoutesInternal(reporting: ReportingCore, logger
3434
security: {
3535
authz: {
3636
enabled: false,
37-
reason: 'This route is opted out from authorization',
37+
reason:
38+
'This route is opted out from authorization because reporting uses its own authorization model.',
3839
},
3940
},
4041
validate: {

‎x-pack/platform/plugins/private/reporting/server/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ export interface ListScheduledReportApiJSON {
129129
created_by: RawScheduledReport['createdBy'];
130130
enabled: RawScheduledReport['enabled'];
131131
jobtype: RawScheduledReport['jobType'];
132+
object_type: RawScheduledReport['meta']['objectType'];
132133
last_run: string | undefined;
133134
next_run: string | undefined;
134135
notification: RawScheduledReport['notification'];

‎x-pack/test/reporting_api_integration/reporting_and_security/disable_scheduled_reports.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export default function ({ getService }: FtrProviderContext) {
9292
errors: [
9393
{
9494
message: `Insufficient privileges to disable scheduled report "${reportId}".`,
95-
status: 403,
95+
status: 404,
9696
id: reportId,
9797
},
9898
],

0 commit comments

Comments
 (0)