fix(auth): clean up rename and null-owner ownership (#4340)

tests/test_auth_session_revocation.py

@@ -80,6 +80,16 @@ def test_password_change_allows_new_password_and_blocks_old_password(tmp_path):
     assert mgr.create_session("alice", "new-password") is not None
 
 
+def test_create_session_trusted_rejects_username_renamed_after_verification(tmp_path):
+    mgr = _make_manager(tmp_path)
+    assert mgr.create_user("admin", "admin-password", is_admin=True)
+
+    assert mgr.verify_password("alice", "old-password") is True
+    assert mgr.rename_user("alice", "alice2", "admin") is True
+
+    assert mgr.create_session_trusted("alice") is None
+
+
 def _change_password_endpoint(auth_manager):
     sys.modules.pop("routes.auth_routes", None)
     _real_core_package()
@@ -92,6 +102,39 @@ def _change_password_endpoint(auth_manager):
     raise AssertionError("change-password route not found")
 
 
+def _login_endpoint(auth_manager):
+    sys.modules.pop("routes.auth_routes", None)
+    _real_core_package()
+    from routes.auth_routes import LoginRequest, setup_auth_routes
+
+    router = setup_auth_routes(auth_manager)
+    for route in router.routes:
+        if getattr(route, "path", None) == "/api/auth/login":
+            return route.endpoint, LoginRequest
+    raise AssertionError("login route not found")
+
+
+def test_login_route_does_not_set_cookie_when_trusted_session_rejects_stale_user(monkeypatch):
+    auth = MagicMock()
+    auth.verify_password.return_value = True
+    auth.totp_enabled.return_value = False
+    auth.create_session_trusted.return_value = None
+    endpoint, LoginRequest = _login_endpoint(auth)
+    monkeypatch.setattr(
+        "routes.auth_routes.asyncio.to_thread",
+        lambda fn, *args, **kwargs: _immediate_to_thread(fn, *args, **kwargs),
+    )
+    request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1"))
+    response = MagicMock()
+    body = LoginRequest(username="alice", password="old-password")
+
+    with pytest.raises(HTTPException) as exc:
+        asyncio.run(endpoint(body=body, request=request, response=response))
+
+    assert exc.value.status_code == 401
+    response.set_cookie.assert_not_called()
+
+
 def test_change_password_route_revokes_other_sessions_after_success(monkeypatch):
     auth = MagicMock()
     auth.get_username_for_token.return_value = "alice"

tests/test_document_session_owner_scope.py

@@ -25,6 +25,7 @@ import routes.document_routes as droutes
 from core.database import Document
 from core.database import Session as DbSession
 from routes.document_helpers import DocumentPatch
+from routes.document_helpers import _owner_session_filter
 
 _TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
 _ENGINE = create_engine(
@@ -141,3 +142,18 @@ async def test_list_documents_filters_foreign_docs_in_visible_session():
         assert bob_doc not in ids
     finally:
         droutes.SessionLocal = previous_session_local
+
+
+def test_owner_session_filter_noops_for_auth_disabled_single_user(monkeypatch):
+    monkeypatch.setenv("AUTH_ENABLED", "false")
+    previous_session_local = _bind_test_db()
+    try:
+        _alice_session, _bob_session, alice_doc, _bob_doc, _legacy_doc = _seed()
+        db = _TS()
+        try:
+            q = db.query(Document).filter(Document.id == alice_doc)
+            assert _owner_session_filter(q, None).first().id == alice_doc
+        finally:
+            db.close()
+    finally:
+        droutes.SessionLocal = previous_session_local

tests/test_personal_upload_isolation.py

@@ -1,5 +1,6 @@
 import os
 from pathlib import Path
+from types import SimpleNamespace
 
 from routes import personal_routes
 
@@ -42,3 +43,44 @@ def test_personal_upload_paths_stay_under_upload_root(tmp_path, monkeypatch):
     assert os.path.commonpath([file_path, upload_dir]) == upload_dir
     assert Path(file_path).name == stored_name
     assert display_name == "env"
+
+
+def test_rename_personal_upload_owner_moves_files_and_rewrites_rag(tmp_path, monkeypatch):
+    monkeypatch.setattr(personal_routes, "UPLOADS_DIR", str(tmp_path))
+
+    old_dir = Path(personal_routes._personal_upload_dir_for_owner("alice"))
+    old_file = old_dir / "note.txt"
+    old_file.write_text("alice private RAG note", encoding="utf-8")
+
+    manager_calls = []
+    rag_calls = []
+    manager = SimpleNamespace(
+        rename_directory=lambda old, new, path_map=None: manager_calls.append((old, new, dict(path_map or {}))),
+    )
+    rag = SimpleNamespace(
+        rename_owner=lambda old, new, path_map=None, path_prefixes=None: rag_calls.append(
+            (old, new, dict(path_map or {}), list(path_prefixes or []))
+        ) or {"success": True, "updated_count": 1},
+    )
+
+    result = personal_routes.rename_personal_upload_owner(
+        "alice",
+        "alice2",
+        personal_docs_manager=manager,
+        rag_manager=rag,
+    )
+
+    new_dir = Path(personal_routes._personal_upload_dir_for_owner("alice2"))
+    new_file = new_dir / "note.txt"
+    assert old_file.exists() is False
+    assert new_file.read_text(encoding="utf-8") == "alice private RAG note"
+    assert result["moved_files"] == 1
+    assert manager_calls == [(str(old_dir), str(new_dir), {str(old_file): str(new_file)})]
+    assert rag_calls == [
+        (
+            "alice",
+            "alice2",
+            {str(old_file): str(new_file)},
+            [(str(old_dir), str(new_dir))],
+        )
+    ]

tests/test_rag_vector_rename_owner.py

@@ -0,0 +1,81 @@
+from src.rag_vector import VectorRAG
+
+
+class _FakeCollection:
+    def __init__(self, docs):
+        self._docs = {
+            doc_id: {"document": document, "metadata": dict(metadata)}
+            for doc_id, document, metadata in docs
+        }
+
+    def count(self):
+        return len(self._docs)
+
+    def get(self, where=None, include=None):
+        rows = []
+        for doc_id, row in self._docs.items():
+            metadata = row["metadata"]
+            if where and any(metadata.get(key) != value for key, value in where.items()):
+                continue
+            rows.append((doc_id, row))
+        return {
+            "ids": [doc_id for doc_id, _row in rows],
+            "documents": [row["document"] for _doc_id, row in rows],
+            "metadatas": [row["metadata"] for _doc_id, row in rows],
+        }
+
+    def update(self, ids, metadatas):
+        for doc_id, metadata in zip(ids, metadatas):
+            self._docs[doc_id]["metadata"] = dict(metadata)
+
+
+def _store(collection):
+    store = VectorRAG.__new__(VectorRAG)
+    store._collection = collection
+    store._lanes = []
+    store._healthy = True
+    return store
+
+
+def test_rename_owner_updates_metadata_used_by_owner_filtered_search(tmp_path):
+    old_dir = tmp_path / "alice"
+    new_dir = tmp_path / "alice2"
+    old_file = old_dir / "note.txt"
+    new_file = new_dir / "note.txt"
+    collection = _FakeCollection([
+        (
+            "doc-old",
+            "private vector note",
+            {
+                "owner": "alice",
+                "source": str(old_file),
+                "directory": str(old_dir),
+            },
+        ),
+        (
+            "doc-other",
+            "other vector note",
+            {
+                "owner": "bob",
+                "source": str(tmp_path / "bob" / "note.txt"),
+            },
+        ),
+    ])
+    store = _store(collection)
+
+    result = store.rename_owner(
+        "alice",
+        "alice2",
+        path_map={str(old_file): str(new_file)},
+        path_prefixes=[(str(old_dir), str(new_dir))],
+    )
+
+    assert result["success"] is True
+    assert result["updated_count"] == 1
+    assert store._keyword_search_fallback("private", k=10, owner="alice") == []
+    renamed = store._keyword_search_fallback("private", k=10, owner="alice2")
+    assert [row["id"] for row in renamed] == ["doc-old"]
+    assert renamed[0]["metadata"]["owner"] == "alice2"
+    assert renamed[0]["metadata"]["source"] == str(new_file)
+    assert renamed[0]["metadata"]["directory"] == str(new_dir)
+    assert store._keyword_search_fallback("other", k=10, owner="bob")[0]["id"] == "doc-other"

tests/test_rename_user_owner_sync.py

@@ -70,12 +70,20 @@ def rename_endpoint(monkeypatch, tmp_path):
     return _route(ar.setup_auth_routes(am), "rename_user"), am, tmp_path
 
 
-def _request(tmp_path, session_manager=None, token="t", research_handler=None, upload_handler=None):
+def _request(
+    tmp_path,
+    session_manager=None,
+    token="t",
+    research_handler=None,
+    upload_handler=None,
+    personal_docs_manager=None,
+):
     state = SimpleNamespace(
         invalidate_token_cache=lambda: None,
         session_manager=session_manager,
         research_handler=research_handler,
         upload_handler=upload_handler,
+        personal_docs_manager=personal_docs_manager,
     )
     return SimpleNamespace(
         cookies={"odysseus_session": token},
@@ -467,6 +475,52 @@ def test_rename_updates_upload_metadata_owner(rename_endpoint):
     assert handler.resolve_upload(upload_id, owner="alice") is None
 
 
+def test_rename_updates_personal_rag_upload_owner(rename_endpoint, monkeypatch):
+    endpoint, _am, tmp_path = rename_endpoint
+    from routes import personal_routes
+
+    monkeypatch.setattr(personal_routes, "UPLOADS_DIR", str(tmp_path / "personal_uploads"))
+    old_dir = Path(personal_routes._personal_upload_dir_for_owner("alice"))
+    old_file = old_dir / "note.txt"
+    old_file.write_text("private RAG note", encoding="utf-8")
+
+    manager_calls = []
+    rag_calls = []
+    rag = SimpleNamespace(
+        rename_owner=lambda old, new, path_map=None, path_prefixes=None: rag_calls.append(
+            (old, new, dict(path_map or {}), list(path_prefixes or []))
+        ) or {"success": True, "updated_count": 1},
+    )
+    personal_docs_manager = SimpleNamespace(
+        rag_manager=rag,
+        rename_directory=lambda old, new, path_map=None: manager_calls.append(
+            (old, new, dict(path_map or {}))
+        ),
+    )
+
+    asyncio.run(
+        endpoint(
+            "alice",
+            SimpleNamespace(username="alice2"),
+            _request(tmp_path, personal_docs_manager=personal_docs_manager),
+        )
+    )
+
+    new_dir = Path(personal_routes._personal_upload_dir_for_owner("alice2"))
+    new_file = new_dir / "note.txt"
+    assert old_file.exists() is False
+    assert new_file.read_text(encoding="utf-8") == "private RAG note"
+    assert manager_calls == [(str(old_dir), str(new_dir), {str(old_file): str(new_file)})]
+    assert rag_calls == [
+        (
+            "alice",
+            "alice2",
+            {str(old_file): str(new_file)},
+            [(str(old_dir), str(new_dir))],
+        )
+    ]
+
+
 # ---------------------------------------------------------------------------
 # 5. Skills (SKILL.md frontmatter + _usage.json sidecar)
 # ---------------------------------------------------------------------------

tests/test_session_list_owner_scope.py

@@ -7,6 +7,7 @@ import sys
 import tempfile
 import types
 import uuid
+from datetime import timedelta
 
 import pytest
 from sqlalchemy import create_engine
@@ -14,6 +15,7 @@ from sqlalchemy.orm import sessionmaker
 from sqlalchemy.pool import NullPool
 
 import core.database as cdb
+from core.database import ChatMessage as DbMessage
 from core.database import Session as DbSession
 
 _TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
@@ -72,3 +74,60 @@ def test_list_sessions_excludes_other_users_sessions(monkeypatch):
     returned_ids = {s["id"] for s in result}
     assert alice_id in returned_ids
     assert bob_id not in returned_ids
+
+
+def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(monkeypatch):
+    import routes.session_routes as sr
+    from unittest.mock import MagicMock
+
+    _stub_multipart_if_missing(monkeypatch)
+    monkeypatch.setenv("AUTH_ENABLED", "false")
+    monkeypatch.setattr(sr, "SessionLocal", _TS)
+    monkeypatch.setattr(sr, "effective_user", lambda request: None)
+
+    sid = str(uuid.uuid4())
+    old_time = cdb.utcnow_naive() - timedelta(hours=2)
+    db = _TS()
+    try:
+        db.query(DbMessage).delete()
+        db.query(DbSession).delete()
+        db.add(DbSession(
+            id=sid,
+            owner="alice",
+            name="New chat",
+            endpoint_url="http://localhost",
+            model="gpt-4",
+            archived=False,
+            message_count=1,
+            created_at=old_time,
+            updated_at=old_time,
+            last_message_at=old_time,
+            last_accessed=old_time,
+        ))
+        db.add(DbMessage(
+            id="m-" + uuid.uuid4().hex,
+            session_id=sid,
+            role="user",
+            content="hi",
+            timestamp=old_time,
+        ))
+        db.commit()
+    finally:
+        db.close()
+
+    session = MagicMock(id=sid, name="New chat", model="gpt-4", endpoint_url="http://localhost", rag=False, archived=False)
+    sm = MagicMock()
+    sm.get_sessions_for_user.return_value = {sid: session}
+    router = sr.setup_session_routes(sm, {})
+    endpoint = next(r.endpoint for r in router.routes
+                    if getattr(r, "path", "") == "/api/sessions/auto-sort"
+                    and "POST" in getattr(r, "methods", set()))
+
+    result = endpoint(request=MagicMock(), skip_llm=True)
+
+    assert result["deleted_throwaway"] == 1
+    db = _TS()
+    try:
+        assert db.query(DbSession).filter(DbSession.id == sid).first() is None
+    finally:
+        db.close()