From 2a944d931d7fbd16cbf95ecd4e75cd368c1eb02b Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Fri, 29 May 2026 08:20:26 +0000 Subject: [PATCH] Search: Exclude deleted files and document per-session scope checks #1307 FileVisibleToSession excludes soft-deleted file rows; PhotoVisibleToSession's UnscopedDb use is documented (archived handling deferred to ScopeVisiblePhotos, full-access short-circuits). Adds TestUserPhotos_ScopeAuthorization pinning that a restricted session may scope only to albums it owns or has shared. --- internal/entity/search/photos.go | 6 +++++- internal/entity/search/photos_scope.go | 10 ++++++++- internal/entity/search/photos_scope_test.go | 24 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/internal/entity/search/photos.go b/internal/entity/search/photos.go index 8ef18ef53..ff1a5639f 100644 --- a/internal/entity/search/photos.go +++ b/internal/entity/search/photos.go @@ -156,7 +156,11 @@ func searchPhotos(frm form.SearchPhotos, sess *entity.Session, resultCols string frm.Hidden = false } - // Visitors and other restricted users can only access shared content. + // Visitors and other restricted users can only access shared content. A non-empty Scope + // bypasses the personal ScopePhotosForSession filter below, so it is gated here instead: + // a restricted session may scope only to an album it owns or has shared. "Restricted" means + // shared-only or unregistered; this assumes any view-capable role lacking library/all access + // also holds access_shared (else HasSharedAccessOnly would not flag it). if frm.Scope != "" && album.CreatedBy != user.UserUID && !sess.HasShare(frm.Scope) && (sess.GetUser().HasSharedAccessOnly(acl.ResourcePhotos) || sess.NotRegistered()) || frm.Scope == "" && acl.Rules.Deny(acl.ResourcePhotos, aclRole, acl.ActionSearch) { event.AuditErr([]string{sess.IP(), "session %s", "%s %s as %s", status.Denied}, sess.RefID, acl.ActionSearch.String(), string(acl.ResourcePhotos), aclRole) diff --git a/internal/entity/search/photos_scope.go b/internal/entity/search/photos_scope.go index d9be35ec7..1002f4fb9 100644 --- a/internal/entity/search/photos_scope.go +++ b/internal/entity/search/photos_scope.go @@ -92,6 +92,10 @@ func PhotoVisibleToSession(photoUID string, sess *entity.Session) (bool, error) return true, nil } + // UnscopedDb is deliberate: the archived (soft-deleted) decision is deferred to + // ScopeVisiblePhotos, which adds "photos.deleted_at IS NULL" for roles without the archive + // permission. Full-access roles short-circuited above and may resolve archived pictures by UID, + // matching GetPhoto / searchPhotos. stmt := ScopeVisiblePhotos(UnscopedDb().Table("photos").Where("photos.photo_uid = ?", photoUID), sess) var count int @@ -111,8 +115,12 @@ func FileVisibleToSession(fileHash string, sess *entity.Session) (bool, error) { return true, nil } + // A soft-deleted file is never accessible, so exclude it explicitly (UnscopedDb does not apply + // the soft-delete scope). ScopeVisiblePhotos handles the photo's archived/private/scope state. stmt := ScopeVisiblePhotos( - UnscopedDb().Table("files").Joins("JOIN photos ON photos.id = files.photo_id").Where("files.file_hash = ?", fileHash), + UnscopedDb().Table("files"). + Joins("JOIN photos ON photos.id = files.photo_id"). + Where("files.file_hash = ? AND files.deleted_at IS NULL", fileHash), sess, ) diff --git a/internal/entity/search/photos_scope_test.go b/internal/entity/search/photos_scope_test.go index da04b7ec8..83b6b5776 100644 --- a/internal/entity/search/photos_scope_test.go +++ b/internal/entity/search/photos_scope_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/photoprism/photoprism/internal/entity" + "github.com/photoprism/photoprism/internal/form" ) // Fixture identifiers used by the scope tests. In the public repo only the admin, @@ -99,6 +100,29 @@ func TestPhotoVisibleToSession(t *testing.T) { }) } +// A non-empty Scope skips the personal ScopePhotosForSession filter, so it must be authorized by +// the album ownership/share gate in searchPhotos: a restricted session may scope only to an album +// it owns or has shared, never to an arbitrary album. +func TestUserPhotos_ScopeAuthorization(t *testing.T) { + const album = "as6sg6bxpogaaba8" // manual album owned by admin, not shared with guests + + t.Run("GuestNonSharedScopeForbidden", func(t *testing.T) { + _, _, err := UserPhotos(form.SearchPhotos{Scope: album}, scopeSession("guest")) + assert.Equal(t, ErrForbidden, err) + }) + t.Run("VisitorSharedScopeAllowed", func(t *testing.T) { + // Unregistered visitor whose session carries a share for the scoped album. + visitor := &entity.Session{} + visitor.SetData(&entity.SessionData{Shares: entity.UIDs{album}}) + _, _, err := UserPhotos(form.SearchPhotos{Scope: album}, visitor) + assert.NoError(t, err) + }) + t.Run("AdminScopeAllowed", func(t *testing.T) { + _, _, err := UserPhotos(form.SearchPhotos{Scope: album}, scopeSession("alice")) + assert.NoError(t, err) + }) +} + func TestFileVisibleToSession(t *testing.T) { t.Run("EmptyHash", func(t *testing.T) { ok, err := FileVisibleToSession("", scopeSession("guest"))