From 406a2e9cf439e2e68d818522ba0acb55db9f7d3d Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Sat, 26 Sep 2026 02:21:16 -0400
Subject: [PATCH] fix(api): never widen the scopes an access token claims

Authenticate ran token claims through Expand, so a token claiming `all`
would have gained every known scope. Only a holder of the signing key
could mint one, but tokens should carry concrete scopes only. Claims now
go through Allowed, which keeps known scopes (and admin only for admins)
and never expands `all`.
---
 core/apiauth/scopes.go               |  7 ++++++-
 core/apiauth/scopes_test.go          | 10 ++++++++++
 core/apiauth/service.go              |  2 +-
 core/apiauth/service_session_test.go | 12 ++++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)

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)