mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-17 10:15:27 -04:00
Attribute API-token sessions to the token owner (effective_user) (#871)
Split 2/4 of the companion bridge (#863 was 1/4). A paired bearer-token caller runs as the sandboxed 'api' pseudo-user, so its sessions were stranded in a separate 'api'-owned silo, invisible to the owner's desktop UI. Add effective_user(): for a bearer token it resolves to the token's real owner (request.state.api_token_owner); for cookie sessions it is identical to get_current_user, so the swap is a no-op for browser users. Route session ownership/attribution in routes/session_routes.py through it. Tests (tests/test_session_owner_attribution.py): - cookie/browser users are unchanged - a bearer token attributes to its owner; with no owner it does NOT escalate - _verify_session_owner: a bearer token for owner A cannot verify owner B's session (404); owner verifies their own; missing -> 404; unauth -> 403
This commit is contained in:
@@ -11,12 +11,12 @@ from core.session_manager import SessionManager
|
|||||||
from core.models import ChatMessage
|
from core.models import ChatMessage
|
||||||
from src.request_models import SessionResponse
|
from src.request_models import SessionResponse
|
||||||
from core.database import Session as DbSession, SessionLocal, Document, GalleryImage
|
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):
|
def _verify_session_owner(request: Request, session_id: str):
|
||||||
"""Verify the current user owns the session. Raises 404 if not."""
|
"""Verify the current user owns the session. Raises 404 if not."""
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
if not user:
|
if not user:
|
||||||
raise HTTPException(403, "Authentication required")
|
raise HTTPException(403, "Authentication required")
|
||||||
db = SessionLocal()
|
db = SessionLocal()
|
||||||
@@ -63,7 +63,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
|
|
||||||
@router.get("/sessions")
|
@router.get("/sessions")
|
||||||
def list_sessions(request: Request):
|
def list_sessions(request: Request):
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
# Lazy purge: incognito sessions are ephemeral by design — wipe leftovers
|
# Lazy purge: incognito sessions are ephemeral by design — wipe leftovers
|
||||||
# from the DB and session_manager so they vanish on the next page refresh.
|
# 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.
|
# 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
|
model_to_use = found
|
||||||
|
|
||||||
sid = str(uuid.uuid4())
|
sid = str(uuid.uuid4())
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
session = session_manager.create_session(
|
session = session_manager.create_session(
|
||||||
session_id=sid,
|
session_id=sid,
|
||||||
name=name or "",
|
name=name or "",
|
||||||
@@ -499,7 +499,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
@router.get("/sessions/archived")
|
@router.get("/sessions/archived")
|
||||||
def list_archived_sessions(request: Request, search: str = "", offset: int = 0, limit: int = 20, sort: str = "recent", model: str = ""):
|
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."""
|
"""List archived sessions for the archive browser."""
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
db = SessionLocal()
|
db = SessionLocal()
|
||||||
try:
|
try:
|
||||||
q = db.query(DbSession).filter(DbSession.archived == True)
|
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")
|
@router.post("/sessions/save")
|
||||||
def sessions_save_now(request: Request):
|
def sessions_save_now(request: Request):
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
if not user:
|
if not user:
|
||||||
raise HTTPException(401, "Not authenticated")
|
raise HTTPException(401, "Not authenticated")
|
||||||
session_manager.save_sessions()
|
session_manager.save_sessions()
|
||||||
@@ -651,7 +651,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
if not OPENAI_API_KEY:
|
if not OPENAI_API_KEY:
|
||||||
raise HTTPException(400, "Server missing OPENAI_API_KEY")
|
raise HTTPException(400, "Server missing OPENAI_API_KEY")
|
||||||
sid = str(uuid.uuid4())
|
sid = str(uuid.uuid4())
|
||||||
user = get_current_user(request)
|
user = effective_user(request)
|
||||||
session = session_manager.create_session(
|
session = session_manager.create_session(
|
||||||
session_id=sid,
|
session_id=sid,
|
||||||
name="",
|
name="",
|
||||||
@@ -791,7 +791,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
users can clean junk without spending tokens.
|
users can clean junk without spending tokens.
|
||||||
"""
|
"""
|
||||||
from src.llm_core import llm_call
|
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)
|
user_sessions = session_manager.get_sessions_for_user(user)
|
||||||
|
|
||||||
# Delete empty and throwaway sessions before sorting
|
# Delete empty and throwaway sessions before sorting
|
||||||
|
|||||||
@@ -10,6 +10,30 @@ def get_current_user(request: Request) -> Optional[str]:
|
|||||||
return getattr(request.state, 'current_user', None)
|
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:
|
def _auth_disabled() -> bool:
|
||||||
"""True when the operator has explicitly turned off auth via .env.
|
"""True when the operator has explicitly turned off auth via .env.
|
||||||
Mirrors the AUTH_ENABLED parse in app.py / core/middleware.py so the
|
Mirrors the AUTH_ENABLED parse in app.py / core/middleware.py so the
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user