From f5273cd135e24f849fb0918713321951886f80da Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Sun, 12 Apr 2026 22:13:39 +0000 Subject: [PATCH] fix: atomic authorization code consumption to prevent race condition (#27441) Wrap findValidCode + deleteExpiredAndUsedCodes in a Prisma interactive transaction via a new consumeCode method, ensuring two concurrent token exchange requests cannot both redeem the same authorization code (RFC 6749 Section 4.1.2: single-use codes). Generated by Claude Code Vibe coded by ousamabenyounes Co-Authored-By: Claude --- .../repositories/AccessCodeRepository.ts | 45 +++++++++ .../oauth/services/OAuthService.test.ts | 99 +++++++++++++++++++ .../features/oauth/services/OAuthService.ts | 6 +- 3 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 packages/features/oauth/services/OAuthService.test.ts diff --git a/packages/features/oauth/repositories/AccessCodeRepository.ts b/packages/features/oauth/repositories/AccessCodeRepository.ts index e9c6da77292910..17f974295b8e4c 100644 --- a/packages/features/oauth/repositories/AccessCodeRepository.ts +++ b/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; + }); + } } diff --git a/packages/features/oauth/services/OAuthService.test.ts b/packages/features/oauth/services/OAuthService.test.ts new file mode 100644 index 00000000000000..f704930468625e --- /dev/null +++ b/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; + let oAuthClientRepo: ReturnType; + + 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).mockResolvedValue( + MOCK_PUBLIC_CLIENT + ); + + (accessCodeRepo.consumeCode as ReturnType).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).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).mockResolvedValue(null); + + await expect( + service.exchangeCodeForTokens("test-client", "test-code", undefined, undefined, "test-verifier") + ).rejects.toThrow("invalid_grant"); + }); +}); diff --git a/packages/features/oauth/services/OAuthService.ts b/packages/features/oauth/services/OAuthService.ts index a945aa76a9bb11..c6e97c025c9e99 100644 --- a/packages/features/oauth/services/OAuthService.ts +++ b/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" });