fix(api): return 401 not 500 for auth errors in CurrentUserRestApi (SC-120417)

[eschutho]

SUMMARY

CurrentUserRestApi's three /api/v1/me/ endpoints — get_me, get_my_roles, and update_me — resolve g.user (a flask-login lazy proxy) inside their method bodies. When flask-login lazily resolves that proxy for a validly-authenticated JWT whose bearer is a real workspace member with no assigned role (Preset Manager's "no access" sentinel), it raises UserLookupError("No access"). A missing/invalid authorization context similarly raises NoAuthorizationError.

Neither exception was caught anywhere in the call chain. It propagated all the way to flask_appbuilder's @safe decorator, whose bare except Exception: log.exception(e); return self.response_500(...) logs a full ERROR traceback and returns a generic 500 — instead of the clean 401 these endpoints are documented to return for unauthenticated/unauthorized callers.

Sentry: SUPERSET-PYTHON-NMA — UserLookupError: No access, culprit CurrentUserRestApi.get_me. 10,471 events / 31 users (firstSeen 2024-07-09, still firing).

Root cause (two overlapping gaps):

1. Regression from fix(api): Fix JWT authentication for /api/v1/me endpoints #36410 (merged 2025-12-11): before that PR, all three methods carried manual try/except NoAuthorizationError: return self.response_401() guards around g.user access. fix(api): Fix JWT authentication for /api/v1/me endpoints #36410 removed them, on the assumption that @protect() fully covers auth failures. It does not — flask_appbuilder's protect() performs no exception handling of its own around the lazy g.user/current_user resolution that happens later, inside the body.
2. Always-missing UserLookupError coverage: even the old guards only caught NoAuthorizationError. UserLookupError (the "member with no role" path, which Preset treats as the expected, routine outcome for those users — not a sync gap) was never handled here at all.

FIX

Add explicit handling around the g.user access in all three CurrentUserRestApi methods, catching both NoAuthorizationError and UserLookupError (both from flask_jwt_extended.exceptions) and returning self.response_401(). This mirrors the established precedent already in this codebase — superset/views/base.py's api decorator — which logs auth-outcome exceptions at WARNING (not ERROR/exc_info-heavy stack spam) before returning 401, reserving ERROR+500 for genuinely unexpected failures.

The change is deliberately surgical:

• @protect()/flask_appbuilder is third-party and untouched.
• Other endpoints in the file (e.g. UserRestApi/avatar) that were never part of this bug are untouched.
• update_me already had a try/except ValidationError block, so the new except clause was added alongside it rather than nesting a second try.

TRADEOFFS

None. This strictly repairs a broken-in-a-worse-way-than-intended auth response: a request that should always have been a 401 was instead emitting a 500 plus a noisy ERROR traceback on every occurrence. No legitimate-user-facing behavior changes — successful requests still return 200 with identical payloads, and genuinely unexpected exceptions still fall through to @safe's ERROR+500 handler unchanged. The only observable differences are (a) the correct 401 status for the affected auth outcomes and (b) WARNING-level (instead of ERROR-level) logging for them.

FOLLOW-UPS

None expected.

TESTING INSTRUCTIONS

New unit tests: tests/unit_tests/views/test_current_user_auth_errors.py.

They drive the real endpoints via the test client (with @protect() satisfied through the full_api_access fixture) and patch the module-level g so that resolving g.user inside each body raises the target exception — exactly as flask-login does in production — then assert the response is 401.

Coverage: get_me, get_my_roles, and update_me, each against both UserLookupError and NoAuthorizationError (6 cases).

Non-vacuous verification (stash-the-fix / keep-the-tests):

# WITH the fix:
$ pytest tests/unit_tests/views/test_current_user_auth_errors.py -q
......                                                                   [100%]
6 passed in 0.96s

# git stash push -- superset/views/users/api.py   (revert only the fix, keep the tests)
$ pytest tests/unit_tests/views/test_current_user_auth_errors.py -q
...
    return self.response(200, result=user_response_schema.dump(g.user))
                                                               ^^^^^^
flask_jwt_extended.exceptions.NoAuthorizationError: Missing Authorization Header
6 failed in 1.18s

The pre-existing tests/unit_tests/views/test_current_user_api.py (4 tests) continues to pass. pre-commit run --files ... (ruff, ruff-format, mypy, pylint) passes on the changed files.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

---

Shortcut: SC-120417
Fixes SUPERSET-PYTHON-NMA

[bito-code-review]

Code Review Agent Run #bd7794

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 5062bb7..5062bb7

  • superset/views/users/api.py
  • tests/unit_tests/views/test_current_user_auth_errors.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.17%. Comparing base (8e66145) to head (5062bb7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #44213   +/-   ##
=======================================
  Coverage   80.17%   80.17%           
=======================================
  Files        2925     2925           
  Lines      172670   172684   +14     
  Branches    40087    40087           
=======================================
+ Hits       138440   138454   +14     
  Misses      31633    31633           
  Partials     2597     2597           

Flag	Coverage Δ
hive	37.33% <17.64%> (-0.01%)	⬇️
mysql	56.86% <47.05%> (-0.01%)	⬇️
postgres	56.88% <47.05%> (-0.01%)	⬇️
presto	39.21% <17.64%> (-0.01%)	⬇️
python	84.56% <100.00%> (+<0.01%)	⬆️
sqlite	56.58% <47.05%> (-0.01%)	⬇️
unit	75.90% <94.11%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eschutho]

@richardfogaca assigning you as reviewer here. This is the auth-error handling on the /api/v1/me/ endpoints in superset/views/users/api.py — part of the Sentry raw-500 → clean-4xx cleanup series (SC-120417). Thanks!

[rusackas]

Checked the precedent claim against views/base.py's api decorator, it's an exact match (same WARNING message, same 401 pattern). Good surgical fix, scoped to just the three methods, and the stash-the-fix verification in the PR body is a nice touch. Approving.