From 3f039866ff27bad57617aa49681ff63b2f95cf27 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Thu, 16 Jul 2026 12:10:44 +0000 Subject: [PATCH] Sharing: Fix single-item access for shared smart albums #5727 Pictures shared only through a folder, moment, calendar, or region link have no photos_albums row, so the by-UID and by-hash visibility checks reported them out of scope and returned "Entity not found" - for example when downloading a single image from a shared folder. The photo and file visibility helpers now fall back to the shared album's filter, matching what browsing and full-album downloads already allow. The fallback runs only after the cheaper personal-scope check misses and skips regular albums, so full-access and regular-share sessions add no queries. --- internal/entity/link_fixtures.go | 29 ++++- internal/entity/search/photos_scope.go | 59 +++++++++- internal/entity/search/photos_scope_test.go | 121 ++++++++++++++++++++ 3 files changed, 205 insertions(+), 4 deletions(-) diff --git a/internal/entity/link_fixtures.go b/internal/entity/link_fixtures.go index aca5cc933..80d1e680e 100644 --- a/internal/entity/link_fixtures.go +++ b/internal/entity/link_fixtures.go @@ -2,10 +2,11 @@ package entity import "time" -var date = time.Date(2050, 3, 6, 2, 6, 51, 0, time.UTC) - +// LinkMap represents a map of share link fixtures keyed by token. type LinkMap map[string]Link +// LinkFixtures provides share link fixtures for use in tests. +// //nolint:gosec // G101: Deterministic fixture tokens for tests only. var LinkFixtures = LinkMap{ "1jxf3jfn2k": { @@ -68,6 +69,30 @@ var LinkFixtures = LinkMap{ CreatedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), ModifiedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), }, + "8jxf3jfn2k": { + LinkUID: "ss62xpryd1ob3gtf", + ShareUID: "as6sg6bipogaaba1", // "april-1990" folder album (smart, path filter). + ShareSlug: "april-1990", + LinkToken: "8jxf3jfn2k", + LinkExpires: 0, + LinkViews: 0, + MaxViews: 0, + HasPassword: false, + CreatedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), + ModifiedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), + }, + "9jxf3jfn2k": { + LinkUID: "ss62xpryd1ob4gtf", + ShareUID: "as6sg6bipogaab11", // "california-usa" state album (smart, place filter). + ShareSlug: "california-usa", + LinkToken: "9jxf3jfn2k", + LinkExpires: 0, + LinkViews: 0, + MaxViews: 0, + HasPassword: false, + CreatedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), + ModifiedAt: time.Date(2020, 3, 6, 2, 6, 51, 0, time.UTC), + }, } // CreateLinkFixtures inserts known entities into the database for testing. diff --git a/internal/entity/search/photos_scope.go b/internal/entity/search/photos_scope.go index b72e3624d..d29fc7246 100644 --- a/internal/entity/search/photos_scope.go +++ b/internal/entity/search/photos_scope.go @@ -5,6 +5,8 @@ import ( "github.com/photoprism/photoprism/internal/auth/acl" "github.com/photoprism/photoprism/internal/entity" + "github.com/photoprism/photoprism/internal/form" + "github.com/photoprism/photoprism/pkg/clean" ) // sessionGrantsPhotos reports whether the session is granted perm on photos. For a client session @@ -115,9 +117,14 @@ func PhotoVisibleToSession(photoUID string, sess *entity.Session) (bool, error) var count int if err := stmt.Count(&count).Error; err != nil { return false, err + } else if count > 0 { + return true, nil } - return count > 0, nil + // Fall back to smart albums shared with the session (folder, moment, calendar, region), whose + // members derive from a filter rather than photos_albums rows and are thus invisible to + // ScopePhotosForSession above. + return sharedSmartAlbumContains(photoUID, sess) } // FileVisibleToSession reports whether the session may access the file with the given hash, @@ -141,7 +148,55 @@ func FileVisibleToSession(fileHash string, sess *entity.Session) (bool, error) { var count int if err := stmt.Count(&count).Error; err != nil { return false, err + } else if count > 0 { + return true, nil } - return count > 0, nil + // Fall back to smart albums shared with the session; the file hash resolves to its photo, so a + // picture shared only through a folder, moment, calendar, or region link stays downloadable. + var f entity.File + if err := UnscopedDb().Table("files").Select("photo_uid"). + Where("file_hash = ? AND deleted_at IS NULL", fileHash). + Limit(1).Scan(&f).Error; err != nil { + return false, err + } + + return sharedSmartAlbumContains(f.PhotoUID, sess) +} + +// sharedSmartAlbumContains reports whether the given photo UID belongs to a smart album shared with +// the session. Smart albums (folder, moment, calendar, region) derive their members from a filter +// rather than photos_albums rows, so ScopePhotosForSession cannot resolve them; this evaluates each +// shared album's filter exactly as the album view does, keeping single-item access aligned with what +// browsing and album downloads already permit. It runs only after the cheaper personal-scope check +// misses and skips regular albums (empty filter, already covered), so full-access and regular-share +// sessions perform no extra queries. +func sharedSmartAlbumContains(photoUID string, sess *entity.Session) (bool, error) { + if photoUID == "" || sess == nil { + return false, nil + } + + for _, albumUID := range sess.SharedUIDs() { + album, err := entity.CachedAlbumByUID(albumUID) + + // Only smart albums (non-empty filter) need the filter-based membership test; regular albums + // are already covered by ScopePhotosForSession's photos_albums predicate. + if err != nil || album.AlbumUID == "" || album.AlbumFilter == "" { + continue + } + + // Reuse the album-scoped search so membership matches browsing; Count 1 limits the query to + // an existence check for the single photo UID. + results, _, searchErr := UserPhotos(form.SearchPhotos{Scope: albumUID, UID: photoUID, Count: 1}, sess) + + if searchErr != nil { + // A single unusable share (e.g. an invalid filter) must not mask the others. + log.Debugf("scope: %s while checking shared album %s", clean.Error(searchErr), clean.Log(albumUID)) + continue + } else if len(results) > 0 { + return true, nil + } + } + + return false, nil } diff --git a/internal/entity/search/photos_scope_test.go b/internal/entity/search/photos_scope_test.go index 19aa10227..a0a6c720e 100644 --- a/internal/entity/search/photos_scope_test.go +++ b/internal/entity/search/photos_scope_test.go @@ -65,6 +65,23 @@ const ( scopePrivatePhotoUID = "ps6sg6be2lvl0y13" // "Photo06", private scopeNormalFileHash = "2cad9168fa6acc5c5c2965ddf6ec465ca42fd818" // file of a non-private photo scopePrivateFileHash = "pcad9a68fa6acc5c5ba965adf6ec465ca42fd917" // "Photo06.png", private photo + + // scopeFolderPhotoUID and scopeFolderFileHash belong to the "april-1990" folder (smart) album, + // whose members derive from a path filter rather than photos_albums rows. + scopeFolderPhotoUID = "ps6sg6be2lvl0yh0" // "Photo03", public, path 1990/04 + scopeFolderFileHash = "pcad9168fa6acc5c5c2965adf6ec465ca42fd818" // primary file of "Photo03" + + // scopeRegularPhotoUID is a non-private member of the regular album shared by scopeRegularShareToken; + // it is resolved by the personal-scope predicate without the smart-album fallback. + scopeRegularPhotoUID = "ps6sg6be2lvl0y21" // photos_albums member of "christmas-2030" + + // Share-link tokens resolve to shared albums; a visitor session derives its shares from them. + //nolint:gosec // G101: deterministic fixture share-link tokens for tests only. + scopeFolderShareToken = "8jxf3jfn2k" // link to "april-1990" folder album (contains 1990/04) + //nolint:gosec // G101: deterministic fixture share-link tokens for tests only. + scopeStateShareToken = "9jxf3jfn2k" // link to "california-usa" state album (excludes 1990/04) + //nolint:gosec // G101: deterministic fixture share-link tokens for tests only. + scopeRegularShareToken = "4jxf3jfn2k" // link to "christmas-2030" regular album (no filter) ) // scopeSession builds an in-memory session for the named user fixture. @@ -74,6 +91,14 @@ func scopeSession(name string) *entity.Session { return s } +// scopeVisitorWithShares builds an unregistered visitor session that has redeemed the given share +// link tokens, so its SharedUIDs resolve from the matching links exactly as in production. +func scopeVisitorWithShares(tokens ...string) *entity.Session { + s := &entity.Session{} + s.SetData(&entity.SessionData{Tokens: tokens}) + return s +} + func TestPhotoSessionSeesEverything(t *testing.T) { t.Run("NilSession", func(t *testing.T) { assert.True(t, PhotoSessionSeesEverything(nil)) @@ -145,6 +170,24 @@ func TestPhotoVisibleToSession(t *testing.T) { assert.NoError(t, err) assert.False(t, ok) }) + t.Run("VisitorSharedFolderAlbum", func(t *testing.T) { + // A picture shared only through a folder (smart) album has no photos_albums row, so it is + // visible only via the shared-album fallback. + ok, err := PhotoVisibleToSession(scopeFolderPhotoUID, scopeVisitorWithShares(scopeFolderShareToken)) + assert.NoError(t, err) + assert.True(t, ok) + }) + t.Run("VisitorWrongSmartAlbum", func(t *testing.T) { + // Sharing a different smart album must not expose the folder picture. + ok, err := PhotoVisibleToSession(scopeFolderPhotoUID, scopeVisitorWithShares(scopeStateShareToken)) + assert.NoError(t, err) + assert.False(t, ok) + }) + t.Run("VisitorNoShares", func(t *testing.T) { + ok, err := PhotoVisibleToSession(scopeFolderPhotoUID, scopeVisitorWithShares()) + assert.NoError(t, err) + assert.False(t, ok) + }) } // A non-empty Scope skips the personal ScopePhotosForSession filter, so it must be authorized by @@ -191,4 +234,82 @@ func TestFileVisibleToSession(t *testing.T) { assert.NoError(t, err) assert.False(t, ok) }) + t.Run("VisitorSharedFolderAlbumFile", func(t *testing.T) { + // The file hash resolves to a picture shared only through a folder (smart) album. + ok, err := FileVisibleToSession(scopeFolderFileHash, scopeVisitorWithShares(scopeFolderShareToken)) + assert.NoError(t, err) + assert.True(t, ok) + }) + t.Run("VisitorWrongSmartAlbumFile", func(t *testing.T) { + ok, err := FileVisibleToSession(scopeFolderFileHash, scopeVisitorWithShares(scopeStateShareToken)) + assert.NoError(t, err) + assert.False(t, ok) + }) +} + +func TestSharedSmartAlbumContains(t *testing.T) { + t.Run("EmptyID", func(t *testing.T) { + ok, err := sharedSmartAlbumContains("", scopeVisitorWithShares(scopeFolderShareToken)) + assert.NoError(t, err) + assert.False(t, ok) + }) + t.Run("NilSession", func(t *testing.T) { + ok, err := sharedSmartAlbumContains(scopeFolderPhotoUID, nil) + assert.NoError(t, err) + assert.False(t, ok) + }) + t.Run("NoShares", func(t *testing.T) { + ok, err := sharedSmartAlbumContains(scopeFolderPhotoUID, scopeVisitorWithShares()) + assert.NoError(t, err) + assert.False(t, ok) + }) + t.Run("PhotoUIDInSharedFolder", func(t *testing.T) { + ok, err := sharedSmartAlbumContains(scopeFolderPhotoUID, scopeVisitorWithShares(scopeFolderShareToken)) + assert.NoError(t, err) + assert.True(t, ok) + }) + t.Run("WrongSmartAlbum", func(t *testing.T) { + ok, err := sharedSmartAlbumContains(scopeFolderPhotoUID, scopeVisitorWithShares(scopeStateShareToken)) + assert.NoError(t, err) + assert.False(t, ok) + }) + t.Run("RegularAlbumSkipped", func(t *testing.T) { + // A shared regular album (empty filter) is skipped here because ScopePhotosForSession already + // covers its photos_albums membership, so the fallback reports no match. + ok, err := sharedSmartAlbumContains(scopeFolderPhotoUID, scopeVisitorWithShares(scopeRegularShareToken)) + assert.NoError(t, err) + assert.False(t, ok) + }) +} + +// BenchmarkPhotoVisibleToSession measures the per-call cost of the single-item visibility check on +// the paths a restricted session hits when the lightbox prefetches photo metadata: the privileged +// short-circuit, the personal-scope hit (regular share), and the smart-album fallback (folder share) +// including the worst case where the fallback searches a shared album and finds no match. +func BenchmarkPhotoVisibleToSession(b *testing.B) { + admin := scopeSession("alice") + folderVisitor := scopeVisitorWithShares(scopeFolderShareToken) + regularVisitor := scopeVisitorWithShares(scopeRegularShareToken) + stateVisitor := scopeVisitorWithShares(scopeStateShareToken) + + b.Run("AdminShortCircuit", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = PhotoVisibleToSession(scopeFolderPhotoUID, admin) + } + }) + b.Run("RegularSharePersonalScopeHit", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = PhotoVisibleToSession(scopeRegularPhotoUID, regularVisitor) + } + }) + b.Run("FolderShareSmartAlbumFallback", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = PhotoVisibleToSession(scopeFolderPhotoUID, folderVisitor) + } + }) + b.Run("OutOfScopeFallbackMiss", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = PhotoVisibleToSession(scopeFolderPhotoUID, stateVisitor) + } + }) }