diff --git a/routes/session_routes.py b/routes/session_routes.py index 5caf7d5..6533444 100644 --- a/routes/session_routes.py +++ b/routes/session_routes.py @@ -11,12 +11,12 @@ from core.session_manager import SessionManager from core.models import ChatMessage from src.request_models import SessionResponse from core.database import Session as DbSession, SessionLocal, Document, GalleryImage -from src.auth_helpers import get_current_user +from src.auth_helpers import get_current_user, effective_user def _verify_session_owner(request: Request, session_id: str): """Verify the current user owns the session. Raises 404 if not.""" - user = get_current_user(request) + user = effective_user(request) if not user: raise HTTPException(403, "Authentication required") db = SessionLocal() @@ -63,7 +63,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ @router.get("/sessions") def list_sessions(request: Request): - user = get_current_user(request) + user = effective_user(request) # Lazy purge: incognito sessions are ephemeral by design — wipe leftovers # from the DB and session_manager so they vanish on the next page refresh. # BUT: skip sessions that were created within the last 10 minutes. @@ -217,7 +217,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ model_to_use = found sid = str(uuid.uuid4()) - user = get_current_user(request) + user = effective_user(request) session = session_manager.create_session( session_id=sid, name=name or "", @@ -499,7 +499,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ @router.get("/sessions/archived") def list_archived_sessions(request: Request, search: str = "", offset: int = 0, limit: int = 20, sort: str = "recent", model: str = ""): """List archived sessions for the archive browser.""" - user = get_current_user(request) + user = effective_user(request) db = SessionLocal() try: q = db.query(DbSession).filter(DbSession.archived == True) @@ -635,7 +635,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ @router.post("/sessions/save") def sessions_save_now(request: Request): - user = get_current_user(request) + user = effective_user(request) if not user: raise HTTPException(401, "Not authenticated") session_manager.save_sessions() @@ -651,7 +651,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ if not OPENAI_API_KEY: raise HTTPException(400, "Server missing OPENAI_API_KEY") sid = str(uuid.uuid4()) - user = get_current_user(request) + user = effective_user(request) session = session_manager.create_session( session_id=sid, name="", @@ -791,7 +791,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_ users can clean junk without spending tokens. """ from src.llm_core import llm_call - user = get_current_user(request) + user = effective_user(request) user_sessions = session_manager.get_sessions_for_user(user) # Delete empty and throwaway sessions before sorting diff --git a/src/auth_helpers.py b/src/auth_helpers.py index 0e9ca23..e9265f4 100644 --- a/src/auth_helpers.py +++ b/src/auth_helpers.py @@ -10,6 +10,30 @@ def get_current_user(request: Request) -> Optional[str]: return getattr(request.state, 'current_user', None) +def effective_user(request: Request): + """The real human behind the request, for ownership/attribution. + + Cookie sessions resolve to the logged-in username. Bearer ``ody_`` callers + come through as the sandboxed pseudo-user "api" so they can't wander into + cookie/user routes by default, but their token was minted by, and belongs + to, a real owner stamped on ``request.state.api_token_owner``. Routes that + should attribute a token's actions to that owner (sessions, chat history) + call this instead of :func:`get_current_user`, so a paired client sees and + creates the SAME data as the owner's desktop UI rather than a separate + "api"-owned silo. + + For cookie sessions this is identical to :func:`get_current_user`, so + swapping a route over is a no-op for browser users. A bearer token with no + owner falls back to :func:`get_current_user` (the "api" pseudo-user), so it + never escalates. + """ + if getattr(request.state, "api_token", False): + owner = getattr(request.state, "api_token_owner", None) + if owner: + return owner + return get_current_user(request) + + def _auth_disabled() -> bool: """True when the operator has explicitly turned off auth via .env. Mirrors the AUTH_ENABLED parse in app.py / core/middleware.py so the diff --git a/tests/test_session_owner_attribution.py b/tests/test_session_owner_attribution.py new file mode 100644 index 0000000..504634c --- /dev/null +++ b/tests/test_session_owner_attribution.py @@ -0,0 +1,113 @@ +"""Tests for token-owner session attribution (effective_user + session routes). + +Proves the two properties the review asked for: + - cookie/browser users are completely unchanged (no-op swap) + - a bearer token for owner A can never read/verify owner B's session, and a + bearer token with no owner does not escalate. + +Follows the direct-helper + mocked-DB style of tests/test_null_owner_gates.py. +""" + +import os +import sys +import types +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +# routes.session_routes imports several heavy modules at import time that blow up +# under conftest's sqlalchemy/* MagicMock stubs (declarative classes). Stub them +# so we can import the module and exercise _verify_session_owner with a mock DB. +_STUBS = { + "core.database": {"Session": MagicMock(), "SessionLocal": MagicMock(), + "Document": MagicMock(), "GalleryImage": MagicMock()}, + "core.session_manager": {"SessionManager": MagicMock()}, + "core.models": {"ChatMessage": MagicMock()}, + "src.request_models": {"SessionResponse": MagicMock()}, +} +for _name, _attrs in _STUBS.items(): + if _name not in sys.modules: + _m = types.ModuleType(_name) + for _k, _v in _attrs.items(): + setattr(_m, _k, _v) + sys.modules[_name] = _m + +from fastapi import HTTPException # noqa: E402 + +from src.auth_helpers import effective_user # noqa: E402 +import routes.session_routes as SR # noqa: E402 + + +def _req(**state): + return SimpleNamespace(state=SimpleNamespace(**state)) + + +# --- effective_user: who a request is attributed to ------------------------ + +def test_cookie_user_is_unchanged(): + # The whole point: browser/cookie callers behave exactly as before. + assert effective_user(_req(api_token=False, current_user="alice")) == "alice" + + +def test_bearer_token_attributes_to_its_owner(): + # A paired phone runs as the "api" pseudo-user but must act as the token owner. + assert effective_user(_req(api_token=True, api_token_owner="alice", current_user="api")) == "alice" + + +def test_bearer_token_without_owner_does_not_escalate(): + # No owner on the token -> falls back to current_user ("api"), never another user. + assert effective_user(_req(api_token=True, api_token_owner=None, current_user="api")) == "api" + + +# --- _verify_session_owner: bearer tokens cannot cross owners --------------- + +def _session_local_returning(owner_value): + """Mock SessionLocal whose query(...).filter(...).first() yields a row with + the given owner (or None for 'no such session').""" + db = MagicMock() + row = None if owner_value is _MISSING else SimpleNamespace(owner=owner_value) + db.query.return_value.filter.return_value.first.return_value = row + return MagicMock(return_value=db) + + +_MISSING = object() + + +def test_bearer_owner_A_cannot_verify_owner_B_session(monkeypatch): + monkeypatch.setattr(SR, "SessionLocal", _session_local_returning("bob")) + req = _req(api_token=True, api_token_owner="alice", current_user="api") + with pytest.raises(HTTPException) as exc: + SR._verify_session_owner(req, "sid-owned-by-bob") + assert exc.value.status_code == 404 + + +def test_owner_can_verify_their_own_session(monkeypatch): + monkeypatch.setattr(SR, "SessionLocal", _session_local_returning("alice")) + req = _req(api_token=True, api_token_owner="alice", current_user="api") + # Should not raise. + SR._verify_session_owner(req, "sid-owned-by-alice") + + +def test_cookie_user_owns_their_session(monkeypatch): + # Cookie path unchanged: alice (cookie) verifies alice's session. + monkeypatch.setattr(SR, "SessionLocal", _session_local_returning("alice")) + req = _req(api_token=False, current_user="alice") + SR._verify_session_owner(req, "sid") + + +def test_missing_session_is_404(monkeypatch): + monkeypatch.setattr(SR, "SessionLocal", _session_local_returning(_MISSING)) + req = _req(api_token=False, current_user="alice") + with pytest.raises(HTTPException) as exc: + SR._verify_session_owner(req, "nope") + assert exc.value.status_code == 404 + + +def test_unauthenticated_caller_rejected(monkeypatch): + req = _req(api_token=False, current_user=None) + with pytest.raises(HTTPException) as exc: + SR._verify_session_owner(req, "sid") + assert exc.value.status_code == 403