mirror of
https://github.com/photoprism/photoprism.git
synced 2026-07-17 16:49:04 +00:00
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.
This commit is contained in:
parent
4946bd2050
commit
3f039866ff
3 changed files with 205 additions and 4 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue