remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-08 02:17:25 +02:00
Code Issues Projects Releases Packages Wiki Activity

refactor(api): reuse existing helpers in API v1 auth

Browse source 
Use gg.V, slice.Map, chi's RequestSize, core/auth's encryption key and
ClientIPRateLimiter instead of local copies; share the grant idle expiry
constant and a Grant.LastActivity helper; pass last use as a time value.
This commit is contained in:
Deluan 2026-09-26 02:33:22 -04:00
commit a41e656706
14 changed files with 70 additions and 71 deletions
Download patch file Download diff file Expand all files Collapse all files

1
consts/consts.go
View file

@ -35,6 +35,7 @@ const (
JWTAPIv1SecretKey = "JWTAPIv1Secret"
JWTIssuer = "ND"
DefaultSessionTimeout = 48 * time.Hour
APIv1GrantIdleExpiry = 90 * 24 * time.Hour
DefaultSmartRefresh = 5 * time.Second
DefaultShareExpiration = 8760 * time.Hour
CookieExpiry = 365 * 24 * 3600 // One year

29
core/apiauth/service.go
View file

@ -10,14 +10,16 @@ import (
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/gg"
)
const (
TokenTTL = time.Hour
IdleExpiry = 90 * 24 * time.Hour
IdleExpiry = consts.APIv1GrantIdleExpiry
cacheTTL = 30 * time.Second
touchInterval = 5 * time.Minute
)
@ -149,11 +151,7 @@ func (s *Service) ResolveGrant(ctx context.Context, secret, ip string) (*Princip
if err != nil {
return nil, err
}
lastActivity := g.CreatedAt
if g.LastUsedAt != nil {
lastActivity = *g.LastUsedAt
}
if !s.now().Before(lastActivity.Add(IdleExpiry)) {
if !s.now().Before(g.LastActivity().Add(IdleExpiry)) {
s.dropGrant(ctx, g.ID)
return nil, model.ErrInvalidAuth
}
@ -166,7 +164,7 @@ func (s *Service) ResolveGrant(ctx context.Context, secret, ip string) (*Princip
return nil, err
}
}
s.touch(ctx, g.ID, ip, g.LastUsedAt)
s.touch(ctx, g.ID, ip, gg.V(g.LastUsedAt))
return &Principal{User: *u, GrantID: g.ID, Scopes: Expand(g.Scopes, u.IsAdmin)}, nil
}
@ -230,10 +228,10 @@ func (s *Service) settleEpoch(ctx context.Context, grantID string) (*model.Grant
return g, u, nil
}
// touch writes last_used at most every touchInterval; the SQL condition keeps that true across nodes.
func (s *Service) touch(ctx context.Context, id, ip string, lastUsed *time.Time) {
// touch writes last_used at most every touchInterval (a zero lastUsed means never); the SQL condition keeps that true across nodes.
func (s *Service) touch(ctx context.Context, id, ip string, lastUsed time.Time) {
now := s.now()
if lastUsed != nil && now.Before(lastUsed.Add(touchInterval)) {
if !lastUsed.IsZero() && now.Before(lastUsed.Add(touchInterval)) {
return
}
if err := s.ds.Grant().Touch(ctx, id, ip, now, now.Add(-touchInterval)); err != nil {
@ -266,11 +264,7 @@ func (s *Service) Authenticate(ctx context.Context, token, ip string) (*Principa
if slices.Contains(c.Scopes, ScopeAdmin) && !u.IsAdmin {
return nil, ErrInsufficientScope
}
var lastUsed *time.Time
if !entry.lastUsedAt.IsZero() {
lastUsed = &entry.lastUsedAt
}
s.touch(ctx, c.GrantID, ip, lastUsed)
s.touch(ctx, c.GrantID, ip, entry.lastUsedAt)
return &Principal{User: *u, GrantID: c.GrantID, Scopes: Allowed(c.Scopes, u.IsAdmin)}, nil
}
@ -294,10 +288,7 @@ func (s *Service) liveGrant(ctx context.Context, id string, u *model.User) (live
return livenessEntry{}, nil, err
}
}
e := livenessEntry{userID: g.UserID, epoch: g.UserEpoch}
if g.LastUsedAt != nil {
e.lastUsedAt = *g.LastUsedAt
}
e := livenessEntry{userID: g.UserID, epoch: g.UserEpoch, lastUsedAt: gg.V(g.LastUsedAt)}
s.cache.put(id, e, now, started)
return e, u, nil
}

17
core/apiauth/signer.go
View file

@ -1,10 +1,8 @@
package apiauth
import (
"cmp"
"context"
"crypto/rand"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
@ -13,8 +11,8 @@ import (
"github.com/go-chi/jwtauth/v5"
"github.com/lestrrat-go/jwx/v3/jwt"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils"
@ -60,7 +58,7 @@ func loadSigner(ctx context.Context, ds model.DataStore, now func() time.Time) (
}
func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
enc, err := utils.Encrypt(ctx, encryptionKey(), newKey())
enc, err := utils.Encrypt(ctx, auth.EncryptionKey(), newKey())
if err != nil {
return "", fmt.Errorf("encrypting API v1 key: %w", err)
}
@ -71,7 +69,7 @@ func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
if err != nil {
return "", fmt.Errorf("reading API v1 key: %w", err)
}
if key, err := utils.Decrypt(ctx, encryptionKey(), stored); err == nil {
if key, err := utils.Decrypt(ctx, auth.EncryptionKey(), stored); err == nil {
return key, nil
}
// A changed PasswordEncryptionKey makes the old key unreadable; replacing it only ends current access tokens.
@ -82,7 +80,7 @@ func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
if err != nil {
return err
}
if k, err := utils.Decrypt(ctx, encryptionKey(), current); err == nil {
if k, err := utils.Decrypt(ctx, auth.EncryptionKey(), current); err == nil {
key = k
return nil
}
@ -90,7 +88,7 @@ func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
if err := tx.Property().Put(ctx, consts.JWTAPIv1SecretKey, enc); err != nil {
return err
}
key, err = utils.Decrypt(ctx, encryptionKey(), enc)
key, err = utils.Decrypt(ctx, auth.EncryptionKey(), enc)
return err
})
if err != nil {
@ -106,11 +104,6 @@ func newKey() string {
return hex.EncodeToString(b)
}
func encryptionKey() []byte {
sum := sha256.Sum256([]byte(cmp.Or(conf.Server.PasswordEncryptionKey, consts.DefaultEncryptionKey)))
return sum[:]
}
func (s *signer) sign(c claims) (string, error) {
_, tok, err := s.auth.Encode(map[string]any{
jwt.SubjectKey: c.UserID,

3
core/apiauth/signer_test.go
View file

@ -8,6 +8,7 @@ import (
"github.com/lestrrat-go/jwx/v3/jwt"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils"
. "github.com/onsi/ginkgo/v2"
@ -34,7 +35,7 @@ var _ = Describe("signer", func() {
})
It("keeps using a stored key created in the older format", func() {
enc, err := utils.Encrypt(ctx, encryptionKey(), "legacy22charskeyABCDEF")
enc, err := utils.Encrypt(ctx, auth.EncryptionKey(), "legacy22charskeyABCDEF")
Expect(err).ToNot(HaveOccurred())
Expect(realDS.Property().Put(ctx, consts.JWTAPIv1SecretKey, enc)).To(Succeed())
Expect(loadKey(ctx, realDS)).To(Equal("legacy22charskeyABCDEF"))

6
core/auth/auth.go
View file

@ -53,7 +53,7 @@ func loadOrCreateSecret(ctx context.Context, ds model.DataStore, key string) str
log.Info(ctx, "Creating new JWT secret", "key", key)
return createNewSecret(ctx, ds, key)
}
if secret, err = utils.Decrypt(ctx, getEncKey(), secret); err != nil {
if secret, err = utils.Decrypt(ctx, EncryptionKey(), secret); err != nil {
log.Error(ctx, "Could not decrypt JWT secret, creating a new one", "key", key, err)
return createNewSecret(ctx, ds, key)
}
@ -171,7 +171,7 @@ func WithAdminUser(ctx context.Context, ds model.DataStore) context.Context {
func createNewSecret(ctx context.Context, ds model.DataStore, key string) string {
secret := id.NewRandom()
encSecret, err := utils.Encrypt(ctx, getEncKey(), secret)
encSecret, err := utils.Encrypt(ctx, EncryptionKey(), secret)
if err != nil {
log.Error(ctx, "Could not encrypt JWT secret", err)
return secret
@ -195,7 +195,7 @@ func DecodeAndVerifyToken(tokenStr string) (jwt.Token, error) {
return jwtauth.VerifyToken(TokenAuth, tokenStr)
}
func getEncKey() []byte {
func EncryptionKey() []byte {
key := cmp.Or(
conf.Server.PasswordEncryptionKey,
consts.DefaultEncryptionKey,

7
model/grant.go
View file

@ -23,6 +23,13 @@ type Grant struct {
LastUsedIP string `structs:"last_used_ip" json:"lastUsedIp"`
}
func (g Grant) LastActivity() time.Time {
if g.LastUsedAt != nil {
return *g.LastUsedAt
}
return g.CreatedAt
}
type Grants []Grant
// Scopes is stored as a single space-separated column.

24
model/grant_test.go Normal file
View file

@ -0,0 +1,24 @@
package model_test
import (
"time"
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("Grant", func() {
Describe("LastActivity", func() {
created := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
It("is the creation time for a grant never used", func() {
Expect(model.Grant{CreatedAt: created}.LastActivity()).To(Equal(created))
})
It("is the last use once the grant was used", func() {
used := created.Add(time.Hour)
Expect(model.Grant{CreatedAt: created, LastUsedAt: &used}.LastActivity()).To(Equal(used))
})
})
})

9
persistence/grant_repository.go
View file

@ -61,12 +61,12 @@ func (r *grantRepository) findOne(ctx context.Context, cond Sqlizer) (*model.Gra
}
// activeForUser skips grants left on an older epoch: they are dead but only deleted when presented.
func (r *grantRepository) activeForUser(userID string, epoch int, idleSince time.Time) Sqlizer {
func activeForUser(userID string, epoch int, idleSince time.Time) Sqlizer {
return And{Eq{"user_id": userID, "user_epoch": epoch}, Expr(grantLastActivity+" >= ?", idleSince.UTC())}
}
func (r *grantRepository) GetAllForUser(ctx context.Context, userID string, epoch int, idleSince time.Time, offset, limit int) (model.Grants, error) {
sel := r.newSelect(ctx).Columns("*").Where(r.activeForUser(userID, epoch, idleSince)).
sel := r.newSelect(ctx).Columns("*").Where(activeForUser(userID, epoch, idleSince)).
OrderBy("last_used_at IS NULL", "last_used_at desc", "created_at desc", "id").
Offset(uint64(offset)).Limit(uint64(limit))
var res model.Grants
@ -75,7 +75,7 @@ func (r *grantRepository) GetAllForUser(ctx context.Context, userID string, epoc
}
func (r *grantRepository) CountForUser(ctx context.Context, userID string, epoch int, idleSince time.Time) (int64, error) {
return r.count(ctx, Select().Where(r.activeForUser(userID, epoch, idleSince)))
return r.count(ctx, Select().Where(activeForUser(userID, epoch, idleSince)))
}
func (r *grantRepository) Delete(ctx context.Context, id string) error {
@ -94,8 +94,7 @@ func (r *grantRepository) DeleteForUser(ctx context.Context, userID, id string)
}
func (r *grantRepository) DeleteOtherEpochs(ctx context.Context, userID string, epoch int) error {
_, err := r.executeSQL(ctx, Delete(r.tableName).Where(And{Eq{"user_id": userID}, NotEq{"user_epoch": epoch}}))
return err
return r.delete(ctx, And{Eq{"user_id": userID}, NotEq{"user_epoch": epoch}})
}
// SetEpoch only moves grants still on fromEpoch, so grants killed by an earlier change never come back.

3
persistence/persistence.go
View file

@ -7,6 +7,7 @@ import (
"sync"
"time"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/db"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
@ -278,7 +279,7 @@ func (s *SQLStore) GC(ctx context.Context, libraryIDs ...int) error {
trace(ctx, "purge non used tags", func() error { return s.tag().(*tagRepository).purgeUnused(ctx) }),
trace(ctx, "remove orphan playlist tracks", func() error { return s.playlist().(*playlistRepository).removeOrphans(ctx) }),
trace(ctx, "purge idle API grants", func() error {
_, err := s.grant().DeleteIdle(ctx, time.Now().Add(-90*24*time.Hour))
_, err := s.grant().DeleteIdle(ctx, time.Now().Add(-consts.APIv1GrantIdleExpiry))
return err
}),
)

12
server/apiv1/api.go
View file

@ -10,6 +10,7 @@ import (
"github.com/getkin/kin-openapi/openapi3"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
"github.com/navidrome/navidrome/api"
"github.com/navidrome/navidrome/core/apiauth"
"github.com/navidrome/navidrome/log"
@ -37,15 +38,6 @@ var gateRulesV1 = gateRules{
noStore: map[string]bool{"login": true, "setupFirstAdmin": true, "createAccessToken": true},
}
func limitBody(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Body != nil {
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
}
next.ServeHTTP(w, r)
})
}
func (rt *Router) routes() http.Handler {
r := chi.NewRouter()
doc, err := openapi3.NewLoader().LoadFromData(api.SpecJSON())
@ -56,7 +48,7 @@ func (rt *Router) routes() http.Handler {
if err != nil {
log.Fatal("API v1: the embedded OpenAPI spec breaks the security rules", err)
}
r.Use(referenceIDMiddleware, problemRecoverer, headAsGet(r), limitBody, g.handler)
r.Use(referenceIDMiddleware, problemRecoverer, headAsGet(r), middleware.RequestSize(maxBodyBytes), g.handler)
r.NotFound(func(w http.ResponseWriter, req *http.Request) {
writeProblemStatus(w, req, http.StatusNotFound, ProblemCodeNotFound, "no such endpoint")
})

5
server/apiv1/auth_handlers.go
View file

@ -5,6 +5,7 @@ import (
"context"
"github.com/navidrome/navidrome/core/apiauth"
"github.com/navidrome/navidrome/utils/gg"
)
const defaultPageSize = 100
@ -35,8 +36,8 @@ func (rt *Router) ListGrants(ctx context.Context, req ListGrantsRequestObject) (
if err != nil {
return nil, err
}
offset := deref(req.Params.OffsetParam)
limit := cmp.Or(deref(req.Params.LimitParam), defaultPageSize)
offset := gg.V(req.Params.OffsetParam)
limit := cmp.Or(gg.V(req.Params.LimitParam), defaultPageSize)
grants, total, err := rt.auth.ListGrants(ctx, p, offset, limit)
if err != nil {
return nil, err

18
server/apiv1/dto.go
View file

@ -3,14 +3,12 @@ package apiv1
import (
"github.com/navidrome/navidrome/core/apiauth"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/gg"
"github.com/navidrome/navidrome/utils/slice"
)
func toScopes(in []string) []Scope {
out := make([]Scope, len(in))
for i, s := range in {
out[i] = Scope(s)
}
return out
return slice.Map(in, func(s string) Scope { return Scope(s) })
}
// fromScopeRequests keeps nil (all scopes) apart from an empty list (no scopes).
@ -58,7 +56,7 @@ func toGrantCreated(i *apiauth.Issued) GrantCreated {
}
func clientMeta(c CredentialsRequest) apiauth.ClientMeta {
return apiauth.ClientMeta{Client: c.Client, Name: deref(c.Name), ClientVersion: deref(c.ClientVersion)}
return apiauth.ClientMeta{Client: c.Client, Name: gg.V(c.Name), ClientVersion: gg.V(c.ClientVersion)}
}
// principal fails closed if the gate did not attach a principal to the context.
@ -68,11 +66,3 @@ func principal(p *apiauth.Principal, ok bool) (*apiauth.Principal, error) {
}
return p, nil
}
func deref[T any](p *T) T {
var zero T
if p == nil {
return zero
}
return *p
}

3
server/apiv1/gate.go
View file

@ -81,8 +81,7 @@ func newGate(doc *openapi3.T, mux chi.Routes, auth authenticator, rules gateRule
}
}
if conf.Server.AuthRequestLimit > 0 {
g.limiter = httprate.LimitBy(conf.Server.AuthRequestLimit, conf.Server.AuthWindowLength,
func(r *http.Request) (string, error) { return server.ClientIP(r), nil },
g.limiter = server.ClientIPRateLimiter(conf.Server.AuthRequestLimit, conf.Server.AuthWindowLength,
httprate.WithLimitHandler(func(w http.ResponseWriter, r *http.Request) {
writeProblemStatus(w, r, http.StatusTooManyRequests, ProblemCodeRateLimited, "too many requests")
}))

4
server/middlewares.go
View file

@ -234,10 +234,10 @@ func trustedProxyPrefixes(list string) []string {
// ClientIPRateLimiter returns a rate limiter keyed by ClientIP, so spoofed forwarding headers
// cannot be rotated for a fresh bucket.
func ClientIPRateLimiter(requestLimit int, windowLength time.Duration) func(http.Handler) http.Handler {
func ClientIPRateLimiter(requestLimit int, windowLength time.Duration, opts ...httprate.Option) func(http.Handler) http.Handler {
return httprate.LimitBy(requestLimit, windowLength, func(r *http.Request) (string, error) {
return ClientIP(r), nil
})
}, opts...)
}
// ClientIP returns the canonical client IP resolved by realIPMiddleware, for keying rate limits. The