mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-25 15:22:24 +01:00
fix(upload): honor no-login mode after account setup (#6528)
* fix(upload): honor no-login mode after account setup * ci: refresh checks against the current dev base * test(upload): add regression tests for unconfigured auth and no-login POST * fix(upload): fail closed on missing auth_manager and preserve cross-user isolation Enforce strict cross-user isolation so that authenticated users cannot access files owned by other accounts when app.state.auth_manager is absent or None. Fail closed with HTTP 503 for non-loopback requests when AUTH_ENABLED=true and authentication infrastructure is unexpectedly unavailable, while preserving intended single-user mode (AUTH_ENABLED=false) and intentional first-run setup mode (auth_manager.is_configured=False). * fix(upload): eliminate fail-open paths for unconfigured manager and loopback transport - Eliminate early return on unconfigured auth_manager (Finding A): fail closed with HTTP 401 Setup required - Eliminate loopback transport bypass when auth_manager is missing (Finding B): fail closed with HTTP 503 - Enforce strict ownership isolation: non-owners and legacy ownerless uploads require verified administrator access - Add regression suites in tests/test_upload_no_login.py covering Finding A, Finding B, bearer tokens, and ownerless files - Explicitly set AUTH_ENABLED=false for unauthenticated tests in tests/test_upload_routes_owner_scope.py * fix(upload): prevent delegated API tokens from inheriting admin privileges and enforce upload scopes - Prevent delegated bearer API credentials from inheriting interactive administrator privileges over other users' files or legacy ownerless files. - Enforce granular upload scopes for delegated credentials: chat and documents:write for upload creation and vision OCR editing; chat, documents:read, and documents:write for downloads, thumbnails, and vision OCR reads. - Reject requests with missing token owners or reserved sentinel identities (api, demo, system, internal-tool) with 403 Forbidden. - Add comprehensive regression tests in tests/test_upload_no_login.py verifying delegated token scope isolation, sentinel rejection, owner isolation, and untouched OCR state on rejected edits. --------- Co-authored-by: PR #6029 Reviewer <reviewer@odysseus-review.local>
This commit is contained in:
co-authored by
PR #6029 Reviewer
parent
b2e717e28f
commit
c74f475679
+76
-25
@@ -21,7 +21,8 @@ from core.database import (
|
||||
Note,
|
||||
Session as DbSession,
|
||||
)
|
||||
from src.auth_helpers import effective_user
|
||||
from src.auth_helpers import effective_user, is_delegated_credential
|
||||
from src.owner_identity import auth_disabled, is_request_sentinel_owner
|
||||
from src.attachment_refs import attachment_refs_from_metadata
|
||||
from src.constants import GENERATED_IMAGES_DIR
|
||||
from src.path_confinement import is_inside
|
||||
@@ -34,6 +35,8 @@ from src.upload_handler import (
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/api/upload", tags=["upload"])
|
||||
UPLOAD_READ_SCOPES = frozenset({"chat", "documents:read", "documents:write"})
|
||||
UPLOAD_WRITE_SCOPES = frozenset({"chat", "documents:write"})
|
||||
UPLOAD_RESPONSE_HEADERS = {"X-Content-Type-Options": "nosniff"}
|
||||
|
||||
def _upload_ids_from_persisted_text(value: object) -> set[str]:
|
||||
@@ -178,6 +181,52 @@ def setup_upload_routes(upload_handler):
|
||||
|
||||
raise HTTPException(404, "File not found")
|
||||
|
||||
def _verify_upload_access(
|
||||
request: Request,
|
||||
file_owner: str | None,
|
||||
*,
|
||||
write: bool = False,
|
||||
) -> None:
|
||||
if auth_disabled():
|
||||
return
|
||||
|
||||
auth_mgr = getattr(request.app.state, "auth_manager", None)
|
||||
if auth_mgr is None:
|
||||
raise HTTPException(503, "Authentication service unavailable")
|
||||
|
||||
if not getattr(auth_mgr, "is_configured", False):
|
||||
raise HTTPException(401, "Setup required")
|
||||
|
||||
current_user = effective_user(request)
|
||||
if not current_user or is_request_sentinel_owner(current_user):
|
||||
raise HTTPException(403, "Access denied")
|
||||
|
||||
# Delegated credentials (bearer API tokens):
|
||||
# Establish token owner identity but NEVER inherit interactive administrator privileges.
|
||||
if is_delegated_credential(request):
|
||||
token_owner = getattr(request.state, "api_token_owner", None)
|
||||
if not token_owner or is_request_sentinel_owner(token_owner):
|
||||
raise HTTPException(403, "API token has no valid owner")
|
||||
|
||||
token_scopes = set(getattr(request.state, "api_token_scopes", []) or [])
|
||||
required_scopes = UPLOAD_WRITE_SCOPES if write else UPLOAD_READ_SCOPES
|
||||
if not token_scopes.intersection(required_scopes):
|
||||
raise HTTPException(403, f"API token missing required scope: {' or '.join(sorted(required_scopes))}")
|
||||
|
||||
# Non-owners and legacy ownerless uploads are strictly forbidden for delegated tokens.
|
||||
if file_owner is None or current_user != file_owner:
|
||||
raise HTTPException(404, "File not found")
|
||||
return
|
||||
|
||||
# Interactive session (cookie-authenticated human):
|
||||
# Owner access
|
||||
if file_owner is not None and current_user == file_owner:
|
||||
return
|
||||
|
||||
# Non-owner or legacy ownerless upload: administrator only
|
||||
if not (hasattr(auth_mgr, "is_admin") and auth_mgr.is_admin(current_user)):
|
||||
raise HTTPException(404, "File not found")
|
||||
|
||||
def _valid_session_id_for_owner(db, session_id: str | None, owner: str | None) -> str | None:
|
||||
if not session_id:
|
||||
return None
|
||||
@@ -280,6 +329,25 @@ def setup_upload_routes(upload_handler):
|
||||
session_id = None
|
||||
if not files:
|
||||
raise HTTPException(400, "No files uploaded")
|
||||
|
||||
if not auth_disabled():
|
||||
app = getattr(request, "app", None)
|
||||
if app is not None:
|
||||
auth_mgr = getattr(getattr(app, "state", None), "auth_manager", None)
|
||||
if auth_mgr is None:
|
||||
raise HTTPException(503, "Authentication service unavailable")
|
||||
if not getattr(auth_mgr, "is_configured", False):
|
||||
raise HTTPException(401, "Setup required")
|
||||
current_user = effective_user(request)
|
||||
if not current_user or is_request_sentinel_owner(current_user):
|
||||
raise HTTPException(403, "Access denied")
|
||||
if is_delegated_credential(request):
|
||||
token_owner = getattr(request.state, "api_token_owner", None)
|
||||
if not token_owner or is_request_sentinel_owner(token_owner):
|
||||
raise HTTPException(403, "API token has no valid owner")
|
||||
token_scopes = set(getattr(request.state, "api_token_scopes", []) or [])
|
||||
if not token_scopes.intersection(UPLOAD_WRITE_SCOPES):
|
||||
raise HTTPException(403, f"API token missing required scope: {' or '.join(sorted(UPLOAD_WRITE_SCOPES))}")
|
||||
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
out = []
|
||||
@@ -303,6 +371,8 @@ def setup_upload_routes(upload_handler):
|
||||
for u in files:
|
||||
try:
|
||||
owner = effective_user(request)
|
||||
if is_request_sentinel_owner(owner):
|
||||
owner = None
|
||||
meta = upload_handler.save_upload(u, client_ip, owner=owner)
|
||||
promoted_gallery_id = _promote_chat_image_to_gallery(meta, owner, session_id, gallery_id)
|
||||
item = {
|
||||
@@ -382,15 +452,8 @@ def setup_upload_routes(upload_handler):
|
||||
info = next((fi for fi in db.values() if fi.get("id") == file_id), None)
|
||||
if info:
|
||||
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 = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
raise HTTPException(403, "Access denied")
|
||||
if file_owner != current_user and not auth_mgr.is_admin(current_user):
|
||||
raise HTTPException(404, "File not found")
|
||||
_verify_upload_access(request, file_owner)
|
||||
path = _resolve_upload_path(file_id)
|
||||
mime = (info or {}).get("mime") or _mt.guess_type(path)[0] or "application/octet-stream"
|
||||
from fastapi.responses import FileResponse
|
||||
@@ -471,15 +534,9 @@ def setup_upload_routes(upload_handler):
|
||||
if not upload_handler.validate_upload_id(file_id):
|
||||
raise HTTPException(400, "Invalid file ID")
|
||||
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 = effective_user(request)
|
||||
file_owner = info.get("owner") if info else None
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
raise HTTPException(403, "Access denied")
|
||||
if file_owner != current_user and not auth_mgr.is_admin(current_user):
|
||||
raise HTTPException(404, "File not found")
|
||||
_verify_upload_access(request, file_owner)
|
||||
current_user = effective_user(request)
|
||||
path = _resolve_upload_path(file_id)
|
||||
import mimetypes as _mt
|
||||
mime = (info or {}).get("mime") or _mt.guess_type(path)[0] or ""
|
||||
@@ -517,15 +574,9 @@ def setup_upload_routes(upload_handler):
|
||||
info = _load_upload_info(file_id)
|
||||
if not info:
|
||||
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 = effective_user(request)
|
||||
file_owner = info.get("owner")
|
||||
if auth_configured:
|
||||
if not current_user:
|
||||
raise HTTPException(403, "Access denied")
|
||||
if file_owner != current_user and not auth_mgr.is_admin(current_user):
|
||||
raise HTTPException(404, "File not found")
|
||||
_verify_upload_access(request, file_owner, write=True)
|
||||
current_user = effective_user(request)
|
||||
_resolve_upload_path(file_id)
|
||||
try:
|
||||
body = await request.json()
|
||||
|
||||
Reference in New Issue
Block a user