You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/09/03 15:13:41 UTC
[PR] fix(security): guard is_guest_user against NoAuthorizationError on unauthenticated error paths [superset]
eschutho opened a new pull request, #43826:
URL: https://github.com/apache/superset/pull/43826
### SUMMARY
Fixes [SUPERSET-PYTHON-15J6](https://preset-inc.sentry.io/issues/7684373355/) — `NoAuthorizationError: Missing JWT in cookies or headers`, **28,783 events** since firstSeen 2026-08-21, still firing daily.
**The Sentry-captured exception is not the original problem — it is a _secondary_ exception raised while Flask is already handling an unrelated, benign exception** (in the sampled events, a 405 `MethodNotAllowed` from a request hitting the wrong HTTP method on a route, before any view/auth code runs). The same crash can happen for any early-in-request-lifecycle `HTTPException`.
#### Root-cause chain
1. Flask raises `MethodNotAllowed` (405) during routing, before any view/auth code runs.
2. The global `@app.errorhandler(HTTPException) def show_http_exception` (`superset/views/error_handling.py`) catches it and calls `json_error_response(...)`.
3. `json_error_response` calls `sanitize_superset_errors()` (`superset/utils/error_sanitization.py`).
4. That calls `is_sanitization_required()` → `security_manager.is_guest_user()`.
5. `is_guest_user()` with no `user` argument calls `get_current_user()` (`superset/tasks/utils.py`), whose `g.user` access forces resolution of flask_login's `current_user` `LocalProxy`.
6. Resolving the proxy invokes the app's registered request loader. For a request with **no guest token, no JWT, and not a public-workspace / MCP-OAuth path**, the loader calls `verify_jwt_in_request()` and lets it raise (by design — a global flask-jwt-extended handler is meant to turn that into a 401 for a real unauthenticated _view_ request).
7. But here the raise happens **inside `show_http_exception`, the error handler for the original 405**. This second exception is not caught by anything and propagates out unhandled — captured by Sentry, and turning a benign 405 into an actual crash/500 for any embedded-enabled deployment, for any unauthenticated request that trips an `HTTPException` path before auth ever runs.
#### The fix
Wrap the current-user resolution inside `SupersetSecurityManager.is_guest_user()` in `try/except NoAuthorizationError: return False`.
A request that carries no JWT and no guest token **definitionally cannot be an embedded guest viewer**, so returning `False` is the semantically correct answer, not merely crash avoidance. `is_guest_user` is the shared choke point behind ~14 call sites; only the error-sanitization path is normally reachable before auth runs, and this fix protects all of them without changing behavior for any already-authenticated caller.
`NoAuthorizationError` is imported from `flask_jwt_extended.exceptions` (the top-level package does not re-export it). The fix lives entirely in `superset/security/manager.py`; `error_sanitization.py`, `error_handling.py`, and the private request-loader are intentionally untouched — their behavior is correct for real view requests.
### TESTING INSTRUCTIONS
Added `tests/unit_tests/security/manager_test.py::test_is_guest_user_no_jwt_returns_false_without_raising`: with `EMBEDDED_SUPERSET` enabled and the current-user resolution raising `NoAuthorizationError`, `is_guest_user()` must return `False` rather than propagate.
Verified the test **fails before** the fix (raises `NoAuthorizationError`) and **passes after**:
```
$ pytest tests/unit_tests/security/manager_test.py tests/unit_tests/utils/test_error_sanitization.py -q
136 passed
$ ruff check superset/security/manager.py tests/unit_tests/security/manager_test.py
All checks passed!
$ ruff format --check superset/security/manager.py tests/unit_tests/security/manager_test.py
2 files already formatted
```
`pre-commit` (mypy, ruff, ruff-format, pylint) clean on the changed files.
### Tradeoffs
**None as a failure-mode change.** This makes `is_guest_user()` never raise where it previously could sometimes crash the request, which is strictly more correct:
- For an unauthenticated request with no JWT/guest token, `False` is the semantically correct return value (such a request cannot be an embedded guest), so no legitimate guest is misclassified.
- For any already-authenticated caller (real guest token or logged-in user), the proxy resolves without raising, so `NoAuthorizationError` is never hit and behavior is byte-for-byte unchanged.
- No security-boundary change: it never grants guest treatment where it wasn't already granted — it only stops a benign error path from escalating into an unhandled 500. Nothing here is undisclosed.
### ADDITIONAL INFORMATION
- Sentry: [SUPERSET-PYTHON-15J6](https://preset-inc.sentry.io/issues/7684373355/) (28,783 events, auto-resolves on merge via `Fixes SUPERSET-PYTHON-15J6`)
- Shortcut: [SC-119741](https://app.shortcut.com/preset/story/119741)
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): guard is_guest_user against NoAuthorizationError on unauthenticated error paths [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43826:
URL: https://github.com/apache/superset/pull/43826#issuecomment-5528106508
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43826?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `83.33333%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 66.22%. Comparing base ([`669a0ce`](https://app.codecov.io/gh/apache/superset/commit/669a0cefb49533961fb7b9bff2cc4a46550a6e04?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0b65d05`](https://app.codecov.io/gh/apache/superset/commit/0b65d054af51b61bf8419f6533d146c617a98fc5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/43826?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/security/manager.py](https://app.codecov.io/gh/apache/superset/pull/43826?src=pr&el=tree&filepath=superset%2Fsecurity%2Fmanager.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | 83.33% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43826?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43826 +/- ##
===========================================
- Coverage 79.41% 66.22% -13.19%
===========================================
Files 2894 2894
Lines 167820 167824 +4
Branches 38863 38863
===========================================
- Hits 133269 111138 -22131
- Misses 32051 54414 +22363
+ Partials 2500 2272 -228
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.78% <16.66%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.67% <16.66%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.36% <83.33%> (-26.50%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.23% <83.33%> (-0.01%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43826/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43826?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): guard is_guest_user against NoAuthorizationError on unauthenticated error paths [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #43826:
URL: https://github.com/apache/superset/pull/43826
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): guard is_guest_user against NoAuthorizationError on unauthenticated error paths [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43826:
URL: https://github.com/apache/superset/pull/43826#issuecomment-5527852581
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org