🐛 Serialise first-login user provisioning behind an advisory lock

packages/twenty-server/src/engine/core-modules/auth/services/sso-user-provisioning.service.spec.ts

@@ -7,12 +7,28 @@ const buildService = (overrides?: {
   existingUser?: unknown;
   workspace?: unknown;
   configuredSubdomain?: string;
+  insideLockExistingUser?: unknown;
 }) => {
+  // Find-or-create runs inside dataSource.transaction. The repository
+  // returned by manager.getRepository(UserEntity) is the same jest
+  // mock as the injected one so existing assertions on
+  // `userRepository.findOne/create/save` keep working unchanged.
   const userRepository = {
     findOne: jest.fn().mockResolvedValue(overrides?.existingUser ?? null),
     create: jest.fn((u) => u),
     save: jest.fn(async (u) => ({ id: 'user-id-1', ...u })),
   };
+
+  const queryFn = jest.fn().mockResolvedValue(undefined);
+  const dataSource = {
+    transaction: jest.fn(async (cb: (m: any) => Promise<unknown>) =>
+      cb({
+        query: queryFn,
+        getRepository: jest.fn(() => userRepository),
+      }),
+    ),
+  };
+
   const workspaceRepository = {
     findOne: jest.fn().mockResolvedValue(
       overrides?.workspace ?? {
@@ -37,6 +53,7 @@ const buildService = (overrides?: {
     workspaceRepository as any,
     userWorkspaceService as any,
     twentyConfigService as any,
+    dataSource as any,
   );
 
   return {
@@ -45,6 +62,8 @@ const buildService = (overrides?: {
     workspaceRepository,
     userWorkspaceService,
     twentyConfigService,
+    dataSource,
+    queryFn,
   };
 };
 
@@ -137,4 +156,71 @@ describe('SsoUserProvisioningService', () => {
       where: { email: 'mixed@case.com' },
     });
   });
+
+  it('should acquire an advisory lock keyed by email before find+create', async () => {
+    // Concurrent-creation race guard: every first-login provisioning
+    // attempt must take a Postgres advisory lock keyed on the email
+    // hash so parallel requests for the same email serialise on the
+    // create. Other emails take different lock keys so distinct
+    // first-logins don't contend.
+    const { service, queryFn, dataSource } = buildService();
+
+    await service.findOrProvision('alice@askii.ai');
+
+    expect(dataSource.transaction).toHaveBeenCalledTimes(1);
+    expect(queryFn).toHaveBeenCalledWith(
+      expect.stringMatching(/pg_advisory_xact_lock\s*\(\s*hashtextextended/),
+      ['alice@askii.ai'],
+    );
+  });
+
+  it('should take the advisory lock before any user lookup', async () => {
+    // The race protection only works if the lock is taken BEFORE the
+    // findOne. If a refactor accidentally moves the findOne above the
+    // lock, two concurrent requests could both pass the miss check
+    // before either takes the lock — race reopens.
+    const callOrder: string[] = [];
+    const { service, userRepository, queryFn } = buildService();
+
+    queryFn.mockImplementation(async () => {
+      callOrder.push('lock');
+    });
+    userRepository.findOne.mockImplementation(async () => {
+      callOrder.push('findOne');
+      return null;
+    });
+
+    await service.findOrProvision('alice@askii.ai');
+
+    expect(callOrder[0]).toBe('lock');
+    expect(callOrder.indexOf('findOne')).toBeGreaterThan(
+      callOrder.indexOf('lock'),
+    );
+  });
+
+  it('should skip create when another request committed a row first', async () => {
+    // Race: this request waited on the advisory lock; while it waited,
+    // another concurrent request committed a User row for the same
+    // email. The lock serialises; once we acquire it, our findOne
+    // sees that row and we skip our own create. Without serialisation
+    // both racing requests would insert duplicate rows.
+    const winningUser = {
+      id: 'committed-by-other-request',
+      email: 'racy@askii.ai',
+    };
+    const { service, userRepository, userWorkspaceService } = buildService({
+      existingUser: winningUser,
+    });
+
+    const result = await service.findOrProvision('racy@askii.ai');
+
+    expect(userRepository.create).not.toHaveBeenCalled();
+    expect(userRepository.save).not.toHaveBeenCalled();
+    expect(result.user).toBe(winningUser);
+    // Membership ensure runs regardless of who won the race — the
+    // workspace join is idempotent on its own.
+    expect(
+      userWorkspaceService.addUserToWorkspaceIfUserNotInWorkspace,
+    ).toHaveBeenCalledWith(winningUser, expect.anything());
+  });
 });

packages/twenty-server/src/engine/core-modules/auth/services/sso-user-provisioning.service.ts

@@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';
 
 import { randomBytes } from 'crypto';
 
-import { Repository } from 'typeorm';
+import { DataSource, Repository } from 'typeorm';
 
 import {
   AuthException,
@@ -35,6 +35,9 @@ export class SsoUserProvisioningService {
     private readonly workspaceRepository: Repository<WorkspaceEntity>,
     private readonly userWorkspaceService: UserWorkspaceService,
     private readonly twentyConfigService: TwentyConfigService,
+    // Used for the advisory-lock transaction that serialises
+    // concurrent first-login provisioning (see findOrCreateUser).
+    private readonly dataSource: DataSource,
   ) {}
 
   async findOrProvision(email: string): Promise<SsoProvisionResult> {
@@ -81,27 +84,56 @@ export class SsoUserProvisioningService {
   }
 
   private async findOrCreateUser(email: string): Promise<UserEntity> {
-    const existing = await this.userRepository.findOne({ where: { email } });
-
-    if (existing) {
-      return existing;
-    }
+    // Concurrent-creation race guard. On first-ever SSO login the SPA
+    // fires multiple parallel API requests with no session cookie set
+    // yet — every request reaches the proxy-login flow, every request
+    // hits the findOne→save below, and before the row is committed the
+    // others have already passed the findOne miss. Without a
+    // serialisation point, N parallel requests insert N duplicate rows
+    // for the same email.
+    //
+    // We don't rely on the UserEntity unique-on-email constraint to
+    // dedupe at the DB level — that surfaces as a 500 on the losers of
+    // the race rather than a graceful return of the existing user. We
+    // serialise the find+create on a Postgres advisory lock keyed by
+    // the email hash. Advisory locks are per-session, transaction-
+    // scoped, no schema cost; different emails take different lock
+    // keys so concurrent first-logins for distinct users don't contend.
+    return await this.dataSource.transaction(async (manager) => {
+      // pg_advisory_xact_lock takes a bigint; hashtextextended returns a
+      // stable bigint hash. Released automatically when the transaction
+      // commits or rolls back.
+      await manager.query(
+        'SELECT pg_advisory_xact_lock(hashtextextended($1, 0))',
+        [email],
+      );
 
-    const unguessablePassword = randomBytes(32).toString('hex');
-    const passwordHash = await hashPassword(unguessablePassword);
-
-    // The SSO IdP already vouched for this email (the only path to the
-    // proxy-login route is through oauth2-proxy, which validates the
-    // session). Mark the user verified at creation so SSO accounts don't
-    // appear semi-verified to gates that read isEmailVerified.
-    const created = this.userRepository.create({
-      email,
-      firstName: '',
-      lastName: '',
-      passwordHash,
-      isEmailVerified: true,
+      // Re-check inside the lock. If a concurrent request beat us to
+      // the create, this returns its row and we skip provisioning.
+      const userRepo = manager.getRepository(UserEntity);
+      const existing = await userRepo.findOne({ where: { email } });
+
+      if (existing) {
+        return existing;
+      }
+
+      const unguessablePassword = randomBytes(32).toString('hex');
+      const passwordHash = await hashPassword(unguessablePassword);
+
+      // The SSO IdP already vouched for this email (the only path to
+      // the proxy-login route is through oauth2-proxy, which validates
+      // the session). Mark the user verified at creation so SSO
+      // accounts don't appear semi-verified to gates that read
+      // isEmailVerified.
+      const created = userRepo.create({
+        email,
+        firstName: '',
+        lastName: '',
+        passwordHash,
+        isEmailVerified: true,
+      });
+
+      return await userRepo.save(created);
     });
-
-    return await this.userRepository.save(created);
   }
 }