diff --git a/internal/hub/api_test.go b/internal/hub/api_test.go index bbe3ff0c..a49ed676 100644 --- a/internal/hub/api_test.go +++ b/internal/hub/api_test.go @@ -6,12 +6,16 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" + "sort" "testing" + "time" beszelTests "github.com/henrygd/beszel/internal/tests" "github.com/henrygd/beszel/internal/migrations" "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/apis" "github.com/pocketbase/pocketbase/core" pbTests "github.com/pocketbase/pocketbase/tests" "github.com/stretchr/testify/require" @@ -26,6 +30,59 @@ func jsonReader(v any) io.Reader { return bytes.NewReader(data) } +type gatedReader struct { + data []byte + started chan struct{} + release chan struct{} + offset int +} + +func (r *gatedReader) Read(p []byte) (int, error) { + if r.offset == 0 { + close(r.started) + <-r.release + } + if r.offset >= len(r.data) { + return 0, io.EOF + } + n := copy(p, r.data[r.offset:]) + r.offset += n + return n, nil +} + +func firstUserTestMux(t *testing.T) (*beszelTests.TestHub, http.Handler) { + t.Helper() + hub, err := beszelTests.NewTestHub(t.TempDir()) + require.NoError(t, err) + _ = hub.StartHub() + + router, err := apis.NewRouter(hub.TestApp) + require.NoError(t, err) + serveEvent := &core.ServeEvent{App: hub.TestApp, Router: router} + + var handler http.Handler + err = hub.TestApp.OnServe().Trigger(serveEvent, func(e *core.ServeEvent) error { + var buildErr error + handler, buildErr = e.Router.BuildMux() + return buildErr + }) + require.NoError(t, err) + require.NotNil(t, handler) + return hub, handler +} + +func postFirstUser(handler http.Handler, email string) *httptest.ResponseRecorder { + body, _ := json.Marshal(map[string]string{ + "email": email, + "password": "password123", + }) + req := httptest.NewRequest(http.MethodPost, "/api/beszel/create-user", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, req) + return recorder +} + func TestApiRoutesAuthentication(t *testing.T) { hub, user := beszelTests.GetHubWithUser(t) defer hub.Cleanup() @@ -789,6 +846,87 @@ func TestFirstUserCreation(t *testing.T) { }) } +func TestFirstUserBootstrapAtomicity(t *testing.T) { + t.Run("concurrent complete requests produce exactly one winner", func(t *testing.T) { + hub, handler := firstUserTestMux(t) + defer hub.Cleanup() + + start := make(chan struct{}) + statuses := make(chan int, 2) + for _, email := range []string{"first@example.com", "second@example.com"} { + go func(email string) { + <-start + statuses <- postFirstUser(handler, email).Code + }(email) + } + close(start) + + got := []int{<-statuses, <-statuses} + sort.Ints(got) + require.Equal(t, []int{http.StatusOK, http.StatusForbidden}, got) + + users, err := hub.FindAllRecords("users") + require.NoError(t, err) + require.Len(t, users, 1) + superusers, err := hub.FindAllRecords(core.CollectionNameSuperusers) + require.NoError(t, err) + require.Len(t, superusers, 1) + require.NotEqual(t, migrations.TempAdminEmail, superusers[0].Email()) + }) + + t.Run("partial body cannot retain stale bootstrap authorization", func(t *testing.T) { + hub, handler := firstUserTestMux(t) + defer hub.Cleanup() + + body, err := json.Marshal(map[string]string{ + "email": "parked@example.com", + "password": "password123", + }) + require.NoError(t, err) + gated := &gatedReader{ + data: body, + started: make(chan struct{}), + release: make(chan struct{}), + } + parkedRequest := httptest.NewRequest(http.MethodPost, "/api/beszel/create-user", gated) + parkedRequest.Header.Set("Content-Type", "application/json") + parkedRecorder := httptest.NewRecorder() + parkedDone := make(chan struct{}) + go func() { + handler.ServeHTTP(parkedRecorder, parkedRequest) + close(parkedDone) + }() + + select { + case <-gated.started: + case <-time.After(2 * time.Second): + t.Fatal("parked request did not begin reading its body") + } + + operatorRecorder := postFirstUser(handler, "operator@example.com") + require.Equal(t, http.StatusOK, operatorRecorder.Code) + lateRecorder := postFirstUser(handler, "late@example.com") + require.Equal(t, http.StatusForbidden, lateRecorder.Code) + + close(gated.release) + select { + case <-parkedDone: + case <-time.After(2 * time.Second): + t.Fatal("parked request did not finish") + } + require.Equal(t, http.StatusForbidden, parkedRecorder.Code) + + users, err := hub.FindAllRecords("users") + require.NoError(t, err) + require.Len(t, users, 1) + require.Equal(t, "operator@example.com", users[0].Email()) + superusers, err := hub.FindAllRecords(core.CollectionNameSuperusers) + require.NoError(t, err) + require.Len(t, superusers, 1) + require.Equal(t, "operator@example.com", superusers[0].Email()) + }) +} + func TestCreateUserEndpointAvailability(t *testing.T) { t.Run("CreateUserEndpoint available when no users exist", func(t *testing.T) { hub, _ := beszelTests.NewTestHub(t.TempDir()) diff --git a/internal/users/users.go b/internal/users/users.go index 9807d0fe..6e091661 100644 --- a/internal/users/users.go +++ b/internal/users/users.go @@ -2,6 +2,7 @@ package users import ( + "errors" "log" "net/http" @@ -15,6 +16,8 @@ type UserManager struct { app core.App } +var errBootstrapUnavailable = errors.New("bootstrap unavailable") + func NewUserManager(app core.App) *UserManager { return &UserManager{ app: app, @@ -59,17 +62,7 @@ func (um *UserManager) InitializeUserSettings(e *core.RecordEvent) error { // Custom API endpoint to create the first user. // Mimics previous default behavior in PocketBase < 0.23.0 allowing user to be created through the Beszel UI. func (um *UserManager) CreateFirstUser(e *core.RequestEvent) error { - // check that there are no users - totalUsers, err := um.app.CountRecords("users") - if err != nil || totalUsers > 0 { - return e.JSON(http.StatusForbidden, map[string]string{"err": "Forbidden"}) - } - // check that there is only one superuser and the email matches the email of the superuser we set up in initial-settings.go - adminUsers, err := um.app.FindAllRecords(core.CollectionNameSuperusers) - if err != nil || len(adminUsers) != 1 || adminUsers[0].GetString("email") != migrations.TempAdminEmail { - return e.JSON(http.StatusForbidden, map[string]string{"err": "Forbidden"}) - } - // create first user using supplied email and password in request body + // Consume the complete body before evaluating the one-time bootstrap state. data := struct { Email string `json:"email"` Password string `json:"password"` @@ -81,26 +74,55 @@ func (um *UserManager) CreateFirstUser(e *core.RequestEvent) error { return e.JSON(http.StatusBadRequest, map[string]string{"err": "Bad request"}) } - collection, _ := um.app.FindCollectionByNameOrId("users") - user := core.NewRecord(collection) - user.SetEmail(data.Email) - user.SetPassword(data.Password) - user.Set("role", "admin") - user.Set("verified", true) - if err := um.app.Save(user); err != nil { - return e.JSON(http.StatusInternalServerError, map[string]string{"err": err.Error()}) + err := um.app.RunInTransaction(func(txApp core.App) error { + totalUsers, err := txApp.CountRecords("users") + if err != nil { + return err + } + if totalUsers > 0 { + return errBootstrapUnavailable + } + + adminUsers, err := txApp.FindAllRecords(core.CollectionNameSuperusers) + if err != nil { + return err + } + if len(adminUsers) != 1 || adminUsers[0].GetString("email") != migrations.TempAdminEmail { + return errBootstrapUnavailable + } + + collection, err := txApp.FindCollectionByNameOrId("users") + if err != nil { + return err + } + user := core.NewRecord(collection) + user.SetEmail(data.Email) + user.SetPassword(data.Password) + user.Set("role", "admin") + user.Set("verified", true) + if err := txApp.Save(user); err != nil { + return err + } + + collection, err = txApp.FindCollectionByNameOrId(core.CollectionNameSuperusers) + if err != nil { + return err + } + adminUser := core.NewRecord(collection) + adminUser.SetEmail(data.Email) + adminUser.SetPassword(data.Password) + if err := txApp.Save(adminUser); err != nil { + return err + } + + return txApp.Delete(adminUsers[0]) + }) + if errors.Is(err, errBootstrapUnavailable) { + return e.JSON(http.StatusForbidden, map[string]string{"err": "Forbidden"}) } - // create superuser using the email of the first user - collection, _ = um.app.FindCollectionByNameOrId(core.CollectionNameSuperusers) - adminUser := core.NewRecord(collection) - adminUser.SetEmail(data.Email) - adminUser.SetPassword(data.Password) - if err := um.app.Save(adminUser); err != nil { - return e.JSON(http.StatusInternalServerError, map[string]string{"err": err.Error()}) - } - // delete the intial superuser - if err := um.app.Delete(adminUsers[0]); err != nil { + if err != nil { return e.JSON(http.StatusInternalServerError, map[string]string{"err": err.Error()}) } + return e.JSON(http.StatusOK, map[string]string{"msg": "User created"}) } diff --git a/internal/users/users_test.go b/internal/users/users_test.go new file mode 100644 index 00000000..e84e84fb --- /dev/null +++ b/internal/users/users_test.go @@ -0,0 +1,117 @@ +//go:build testing + +package users_test + +import ( + "errors" + "io" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/henrygd/beszel/internal/migrations" + beszelTests "github.com/henrygd/beszel/internal/tests" + "github.com/henrygd/beszel/internal/users" + "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/tools/router" + "github.com/stretchr/testify/require" +) + +type blockedBody struct { + io.Reader + entered chan struct{} + resume chan struct{} +} + +func (b *blockedBody) Read(p []byte) (int, error) { + if b.entered != nil { + close(b.entered) + b.entered = nil + <-b.resume + } + return b.Reader.Read(p) +} + +func TestCreateFirstUserAtomic(t *testing.T) { + for _, scenario := range []string{"parked body", "concurrent requests", "rollback"} { + t.Run(scenario, func(t *testing.T) { + h, err := beszelTests.NewTestHub(t.TempDir()) + require.NoError(t, err) + defer h.Cleanup() + h.StartHub() + um := users.NewUserManager(h.App) + invoke := func(body io.Reader) int { + req := httptest.NewRequest("POST", "/api/beszel/create-user", body) + req.Header.Set("Content-Type", "application/json") + res := httptest.NewRecorder() + if err := um.CreateFirstUser(&core.RequestEvent{App: h.App, Event: router.Event{Request: req, Response: res}}); err != nil { + t.Error(err) + } + return res.Code + } + body := func(email string) io.Reader { + return strings.NewReader(`{"email":"` + email + `","password":"password12345"}`) + } + await := func(results <-chan int) int { + select { + case status := <-results: + return status + case <-time.After(10 * time.Second): + t.Fatal("request did not finish") + return 0 + } + } + switch scenario { + case "parked body": + entered, resume := make(chan struct{}), make(chan struct{}) + defer func() { + select { + case <-resume: + default: + close(resume) + } + }() + result := make(chan int, 1) + go func() { result <- invoke(&blockedBody{body("attacker@example.com"), entered, resume}) }() + select { + case <-entered: + case <-time.After(10 * time.Second): + t.Fatal("request did not reach body parsing") + } + require.Equal(t, 200, invoke(body("operator@example.com"))) + close(resume) + require.Equal(t, 403, await(result)) + case "concurrent requests": + start := make(chan struct{}) + results := make(chan int, 2) + for _, email := range []string{"one@example.com", "two@example.com"} { + go func() { <-start; results <- invoke(body(email)) }() + } + close(start) + require.ElementsMatch(t, []int{200, 403}, []int{await(results), await(results)}) + case "rollback": + hook := h.OnRecordCreate(core.CollectionNameSuperusers).BindFunc(func(e *core.RecordEvent) error { + return errors.New("injected superuser creation failure") + }) + require.Equal(t, 500, invoke(body("operator@example.com"))) + count, err := h.CountRecords("users") + require.NoError(t, err) + require.Zero(t, count) + admins, err := h.FindAllRecords(core.CollectionNameSuperusers) + require.NoError(t, err) + require.Len(t, admins, 1) + require.Equal(t, migrations.TempAdminEmail, admins[0].Email()) + h.OnRecordCreate(core.CollectionNameSuperusers).Unbind(hook) + require.Equal(t, 200, invoke(body("operator@example.com"))) + } + count, err := h.CountRecords("users") + require.NoError(t, err) + require.EqualValues(t, 1, count) + admins, err := h.FindAllRecords(core.CollectionNameSuperusers) + require.NoError(t, err) + require.Len(t, admins, 1) + require.NotEqual(t, migrations.TempAdminEmail, admins[0].Email()) + }) + } +}