fitzee opened a new pull request, #45698: URL: https://github.com/apache/superset/pull/45698
### SUMMARY A scheduled report's chart or dashboard access was checked only as the **saving user**. The identity the report actually runs as (the legacy `ALERT_REPORTS_EXECUTORS` resolution, e.g. a fixed service account, or the SIP-209 "Run As" user) was never checked. If that identity couldn't see the target (for example, because access came from grants only the creator holds), the report saved successfully but could never deliver: - **Dashboards with a tab anchor, URL params, or native filters:** building the permalink as the executor raised a bare `DashboardAccessDeniedError` *before* the execution was claimed. No execution log row was written, the report state never changed, and no one was notified. The reason only reached the Celery worker log. The editors' failure notification also built that permalink as the executor, so it failed too. - **Plain dashboard URLs:** the dashboard page returned a 404 to the executor and the run failed as a generic screenshot error, with no access reason. This PR: 1. **Validates the execution identity at save time.** On create/update, resolve the executor the saved schedule will run as and check the chart/dashboard under that identity. A denial returns a field-level 422 on `dashboard`/`chart`. Exempt: paused schedules (so a broken schedule can always be paused), alerts without attachments, executors that can't be resolved until run time, and edits that leave the target, executor, delivery, and active state unchanged. The executor's username is shown only to admins or to a saver who is the executor; everyone else gets a generic message. 2. **Records executor access failures at execution.** Dashboard access is checked as the executor before any URL is built. A denial raises `ReportScheduleExecutorAccessDeniedError`, which names the executor and the security manager's specific reason. The pre-claim permalink pre-commit step skips this denial, so the claimed state machine records it as an ERROR log row and ERROR state and notifies editors. The failure notification falls back to a plain dashboard link. The denial is not retried. 3. **Keeps public responses unchanged.** `DashboardDAO.get_by_id_or_slug` and the permalink API/view are untouched and still return the generic "You don't have access to this dashboard." The specific reason is read from the denial's `__cause__` only inside report execution, so a custom security manager's denial text never reaches users without access. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend only) ### TESTING INSTRUCTIONS Automated: - `tests/unit_tests/commands/report/executor_target_access_test.py` (new, 17 tests): save-time executor resolution and checks, exemptions, username disclosure, execution-time conversion of the denial using `__cause__`, and the pre-commit skip. - `tests/integration_tests/reports/api_tests.py`: create/update returns 422 when the saving user can access the dashboard but the configured executor can't; a paused report still saves; an admin-chosen "Run As" user without access returns 422. These returned 201/200 before the fix. - `tests/integration_tests/reports/commands_tests.py`: tab-permalink and plain-URL dashboard reports whose executor lacks access end with an ERROR log row, ERROR state, and an editors' email carrying the specific reason. Before the fix, no log row was written. - `tests/integration_tests/dashboards/permalink/api_tests.py`: with a security manager that returns a specific denial reason, the permalink API and the `/superset/dashboard/p/<key>/` view still return the generic message. Manual: 1. Configure `ALERT_REPORTS_EXECUTORS` with a fixed user that cannot access dashboard D, while your own user can. 2. Create a report on D → expect a 422 on the dashboard field (previously saved fine). 3. For an existing report in that state, trigger execution → expect an ERROR entry in the report's execution log naming the access failure, and a failure email to editors (previously no log entry for dashboards with tabs/filters). Not covered: a real-webdriver run of the old plain-URL 404 behaviour. With this change the access check fails before the browser loads anything, so that path is no longer reached for this case. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
