diff --git a/routes/gallery_helpers.py b/routes/gallery_helpers.py index 5cab62791..e4005b8a7 100644 --- a/routes/gallery_helpers.py +++ b/routes/gallery_helpers.py @@ -11,6 +11,7 @@ from typing import Dict, Any, Optional from pydantic import BaseModel from core.database import GalleryImage +from src.auth_helpers import _auth_disabled logger = logging.getLogger(__name__) @@ -120,19 +121,18 @@ def _image_to_dict(img: GalleryImage, session_name: str = None) -> Dict[str, Any } -def _owner_filter(q, user): +def _owner_filter(q, user, model_cls=GalleryImage): """Apply owner filtering to a gallery query. - When auth is disabled (single-user mode) get_current_user returns None - and there is no per-user scoping. The main library list and stats already - treat None as "show everything" (`if user is not None`), so this helper - must too — otherwise the tag/model filter sidebars come back empty and the - tag-cleanup endpoints (clear-user-tags, clear-ai-tags, dedupe-tags) - silently affect zero rows in the most common self-hosted deployment. + ``get_current_user`` returns None both in auth-disabled single-user mode + and when auth is enabled but no current user was resolved. Preserve the + single-user behavior, but fail closed for auth-enabled null-user states. """ - if user is None: + if user is not None: + return q.filter(model_cls.owner == user) + if _auth_disabled(): return q - return q.filter(GalleryImage.owner == user) + return q.filter(False) diff --git a/routes/gallery_routes.py b/routes/gallery_routes.py index 43999344e..feadc2ec8 100644 --- a/routes/gallery_routes.py +++ b/routes/gallery_routes.py @@ -476,8 +476,7 @@ def setup_gallery_routes() -> APIRouter: .outerjoin(DbSession, GalleryImage.session_id == DbSession.id) .filter(GalleryImage.is_active == True) ) - if user is not None: - q = q.filter(GalleryImage.owner == user) + q = _owner_filter(q, user) # Search filter (prompt + tags + ai_tags) if search: @@ -579,28 +578,26 @@ def setup_gallery_routes() -> APIRouter: db = SessionLocal() try: q = db.query(GalleryAlbum) - if user: - q = q.filter(GalleryAlbum.owner == user) + q = _owner_filter(q, user, GalleryAlbum) albums = q.order_by(GalleryAlbum.created_at.desc()).all() result = [] for a in albums: _count_q = db.query(GalleryImage).filter( GalleryImage.album_id == a.id, GalleryImage.is_active == True ) - if user: - _count_q = _count_q.filter(GalleryImage.owner == user) + _count_q = _owner_filter(_count_q, user) count = _count_q.count() cover_url = None if a.cover_id: - cover = db.query(GalleryImage).filter(GalleryImage.id == a.cover_id).first() + cover_q = db.query(GalleryImage).filter(GalleryImage.id == a.cover_id) + cover = _owner_filter(cover_q, user).first() if cover: cover_url = f"/api/generated-image/{cover.filename}" elif count > 0: _cover_q = db.query(GalleryImage).filter( GalleryImage.album_id == a.id, GalleryImage.is_active == True ) - if user: - _cover_q = _cover_q.filter(GalleryImage.owner == user) + _cover_q = _owner_filter(_cover_q, user) first = _cover_q.order_by(GalleryImage.created_at.desc()).first() if first: cover_url = f"/api/generated-image/{first.filename}" @@ -643,10 +640,9 @@ def setup_gallery_routes() -> APIRouter: base = db.query(GalleryImage).filter(GalleryImage.is_active == True) size_q = db.query(func.sum(GalleryImage.file_size)).filter(GalleryImage.is_active == True) album_q = db.query(GalleryAlbum) - if user: - base = base.filter(GalleryImage.owner == user) - size_q = size_q.filter(GalleryImage.owner == user) - album_q = album_q.filter(GalleryAlbum.owner == user) + base = _owner_filter(base, user) + size_q = _owner_filter(size_q, user) + album_q = _owner_filter(album_q, user, GalleryAlbum) total = base.count() total_size = size_q.scalar() or 0 fav_count = base.filter(GalleryImage.favorite == True).count() @@ -674,8 +670,7 @@ def setup_gallery_routes() -> APIRouter: GalleryImage.is_active == True, (GalleryImage.ai_tags == None) | (GalleryImage.ai_tags == ""), ) - if user: - q = q.filter(GalleryImage.owner == user) + q = _owner_filter(q, user) if album_id: q = q.filter(GalleryImage.album_id == album_id) untagged = q.count() diff --git a/tests/test_gallery_album_owner_scope.py b/tests/test_gallery_album_owner_scope.py index 143d4eda9..dcd3c13bd 100644 --- a/tests/test_gallery_album_owner_scope.py +++ b/tests/test_gallery_album_owner_scope.py @@ -40,9 +40,12 @@ def test_upload_validates_target_album_ownership(): def test_list_albums_count_and_cover_are_owner_scoped(): fns = _function_sources() body = fns["list_albums"] - # Both the per-album image count and the cover-fallback query must owner-scope - # by GalleryImage.owner (the album list itself already filters by owner). - assert body.count("GalleryImage.owner == user") >= 2 + # The album list, per-album image count, explicit cover, and cover-fallback + # queries should all share the same gallery owner policy. + assert "q = _owner_filter(q, user, GalleryAlbum)" in body + assert "_count_q = _owner_filter(_count_q, user)" in body + assert "cover = _owner_filter(cover_q, user).first()" in body + assert "_cover_q = _owner_filter(_cover_q, user)" in body def test_delete_album_cleanup_is_owner_scoped(): diff --git a/tests/test_gallery_null_user_routes.py b/tests/test_gallery_null_user_routes.py new file mode 100644 index 000000000..63967a958 --- /dev/null +++ b/tests/test_gallery_null_user_routes.py @@ -0,0 +1,149 @@ +import uuid + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import NullPool + +import core.database as cdb +from core.database import GalleryAlbum, GalleryImage +import routes.gallery_routes as gallery_routes + + +def _client_with_gallery(monkeypatch, tmp_path): + engine = create_engine( + f"sqlite:///{tmp_path / 'gallery.db'}", + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + cdb.Base.metadata.create_all(engine) + session_factory = sessionmaker(bind=engine, autoflush=False, autocommit=False) + monkeypatch.setattr(gallery_routes, "SessionLocal", session_factory) + + db = session_factory() + try: + db.add_all( + [ + GalleryAlbum(id="album-alice", name="Alice album", owner="alice"), + GalleryAlbum(id="album-bob", name="Bob album", owner="bob"), + GalleryImage( + id="img-alice", + filename=f"{uuid.uuid4().hex}.png", + prompt="alice prompt", + model="model-a", + tags="alice-tag", + ai_tags="", + owner="alice", + album_id="album-alice", + is_active=True, + file_size=10, + ), + GalleryImage( + id="img-bob", + filename=f"{uuid.uuid4().hex}.png", + prompt="bob prompt", + model="model-b", + tags="bob-tag", + ai_tags="", + owner="bob", + album_id="album-bob", + is_active=True, + file_size=20, + ), + ] + ) + db.commit() + finally: + db.close() + + app = FastAPI() + app.include_router(gallery_routes.setup_gallery_routes()) + return TestClient(app) + + +def test_auth_enabled_null_user_gallery_routes_fail_closed(monkeypatch, tmp_path): + monkeypatch.setenv("AUTH_ENABLED", "true") + client = _client_with_gallery(monkeypatch, tmp_path) + + library = client.get("/api/gallery/library").json() + assert library["items"] == [] + assert library["total"] == 0 + assert library["total_tagged"] == 0 + assert library["tags"] == [] + assert library["models"] == [] + + shuffled = client.get("/api/gallery/library", params={"sort": "shuffle"}).json() + assert shuffled["items"] == [] + assert shuffled["total"] == 0 + + assert client.get("/api/gallery/tags").json() == {"tags": []} + assert client.get("/api/gallery/albums").json() == {"albums": []} + assert client.get("/api/gallery/stats").json() == { + "total_photos": 0, + "total_size": 0, + "total_size_human": "0.0 B", + "favorites": 0, + "albums": 0, + } + assert client.post("/api/gallery/ai-tag-batch").json() == { + "ok": True, + "queued": 0, + "total_untagged": 0, + "image_ids": [], + } + + +def test_auth_disabled_null_user_gallery_routes_keep_single_user_mode(monkeypatch, tmp_path): + monkeypatch.setenv("AUTH_ENABLED", "false") + client = _client_with_gallery(monkeypatch, tmp_path) + + library = client.get("/api/gallery/library").json() + assert {item["id"] for item in library["items"]} == {"img-alice", "img-bob"} + assert library["total"] == 2 + assert library["tags"] == ["alice-tag", "bob-tag"] + assert library["models"] == ["model-a", "model-b"] + + assert client.get("/api/gallery/tags").json() == {"tags": ["alice-tag", "bob-tag"]} + assert len(client.get("/api/gallery/albums").json()["albums"]) == 2 + assert client.get("/api/gallery/stats").json() == { + "total_photos": 2, + "total_size": 30, + "total_size_human": "30.0 B", + "favorites": 0, + "albums": 2, + } + batch = client.post("/api/gallery/ai-tag-batch").json() + assert batch["ok"] is True + assert batch["queued"] == 2 + assert batch["total_untagged"] == 2 + assert set(batch["image_ids"]) == {"img-alice", "img-bob"} + + +def test_authenticated_gallery_routes_remain_owner_scoped(monkeypatch, tmp_path): + monkeypatch.setenv("AUTH_ENABLED", "true") + monkeypatch.setattr(gallery_routes, "get_current_user", lambda request: "alice") + client = _client_with_gallery(monkeypatch, tmp_path) + + library = client.get("/api/gallery/library").json() + assert [item["id"] for item in library["items"]] == ["img-alice"] + assert library["total"] == 1 + assert library["tags"] == ["alice-tag"] + assert library["models"] == ["model-a"] + + assert client.get("/api/gallery/tags").json() == {"tags": ["alice-tag"]} + albums = client.get("/api/gallery/albums").json()["albums"] + assert [album["id"] for album in albums] == ["album-alice"] + assert client.get("/api/gallery/stats").json() == { + "total_photos": 1, + "total_size": 10, + "total_size_human": "10.0 B", + "favorites": 0, + "albums": 1, + } + assert client.post("/api/gallery/ai-tag-batch").json() == { + "ok": True, + "queued": 1, + "total_untagged": 1, + "image_ids": ["img-alice"], + } diff --git a/tests/test_gallery_owner_filter_single_user.py b/tests/test_gallery_owner_filter_single_user.py index dc3211bf8..7032410c6 100644 --- a/tests/test_gallery_owner_filter_single_user.py +++ b/tests/test_gallery_owner_filter_single_user.py @@ -1,11 +1,8 @@ -"""_owner_filter must not blank out the gallery in single-user mode. +"""_owner_filter must separate single-user mode from anonymous callers. -When AUTH_ENABLED=false, get_current_user returns None. The gallery main -list and stats treat None as "show all images" (`if user is not None`), but -_owner_filter returned q.filter(False) (zero rows) for None. So the tag and -model filter chips were always empty and clear-user-tags / clear-ai-tags / -dedupe-tags silently no-oped. _owner_filter must match the main list: no -filter when user is None, owner-scoped otherwise. +When AUTH_ENABLED=false, get_current_user returns None and gallery routes should +stay all-visible. When AUTH_ENABLED=true and no current user resolves, the same +None means an anonymous caller and gallery queries must fail closed. """ import tempfile import uuid @@ -36,7 +33,8 @@ def _seed(*owners): db.close() -def test_none_user_returns_all_rows(): +def test_none_user_returns_all_rows(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "false") _seed(None, None, "alice") db = _TS() try: @@ -54,3 +52,13 @@ def test_named_user_is_still_scoped(): assert _owner_filter(db.query(GalleryImage), "bob").count() == 1 finally: db.close() + + +def test_none_user_blocks_when_auth_is_enabled(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "true") + _seed(None, "alice", "bob") + db = _TS() + try: + assert _owner_filter(db.query(GalleryImage), None).count() == 0 + finally: + db.close() diff --git a/tests/test_null_owner_gates.py b/tests/test_null_owner_gates.py index 3ff6949da..deada7e54 100644 --- a/tests/test_null_owner_gates.py +++ b/tests/test_null_owner_gates.py @@ -153,11 +153,20 @@ def test_document_owner_filter_applies_owner_clause(): # gallery._owner_filter # --------------------------------------------------------------------------- -def test_gallery_owner_filter_allows_single_user_mode(): +def test_gallery_owner_filter_blocks_anonymous(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "true") + from routes.gallery_routes import _owner_filter + fake_q = MagicMock() + out = _owner_filter(fake_q, user=None) + fake_q.filter.assert_called_once_with(False) + assert out is fake_q.filter.return_value + + +def test_gallery_owner_filter_allows_single_user_mode(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "false") from routes.gallery_routes import _owner_filter fake_q = MagicMock() out = _owner_filter(fake_q, user=None) - # user=None means single-user/auth-disabled mode: return q unchanged, no filter. fake_q.filter.assert_not_called() assert out is fake_q