Skip to content

test(auth): pin proxy_user > jwt_user precedence in current_active_user#22

Merged
awais786 merged 2 commits into
foss-mainfrom
fix/proxy-auth-stale-session-on-user-switch
May 16, 2026
Merged

test(auth): pin proxy_user > jwt_user precedence in current_active_user#22
awais786 merged 2 commits into
foss-mainfrom
fix/proxy-auth-stale-session-on-user-switch

Conversation

@awais786
Copy link
Copy Markdown

Summary

SurfSense is architecturally immune to the cross-app stale-session-on-user-switch bug class that affected Pressingly/plane#29 / Pressingly/outline#19 / Pressingly/penpot#18 / Pressingly/twenty#8. The reason: current_active_user in app.users resolves the upstream identity (request.state.proxy_user, set per-request by ProxyAuthMiddleware from X-Auth-Request-Email) before falling back to the persisted JWT identity. The other apps had to add an explicit "compare upstream vs session, flush on mismatch" middleware. SurfSense's existing precedence already gives the same property for free.

That precedence IS the contract. This PR adds 5 unit tests that pin it so a future refactor cannot silently re-introduce the bug.

The contract

async def current_active_user(request, jwt_user=Depends(...)):
    proxy_user = getattr(request.state, "proxy_user", None)
    if proxy_user is not None:
        return proxy_user        # ← upstream wins
    if jwt_user is not None:
        return jwt_user          # ← fallback (header-absent paths)
    raise 401

If a future "simplify auth to JWT-only" change removes the proxy_user branch, type-checks pass and the regression ships. These tests catch it.

Cross-app reference

Test plan

pytest surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py -v — 5 cases, all passing:

  • test_proxy_user_wins_when_both_proxy_and_jwt_present — the user-switch scenario: proxy says alice, JWT says bob (stale), result MUST be alice
  • test_falls_back_to_jwt_when_proxy_user_absent — header-absent is NOT a logout signal per spec; JWT serves the request
  • test_raises_401_when_neither_proxy_nor_jwt_present — sanity
  • test_optional_user_returns_proxy_when_both_present — same precedence in the optional variant
  • test_optional_user_returns_none_when_neither_present — optional variant returns None instead of raising

Why this is a test-only PR

No production-code change. SurfSense's current_active_user already implements the correct precedence; the architectural-immunity audit in surfsense-security.md confirms it. This PR's contribution is the regression guard — a test suite that fails if anyone flips the precedence or removes the proxy_user check.

Out of scope

This is the SurfSense leg of the cross-app stale-session-on-user-switch family. The other four apps' fixes:

  • Plane: #29django.contrib.auth.logout(request) on mismatch
  • Outline: #19 — throw 401 → outer catch clears accessToken cookie
  • Penpot: #18 — three-path handling (re-key / 403 / drop+expire)
  • Twenty: #8clearCookie('tokenPair') before guard returns false

🤖 Generated with Claude Code

Regression-guard for SurfSense's architectural immunity to the
cross-app stale-session-on-user-switch class of bug. SurfSense doesn't
need an explicit Rule-2-style "compare upstream identity vs session
identity → flush on mismatch" middleware (like Plane MODSetter#29, Outline #19,
Penpot #18, Twenty #8 do) because `current_active_user` in app.users
already resolves to the upstream identity (proxy_user) over the
persisted session (jwt_user) whenever both are present.

That precedence IS the contract. If a future refactor flips it (or
removes the proxy_user check during a "simplify auth" pass) the
stale-session bug class is silently re-introduced — and type-checks
pass, so it would ship.

These five tests pin the contract:

  1. proxy_user wins when both proxy_user and jwt_user are present
     with different identities (the user-switch scenario)
  2. Falls back to jwt_user when proxy_user is absent (header-absent
     is NOT a logout signal — internal calls, OPTIONS preflight,
     direct backend hits at 127.0.0.1 legitimately arrive without
     a proxy header)
  3. Raises 401 when neither is present (sanity)
  4. Same precedence for current_optional_user
  5. current_optional_user returns None (does not raise) when neither
     is present

Cross-app contract:
  awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md
SurfSense's architectural-immunity reasoning:
  awais786/sso-rules-moneta:surfsense-security.md

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds regression tests to pin current_active_user / current_optional_user precedence so proxy-auth identity wins over stale JWT identity.

Changes:

  • Adds five async unit tests covering proxy-vs-JWT precedence, JWT fallback, unauthenticated rejection, and optional-user behavior.
  • Uses lightweight request/user helpers and mocks SMB auto-join side effects where needed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return user


@pytest.mark.asyncio
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in commit 27a1bc3: added a module-level pytestmark = pytest.mark.unit to surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py so these tests are included by the existing pytest -m unit CI selection.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

@awais786 awais786 merged commit 092a1e6 into foss-main May 16, 2026
8 of 10 checks passed
@awais786 awais786 deleted the fix/proxy-auth-stale-session-on-user-switch branch May 16, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants