fix(server): create the first admin inside one locked transaction

core/auth/auth_test.go

@@ -15,6 +15,7 @@ import (
 )
 
 func TestAuth(t *testing.T) {
+	tests.Init(t, false)
 	log.SetLevel(log.LevelFatal)
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Auth Test Suite")

core/auth/first_admin.go

@@ -0,0 +1,40 @@
+package auth
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"time"
+
+	"github.com/navidrome/navidrome/log"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/model/id"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
+)
+
+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)
+	if err != nil {
+		return nil, fmt.Errorf("counting users: %w", 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)
+}

core/auth/first_admin_test.go

@@ -0,0 +1,97 @@
+package auth_test
+
+import (
+	"context"
+	"path/filepath"
+	"sync"
+	"time"
+
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
+	"github.com/navidrome/navidrome/core/auth"
+	"github.com/navidrome/navidrome/db"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/persistence"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("CreateFirstAdmin", Ordered, func() {
+	var ctx context.Context
+	var ds model.DataStore
+
+	BeforeAll(func() {
+		DeferCleanup(configtest.SetupConfig())
+		conf.Server.DbPath = filepath.Join(GinkgoT().TempDir(), "first-admin.db") + "?_journal_mode=WAL&_foreign_keys=on&_busy_timeout=5000"
+		DeferCleanup(db.Init(GinkgoT().Context()))
+		ds = persistence.New(db.Db())
+	})
+
+	BeforeEach(func() {
+		ctx = GinkgoT().Context()
+		_, err := db.Db().ExecContext(ctx, "delete from user")
+		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 })
+	}
+
+	It("creates an admin with a title-cased name and returns it with its id", func() {
+		u, err := create("john")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(u.ID).ToNot(BeEmpty())
+		Expect(u.IsAdmin).To(BeTrue())
+		Expect(u.Name).To(Equal("John"))
+
+		stored, err := ds.User().FindByUsernameWithPassword(ctx, "john")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(stored.Password).To(Equal("secret"))
+	})
+
+	It("refuses once any user exists", func() {
+		_, err := create("first")
+		Expect(err).ToNot(HaveOccurred())
+		_, err = create("second")
+		Expect(err).To(MatchError(auth.ErrSetupComplete))
+	})
+
+	It("lets exactly one of two concurrent setups win", func() {
+		var wg sync.WaitGroup
+		errs := make([]error, 2)
+		for i, name := range []string{"racer-a", "racer-b"} {
+			wg.Add(1)
+			go func() {
+				defer GinkgoRecover()
+				defer wg.Done()
+				_, errs[i] = createWith(name, func(tx model.DataStore) model.DataStore { return slowCountDS{tx} })
+			}()
+		}
+		wg.Wait()
+		Expect(errs).To(ContainElement(BeNil()))
+		Expect(errs).To(ContainElement(MatchError(auth.ErrSetupComplete)))
+		Expect(ds.User().CountAll(ctx)).To(Equal(int64(1)))
+	})
+})
+
+type slowCountDS struct{ model.DataStore }
+
+func (d slowCountDS) User() model.UserRepository { return slowCountUsers{d.DataStore.User()} }
+
+type slowCountUsers struct{ model.UserRepository }
+
+// Holds the transaction open after counting, so an unlocked count would interleave with the other racer.
+func (u slowCountUsers) CountAll(ctx context.Context, opts ...model.QueryOptions) (int64, error) {
+	n, err := u.UserRepository.CountAll(ctx, opts...)
+	time.Sleep(50 * time.Millisecond)
+	return n, err
+}

server/auth.go

@@ -13,7 +13,6 @@ import (
 	"slices"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/deluan/rest"
 	"github.com/go-chi/jwtauth/v5"
@@ -26,8 +25,6 @@ import (
 	"github.com/navidrome/navidrome/model/id"
 	"github.com/navidrome/navidrome/model/request"
 	"github.com/navidrome/navidrome/utils/gravatar"
-	"golang.org/x/text/cases"
-	"golang.org/x/text/language"
 )
 
 var (
@@ -127,16 +124,14 @@ func createAdmin(ds model.DataStore) func(w http.ResponseWriter, r *http.Request
 			_ = rest.RespondWithError(w, http.StatusUnprocessableEntity, err.Error())
 			return
 		}
-		c, err := ds.User().CountAll(r.Context())
-		if err != nil {
-			_ = rest.RespondWithError(w, http.StatusInternalServerError, err.Error())
-			return
-		}
-		if c > 0 {
+		err = ds.WithTxImmediate(func(tx model.DataStore) error {
+			_, err := auth.CreateFirstAdmin(r.Context(), tx, username, password)
+			return err
+		})
+		if errors.Is(err, auth.ErrSetupComplete) {
 			_ = rest.RespondWithError(w, http.StatusForbidden, "Cannot create another first admin")
 			return
 		}
-		err = createAdminUser(r.Context(), ds, username, password)
 		if err != nil {
 			_ = rest.RespondWithError(w, http.StatusInternalServerError, err.Error())
 			return
@@ -145,26 +140,6 @@ func createAdmin(ds model.DataStore) func(w http.ResponseWriter, r *http.Request
 	}
 }
 
-func createAdminUser(ctx context.Context, ds model.DataStore, username, password string) error {
-	log.Warn(ctx, "Creating initial user", "user", username)
-	caser := cases.Title(language.Und)
-	initialUser := model.User{
-		ID:          id.NewRandom(),
-		UserName:    username,
-		Name:        caser.String(username),
-		Email:       "",
-		NewPassword: password,
-		IsAdmin:     true,
-		LastLoginAt: new(time.Now()),
-	}
-	err := ds.User().Put(ctx, &initialUser)
-	if err != nil {
-		log.Error(ctx, "Could not create initial user", "user", initialUser.UserName, err)
-		return fmt.Errorf("creating initial user: %w", err)
-	}
-	return nil
-}
-
 func validateLogin(ctx context.Context, userRepo model.UserRepository, userName, password string) (*model.User, error) {
 	u, err := userRepo.FindByUsernameWithPassword(ctx, userName)
 	if errors.Is(err, model.ErrNotFound) {

server/auth_test.go

@@ -74,14 +74,30 @@ var _ = Describe("Auth", func() {
 			})
 		})
 
-		Describe("createAdminUser", func() {
+		Describe("CreateFirstAdmin", func() {
 			It("returns the error when the user cannot be saved", func() {
-				ds = &tests.MockDataStore{MockedUser: &tests.MockedUserRepo{Error: errors.New("db is down")}}
-				err := createAdminUser(context.Background(), ds, "johndoe", "secret")
+				failing := dsWithFailingPut(errors.New("db is down"))
+				err := failing.WithTxImmediate(func(tx model.DataStore) error {
+					_, err := auth.CreateFirstAdmin(ctx, tx, "johndoe", "secret")
+					return err
+				})
 				Expect(err).To(MatchError(ContainSubstring("db is down")))
 			})
 		})
 
+		Describe("createAdmin when a user already exists", func() {
+			It("responds 403", func() {
+				req = httptest.NewRequest("POST", "/createAdmin", strings.NewReader(`{"username":"another", "password":"secret"}`))
+				resp = httptest.NewRecorder()
+				Expect(ds.User().Put(ctx, &model.User{UserName: "johndoe", NewPassword: "secret"})).To(Succeed())
+
+				createAdmin(ds)(resp, req)
+
+				Expect(resp.Code).To(Equal(http.StatusForbidden))
+				Expect(resp.Body.String()).To(ContainSubstring("Cannot create another first admin"))
+			})
+		})
+
 		Describe("createAdmin when the user cannot be stored", func() {
 			It("responds 500 rather than falling through to login", func() {
 				failing := dsWithFailingPut(errors.New("db is down"))