fix(share): enforce per-user ownership on share reads

Share repository read methods (Get, GetAll, Read, ReadAll, Exists, Count,
CountAll) did not apply an owner filter, so non-admin users saw shares
belonging to other users. The write paths already enforced per-user ownership;
this brings reads in line with them.

Add an addRestriction()/ownerFilter() based scope to share reads, keeping
admins and the headless public-share resolution path unrestricted. Route share
and player Delete through a new base-repo deleteOwned() primitive that applies
the ownership predicate in the DELETE's WHERE clause (atomic, no select-then-
delete window) and classifies a zero-row result as permission-denied vs
not-found, mirroring updateOwned. The addRestriction helper and the write-miss
classifier are hoisted onto the base repository so player and share share one
implementation.

Also map rest.ErrPermissionDenied and rest.ErrNotFound in the Subsonic error
handler so ownership/not-found failures from the rest-backed repositories
return the proper Subsonic codes (50 / 70) instead of a generic error.

Covered by unit tests (persistence, subsonic error mapping) and an end-to-end
cross-user sharing isolation test.

persistence/player_repository.go

@@ -62,17 +62,6 @@ func (r *playerRepository) newRestSelect(options ...model.QueryOptions) SelectBu
 	return s.Where(r.addRestriction())
 }
 
-func (r *playerRepository) addRestriction(sql ...Sqlizer) Sqlizer {
-	s := And{}
-	if len(sql) > 0 {
-		s = append(s, sql[0])
-	}
-	if owner := r.ownerFilter(); owner != nil {
-		s = append(s, owner)
-	}
-	return s
-}
-
 func (r *playerRepository) CountByClient(options ...model.QueryOptions) (map[string]int64, error) {
 	sel := r.newSelect(options...).
 		Columns(
@@ -152,12 +141,7 @@ func (r *playerRepository) Update(id string, entity any, cols ...string) error {
 }
 
 func (r *playerRepository) Delete(id string) error {
-	filter := r.addRestriction(And{Eq{"player.id": id}})
-	err := r.delete(filter)
-	if errors.Is(err, model.ErrNotFound) {
-		return rest.ErrNotFound
-	}
-	return err
+	return r.deleteOwned(id)
 }
 
 var _ model.PlayerRepository = (*playerRepository)(nil)

persistence/player_repository_test.go

@@ -110,33 +110,43 @@ var _ = Describe("PlayerRepository", func() {
 		})
 
 		Describe("Delete", func() {
-			DescribeTable("item type", func(player model.Player) {
-				err := repo.Delete(player.ID)
+			It("deletes a player owned by the current user", func() {
+				err := repo.Delete(userPlayer.ID)
 				Expect(err).To(BeNil())
 
-				isReal := player.UserId != ""
-				canDelete := admin || player.UserId == userPlayer.UserId
-
 				count, err := repo.Count()
 				Expect(err).To(BeNil())
+				Expect(count).To(Equal(baseCount - 1))
 
-				if isReal && canDelete {
-					Expect(count).To(Equal(baseCount - 1))
-				} else {
-					Expect(count).To(Equal(baseCount))
-				}
+				_, err = repo.Get(userPlayer.ID)
+				Expect(err).To(Equal(model.ErrNotFound))
+			})
 
-				item, err := repo.Get(player.ID)
-				if !isReal || canDelete {
+			It("does not delete another user's player when not admin", func() {
+				err := repo.Delete(otherPlayer.ID)
+
+				if admin {
+					// Admins may delete any player.
+					Expect(err).To(BeNil())
+					Expect(repo.Count()).To(Equal(baseCount - 1))
+					_, err = repo.Get(otherPlayer.ID)
 					Expect(err).To(Equal(model.ErrNotFound))
 				} else {
-					Expect(*item).To(Equal(player))
+					// The ownership-restricted delete matches no owned row, so it reports
+					// permission-denied and leaves the other user's player untouched.
+					Expect(err).To(Equal(rest.ErrPermissionDenied))
+					Expect(repo.Count()).To(Equal(baseCount))
+					item, err := repo.Get(otherPlayer.ID)
+					Expect(err).To(BeNil())
+					Expect(*item).To(Equal(otherPlayer))
 				}
-			},
-				Entry("same user", userPlayer),
-				Entry("other item", otherPlayer),
-				Entry("fake item", model.Player{}),
-			)
+			})
+
+			It("returns not-found for a nonexistent player", func() {
+				err := repo.Delete("i don't exist")
+				Expect(err).To(Equal(rest.ErrNotFound))
+				Expect(repo.Count()).To(Equal(baseCount))
+			})
 		})
 
 		Describe("Read", func() {

persistence/share_repository.go

@@ -30,51 +30,18 @@ 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
-}
-
-// TODO: this still uses the legacy checkOwnership SELECT-then-delete pattern (a TOCTOU window),
-// the same shape removed from Update. Once a base-repo deleteOwned exists (built on ownerFilter,
-// mirroring updateOwned), route Delete through it and drop checkOwnership entirely. playerRepository
-// .Delete (which restricts via addRestriction) should adopt the same primitive.
 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
-	}
-	return err
+	return r.deleteOwned(id)
 }
 
 func (r *shareRepository) selectShare(options ...model.QueryOptions) SelectBuilder {
 	return r.newSelect(options...).Join("user u on u.id = share.user_id").
-		Columns("share.*", "user_name as username")
+		Columns("share.*", "user_name as username").
+		Where(r.addRestriction())
 }
 
 func (r *shareRepository) Exists(id string) (bool, error) {
-	return r.exists(Eq{"id": id})
+	return r.exists(r.addRestriction(And{Eq{"id": id}}))
 }
 
 func (r *shareRepository) Get(id string) (*model.Share, error) {

persistence/share_repository_test.go

@@ -20,7 +20,7 @@ var _ = Describe("ShareRepository", func() {
 
 	BeforeEach(func() {
 		DeferCleanup(configtest.SetupConfig())
-		ctx = request.WithUser(log.NewContext(context.TODO()), adminUser)
+		ctx = request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
 		repo = NewShareRepository(ctx, GetDBXBuilder())
 
 		// Insert the admin user into the database (required for foreign key constraint)
@@ -38,7 +38,7 @@ var _ = Describe("ShareRepository", func() {
 		Context("Repository creation and basic operations", func() {
 			It("should create repository successfully with no user context", func() {
 				// Create repository with no user context (headless)
-				headlessRepo := NewShareRepository(context.Background(), GetDBXBuilder())
+				headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
 				Expect(headlessRepo).ToNot(BeNil())
 			})
 
@@ -60,7 +60,7 @@ var _ = Describe("ShareRepository", func() {
 				Expect(err).ToNot(HaveOccurred())
 
 				// Headless process should see all shares
-				headlessRepo := NewShareRepository(context.Background(), GetDBXBuilder())
+				headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
 				shares, err := headlessRepo.GetAll()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -92,7 +92,7 @@ var _ = Describe("ShareRepository", func() {
 				Expect(err).ToNot(HaveOccurred())
 
 				// Headless process should be able to get the share
-				headlessRepo := NewShareRepository(context.Background(), GetDBXBuilder())
+				headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
 				share, err := headlessRepo.Get(shareID)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(share.ID).To(Equal(shareID))
@@ -155,7 +155,7 @@ var _ = Describe("ShareRepository", func() {
 		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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), ownerUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Delete("own-share-del")
 				Expect(err).ToNot(HaveOccurred())
@@ -163,15 +163,21 @@ var _ = Describe("ShareRepository", func() {
 
 			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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), otherUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Delete("other-share-del")
 				Expect(err).To(Equal(rest.ErrPermissionDenied))
+
+				// The share was not deleted: the owner can still read it.
+				ownerCtx := request.WithUser(log.NewContext(GinkgoT().Context()), ownerUser)
+				ownerRepo := NewShareRepository(ownerCtx, GetDBXBuilder())
+				_, err = ownerRepo.(rest.Repository).Read("other-share-del")
+				Expect(err).ToNot(HaveOccurred())
 			})
 
 			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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Delete("admin-del-share")
 				Expect(err).ToNot(HaveOccurred())
@@ -179,7 +185,7 @@ var _ = Describe("ShareRepository", func() {
 
 			It("allows headless context (no user) to delete a share", func() {
 				insertShare("headless-del-share", ownerUser.ID)
-				repo := NewShareRepository(context.Background(), GetDBXBuilder())
+				repo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
 				err := repo.(rest.Persistable).Delete("headless-del-share")
 				Expect(err).ToNot(HaveOccurred())
 			})
@@ -188,7 +194,7 @@ var _ = Describe("ShareRepository", func() {
 		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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), ownerUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Update("own-share-upd", &model.Share{Description: "Updated"}, "description")
 				Expect(err).ToNot(HaveOccurred())
@@ -196,7 +202,7 @@ var _ = Describe("ShareRepository", func() {
 
 			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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), otherUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Update("other-share-upd", &model.Share{Description: "Hacked"}, "description")
 				Expect(err).To(Equal(rest.ErrPermissionDenied))
@@ -204,7 +210,7 @@ var _ = Describe("ShareRepository", func() {
 
 			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)
+				ctx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
 				repo := NewShareRepository(ctx, GetDBXBuilder())
 				err := repo.(rest.Persistable).Update("admin-upd-share", &model.Share{Description: "Admin Updated"}, "description")
 				Expect(err).ToNot(HaveOccurred())
@@ -212,7 +218,7 @@ var _ = Describe("ShareRepository", func() {
 
 			It("allows headless context (no user) to update a share", func() {
 				insertShare("headless-upd-share", ownerUser.ID)
-				repo := NewShareRepository(context.Background(), GetDBXBuilder())
+				repo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
 				err := repo.(rest.Persistable).Update("headless-upd-share", &model.Share{Description: "Headless"}, "description")
 				Expect(err).ToNot(HaveOccurred())
 			})
@@ -255,5 +261,135 @@ var _ = Describe("ShareRepository", func() {
 				Expect(got.(*model.Share).UserID).To(Equal(ownerUser.ID))
 			})
 		})
+
+		Describe("Read scoping", func() {
+			BeforeEach(func() {
+				// Persist owner/other users so the JOIN in selectShare resolves.
+				ur := NewUserRepository(ctx, GetDBXBuilder())
+				Expect(ur.Put(&ownerUser)).To(Succeed())
+				Expect(ur.Put(&otherUser)).To(Succeed())
+
+				insertShare("share-owner-1", ownerUser.ID)
+				insertShare("share-owner-2", ownerUser.ID)
+				insertShare("share-other-1", otherUser.ID)
+			})
+
+			Context("non-admin user", func() {
+				var nonAdminRepo model.ShareRepository
+				var nonAdminRest rest.Repository
+
+				BeforeEach(func() {
+					nonAdminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), ownerUser)
+					nonAdminRepo = NewShareRepository(nonAdminCtx, GetDBXBuilder())
+					nonAdminRest = nonAdminRepo.(rest.Repository)
+				})
+
+				It("GetAll returns only own shares", func() {
+					shares, err := nonAdminRepo.GetAll()
+					Expect(err).ToNot(HaveOccurred())
+					ids := make([]string, len(shares))
+					for i, s := range shares {
+						ids[i] = s.ID
+					}
+					Expect(ids).To(ConsistOf("share-owner-1", "share-owner-2"))
+				})
+
+				It("ReadAll returns only own shares", func() {
+					res, err := nonAdminRest.ReadAll()
+					Expect(err).ToNot(HaveOccurred())
+					shares := res.(model.Shares)
+					ids := make([]string, len(shares))
+					for i, s := range shares {
+						ids[i] = s.ID
+					}
+					Expect(ids).To(ConsistOf("share-owner-1", "share-owner-2"))
+				})
+
+				It("Get returns own share", func() {
+					s, err := nonAdminRepo.Get("share-owner-1")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(s.ID).To(Equal("share-owner-1"))
+				})
+
+				It("Get returns ErrNotFound for another user's share", func() {
+					_, err := nonAdminRepo.Get("share-other-1")
+					Expect(err).To(MatchError(model.ErrNotFound))
+				})
+
+				It("Read returns ErrNotFound for another user's share", func() {
+					_, err := nonAdminRest.Read("share-other-1")
+					Expect(err).To(MatchError(model.ErrNotFound))
+				})
+
+				It("Exists returns true for own share", func() {
+					exists, err := nonAdminRepo.Exists("share-owner-1")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(exists).To(BeTrue())
+				})
+
+				It("Exists returns false for another user's share", func() {
+					exists, err := nonAdminRepo.Exists("share-other-1")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(exists).To(BeFalse())
+				})
+
+				It("CountAll counts only own shares", func() {
+					count, err := nonAdminRepo.CountAll()
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(BeNumerically("==", 2))
+				})
+
+				It("Count (rest) counts only own shares", func() {
+					count, err := nonAdminRest.Count()
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(BeNumerically("==", 2))
+				})
+			})
+
+			Context("admin user", func() {
+				It("GetAll returns all shares", func() {
+					adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+					adminRepo := NewShareRepository(adminCtx, GetDBXBuilder())
+					shares, err := adminRepo.GetAll()
+					Expect(err).ToNot(HaveOccurred())
+					ids := make([]string, len(shares))
+					for i, s := range shares {
+						ids[i] = s.ID
+					}
+					Expect(ids).To(ConsistOf("share-owner-1", "share-owner-2", "share-other-1"))
+				})
+
+				It("CountAll counts all shares", func() {
+					adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+					adminRepo := NewShareRepository(adminCtx, GetDBXBuilder())
+					count, err := adminRepo.CountAll()
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(BeNumerically("==", 3))
+				})
+			})
+
+			Context("headless context (public share route)", func() {
+				It("GetAll returns all shares", func() {
+					headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
+					shares, err := headlessRepo.GetAll()
+					Expect(err).ToNot(HaveOccurred())
+					Expect(shares).To(HaveLen(3))
+				})
+
+				It("Get returns another user's share", func() {
+					headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
+					s, err := headlessRepo.Get("share-other-1")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(s.ID).To(Equal("share-other-1"))
+				})
+
+				It("Exists returns true for any share", func() {
+					headlessRepo := NewShareRepository(GinkgoT().Context(), GetDBXBuilder())
+					exists, err := headlessRepo.Exists("share-other-1")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(exists).To(BeTrue())
+				})
+			})
+		})
 	})
 })

persistence/sql_base_repository.go

@@ -61,6 +61,9 @@ func loggedUser(ctx context.Context) *model.User {
 // ownerFilter returns the predicate restricting access to rows owned by the logged-in user, for
 // tables with a user_id column. It returns nil for admins and for headless/system contexts (invalid
 // user), meaning "no ownership restriction". Callers should skip the WHERE clause when it is nil.
+//
+// The predicate uses an unqualified user_id, so it only works on queries where that column is
+// unambiguous (no join introducing a second user_id).
 func (r sqlRepository) ownerFilter() Sqlizer {
 	if usr := loggedUser(r.ctx); !usr.IsAdmin && usr.ID != invalidUserId {
 		return Eq{"user_id": usr.ID}
@@ -68,6 +71,20 @@ func (r sqlRepository) ownerFilter() Sqlizer {
 	return nil
 }
 
+// addRestriction combines an optional caller predicate with the ownership filter, producing the
+// WHERE clause for owner-scoped reads. For admins and headless contexts ownerFilter() is nil and
+// only the caller's predicate (if any) remains.
+func (r sqlRepository) addRestriction(sql ...Sqlizer) Sqlizer {
+	s := And{}
+	if len(sql) > 0 {
+		s = append(s, sql[0])
+	}
+	if owner := r.ownerFilter(); owner != nil {
+		s = append(s, owner)
+	}
+	return s
+}
+
 func (r *sqlRepository) registerModel(instance any, filters map[string]filterFunc) {
 	if r.tableName == "" {
 		r.tableName = strings.TrimPrefix(reflect.TypeOf(instance).String(), "*model.")
@@ -402,29 +419,47 @@ func (r sqlRepository) updateOwned(id string, m any, colsToUpdate ...string) err
 	}
 	updateValues := filterUpdateValues(values, id, colsToUpdate...)
 	delete(updateValues, "user_id") // ownership is immutable on update
-	update := Update(r.tableName).Where(Eq{"id": id}).SetMap(updateValues)
-	if owner := r.ownerFilter(); owner != nil {
-		update = update.Where(owner)
-	}
+	update := Update(r.tableName).Where(r.addRestriction(Eq{"id": id})).SetMap(updateValues)
 	count, err := r.executeSQL(update)
 	if err != nil {
 		return err
 	}
 	if count == 0 {
-		// The update matched no row: either the id is missing, or it exists but is owned by
-		// someone else. Disambiguate to return the more accurate error.
-		exists, err := r.exists(Eq{"id": id})
-		if err != nil {
-			return err
-		}
-		if exists {
-			return rest.ErrPermissionDenied
-		}
-		return rest.ErrNotFound
+		return r.classifyOwnedWriteMiss(id)
 	}
 	return nil
 }
 
+// deleteOwned performs an atomic, ownership-restricted delete of the row identified by id, for
+// repositories whose table has a user_id column. Non-admins can only delete rows they own: the
+// ownership predicate is part of the DELETE's WHERE clause, so a row owned by another user simply
+// does not match and is left untouched. The failure path mirrors updateOwned (see
+// classifyOwnedWriteMiss), so there is no TOCTOU on the delete.
+func (r sqlRepository) deleteOwned(id string) error {
+	count, err := r.executeSQL(Delete(r.tableName).Where(r.addRestriction(Eq{"id": id})))
+	if err != nil {
+		return err
+	}
+	if count == 0 {
+		return r.classifyOwnedWriteMiss(id)
+	}
+	return nil
+}
+
+// classifyOwnedWriteMiss explains why an ownership-filtered write (updateOwned/deleteOwned) matched
+// no row: rest.ErrPermissionDenied if the row exists but is owned by another user, otherwise
+// rest.ErrNotFound. It runs only on the failure path (count == 0), where no write occurred.
+func (r sqlRepository) classifyOwnedWriteMiss(id string) error {
+	exists, err := r.exists(Eq{"id": id})
+	if err != nil {
+		return err
+	}
+	if exists {
+		return rest.ErrPermissionDenied
+	}
+	return rest.ErrNotFound
+}
+
 func (r sqlRepository) count(countQuery SelectBuilder, options ...model.QueryOptions) (int64, error) {
 	countQuery = countQuery.
 		RemoveColumns().Columns("count(distinct " + r.tableName + ".id) as count").

server/e2e/subsonic_sharing_test.go

@@ -125,3 +125,82 @@ var _ = Describe("Sharing Endpoints", Ordered, func() {
 		Expect(resp.Error).ToNot(BeNil())
 	})
 })
+
+var _ = Describe("Sharing Cross-User Isolation", Ordered, func() {
+	var userA, userB model.User
+	var shareID string
+	var albumID string
+
+	BeforeAll(func() {
+		conf.Server.EnableSharing = true
+		setupTestDB()
+
+		userA = createUser("share-user-a", "share-user-a", "Share User A", false)
+		userB = createUser("share-user-b", "share-user-b", "Share User B", false)
+
+		albums, err := ds.Album(ctx).GetAll(model.QueryOptions{
+			Filters: squirrel.Eq{"album.name": "Abbey Road"},
+		})
+		Expect(err).ToNot(HaveOccurred())
+		Expect(albums).ToNot(BeEmpty())
+		albumID = albums[0].ID
+
+		resp := doReqWithUser(userA, "createShare", "id", albumID, "description", "User A's share")
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		Expect(resp.Shares.Share).To(HaveLen(1))
+		shareID = resp.Shares.Share[0].ID
+		Expect(resp.Shares.Share[0].Username).To(Equal(userA.UserName))
+	})
+
+	It("userB's getShares does not leak userA's share", func() {
+		resp := doReqWithUser(userB, "getShares")
+
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		Expect(resp.Shares).ToNot(BeNil())
+		Expect(resp.Shares.Share).To(BeEmpty())
+	})
+
+	It("userA still sees own share", func() {
+		resp := doReqWithUser(userA, "getShares")
+
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		Expect(resp.Shares.Share).To(HaveLen(1))
+		Expect(resp.Shares.Share[0].ID).To(Equal(shareID))
+		Expect(resp.Shares.Share[0].Description).To(Equal("User A's share"))
+	})
+
+	It("admin sees userA's share", func() {
+		resp := doReqWithUser(adminUser, "getShares")
+
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		ids := make([]string, len(resp.Shares.Share))
+		for i, s := range resp.Shares.Share {
+			ids[i] = s.ID
+		}
+		Expect(ids).To(ContainElement(shareID))
+	})
+
+	It("userB cannot updateShare on userA's share", func() {
+		resp := doReqWithUser(userB, "updateShare", "id", shareID, "description", "hijacked")
+
+		Expect(resp.Status).To(Equal(responses.StatusFailed))
+		Expect(resp.Error).ToNot(BeNil())
+
+		// Confirm description unchanged for userA.
+		check := doReqWithUser(userA, "getShares")
+		Expect(check.Shares.Share).To(HaveLen(1))
+		Expect(check.Shares.Share[0].Description).To(Equal("User A's share"))
+	})
+
+	It("userB cannot deleteShare on userA's share", func() {
+		resp := doReqWithUser(userB, "deleteShare", "id", shareID)
+
+		Expect(resp.Status).To(Equal(responses.StatusFailed))
+		Expect(resp.Error).ToNot(BeNil())
+
+		// Confirm share still present for userA.
+		check := doReqWithUser(userA, "getShares")
+		Expect(check.Shares.Share).To(HaveLen(1))
+		Expect(check.Shares.Share[0].ID).To(Equal(shareID))
+	})
+})

server/subsonic/api.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"strconv"
 
+	"github.com/deluan/rest"
 	"github.com/go-chi/chi/v5"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/core"
@@ -301,9 +302,9 @@ func mapToSubsonicError(err error) subError {
 		err = newError(responses.ErrorMissingParameter, err.Error())
 	case errors.Is(err, req.ErrInvalidParam):
 		err = newError(responses.ErrorGeneric, err.Error())
-	case errors.Is(err, model.ErrNotFound):
+	case errors.Is(err, model.ErrNotFound), errors.Is(err, rest.ErrNotFound):
 		err = newError(responses.ErrorDataNotFound, "data not found")
-	case errors.Is(err, model.ErrNotAuthorized):
+	case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, rest.ErrPermissionDenied):
 		err = newError(responses.ErrorAuthorizationFail)
 	case errors.Is(err, stream.ErrTooManyTranscodes):
 		err = newError(responses.ErrorGeneric, "too many concurrent transcodes, please retry shortly")

server/subsonic/api_test.go

@@ -4,13 +4,16 @@ import (
 	"context"
 	"encoding/json"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"math"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 
+	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/core/stream"
+	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -187,3 +190,24 @@ var _ = Describe("sendResponse", func() {
 		Expect(pointer).To(Equal(responses.ErrorDataNotFound))
 	})
 })
+
+var _ = Describe("mapToSubsonicError", func() {
+	DescribeTable("maps repository errors to the correct Subsonic error code",
+		func(err error, expectedCode int32) {
+			subErr := mapToSubsonicError(err)
+			Expect(subErr.code).To(Equal(expectedCode))
+		},
+		Entry("rest.ErrPermissionDenied -> not authorized (50)",
+			rest.ErrPermissionDenied, responses.ErrorAuthorizationFail),
+		Entry("rest.ErrNotFound -> data not found (70)",
+			rest.ErrNotFound, responses.ErrorDataNotFound),
+		Entry("model.ErrNotAuthorized -> not authorized (50)",
+			model.ErrNotAuthorized, responses.ErrorAuthorizationFail),
+		Entry("model.ErrNotFound -> data not found (70)",
+			model.ErrNotFound, responses.ErrorDataNotFound),
+		Entry("wrapped rest.ErrPermissionDenied is still mapped",
+			fmt.Errorf("update share: %w", rest.ErrPermissionDenied), responses.ErrorAuthorizationFail),
+		Entry("unknown error -> generic (0)",
+			errors.New("boom"), responses.ErrorGeneric),
+	)
+})