diff --git a/core/apiauth/scopes.go b/core/apiauth/scopes.go index bc3ac9e27..62c840f1f 100644 --- a/core/apiauth/scopes.go +++ b/core/apiauth/scopes.go @@ -51,7 +51,12 @@ func Expand(granted []string, isAdmin bool) []string { } out = append(out, s) } - out = slices.DeleteFunc(out, func(s string) bool { + return Allowed(out, isAdmin) +} + +// Allowed keeps the concrete scopes the user may hold now; unlike Expand it never widens `all`. +func Allowed(scopes []string, isAdmin bool) []string { + out := slices.DeleteFunc(slices.Clone(scopes), func(s string) bool { return !known(s) || (s == ScopeAdmin && !isAdmin) }) return normalize(out) diff --git a/core/apiauth/scopes_test.go b/core/apiauth/scopes_test.go index dc0183b24..23826abd3 100644 --- a/core/apiauth/scopes_test.go +++ b/core/apiauth/scopes_test.go @@ -39,6 +39,16 @@ var _ = Describe("scopes", func() { }) }) + Describe("Allowed", func() { + It("never widens all", func() { + Expect(Allowed([]string{ScopeAll}, true)).To(BeEmpty()) + }) + It("keeps known scopes, and admin only for admins", func() { + Expect(Allowed([]string{"read", "retired", "admin"}, false)).To(Equal([]string{"read"})) + Expect(Allowed([]string{"read", "admin"}, true)).To(Equal([]string{"admin", "read"})) + }) + }) + Describe("Attenuate", func() { available := []string{"playlists:write", "read"} It("returns everything when no subset is asked", func() { diff --git a/core/apiauth/service.go b/core/apiauth/service.go index 29b5075b3..5082017b8 100644 --- a/core/apiauth/service.go +++ b/core/apiauth/service.go @@ -271,7 +271,7 @@ func (s *Service) Authenticate(ctx context.Context, token, ip string) (*Principa lastUsed = &entry.lastUsedAt } s.touch(ctx, c.GrantID, ip, lastUsed) - return &Principal{User: *u, GrantID: c.GrantID, Scopes: Expand(c.Scopes, u.IsAdmin)}, nil + return &Principal{User: *u, GrantID: c.GrantID, Scopes: Allowed(c.Scopes, u.IsAdmin)}, nil } // liveGrant trusts the cache only while its epoch matches; a mismatch is settled from one consistent read. diff --git a/core/apiauth/service_session_test.go b/core/apiauth/service_session_test.go index de0d19033..b871f279b 100644 --- a/core/apiauth/service_session_test.go +++ b/core/apiauth/service_session_test.go @@ -133,6 +133,18 @@ var _ = Describe("Service: sessions", func() { Expect(err).To(MatchError(model.ErrInvalidAuth)) }) + It("grants no scopes to a signed token claiming all", func() { + u := createUser(ctx, "pw", true) + _, p, _ := login(u) + sg, err := svc.signer() + Expect(err).ToNot(HaveOccurred()) + tok, err := sg.sign(claims{UserID: u.ID, GrantID: p.GrantID, Scopes: []string{ScopeAll, "unknown"}, IssuedAt: now, ExpiresAt: now.Add(TokenTTL)}) + Expect(err).ToNot(HaveOccurred()) + got, err := svc.Authenticate(ctx, tok, "") + Expect(err).ToNot(HaveOccurred()) + Expect(got.Scopes).To(BeEmpty()) + }) + It("rejects a token whose grant belongs to another user, even across an epoch change", func() { alice := createUser(ctx, "pw", false) bob := createUser(ctx, "pw", false)