From 158721ea604cac255a7724d2084ab753b77f90e1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 26 Sep 2026 01:01:31 -0400 Subject: [PATCH] fix(server): create the first admin inside one locked transaction --- core/auth/auth_test.go | 1 + core/auth/first_admin.go | 40 +++++++++++++++ core/auth/first_admin_test.go | 97 +++++++++++++++++++++++++++++++++++ server/auth.go | 35 ++----------- server/auth_test.go | 22 ++++++-- 5 files changed, 162 insertions(+), 33 deletions(-) create mode 100644 core/auth/first_admin.go create mode 100644 core/auth/first_admin_test.go diff --git a/core/auth/auth_test.go b/core/auth/auth_test.go index c86dcd08c..05da7ec65 100644 --- a/core/auth/auth_test.go +++ b/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") diff --git a/core/auth/first_admin.go b/core/auth/first_admin.go new file mode 100644 index 000000000..319b325eb --- /dev/null +++ b/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) +} diff --git a/core/auth/first_admin_test.go b/core/auth/first_admin_test.go new file mode 100644 index 000000000..d22cea2f2 --- /dev/null +++ b/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 +} diff --git a/server/auth.go b/server/auth.go index 3e58359da..dc70c5048 100644 --- a/server/auth.go +++ b/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) { diff --git a/server/auth_test.go b/server/auth_test.go index 1095fafc9..e016de5e1 100644 --- a/server/auth_test.go +++ b/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"))