mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
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`.
This commit is contained in:
parent
3e3b73a9cc
commit
406a2e9cf4
4 changed files with 29 additions and 2 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue