nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-31 07:30:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(playlists): drop redundant entity-based owner-permission guard

Browse Source 
The case-insensitive sentFields predicate already prevents case-variant
JSON keys like {"OwnerId":"x"} from bypassing the ownerChanged check, so
the duplicated entity-content guard is no longer load-bearing.

Strengthen the regression test into a DescribeTable covering canonical,
PascalCase, all-upper, and all-lower spellings to lock in the
case-insensitive contract.
This commit is contained in:
Deluan 2026-05-27 23:24:43 -03:00
parent 3060eef63f
commit 65bd4df49c
2 changed files with 19 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
core/playlists/rest_adapter.go
View File

@ -106,11 +106,7 @@ func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity
usr, _ := request.UserFrom(ctx)
ownerChanged := sent("ownerId") && entity.OwnerID != "" && entity.OwnerID != current.OwnerID
// Permission check uses the deserialized entity directly (not gated by `sent`)
// so a non-admin can't smuggle in an owner change via a case-variant JSON key
// like {"OwnerId":"x"} — Go's json decoder is case-insensitive on field match
// but rest.Put's field-name extraction is case-sensitive.
if !usr.IsAdmin && entity.OwnerID != "" && entity.OwnerID != current.OwnerID {
if !usr.IsAdmin && ownerChanged {
return rest.ErrPermissionDenied
}

29
core/playlists/rest_adapter_test.go
View File

@ -125,17 +125,24 @@ var _ = Describe("REST Adapter", func() {
Expect(err).To(Equal(rest.ErrPermissionDenied))
})
It("denies regular user even when ownerId arrives under a case-variant JSON key", func() {
// rest.Put's field-name extraction is case-sensitive, but Go's json
// decoder is case-insensitive on struct fields, so {"OwnerId":"x"}
// populates entity.OwnerID while cols carries "OwnerId" instead of
// "ownerId". The permission gate must still fire.
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
repo = ps.NewRepository(ctx).(rest.Persistable)
pls := &model.Playlist{OwnerID: "other-user"}
err := repo.Update("pls-1", pls, "OwnerId")
Expect(err).To(Equal(rest.ErrPermissionDenied))
})
DescribeTable("denies regular user from changing ownership under any case-variant JSON key",
func(colName string) {
// rest.Put's field-name extraction is case-sensitive, but Go's
// json decoder is case-insensitive on struct fields, so any
// {"OwnerId":"x"} / {"OWNERID":"x"} / {"ownerid":"x"} populates
// entity.OwnerID. sentFields normalizes both sides so the
// permission gate fires regardless of casing.
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
repo = ps.NewRepository(ctx).(rest.Persistable)
pls := &model.Playlist{OwnerID: "other-user"}
err := repo.Update("pls-1", pls, colName)
Expect(err).To(Equal(rest.ErrPermissionDenied))
},
Entry("canonical camelCase", "ownerId"),
Entry("PascalCase", "OwnerId"),
Entry("all upper", "OWNERID"),
Entry("all lower", "ownerid"),
)
It("updates smart playlist rules", func() {
mockPlsRepo.Data["smart-1"] = &model.Playlist{