diff --git a/persistence/share_repository.go b/persistence/share_repository.go index 9343e3e7..41510964 100644 --- a/persistence/share_repository.go +++ b/persistence/share_repository.go @@ -30,7 +30,33 @@ func NewShareRepository(ctx context.Context, db dbx.Builder) model.ShareReposito return r } +// TODO: Ownership checks should be moved to the service layer (core/share.go) +func (r *shareRepository) checkOwnership(id string) error { + usr := loggedUser(r.ctx) + if usr.IsAdmin || usr.ID == invalidUserId { + return nil + } + sel := r.newSelect().Columns("user_id").Where(Eq{"id": id}) + var share struct { + UserID string `db:"user_id"` + } + err := r.queryOne(sel, &share) + if err != nil { + if errors.Is(err, model.ErrNotFound) { + return rest.ErrNotFound + } + return err + } + if share.UserID != usr.ID { + return rest.ErrPermissionDenied + } + return nil +} + func (r *shareRepository) Delete(id string) error { + if err := r.checkOwnership(id); err != nil { + return err + } err := r.delete(Eq{"id": id}) if errors.Is(err, model.ErrNotFound) { return rest.ErrNotFound @@ -140,7 +166,9 @@ func sortByIdPosition(mfs model.MediaFiles, ids []string) model.MediaFiles { func (r *shareRepository) Update(id string, entity any, cols ...string) error { s := entity.(*model.Share) - // TODO Validate record + if err := r.checkOwnership(id); err != nil { + return err + } s.ID = id s.UpdatedAt = time.Now() cols = append(cols, "updated_at") diff --git a/persistence/share_repository_test.go b/persistence/share_repository_test.go index 96fd0c2b..6988f323 100644 --- a/persistence/share_repository_test.go +++ b/persistence/share_repository_test.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/deluan/rest" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -130,4 +131,91 @@ var _ = Describe("ShareRepository", func() { Expect(share.Albums).To(BeEmpty()) }) }) + + Describe("Ownership Checks", func() { + var ownerUser = model.User{ID: "2222", UserName: "regular-user"} + var otherUser = model.User{ID: "3333", UserName: "third-user"} + + insertShare := func(shareID, userID string) { + _, err := GetDBXBuilder().NewQuery(` + INSERT INTO share (id, user_id, description, resource_type, resource_ids, created_at, updated_at) + VALUES ({:id}, {:user}, {:desc}, {:type}, {:ids}, {:created}, {:updated}) + `).Bind(map[string]any{ + "id": shareID, + "user": userID, + "desc": "Test Share", + "type": "media_file", + "ids": "1001", + "created": time.Now(), + "updated": time.Now(), + }).Execute() + Expect(err).ToNot(HaveOccurred()) + } + + Describe("Delete", func() { + It("allows a non-admin user to delete their own share", func() { + insertShare("own-share-del", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), ownerUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Delete("own-share-del") + Expect(err).ToNot(HaveOccurred()) + }) + + It("denies a non-admin user from deleting another user's share", func() { + insertShare("other-share-del", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), otherUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Delete("other-share-del") + Expect(err).To(Equal(rest.ErrPermissionDenied)) + }) + + It("allows an admin to delete any user's share", func() { + insertShare("admin-del-share", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), adminUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Delete("admin-del-share") + Expect(err).ToNot(HaveOccurred()) + }) + + It("allows headless context (no user) to delete a share", func() { + insertShare("headless-del-share", ownerUser.ID) + repo := NewShareRepository(context.Background(), GetDBXBuilder()) + err := repo.(rest.Persistable).Delete("headless-del-share") + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Describe("Update", func() { + It("allows a non-admin user to update their own share", func() { + insertShare("own-share-upd", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), ownerUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Update("own-share-upd", &model.Share{Description: "Updated"}, "description") + Expect(err).ToNot(HaveOccurred()) + }) + + It("denies a non-admin user from updating another user's share", func() { + insertShare("other-share-upd", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), otherUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Update("other-share-upd", &model.Share{Description: "Hacked"}, "description") + Expect(err).To(Equal(rest.ErrPermissionDenied)) + }) + + It("allows an admin to update any user's share", func() { + insertShare("admin-upd-share", ownerUser.ID) + ctx := request.WithUser(log.NewContext(context.TODO()), adminUser) + repo := NewShareRepository(ctx, GetDBXBuilder()) + err := repo.(rest.Persistable).Update("admin-upd-share", &model.Share{Description: "Admin Updated"}, "description") + Expect(err).ToNot(HaveOccurred()) + }) + + It("allows headless context (no user) to update a share", func() { + insertShare("headless-upd-share", ownerUser.ID) + repo := NewShareRepository(context.Background(), GetDBXBuilder()) + err := repo.(rest.Persistable).Update("headless-upd-share", &model.Share{Description: "Headless"}, "description") + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) })