mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-08-19 03:16:28 +00:00
fix: race condition in initial admin setup
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user