diff --git a/consts/consts.go b/consts/consts.go index c97c72cab..0808bff79 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -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 diff --git a/core/apiauth/service.go b/core/apiauth/service.go index 5082017b8..0bd3a22bc 100644 --- a/core/apiauth/service.go +++ b/core/apiauth/service.go @@ -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 } diff --git a/core/apiauth/signer.go b/core/apiauth/signer.go index e4814d3c8..6290b7130 100644 --- a/core/apiauth/signer.go +++ b/core/apiauth/signer.go @@ -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, diff --git a/core/apiauth/signer_test.go b/core/apiauth/signer_test.go index b749d6193..7d3d622b9 100644 --- a/core/apiauth/signer_test.go +++ b/core/apiauth/signer_test.go @@ -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")) diff --git a/core/auth/auth.go b/core/auth/auth.go index 1bdc917da..43314a7c1 100644 --- a/core/auth/auth.go +++ b/core/auth/auth.go @@ -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, diff --git a/model/grant.go b/model/grant.go index f1475c93f..1e8fe46e1 100644 --- a/model/grant.go +++ b/model/grant.go @@ -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. diff --git a/model/grant_test.go b/model/grant_test.go new file mode 100644 index 000000000..dd59a2422 --- /dev/null +++ b/model/grant_test.go @@ -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)) + }) + }) +}) diff --git a/persistence/grant_repository.go b/persistence/grant_repository.go index fcd58bfa3..0e561315c 100644 --- a/persistence/grant_repository.go +++ b/persistence/grant_repository.go @@ -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. diff --git a/persistence/persistence.go b/persistence/persistence.go index 8feda0c8f..9656be178 100644 --- a/persistence/persistence.go +++ b/persistence/persistence.go @@ -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 }), ) diff --git a/server/apiv1/api.go b/server/apiv1/api.go index ac6831f81..53076063a 100644 --- a/server/apiv1/api.go +++ b/server/apiv1/api.go @@ -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") }) diff --git a/server/apiv1/auth_handlers.go b/server/apiv1/auth_handlers.go index 543b1e446..26eecd386 100644 --- a/server/apiv1/auth_handlers.go +++ b/server/apiv1/auth_handlers.go @@ -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 diff --git a/server/apiv1/dto.go b/server/apiv1/dto.go index 10ec338b1..9cab02595 100644 --- a/server/apiv1/dto.go +++ b/server/apiv1/dto.go @@ -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 -} diff --git a/server/apiv1/gate.go b/server/apiv1/gate.go index 1002de01e..527ec2bcc 100644 --- a/server/apiv1/gate.go +++ b/server/apiv1/gate.go @@ -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") })) diff --git a/server/middlewares.go b/server/middlewares.go index 3d8199187..149ddbfc3 100644 --- a/server/middlewares.go +++ b/server/middlewares.go @@ -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