refactor(api): tighten API v1 auth internals and first-admin setup

Load the signer lock-free once cached, read the signing key before
generating one, cap the liveness cache at its limit, share one
grantable-scope predicate, and let CreateFirstAdmin open its own locked
transaction with an optional in-transaction follow-up.

core/apiauth/cache.go

@@ -55,6 +55,13 @@ func (c *livenessCache) put(id string, e livenessEntry, now time.Time, started u
 	if len(c.entries) >= maxLivenessEntries {
 		maps.DeleteFunc(c.entries, func(_ string, v livenessEntry) bool { return !now.Before(v.expires) })
 	}
+	if _, refresh := c.entries[id]; !refresh && len(c.entries) >= maxLivenessEntries {
+		// Dropping an arbitrary live entry only costs that grant one extra DB read.
+		for k := range c.entries {
+			delete(c.entries, k)
+			break
+		}
+	}
 	if len(c.evicted) >= maxLivenessEntries {
 		clear(c.evicted)
 		c.floor = c.gen

core/apiauth/cache_test.go

@@ -81,4 +81,17 @@ var _ = Describe("livenessCache", func() {
 		c.put("fresh", livenessEntry{}, t0.Add(time.Minute), c.begin())
 		Expect(c.len()).To(Equal(1))
 	})
+
+	It("never grows past its cap, even when every entry is live", func() {
+		for i := range maxLivenessEntries {
+			c.put(fmt.Sprint(i), livenessEntry{}, t0, c.begin())
+		}
+		c.put("fresh", livenessEntry{}, t0, c.begin())
+		Expect(c.len()).To(Equal(maxLivenessEntries))
+		_, ok := c.get("fresh", t0)
+		Expect(ok).To(BeTrue())
+
+		c.put("fresh", livenessEntry{userID: "u1"}, t0, c.begin())
+		Expect(c.len()).To(Equal(maxLivenessEntries))
+	})
 })

core/apiauth/scopes.go

@@ -19,6 +19,10 @@ func known(s string) bool {
 	return slices.Contains(KnownScopes, s)
 }
 
+func grantable(s string, isAdmin bool) bool {
+	return known(s) && (s != ScopeAdmin || isAdmin)
+}
+
 func normalize(in []string) []string {
 	out := slices.Clone(in)
 	slices.Sort(out)
@@ -32,9 +36,7 @@ func Entitled(requested []string, isAdmin bool) []string {
 	}
 	var out []string
 	for _, s := range requested {
-		switch {
-		case s == ScopeAdmin && !isAdmin:
-		case s == ScopeAll || known(s):
+		if s == ScopeAll || grantable(s, isAdmin) {
 			out = append(out, s)
 		}
 	}
@@ -56,9 +58,7 @@ func Expand(granted []string, isAdmin bool) []string {
 
 // 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)
-	})
+	out := slices.DeleteFunc(slices.Clone(scopes), func(s string) bool { return !grantable(s, isAdmin) })
 	return normalize(out)
 }
 

core/apiauth/service.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"slices"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/navidrome/navidrome/conf"
@@ -60,7 +61,7 @@ type Service struct {
 	cache    *livenessCache
 	now      func() time.Time
 	signerMu sync.Mutex
-	sg       *signer
+	sg       atomic.Pointer[signer]
 }
 
 func New(ds model.DataStore) *Service {
@@ -77,16 +78,19 @@ func New(ds model.DataStore) *Service {
 
 // signer loads the key on first use, so building the router never touches the database; only a success is kept.
 func (s *Service) signer() (*signer, error) {
+	if sg := s.sg.Load(); sg != nil {
+		return sg, nil
+	}
 	s.signerMu.Lock()
 	defer s.signerMu.Unlock()
-	if s.sg != nil {
-		return s.sg, nil
+	if sg := s.sg.Load(); sg != nil {
+		return sg, nil
 	}
 	sg, err := loadSigner(context.Background(), s.ds, func() time.Time { return s.now() })
 	if err != nil {
 		return nil, err
 	}
-	s.sg = sg
+	s.sg.Store(sg)
 	return sg, nil
 }
 
@@ -111,15 +115,15 @@ func (s *Service) Login(ctx context.Context, username, password string, meta Cli
 
 func (s *Service) Setup(ctx context.Context, username, password string, meta ClientMeta, scopes []string) (*Issued, error) {
 	var issued *Issued
-	err := s.ds.WithTxImmediate(func(tx model.DataStore) error {
-		u, err := auth.CreateFirstAdmin(ctx, tx, username, password)
-		if err != nil {
-			return err
-		}
+	_, err := auth.CreateFirstAdmin(ctx, s.ds, username, password, func(tx model.DataStore, u *model.User) error {
+		var err error
 		issued, err = s.issue(ctx, tx, *u, "setup", meta, scopes)
 		return err
 	})
-	return issued, err
+	if err != nil {
+		return nil, err
+	}
+	return issued, nil
 }
 
 // issue stores a grant bound to the epoch read with the user, so a racing password change leaves it dead.

core/apiauth/service_test.go

@@ -30,12 +30,12 @@ type flakyProps struct {
 	ds *flakyPropsDS
 }
 
-func (p *flakyProps) PutIfAbsent(ctx context.Context, id, value string) error {
+func (p *flakyProps) Get(ctx context.Context, id string) (string, error) {
 	if p.ds.failures > 0 {
 		p.ds.failures--
-		return errFlakyProps
+		return "", errFlakyProps
 	}
-	return p.PropertyRepository.PutIfAbsent(ctx, id, value)
+	return p.PropertyRepository.Get(ctx, id)
 }
 
 var meta = ClientMeta{Name: "Living room", Client: "TestApp", ClientVersion: "1.0"}

core/apiauth/signer.go

@@ -58,14 +58,10 @@ 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, auth.EncryptionKey(), newKey())
-	if err != nil {
-		return "", fmt.Errorf("encrypting API v1 key: %w", err)
-	}
-	if err := ds.Property().PutIfAbsent(ctx, consts.JWTAPIv1SecretKey, enc); err != nil {
-		return "", fmt.Errorf("storing API v1 key: %w", err)
-	}
 	stored, err := ds.Property().Get(ctx, consts.JWTAPIv1SecretKey)
+	if errors.Is(err, model.ErrNotFound) {
+		stored, err = createKey(ctx, ds)
+	}
 	if err != nil {
 		return "", fmt.Errorf("reading API v1 key: %w", err)
 	}
@@ -85,11 +81,15 @@ func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
 			return nil
 		}
 		log.Warn(ctx, "Could not decrypt API v1 key, replacing it")
+		k, enc, err := newEncryptedKey(ctx)
+		if err != nil {
+			return err
+		}
 		if err := tx.Property().Put(ctx, consts.JWTAPIv1SecretKey, enc); err != nil {
 			return err
 		}
-		key, err = utils.Decrypt(ctx, auth.EncryptionKey(), enc)
-		return err
+		key = k
+		return nil
 	})
 	if err != nil {
 		return "", fmt.Errorf("replacing API v1 key: %w", err)
@@ -97,6 +97,26 @@ func loadKey(ctx context.Context, ds model.DataStore) (string, error) {
 	return key, nil
 }
 
+// createKey re-reads after the insert-if-absent, so nodes racing to create the key agree on the winner.
+func createKey(ctx context.Context, ds model.DataStore) (string, error) {
+	_, enc, err := newEncryptedKey(ctx)
+	if err != nil {
+		return "", err
+	}
+	if err := ds.Property().PutIfAbsent(ctx, consts.JWTAPIv1SecretKey, enc); err != nil {
+		return "", fmt.Errorf("storing API v1 key: %w", err)
+	}
+	return ds.Property().Get(ctx, consts.JWTAPIv1SecretKey)
+}
+
+func newEncryptedKey(ctx context.Context) (key, enc string, err error) {
+	key = newKey()
+	if enc, err = utils.Encrypt(ctx, auth.EncryptionKey(), key); err != nil {
+		return "", "", fmt.Errorf("encrypting API v1 key: %w", err)
+	}
+	return key, enc, nil
+}
+
 // newKey returns 256 random bits, the minimum RFC 7518 asks of an HS256 key.
 func newKey() string {
 	b := make([]byte, 32)

core/auth/first_admin.go

@@ -15,26 +15,40 @@ import (
 
 var ErrSetupComplete = errors.New("setup already complete")
 
-// CreateFirstAdmin must run inside ds.WithTxImmediate, so the count and the insert cannot interleave.
-func CreateFirstAdmin(ctx context.Context, tx model.DataStore, username, password string) (*model.User, error) {
-	count, err := tx.User().CountAll(ctx)
+// CreateFirstAdmin counts and inserts in one locked transaction, so racing setups cannot both win.
+// then, if not nil, runs in that same transaction with the new user.
+func CreateFirstAdmin(ctx context.Context, ds model.DataStore, username, password string, then func(tx model.DataStore, u *model.User) error) (*model.User, error) {
+	var created *model.User
+	err := ds.WithTxImmediate(func(tx model.DataStore) error {
+		count, err := tx.User().CountAll(ctx)
+		if err != nil {
+			return fmt.Errorf("counting users: %w", err)
+		}
+		if count > 0 {
+			return ErrSetupComplete
+		}
+		log.Warn(ctx, "Creating initial user", "user", username)
+		u := model.User{
+			ID:          id.NewRandom(),
+			UserName:    username,
+			Name:        cases.Title(language.Und).String(username),
+			NewPassword: password,
+			IsAdmin:     true,
+			LastLoginAt: new(time.Now()),
+		}
+		if err := tx.User().Put(ctx, &u); err != nil {
+			return fmt.Errorf("creating initial user: %w", err)
+		}
+		if created, err = tx.User().Get(ctx, u.ID); err != nil {
+			return err
+		}
+		if then != nil {
+			return then(tx, created)
+		}
+		return nil
+	})
 	if err != nil {
-		return nil, fmt.Errorf("counting users: %w", err)
+		return nil, err
 	}
-	if count > 0 {
-		return nil, ErrSetupComplete
-	}
-	log.Warn(ctx, "Creating initial user", "user", username)
-	u := model.User{
-		ID:          id.NewRandom(),
-		UserName:    username,
-		Name:        cases.Title(language.Und).String(username),
-		NewPassword: password,
-		IsAdmin:     true,
-		LastLoginAt: new(time.Now()),
-	}
-	if err := tx.User().Put(ctx, &u); err != nil {
-		return nil, fmt.Errorf("creating initial user: %w", err)
-	}
-	return tx.User().Get(ctx, u.ID)
+	return created, nil
 }

core/auth/first_admin_test.go

@@ -2,6 +2,7 @@ package auth_test
 
 import (
 	"context"
+	"errors"
 	"path/filepath"
 	"sync"
 	"time"
@@ -33,17 +34,8 @@ var _ = Describe("CreateFirstAdmin", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
-	createWith := func(name string, wrap func(model.DataStore) model.DataStore) (*model.User, error) {
-		var u *model.User
-		err := ds.WithTxImmediate(func(tx model.DataStore) error {
-			var err error
-			u, err = auth.CreateFirstAdmin(ctx, wrap(tx), name, "secret")
-			return err
-		})
-		return u, err
-	}
 	create := func(name string) (*model.User, error) {
-		return createWith(name, func(tx model.DataStore) model.DataStore { return tx })
+		return auth.CreateFirstAdmin(ctx, ds, name, "secret", nil)
 	}
 
 	It("creates an admin with a title-cased name and returns it with its id", func() {
@@ -65,6 +57,19 @@ var _ = Describe("CreateFirstAdmin", Ordered, func() {
 		Expect(err).To(MatchError(auth.ErrSetupComplete))
 	})
 
+	It("runs then in the same transaction, rolling the user back when it fails", func() {
+		boom := errors.New("boom")
+		var seen string
+		_, err := auth.CreateFirstAdmin(ctx, ds, "john", "secret", func(tx model.DataStore, u *model.User) error {
+			seen = u.ID
+			Expect(tx.User().CountAll(ctx)).To(Equal(int64(1)))
+			return boom
+		})
+		Expect(err).To(MatchError(boom))
+		Expect(seen).ToNot(BeEmpty())
+		Expect(ds.User().CountAll(ctx)).To(BeZero())
+	})
+
 	It("lets exactly one of two concurrent setups win", func() {
 		var wg sync.WaitGroup
 		errs := make([]error, 2)
@@ -73,7 +78,7 @@ var _ = Describe("CreateFirstAdmin", Ordered, func() {
 			go func() {
 				defer GinkgoRecover()
 				defer wg.Done()
-				_, errs[i] = createWith(name, func(tx model.DataStore) model.DataStore { return slowCountDS{tx} })
+				_, errs[i] = auth.CreateFirstAdmin(ctx, slowCountDS{ds}, name, "secret", nil)
 			}()
 		}
 		wg.Wait()
@@ -87,6 +92,10 @@ type slowCountDS struct{ model.DataStore }
 
 func (d slowCountDS) User() model.UserRepository { return slowCountUsers{d.DataStore.User()} }
 
+func (d slowCountDS) WithTxImmediate(block func(tx model.DataStore) error, scope ...string) error {
+	return d.DataStore.WithTxImmediate(func(tx model.DataStore) error { return block(slowCountDS{tx}) }, scope...)
+}
+
 type slowCountUsers struct{ model.UserRepository }
 
 // Holds the transaction open after counting, so an unlocked count would interleave with the other racer.

server/apiv1/auth_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/core/auth"
-	"github.com/navidrome/navidrome/model"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -82,10 +81,7 @@ var _ = Describe("auth endpoints", func() {
 		go func() {
 			defer GinkgoRecover()
 			defer wg.Done()
-			v0Err = realDS.WithTxImmediate(func(tx model.DataStore) error { // what v0 /auth/createAdmin runs
-				_, err := auth.CreateFirstAdmin(ctx, tx, "v0admin", "pw")
-				return err
-			})
+			_, v0Err = auth.CreateFirstAdmin(ctx, realDS, "v0admin", "pw", nil) // what v0 /auth/createAdmin runs
 		}()
 		wg.Wait()
 		Expect(realDS.User().CountAll(ctx)).To(Equal(int64(1)))

server/auth.go

@@ -124,10 +124,7 @@ func createAdmin(ds model.DataStore) func(w http.ResponseWriter, r *http.Request
 			_ = rest.RespondWithError(w, http.StatusUnprocessableEntity, err.Error())
 			return
 		}
-		err = ds.WithTxImmediate(func(tx model.DataStore) error {
-			_, err := auth.CreateFirstAdmin(r.Context(), tx, username, password)
-			return err
-		})
+		_, err = auth.CreateFirstAdmin(r.Context(), ds, username, password, nil)
 		if errors.Is(err, auth.ErrSetupComplete) {
 			_ = rest.RespondWithError(w, http.StatusForbidden, "Cannot create another first admin")
 			return

server/auth_test.go

@@ -77,10 +77,7 @@ var _ = Describe("Auth", func() {
 		Describe("CreateFirstAdmin", func() {
 			It("returns the error when the user cannot be saved", func() {
 				failing := dsWithFailingPut(errors.New("db is down"))
-				err := failing.WithTxImmediate(func(tx model.DataStore) error {
-					_, err := auth.CreateFirstAdmin(ctx, tx, "johndoe", "secret")
-					return err
-				})
+				_, err := auth.CreateFirstAdmin(ctx, failing, "johndoe", "secret", nil)
 				Expect(err).To(MatchError(ContainSubstring("db is down")))
 			})
 		})