Abdulrehman-PIAIC80387 opened a new pull request, #42611: URL: https://github.com/apache/superset/pull/42611
### SUMMARY Fixes #33584. `get_executor` in `superset/tasks/utils.py` walks the configured executor priority list (CREATOR → MODIFIER → EDITOR → …) and returns the first matching owner/creator/modifier without checking whether that user is active. When the picked user was later deactivated, their username lands in `login_user()` in `MachineAuthProviderUser.get_auth_cookies`, which silently returns `False` for inactive users. No session cookie is set; the downstream screenshot / CSV / dashboard-render request runs with no auth and surfaces as an opaque `ReportScheduleCsvFailedError`. Per @rusackas on the [issue thread](https://github.com/apache/superset/issues/33584), the fix is to have `get_executor` skip inactive owners so the priority chain falls through to an active one (or raises the existing `ExecutorNotFoundError` if none exist). ### FIX Adds a small `_is_active(user)` predicate and applies it at every direct-user return site in `get_executor`: - `CREATOR`, `MODIFIER`, both `_EDITOR` variants - `EDITOR`'s modifier/creator/editor-users fallback chain Intentionally left unchanged: - **`_get_indirect_editor_user`** — already filters on `User.active.is_(True)` - **`FixedExecutor` / `CURRENT_USER`** — not `ab_user`-backed at this layer - **`login_user(force=True)`** — considered and rejected: bypassing the FAB active-user check would let deactivated users execute schedules on their behalf, which is a regression, not a fix ### BEHAVIOR MATRIX | Scenario | Before | After | |---|---|---| | Creator active | uses creator ✓ | uses creator ✓ (unchanged) | | Creator inactive, modifier active | picks creator → auth fails → `ReportScheduleCsvFailedError` | uses modifier ✓ | | All owners inactive | picks first inactive → auth fails → `ReportScheduleCsvFailedError` | `ExecutorNotFoundError` (actionable) | | Indirect (role/group) editor | filtered by `is_active` ✓ | filtered by `is_active` ✓ (unchanged) | ### TESTING INSTRUCTIONS Manual (requires a working Alerts & Reports setup): 1. Create a scheduled report as user A 2. Add user B as an owner 3. Deactivate user A (`UPDATE ab_user SET active=false WHERE username='A'`) 4. Trigger the schedule 5. **Before:** `ReportScheduleCsvFailedError` in logs, no email 6. **After:** report sends; the executor log line names user B Automated: ```bash pytest tests/unit_tests/tasks/test_utils.py::test_get_executor -v ``` Four new parametrized cases: inactive-creator-falls-through, all-inactive-raises, EDITOR-skips-inactive-editor-user, CREATOR_EDITOR-inactive-editor-falls-through. Existing cases stay green because the `_get_users` / `_make_user_subject` helpers now carry an explicit `active` flag defaulted to `True`. ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes #33584 - [x] Required feature flags: none - [x] Changes UI: no - [x] Includes DB Migration: no - [x] Includes CLI or Node.js commands: no - [x] Breaking change: no -- 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]
