[Response Ops][Reporting] Scheduled Reports - List and Disable API by ymao1 · Pull Request #220922 · elastic/kibana

ymao1 · 2025-05-19T17:26:18Z

Note

This PR will be merged into a feature branch

Summary

Adds internal APIs for listing and disabling scheduled reports. For this initial iteration, we are not adding an enable API.

This adds a Manage Scheduled Reports feature privilege that, if the user has it, allows them to view and disable the scheduled reports of any user in the current space.

To Verify

Create a user (user1) with the ability to generate reports but not manage reports. Use the schedule API to schedule some reports as this user.
Create another user (user2) with the ability to manage reports. Use the schedule API to schedule some reports as this user.
As user1, use the Dev Console to list scheduled reports. You should only see those created by user1. user1 should also be able to disable their own reports.
As user2, use the Dev Console to list scheduled reports. You should see reports from both user1 and user2. user2 should be able to disable a report created by user1.

…apping'

…-fix'

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

…te --fix'

… src/core/server/integration_tests/ci_checks'

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

… src/core/server/integration_tests/ci_checks'

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

…heduled-reports-task

SiddharthMantri

One nit regarding the authz config for the new route.

SiddharthMantri · 2025-06-05T15:02:41Z

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

+        security: {
+          authz: {
+            enabled: false,
+            reason: 'This route is opted out from authorization',


Nit: Can this be expanded to better explain the reasoning for opting out?

Updated in 6030bf0

umbopepato · 2025-06-09T14:56:04Z

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

+  next_run: string | undefined;
+  notification: RawScheduledReport['notification'];
+  schedule: RruleSchedule;
+  title: RawScheduledReport['title'];


This title is extracted from the jobParams, right?

Yes, it currently is but we could also make it an explicit field set in the schedule API if that is preferable.

Oh no worries, I receive the title from the sharing data before creating the jobParams string so no need to specify it separately 🙂

src/platform/packages/private/kbn-reporting/common/routes.ts

umbopepato

Tested locally, works as expected, LGTM! 🚀
(Just dropped a few comments)

pmuellr · 2025-06-10T02:52:29Z

I tried out Verify steps with user1 and user2 and everything worked as expected. However, when I use a job id from user2 while logged in with user1 and do a bulk disable, I get the following:

{
  "scheduled_report_ids": [],
  "errors": [
    {
      "message": "Insufficient privileges to disable scheduled report \"c348b993-d106-482f-b538-3c9aee190a9f\".",
      "status": 403,
      "id": "c348b993-d106-482f-b538-3c9aee190a9f"
    }
  ],
  "total": 1
}

I'm wondering if we should return a 404 instead?

If we go that route, perhaps we can log the message we're currently generating as a WARN, since it's obviously useful info ...

…heduled-reports-find

ymao1 · 2025-06-10T13:47:32Z

I'm wondering if we should return a 404 instead?

Updated in 6030bf0

pmuellr

LGTM, left some comments

pmuellr · 2025-06-10T14:00:39Z

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

+
+export function scheduledQueryFactory(reportingCore: ReportingCore): ScheduledQueryFactory {
+  return {
+    async list(req, res, user, page = 1, size = DEFAULT_SCHEDULED_REPORT_LIST_SIZE) {


Feels like we should put each route in a separate file, but perhaps we can plan on doing that when we add the next set of APIs (enable?)

Sounds good. I was following what already existed for the jobs APIs but I guess we can start doing things the way we're used to in response ops now :)

pmuellr · 2025-06-10T14:12:31Z

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

+            });
+          } else {
+            // check if user is allowed to update this scheduled report
+            if (so.attributes.createdBy !== username && !canManageReporting) {


if username is false, presumably we'd always go through this if block, maybe we could exit early instead?

pmuellr · 2025-06-10T14:16:19Z

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

+            // check if user is allowed to update this scheduled report
+            if (so.attributes.createdBy !== username && !canManageReporting) {
+              bulkErrors.push({
+                message: `Insufficient privileges to disable scheduled report "${so.id}".`,


I think the intention with a 404 is to not leak the existance of resources the client doesn't have access to. So the message should probably be "not found". Having the logged warn is good though, since it's ok to "leak" that info in the logs.

Is it considered a leak though? If we try to access a saved object that doesn't exist, we get a Saved object not found error that returns ${type}/${id} in the message. This only returns the ID that we're trying to disable, not any additional information about it.

I think I understand what you're saying...I'll update the message to be a generic not found.

Updated in fbac095

Ya, I think my general experience with things is when you don't have access (haven't logged in), results tend to look like "I can't find that" - but not sure if we have guidelines about things like that ...

pmuellr · 2025-06-10T14:22:08Z

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

+          page,
+          perPage: size,
+          ...(!canManageReporting
+            ? { filter: `scheduled_report.attributes.createdBy: "${username}"` }


username can be false, at least type-wise, so in such cases (API keys?), this would search for reports by false.

Good catch! We actually prevent scheduling reports if security is disabled and api keys are not available because we need that API key for scheduled reports. I probably carried over that typing from the normal reports so I'll fix the typing.

Fixed the types, added a double check that username is specified and added a functional test to ensure we're getting an error when scheduling reports with security disabled: 1f6abbb

elasticmachine · 2025-06-10T18:19:38Z

💚 Build Succeeded

Buildkite Build
Commit: fbac095

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/reporting-common`	94	97	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	1.8MB	1.8MB	+78.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`reporting`	50.7KB	50.8KB	+78.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/reporting-common`	104	107	+3

ESLint disabled line counts

id	before	after	diff
`@kbn/test-suites-xpack`	666	669	+3

References to deprecated APIs

id	before	after	diff
`reporting`	7	9	+2

Total ESLint disabled count

id	before	after	diff
`@kbn/test-suites-xpack`	687	690	+3

History

💚 Build #307036 succeeded 6030bf0
💛 Build #306273 was flaky dac0556
💚 Build #303911 succeeded 83ad071
💛 Build #303207 was flaky 2242eee
💚 Build #301824 succeeded c32bea7
💛 Build #301505 was flaky 5432b88

cc @ymao1

Resolves #216313 ## Summary This is a feature branch that contains the following commits. Each individual linked PR contains a summary and verification instructions. * Schedule API - #219771 * Scheduled report task runner - #219770 * List and disable API - #220922 * Audit logging - #221846 * Send scheduled report emails - #220539 * Commit to check license - f5f9d9d * Update to list API response format - #224262 --------- Co-authored-by: Ersin Erdal <ersin.erdal@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alexi Doak <109488926+doakalexi@users.noreply.github.com>

Resolves elastic#216313 ## Summary This is a feature branch that contains the following commits. Each individual linked PR contains a summary and verification instructions. * Schedule API - elastic#219771 * Scheduled report task runner - elastic#219770 * List and disable API - elastic#220922 * Audit logging - elastic#221846 * Send scheduled report emails - elastic#220539 * Commit to check license - elastic@f5f9d9d * Update to list API response format - elastic#224262 --------- Co-authored-by: Ersin Erdal <ersin.erdal@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alexi Doak <109488926+doakalexi@users.noreply.github.com> (cherry picked from commit a409627) # Conflicts: # src/platform/packages/private/kbn-reporting/common/routes.ts # x-pack/platform/plugins/private/canvas/server/feature.test.ts # x-pack/platform/plugins/private/reporting/server/core.ts # x-pack/platform/plugins/private/reporting/server/features.ts # x-pack/platform/plugins/shared/features/server/__snapshots__/oss_features.test.ts.snap # x-pack/platform/test/api_integration/apis/features/features/features.ts # x-pack/test_serverless/api_integration/test_suites/chat/platform_security/authorization.ts # x-pack/test_serverless/api_integration/test_suites/observability/platform_security/authorization.ts # x-pack/test_serverless/api_integration/test_suites/search/platform_security/authorization.ts # x-pack/test_serverless/api_integration/test_suites/security/platform_security/authorization.ts

ersin-erdal and others added 30 commits April 9, 2025 18:38

Support rrule for task scheduling

a74e49d

remove redundant return

6163f7e

add functional tests

20ecece

[CI] Auto-commit changed files from 'node scripts/styled_components_m…

c29a08e

…apping'

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

de20db1

…-fix'

fix linting

65f0960

Merge branch '216308-support-rrule-for-task-scheduling' of github.com…

2dde5f6

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

fix linter

717cadb

[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…

72b108e

…te --fix'

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

38e69ef

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

c9e71ba

… src/core/server/integration_tests/ci_checks'

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

5f6fad9

Allowing shared concurrency task types

d05d9c8

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

4a04b90

set second to [0]

249f14c

Merge branch '216308-support-rrule-for-task-scheduling' of github.com…

4c60d8e

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

testing schedule is more resilient

4a14142

remove "only"

4ec1bca

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

1275f1e

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

8aff2c9

Merge branch 'main' into shared-concurrency-tasks

04b70a2

Merge branch 'main' into shared-concurrency-tasks

c9547ae

throw when interval is not provided for the next run calculation

850ec64

Merge branch '216308-support-rrule-for-task-scheduling' of github.com…

10c40e5

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

Merge branch 'main' into 216308-support-rrule-for-task-scheduling

af75dfb

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

95cf333

… src/core/server/integration_tests/ci_checks'

update read me

d5227aa

Merge branch '216308-support-rrule-for-task-scheduling' of github.com…

2a1482e

…:ersin-erdal/kibana into 216308-support-rrule-for-task-scheduling

Merge branch 'main' into shared-concurrency-tasks

372d4c7

update read me

62fe072

ymao1 added 5 commits May 23, 2025 09:50

Merge branch 'scheduled-reports' of github.com:elastic/kibana into sc…

d60bbff

…heduled-reports-task

Merge branch 'scheduled-reports' of github.com:elastic/kibana into sc…

3955376

…heduled-reports-task

Merge branch 'scheduled-reports-task' into scheduled-reports-find

2242eee

Merge

c3cb16e

Cleanup

83ad071

ymao1 self-assigned this May 30, 2025

ymao1 marked this pull request as ready for review May 31, 2025 14:42

ymao1 requested review from a team as code owners May 31, 2025 14:42

ymao1 requested review from js-jankisalvi, pmuellr and umbopepato May 31, 2025 14:42

SiddharthMantri self-requested a review June 4, 2025 16:24

SiddharthMantri approved these changes Jun 5, 2025

View reviewed changes

Merge

dac0556

umbopepato reviewed Jun 9, 2025

View reviewed changes

src/platform/packages/private/kbn-reporting/common/routes.ts Show resolved Hide resolved

umbopepato approved these changes Jun 9, 2025

View reviewed changes

ymao1 added 2 commits June 10, 2025 07:54

Merge branch 'scheduled-reports' of github.com:elastic/kibana into sc…

c1a94bd

…heduled-reports-find

PR feedback

6030bf0

pmuellr approved these changes Jun 10, 2025

View reviewed changes

ymao1 added 2 commits June 10, 2025 12:26

PR feedback

1f6abbb

PR feedback

fbac095

ymao1 merged commit c6e35e7 into elastic:scheduled-reports Jun 10, 2025
10 checks passed

ymao1 deleted the scheduled-reports-find branch June 17, 2025 11:50

Conversation

ymao1 commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

To Verify

Uh oh!

SiddharthMantri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

umbopepato left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmuellr commented Jun 10, 2025

Uh oh!

ymao1 commented Jun 10, 2025

Uh oh!

pmuellr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jun 10, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ymao1 commented May 19, 2025 •

edited

Loading

umbopepato left a comment •

edited

Loading