mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-16 09:45:24 -04:00
fix(api): attribute bearer-token actions to the token owner on owner-scoped routes (#4054)
* fix(api): attribute bearer-token actions to the token owner on owner-scoped routes Owner-scoped chat, session, and upload routes called get_current_user(), which resolves a bearer ody_ API token to the sandboxed "api" pseudo-user. A paired API-token client (companion, CLI, IDE extension) therefore saw and created a separate "api"-owned silo instead of the owner's data. effective_user() already exists for exactly this: it attributes a token's actions to request.state.api_token_owner, is identical to get_current_user() for cookie sessions, and falls back safely when a token has no owner. session_routes.py was already migrated; this completes the migration for the remaining owner-scoped routes: - chat_helpers.py: chat-privilege enforcement, message attribution, prefs/context - chat_routes.py: orphaned-endpoint owner, session-auth owner, message search - upload_routes.py: upload owner attribution + access checks The /api/models swap is intentionally omitted: #4292 already migrated it to effective_user (plus the chat-scope gate and ownerless-token 403), so this PR keeps dev's version of routes/model_routes.py unchanged. chat_routes.py keeps importing get_current_user for the workspace owner gate; session_routes.py drops the now-unused import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: target effective_user in auth monkeypatches and owner-scope assertion The owner-scoped routes now call effective_user() instead of get_current_user(), so the tests that stubbed get_current_user (or asserted on it) follow suit: - test_chat_helpers.py, test_review_regressions.py, test_kv_cache_invalidation_2927.py: monkeypatch effective_user - test_session_endpoint_owner_scope.py: assert the owner-scope guard uses effective_user(request) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Fahim
@@ -14,7 +14,7 @@ from core.database import Session as DBSession, ModelEndpoint
|
||||
from src.llm_core import normalize_model_id
|
||||
from src.endpoint_resolver import normalize_base
|
||||
from src.context_compactor import maybe_compact, trim_for_context
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user
|
||||
from src.prompt_security import untrusted_context_message
|
||||
from routes.prefs_routes import _load_for_user as load_prefs_for_user
|
||||
|
||||
@@ -78,7 +78,7 @@ def _enforce_chat_privileges(request, sess) -> None:
|
||||
which means unrestricted allowed_models / zero cap -> no-op for them.
|
||||
"""
|
||||
try:
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
except Exception:
|
||||
user = None
|
||||
if not user:
|
||||
@@ -350,7 +350,7 @@ def fire_message_event(request, webhook_manager, session_id: str, sess, message:
|
||||
"session_id": session_id, "model": sess.model, "message": message[:2000],
|
||||
}))
|
||||
from src.event_bus import fire_event
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
fire_event("message_sent", user)
|
||||
|
||||
|
||||
@@ -577,7 +577,7 @@ async def build_chat_context(
|
||||
fire_message_event(request, webhook_manager, session_id, sess, message, compare_mode)
|
||||
|
||||
# Resolve user prefs
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
uprefs = load_prefs_for_user(user)
|
||||
|
||||
# Memory enabled?
|
||||
|
||||
@@ -23,7 +23,7 @@ from src.endpoint_resolver import normalize_base as _normalize_base, build_chat_
|
||||
from src.session_search import search_session_messages
|
||||
from src.prompt_security import untrusted_context_message
|
||||
from core.exceptions import SessionNotFoundError
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user, get_current_user
|
||||
from routes.session_routes import _verify_session_owner
|
||||
from routes.document_helpers import _owner_session_filter
|
||||
from core.database import SessionLocal, get_session_mode, set_session_mode
|
||||
@@ -363,7 +363,7 @@ def setup_chat_routes(
|
||||
sess = session_manager.get_session(session)
|
||||
except KeyError:
|
||||
raise HTTPException(404, f"Session '{session}' not found")
|
||||
owner = get_current_user(request)
|
||||
owner = effective_user(request)
|
||||
if _clear_orphaned_session_endpoint(sess, owner=owner):
|
||||
raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.")
|
||||
|
||||
@@ -603,7 +603,7 @@ def setup_chat_routes(
|
||||
# but BEFORE loading. Prevents cross-user session hijack.
|
||||
_verify_session_owner(request, session)
|
||||
sess = session_manager.get_session(session)
|
||||
owner = get_current_user(request)
|
||||
owner = effective_user(request)
|
||||
if _clear_orphaned_session_endpoint(sess, owner=owner):
|
||||
raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.")
|
||||
# Issue #587: picker shows a model from the endpoint cache but
|
||||
@@ -634,7 +634,7 @@ def setup_chat_routes(
|
||||
_enforce_chat_privileges(request, sess)
|
||||
|
||||
# Ensure session has auth headers
|
||||
resolve_session_auth(sess, session, owner=get_current_user(request))
|
||||
resolve_session_auth(sess, session, owner=effective_user(request))
|
||||
|
||||
# Check for research_pending BEFORE mode persist overwrites it
|
||||
do_research = str(use_research).lower() == "true"
|
||||
@@ -1485,7 +1485,7 @@ def setup_chat_routes(
|
||||
if not q or not q.strip():
|
||||
return []
|
||||
|
||||
_user = get_current_user(request)
|
||||
_user = effective_user(request)
|
||||
return [
|
||||
result.to_dict()
|
||||
for result in search_session_messages(
|
||||
|
||||
@@ -11,7 +11,7 @@ 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, utcnow_naive
|
||||
from src.auth_helpers import get_current_user, effective_user, _auth_disabled, owner_filter
|
||||
from src.auth_helpers import effective_user, _auth_disabled, owner_filter
|
||||
from src.session_actions import is_session_recently_active
|
||||
|
||||
|
||||
@@ -328,7 +328,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
||||
endpoint_id: str = Form(""),
|
||||
):
|
||||
skip_val = str(skip_validation).lower() == "true"
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
endpoint_api_key = ""
|
||||
endpoint_base_url = ""
|
||||
_reject_raw_endpoint_url_for_non_admin(request, user, endpoint_id, endpoint_url)
|
||||
@@ -477,7 +477,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
||||
db.close()
|
||||
# Switch model/endpoint mid-session
|
||||
if model is not None and endpoint_url is not None:
|
||||
user = get_current_user(request)
|
||||
user = effective_user(request)
|
||||
_reject_raw_endpoint_url_for_non_admin(request, user, endpoint_id, endpoint_url)
|
||||
endpoint_api_key = ""
|
||||
endpoint_base_url = ""
|
||||
|
||||
@@ -7,7 +7,7 @@ from fastapi import APIRouter, Request, File, UploadFile, HTTPException
|
||||
from typing import List
|
||||
import logging
|
||||
from core.middleware import require_admin
|
||||
from src.auth_helpers import get_current_user
|
||||
from src.auth_helpers import effective_user
|
||||
from src.upload_handler import count_recent_uploads
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -78,7 +78,7 @@ def setup_upload_routes(upload_handler):
|
||||
|
||||
for u in files:
|
||||
try:
|
||||
meta = upload_handler.save_upload(u, client_ip, owner=get_current_user(request))
|
||||
meta = upload_handler.save_upload(u, client_ip, owner=effective_user(request))
|
||||
out.append({
|
||||
"id": meta["id"],
|
||||
"name": meta["name"],
|
||||
@@ -138,7 +138,7 @@ def setup_upload_routes(upload_handler):
|
||||
original_name = info.get("name", file_id)
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
@@ -204,7 +204,7 @@ def setup_upload_routes(upload_handler):
|
||||
info = _load_upload_info(file_id)
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
@@ -247,7 +247,7 @@ def setup_upload_routes(upload_handler):
|
||||
raise HTTPException(404, "File not found")
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
auth_configured = bool(auth_mgr and auth_mgr.is_configured)
|
||||
current_user = get_current_user(request)
|
||||
current_user = effective_user(request)
|
||||
file_owner = info.get("owner")
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
|
||||
@@ -30,7 +30,7 @@ class _Session:
|
||||
|
||||
|
||||
def test_allowed_models_legacy_empty_list_remains_unrestricted(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
_enforce_chat_privileges(
|
||||
_Request({"allowed_models": [], "max_messages_per_day": 0}),
|
||||
@@ -39,7 +39,7 @@ def test_allowed_models_legacy_empty_list_remains_unrestricted(monkeypatch):
|
||||
|
||||
|
||||
def test_allowed_models_explicit_empty_restricted_list_blocks_all_models(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
_enforce_chat_privileges(
|
||||
@@ -56,7 +56,7 @@ def test_allowed_models_explicit_empty_restricted_list_blocks_all_models(monkeyp
|
||||
|
||||
|
||||
def test_allowed_models_nonempty_list_still_restricts_without_new_flag(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
_enforce_chat_privileges(
|
||||
_Request({"allowed_models": ["provider/model-a"], "max_messages_per_day": 0}),
|
||||
@@ -70,7 +70,7 @@ def test_allowed_models_nonempty_list_still_restricts_without_new_flag(monkeypat
|
||||
|
||||
|
||||
def test_no_restriction_allows_any_model(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
privs = {"allowed_models": [], "block_all_models": False, "max_messages_per_day": 0}
|
||||
_enforce_chat_privileges(_Request(privs), _Session("provider/model-a"))
|
||||
@@ -78,7 +78,7 @@ def test_no_restriction_allows_any_model(monkeypatch):
|
||||
|
||||
|
||||
def test_specific_allowlist_blocks_models_outside_it(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
privs = {
|
||||
"allowed_models": ["gpt-4"],
|
||||
@@ -92,7 +92,7 @@ def test_specific_allowlist_blocks_models_outside_it(monkeypatch):
|
||||
|
||||
|
||||
def test_block_all_models_blocks_regardless_of_allowed_models_contents(monkeypatch):
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "alice")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "alice")
|
||||
|
||||
# Even if allowed_models contains entries, block_all_models wins.
|
||||
privs = {
|
||||
@@ -111,7 +111,7 @@ def test_block_all_models_blocks_regardless_of_allowed_models_contents(monkeypat
|
||||
def test_admin_user_is_never_blocked(monkeypatch):
|
||||
from core.auth import ADMIN_PRIVILEGES
|
||||
|
||||
monkeypatch.setattr("routes.chat_helpers.get_current_user", lambda request: "admin")
|
||||
monkeypatch.setattr("routes.chat_helpers.effective_user", lambda request: "admin")
|
||||
|
||||
class _AdminAuthManager:
|
||||
def get_privileges(self, username):
|
||||
|
||||
@@ -79,7 +79,7 @@ def _build_context_harness(monkeypatch, chat_helpers, history):
|
||||
monkeypatch.setattr(chat_helpers, "extract_preset", fake_extract_preset)
|
||||
monkeypatch.setattr(chat_helpers, "add_user_message", fake_add_user_message)
|
||||
monkeypatch.setattr(chat_helpers, "load_prefs_for_user", lambda user: {})
|
||||
monkeypatch.setattr(chat_helpers, "get_current_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "effective_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "normalize_model_id", lambda endpoint_url, model, **kwargs: None)
|
||||
monkeypatch.setattr(chat_helpers, "maybe_compact", fake_maybe_compact)
|
||||
monkeypatch.setattr(chat_helpers, "trim_for_context", lambda messages, context_length: messages)
|
||||
|
||||
@@ -385,7 +385,7 @@ async def test_build_chat_context_incognito_does_not_duplicate_current_user_mess
|
||||
monkeypatch.setattr(chat_helpers, "extract_preset", fake_extract_preset)
|
||||
monkeypatch.setattr(chat_helpers, "add_user_message", fake_add_user_message)
|
||||
monkeypatch.setattr(chat_helpers, "load_prefs_for_user", lambda user: {})
|
||||
monkeypatch.setattr(chat_helpers, "get_current_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "effective_user", lambda request: "tester")
|
||||
monkeypatch.setattr(chat_helpers, "normalize_model_id", lambda endpoint_url, model, **kwargs: None)
|
||||
monkeypatch.setattr(chat_helpers, "maybe_compact", fake_maybe_compact)
|
||||
monkeypatch.setattr(chat_helpers, "trim_for_context", lambda messages, context_length: messages)
|
||||
|
||||
@@ -52,6 +52,6 @@ def test_chat_endpoint_recovery_paths_are_owner_scoped():
|
||||
assert "def _clear_orphaned_session_endpoint(sess, owner:" in chat_routes
|
||||
assert "def _recover_empty_session_model(sess, session_id: str, owner:" in chat_routes
|
||||
assert "q = owner_filter(q, ModelEndpoint, owner)" in chat_routes
|
||||
assert "resolve_session_auth(sess, session, owner=get_current_user(request))" in chat_routes
|
||||
assert "resolve_session_auth(sess, session, owner=effective_user(request))" in chat_routes
|
||||
assert "def resolve_session_auth(sess, session_id: str, owner:" in chat_helpers
|
||||
assert "update_q = update_q.filter(DBSession.owner == owner)" in chat_helpers
|
||||
|
||||
Reference in New Issue
Block a user