diff --git a/backend/internal/usersignup/service.go b/backend/internal/usersignup/service.go index 0a26f6b3..1d2ffed9 100644 --- a/backend/internal/usersignup/service.go +++ b/backend/internal/usersignup/service.go @@ -148,6 +148,13 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, config *appconfig.AppC tx.Rollback() }() + // We lock the users table to prevent concurrent initial admin setups from racing to create the first user + // This is only necessary for Postgres, since SQLite serializes all writes anyway + if err := lockInitialAdminSetup(ctx, tx); err != nil { + return model.User{}, "", err + } + + // Reject setup when a committed user already exists setupCompleted, err := s.isInitialAdminSetupCompleted(ctx, tx) if err != nil { return model.User{}, "", err @@ -156,6 +163,7 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, config *appconfig.AppC return model.User{}, "", &common.SetupNotAvailableError{} } + // Build the first user with administrator privileges userToCreate := dto.UserCreateDto{ FirstName: signUpData.FirstName, LastName: signUpData.LastName, @@ -170,6 +178,7 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, config *appconfig.AppC return model.User{}, "", err } + // Issue the setup session before committing so failures roll back the transaction token, err := s.signer.GenerateAccessToken(user, authenticationMethodOneTimePassword, config.SessionDuration.AsDurationMinutes()) if err != nil { return model.User{}, "", err @@ -183,6 +192,18 @@ func (s *Service) SignUpInitialAdmin(ctx context.Context, config *appconfig.AppC return user, token, nil } +func lockInitialAdminSetup(ctx context.Context, tx *gorm.DB) error { + if tx.Name() != "postgres" { + return nil + } + + if err := tx.WithContext(ctx).Exec("LOCK TABLE users IN SHARE ROW EXCLUSIVE MODE").Error; err != nil { + return fmt.Errorf("failed to lock users table for initial admin setup: %w", err) + } + + return nil +} + func (s *Service) IsInitialAdminSetupCompleted(ctx context.Context) (bool, error) { return s.isInitialAdminSetupCompleted(ctx, s.db) } diff --git a/backend/internal/usersignup/service_test.go b/backend/internal/usersignup/service_test.go index bc3a424b..3af38502 100644 --- a/backend/internal/usersignup/service_test.go +++ b/backend/internal/usersignup/service_test.go @@ -118,6 +118,46 @@ func TestSignUpRejectsInvalidToken(t *testing.T) { require.ErrorAs(t, err, &invalidErr) } +func TestSignUpInitialAdminCreatesAdmin(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + svc := newSignupServiceForTest(t, db, fakeUserCreator{user: model.User{Base: model.Base{ID: "new-admin"}}}) + config := appconfig.NewTestConfig(nil) + + // Complete setup and return the generated administrator session + user, accessToken, err := svc.SignUpInitialAdmin(t.Context(), config, signUpDto{Username: "new-admin"}) + require.NoError(t, err) + require.Equal(t, "new-admin", user.ID) + require.Equal(t, "access-token", accessToken) +} + +func TestSignUpInitialAdminAllowsRetryAfterFailure(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + boom := errors.New("could not create initial admin") + svc := newSignupServiceForTest(t, db, fakeUserCreator{err: boom}) + config := appconfig.NewTestConfig(nil) + + // Fail the first setup transaction before it can commit + _, _, err := svc.SignUpInitialAdmin(t.Context(), config, signUpDto{Username: "failed-admin"}) + require.ErrorIs(t, err, boom) + + // Confirm a later setup can complete after the failed transaction rolls back + svc.userCreator = fakeUserCreator{user: model.User{Base: model.Base{ID: "new-admin"}}} + user, _, err := svc.SignUpInitialAdmin(t.Context(), config, signUpDto{Username: "new-admin"}) + require.NoError(t, err) + require.Equal(t, "new-admin", user.ID) +} + +func TestSignUpInitialAdminRejectsExistingInstallation(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + require.NoError(t, db.Create(&model.User{Username: "existing-admin", IsAdmin: true}).Error) + svc := newSignupServiceForTest(t, db, fakeUserCreator{user: model.User{Base: model.Base{ID: "new-admin"}}}) + + // Reject setup when the installation already contains a user + _, _, err := svc.SignUpInitialAdmin(t.Context(), appconfig.NewTestConfig(nil), signUpDto{Username: "new-admin"}) + var setupNotAvailableErr *common.SetupNotAvailableError + require.ErrorAs(t, err, &setupNotAvailableErr) +} + // listAllOptions returns list options that return every token on a single page. func listAllOptions() utils.ListRequestOptions { var opts utils.ListRequestOptions diff --git a/tests/specs/user-signup.spec.ts b/tests/specs/user-signup.spec.ts index 2871bfc1..7b2129cd 100644 --- a/tests/specs/user-signup.spec.ts +++ b/tests/specs/user-signup.spec.ts @@ -81,6 +81,37 @@ test.describe('Initial User Signup', () => { await expect(page.getByText('Set up your passkey')).toBeVisible(); }); + test('Initial Signup - concurrent requests create one administrator', async ({ request }) => { + await cleanupBackend({ skipSeed: true }); + + const requestCount = 20; + const responses = await Promise.all( + Array.from({ length: requestCount }, (_, index) => + request.post('/api/signup/setup', { + data: { + username: `race-admin-${index}`, + email: `race-admin-${index}@example.invalid`, + firstName: 'Race', + lastName: `${index}` + } + }) + ) + ); + + const successfulResponses = responses.filter((response) => response.status() === 200); + const rejectedResponses = responses.filter((response) => response.status() === 404); + + expect(successfulResponses).toHaveLength(1); + expect(rejectedResponses).toHaveLength(requestCount - 1); + await expect(successfulResponses[0].json()).resolves.toMatchObject({ isAdmin: true }); + + const usersResponse = await request.get('/api/users'); + expect(usersResponse.status()).toBe(200); + const users = await usersResponse.json(); + expect(users.data).toHaveLength(1); + expect(users.data[0]).toMatchObject({ isAdmin: true }); + }); + test('Initial Signup - setup route unavailable after completion', async ({ page }) => { await cleanupBackend(); await page.goto('/setup');