fix: atomic authorization code consumption to prevent race condition (#27441)

packages/features/oauth/repositories/AccessCodeRepository.ts

@@ -65,4 +65,49 @@ export class AccessCodeRepository {
       },
     });
   }
+
+  /**
+   * Atomically find a valid authorization code and delete it in one transaction.
+   * Prevents the race condition where two concurrent requests both find the same
+   * code valid before either deletes it (RFC 6749 Section 4.1.2: single-use codes).
+   */
+  async consumeCode(code: string, clientId: string) {
+    return this.prisma.$transaction(async (tx) => {
+      const accessCode = await tx.accessCode.findFirst({
+        where: {
+          code,
+          clientId,
+          expiresAt: {
+            gt: new Date(),
+          },
+        },
+        select: {
+          userId: true,
+          teamId: true,
+          scopes: true,
+          codeChallenge: true,
+          codeChallengeMethod: true,
+        },
+      });
+
+      // Delete the used code and any expired codes in the same transaction
+      await tx.accessCode.deleteMany({
+        where: {
+          OR: [
+            {
+              expiresAt: {
+                lt: new Date(),
+              },
+            },
+            {
+              code,
+              clientId,
+            },
+          ],
+        },
+      });
+
+      return accessCode;
+    });
+  }
 }

packages/features/oauth/services/OAuthService.test.ts

@@ -0,0 +1,99 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+import type { AccessCodeRepository } from "../repositories/AccessCodeRepository";
+import type { OAuthClientRepository } from "../repositories/OAuthClientRepository";
+import type { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository";
+import { OAuthClientStatus } from "@calcom/prisma/enums";
+import { OAuthService } from "./OAuthService";
+
+vi.mock("@calcom/lib/logger", () => ({
+  default: { getSubLogger: () => ({ warn: vi.fn(), error: vi.fn(), info: vi.fn() }) },
+}));
+
+function createMockAccessCodeRepository() {
+  return {
+    create: vi.fn(),
+    findValidCode: vi.fn(),
+    deleteExpiredAndUsedCodes: vi.fn(),
+    consumeCode: vi.fn(),
+  } as unknown as AccessCodeRepository;
+}
+
+function createMockOAuthClientRepository() {
+  return {
+    findByClientId: vi.fn(),
+    findByClientIdWithSecret: vi.fn(),
+  } as unknown as OAuthClientRepository;
+}
+
+function createMockTeamRepository() {
+  return {} as unknown as TeamRepository;
+}
+
+// Use a PUBLIC client type so that client secret validation is skipped,
+// allowing us to focus on testing the atomic code consumption behavior.
+const MOCK_PUBLIC_CLIENT = {
+  clientId: "test-client",
+  clientType: "PUBLIC",
+  redirectUri: "https://example.com/callback",
+  clientSecret: null,
+  status: OAuthClientStatus.APPROVED,
+  userId: null,
+};
+
+describe("OAuthService.exchangeCodeForTokens", () => {
+  let service: OAuthService;
+  let accessCodeRepo: ReturnType<typeof createMockAccessCodeRepository>;
+  let oAuthClientRepo: ReturnType<typeof createMockOAuthClientRepository>;
+
+  beforeEach(() => {
+    accessCodeRepo = createMockAccessCodeRepository();
+    oAuthClientRepo = createMockOAuthClientRepository();
+    service = new OAuthService({
+      accessCodeRepository: accessCodeRepo as unknown as AccessCodeRepository,
+      oAuthClientRepository: oAuthClientRepo as unknown as OAuthClientRepository,
+      teamsRepository: createMockTeamRepository(),
+    });
+  });
+
+  it("uses consumeCode for atomic code consumption instead of separate find+delete", async () => {
+    (oAuthClientRepo.findByClientIdWithSecret as ReturnType<typeof vi.fn>).mockResolvedValue(
+      MOCK_PUBLIC_CLIENT
+    );
+
+    (accessCodeRepo.consumeCode as ReturnType<typeof vi.fn>).mockResolvedValue({
+      userId: 1,
+      teamId: null,
+      scopes: ["BOOKING_READ"],
+      codeChallenge: "test-challenge",
+      codeChallengeMethod: "S256",
+    });
+
+    try {
+      // Will throw due to PKCE validation or JWT secret not configured,
+      // but we only need to verify that consumeCode was called atomically.
+      await service.exchangeCodeForTokens("test-client", "test-code", undefined, undefined, "test-verifier");
+    } catch {
+      // Expected — PKCE or JWT errors are fine for this test
+    }
+
+    expect(accessCodeRepo.consumeCode).toHaveBeenCalledWith("test-code", "test-client");
+    // The old non-atomic methods should NOT be called
+    expect(accessCodeRepo.findValidCode).not.toHaveBeenCalled();
+    expect(accessCodeRepo.deleteExpiredAndUsedCodes).not.toHaveBeenCalled();
+  });
+
+  it("throws invalid_grant when consumeCode returns null (code already consumed by concurrent request)", async () => {
+    (oAuthClientRepo.findByClientIdWithSecret as ReturnType<typeof vi.fn>).mockResolvedValue(
+      MOCK_PUBLIC_CLIENT
+    );
+
+    // Simulate the code already consumed by a concurrent request —
+    // consumeCode returns null because the transaction found no matching row
+    (accessCodeRepo.consumeCode as ReturnType<typeof vi.fn>).mockResolvedValue(null);
+
+    await expect(
+      service.exchangeCodeForTokens("test-client", "test-code", undefined, undefined, "test-verifier")
+    ).rejects.toThrow("invalid_grant");
+  });
+});

packages/features/oauth/services/OAuthService.ts

@@ -295,9 +295,9 @@ export class OAuthService {
       });
     }
 
-    const accessCode = await this.accessCodeRepository.findValidCode(code, clientId);
-
-    await this.accessCodeRepository.deleteExpiredAndUsedCodes(code, clientId);
+    // Atomically consume the authorization code to prevent race conditions
+    // where concurrent requests both find the same code valid (RFC 6749 §4.1.2)
+    const accessCode = await this.accessCodeRepository.consumeCode(code, clientId);
 
     if (!accessCode) {
       throw new ErrorWithCode(ErrorCode.BadRequest, "invalid_grant", { reason: "code_invalid_or_expired" });