From 80dd926f2d88ffab9df62411ae5802e2feb9a616 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sat, 7 Oct 2023 17:33:04 +0200 Subject: [PATCH] Share: Improve query validation in the search and albums API Signed-off-by: Michael Mayer --- internal/api/albums.go | 99 +++++++++++++++++++++++++++---- internal/entity/auth_user.go | 18 +++++- internal/entity/auth_user_test.go | 6 ++ internal/search/photos.go | 2 +- internal/search/photos_geo.go | 2 +- 5 files changed, 111 insertions(+), 16 deletions(-) diff --git a/internal/api/albums.go b/internal/api/albums.go index 318c4ad2f..e8f8ede45 100644 --- a/internal/api/albums.go +++ b/internal/api/albums.go @@ -49,8 +49,17 @@ func GetAlbum(router *gin.RouterGroup) { return } - id := clean.UID(c.Param("uid")) - a, err := query.AlbumByUID(id) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) @@ -129,7 +138,16 @@ func UpdateAlbum(router *gin.RouterGroup) { return } + // Get sanitized album UID from request path. uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. a, err := query.AlbumByUID(uid) if err != nil { @@ -184,9 +202,17 @@ func DeleteAlbum(router *gin.RouterGroup) { return } - id := clean.UID(c.Param("uid")) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) - a, err := query.AlbumByUID(id) + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) @@ -211,7 +237,7 @@ func DeleteAlbum(router *gin.RouterGroup) { return } - // PublishAlbumEvent(EntityDeleted, id, c) + // PublishAlbumEvent(EntityDeleted, uid, c) UpdateClientConfig() @@ -237,8 +263,17 @@ func LikeAlbum(router *gin.RouterGroup) { return } - id := clean.UID(c.Param("uid")) - a, err := query.AlbumByUID(id) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) @@ -252,7 +287,7 @@ func LikeAlbum(router *gin.RouterGroup) { UpdateClientConfig() - PublishAlbumEvent(EntityUpdated, id, c) + PublishAlbumEvent(EntityUpdated, uid, c) // Update album YAML backup. SaveAlbumAsYaml(a) @@ -276,8 +311,17 @@ func DislikeAlbum(router *gin.RouterGroup) { return } - id := clean.UID(c.Param("uid")) - a, err := query.AlbumByUID(id) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) @@ -291,7 +335,7 @@ func DislikeAlbum(router *gin.RouterGroup) { UpdateClientConfig() - PublishAlbumEvent(EntityUpdated, id, c) + PublishAlbumEvent(EntityUpdated, uid, c) // Update album YAML backup. SaveAlbumAsYaml(a) @@ -311,7 +355,17 @@ func CloneAlbums(router *gin.RouterGroup) { return } - a, err := query.AlbumByUID(clean.UID(c.Param("uid"))) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) @@ -376,7 +430,16 @@ func AddPhotosToAlbum(router *gin.RouterGroup) { return } + // Get sanitized album UID from request path. uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. a, err := query.AlbumByUID(uid) if err != nil { @@ -443,7 +506,17 @@ func RemovePhotosFromAlbum(router *gin.RouterGroup) { return } - a, err := query.AlbumByUID(clean.UID(c.Param("uid"))) + // Get sanitized album UID from request path. + uid := clean.UID(c.Param("uid")) + + // Visitors and other restricted users can only access shared content. + if (s.User().HasSharedAccessOnly(acl.ResourceAlbums) || s.NotRegistered()) && !s.HasShare(uid) { + AbortForbidden(c) + return + } + + // Find album by UID. + a, err := query.AlbumByUID(uid) if err != nil { AbortAlbumNotFound(c) diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index afd46e7bf..84e9c9f0b 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -731,6 +731,15 @@ func (m *User) IsVisitor() bool { return m.AclRole() == acl.RoleVisitor || m.ID == Visitor.ID } +// HasSharedAccessOnly checks if the user as only access to shared resources. +func (m *User) HasSharedAccessOnly(resource acl.Resource) bool { + if acl.Resources.Deny(resource, m.AclRole(), acl.AccessShared) { + return false + } + + return acl.Resources.DenyAll(resource, m.AclRole(), acl.Permissions{acl.AccessAll, acl.AccessLibrary}) +} + // IsUnknown checks if the user is unknown. func (m *User) IsUnknown() bool { return !rnd.IsUID(m.UserUID, UserUID) || m.ID == UnknownUser.ID || m.UserUID == UnknownUser.UserUID @@ -953,10 +962,17 @@ func (m *User) HasShares() bool { // HasShare if a uid was shared with the user. func (m *User) HasShare(uid string) bool { - if !m.IsRegistered() || m.NoShares() { + if !m.IsRegistered() { return false } + // Check cashed list of user shares. + if m.UserShares.Contains(uid) { + return true + } + + // Refresh cache and try again if not found. + m.RefreshShares() return m.UserShares.Contains(uid) } diff --git a/internal/entity/auth_user_test.go b/internal/entity/auth_user_test.go index bd7abff3e..187cbf35c 100644 --- a/internal/entity/auth_user_test.go +++ b/internal/entity/auth_user_test.go @@ -43,6 +43,8 @@ func TestFindLocalUser(t *testing.T) { assert.Equal(t, acl.RoleAdmin, m.AclRole()) assert.Equal(t, "", m.Attr()) assert.False(t, m.IsVisitor()) + assert.False(t, m.HasSharedAccessOnly(acl.ResourcePhotos)) + assert.False(t, m.HasSharedAccessOnly(acl.ResourceAlbums)) assert.True(t, m.SuperAdmin) assert.True(t, m.CanLogin) assert.True(t, m.CanInvite) @@ -66,6 +68,8 @@ func TestFindLocalUser(t *testing.T) { assert.Equal(t, acl.RoleAdmin, m.AclRole()) assert.NotEqual(t, acl.RoleVisitor, m.AclRole()) assert.False(t, m.IsVisitor()) + assert.False(t, m.HasSharedAccessOnly(acl.ResourcePhotos)) + assert.False(t, m.HasSharedAccessOnly(acl.ResourceAlbums)) assert.True(t, m.CanLogin) assert.NotEmpty(t, m.CreatedAt) assert.NotEmpty(t, m.UpdatedAt) @@ -85,6 +89,8 @@ func TestFindLocalUser(t *testing.T) { assert.Equal(t, "bob@example.com", m.UserEmail) assert.False(t, m.SuperAdmin) assert.False(t, m.IsVisitor()) + assert.False(t, m.HasSharedAccessOnly(acl.ResourcePhotos)) + assert.False(t, m.HasSharedAccessOnly(acl.ResourceAlbums)) assert.True(t, m.CanLogin) assert.NotEmpty(t, m.CreatedAt) assert.NotEmpty(t, m.UpdatedAt) diff --git a/internal/search/photos.go b/internal/search/photos.go index a126a9b8b..c8f674493 100644 --- a/internal/search/photos.go +++ b/internal/search/photos.go @@ -146,7 +146,7 @@ func searchPhotos(f form.SearchPhotos, sess *entity.Session, resultCols string) } // Visitors and other restricted users can only access shared content. - if f.Scope != "" && !sess.HasShare(f.Scope) && (sess.IsVisitor() || sess.NotRegistered()) || + if f.Scope != "" && !sess.HasShare(f.Scope) && (sess.User().HasSharedAccessOnly(acl.ResourcePhotos) || sess.NotRegistered()) || f.Scope == "" && acl.Resources.Deny(acl.ResourcePhotos, aclRole, acl.ActionSearch) { event.AuditErr([]string{sess.IP(), "session %s", "%s %s as %s", "denied"}, sess.RefID, acl.ActionSearch.String(), string(acl.ResourcePhotos), aclRole) return PhotoResults{}, 0, ErrForbidden diff --git a/internal/search/photos_geo.go b/internal/search/photos_geo.go index e381120df..3657ac360 100644 --- a/internal/search/photos_geo.go +++ b/internal/search/photos_geo.go @@ -129,7 +129,7 @@ func UserPhotosGeo(f form.SearchPhotosGeo, sess *entity.Session) (results GeoRes } // Visitors and other restricted users can only access shared content. - if f.Scope != "" && !sess.HasShare(f.Scope) && (sess.IsVisitor() || sess.NotRegistered()) || + if f.Scope != "" && !sess.HasShare(f.Scope) && (sess.User().HasSharedAccessOnly(acl.ResourcePlaces) || sess.NotRegistered()) || f.Scope == "" && acl.Resources.Deny(acl.ResourcePlaces, aclRole, acl.ActionSearch) { event.AuditErr([]string{sess.IP(), "session %s", "%s %s as %s", "denied"}, sess.RefID, acl.ActionSearch.String(), string(acl.ResourcePlaces), aclRole) return GeoResults{}, ErrForbidden