From c321a6c07ad14ba2e2c1919f936575b94e6cf14d Mon Sep 17 00:00:00 2001 From: Lauris Skraucis Date: Thu, 19 Feb 2026 12:11:46 +0100 Subject: [PATCH] feat: owner can test non accepted OAuth client (#27525) * refactor: combine exchange and refresh into token endpoint * refactor: controller error handling * refactor: use snake_case * refactor: use snake_case * refactor: use snake case * refactor: token endpoint accepts application/x-www-form-urlencoded * refactor: token endpoint accepts application/x-www-form-urlencoded * refactor: flat token response data * refactor: error structure * refactor: client_id in the body * fix: address Cubic AI review feedback on OAuth2 endpoints - Fix getClient endpoint to use proper REST API error format instead of OAuth token error format (confidence 9/10) - Add missing space after comma in error format string in token.input.pipe.ts (confidence 9/10) - Support both camelCase and snake_case inputs in authorize endpoint for backward compatibility (confidence 10/10) - Restore legacy /exchange and /refresh endpoints alongside new /token endpoint for backward compatibility (confidence 10/10) - Add OAuth2TokensResponseDto for legacy endpoint wrapped responses - Add OAuth2LegacyExchangeInput and OAuth2LegacyRefreshInput for legacy endpoints Co-Authored-By: unknown <> * fix: address additional Cubic AI feedback on OAuth2 endpoints - Log errors when status code >= 500 in handleClientError (confidence 9/10) - Add Cache-Control: no-store and Pragma: no-cache headers to legacy /exchange and /refresh endpoints (confidence 9/10) Co-Authored-By: unknown <> * docs * Revert "fix: address additional Cubic AI feedback on OAuth2 endpoints" This reverts commit 39cc4aa3ebb9e59a171541d7010398425995ed89. * Revert "fix: address Cubic AI review feedback on OAuth2 endpoints" This reverts commit 97bf593186db04c0859f9ca30950c9e3e524019d. * docs * fix: address Cubic AI review feedback on OAuth2 endpoints - Fix getClient to use handleClientError instead of handleTokenError (confidence 10) - Restore legacy /exchange and /refresh endpoints for backward compatibility (confidence 9) - Fix RFC 6749 error format: use human-readable messages in error_description (confidence 9) - Fix errorDescription in OAuthService to use OAUTH_ERROR_REASONS mapping (confidence 9) Co-Authored-By: unknown <> * fix: address additional Cubic AI feedback on OAuth2 endpoints - Fix security issue: Replace 'CALENDSO_ENCRYPTION_KEY is not set' with generic 'Internal server configuration error' message (confidence 10/10) - Fix backward compatibility: Create OAuth2LegacyTokensDto with camelCase properties for legacy /exchange and /refresh endpoints (confidence 9/10) - Skipped: RFC 6749 error field issue (confidence 8/10, below threshold) Co-Authored-By: unknown <> * e2e * Revert "fix: address additional Cubic AI feedback on OAuth2 endpoints" This reverts commit a080e93f07aaf5a7dcf81fe605012cb7ebcdc192. * Revert "fix: address Cubic AI review feedback on OAuth2 endpoints" This reverts commit 04986a16c981521ca97069152457bf521a9ee45f. * fix: re-apply Cubic AI review feedback on OAuth2 endpoints - Restore OAuth2LegacyExchangeInput and OAuth2LegacyRefreshInput classes - Restore legacy /exchange and /refresh endpoints in OAuth2Controller - Restore OAuth2LegacyTokensDto and OAuth2TokensResponseDto classes - Restore OAUTH_ERROR_DESCRIPTIONS mapping in oauth2-error.service.ts - Restore OAUTH_ERROR_REASONS lookup in OAuthService.ts mapErrorToOAuthError - Fix encryption_key_missing error to not expose internal env var name Addresses Cubic AI feedback with confidence >= 9/10: - Comment 32 (9/10): Legacy endpoints and input classes - Comment 34 (9/10): Error description mapping in OAuthService - Comment 35 (10/10): OAUTH_ERROR_DESCRIPTIONS in error service Skipped (confidence < 9/10): - Comment 33 (8/10): getClient handleTokenError vs handleClientError Co-Authored-By: unknown <> * Revert "fix: re-apply Cubic AI review feedback on OAuth2 endpoints" This reverts commit 416bef9c931d9a7ed78c65a70a3425550d61b151. * delete unused file * fix: e2e tests * address cubic review * fix: address Cubic AI review feedback on OAuth2 exception filter - Fix header case sensitivity: use lowercase 'x-request-id' instead of 'X-Request-Id' since Express lowercases all request headers - Redact request body in error logs to prevent exposing sensitive OAuth2 credentials like client_secret, password, and refresh_token Co-Authored-By: unknown <> * docs: api v2 oauth controller docs * chore: remove authorize endpoint * feat: owner can test non accepted OAuth client * fix: remove sensitive data from OAuth2 exception logs Remove Authorization header and userEmail from error logs in OAuth2HttpExceptionFilter to avoid logging sensitive information. Addresses Cubic AI review feedback (confidence 9/10). Co-Authored-By: unknown <> * fix: e2e --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../controllers/oauth2.controller.e2e-spec.ts | 284 ++++++++++++++++-- .../filters/oauth2-http-exception.filter.ts | 7 +- .../oauth2-client.repository.fixture.ts | 9 + .../oauth/view/OAuthClientDetailsDialog.tsx | 16 +- .../oauth-authorize-approval-status.e2e.ts | 73 ++++- .../oauth/oauth-refresh-tokens.e2e.ts | 24 +- apps/web/public/static/locales/en/common.json | 5 +- .../repositories/OAuthClientRepository.ts | 1 + .../features/oauth/services/OAuthService.ts | 52 +++- .../server/routers/viewer/oAuth/_router.tsx | 6 +- .../oAuth/generateAuthCode.handler.test.ts | 109 ++++++- .../getClientForAuthorization.handler.ts | 13 +- 12 files changed, 532 insertions(+), 67 deletions(-) diff --git a/apps/api/v2/src/modules/auth/oauth2/controllers/oauth2.controller.e2e-spec.ts b/apps/api/v2/src/modules/auth/oauth2/controllers/oauth2.controller.e2e-spec.ts index 6669500439775d..5c63af99464f82 100644 --- a/apps/api/v2/src/modules/auth/oauth2/controllers/oauth2.controller.e2e-spec.ts +++ b/apps/api/v2/src/modules/auth/oauth2/controllers/oauth2.controller.e2e-spec.ts @@ -1,11 +1,12 @@ import { generateSecret } from "@calcom/platform-libraries"; import type { Membership, Team, User } from "@calcom/prisma/client"; -import { AccessScope, OAuthClientType } from "@calcom/prisma/enums"; -import { INestApplication } from "@nestjs/common"; -import { NestExpressApplication } from "@nestjs/platform-express"; -import { Test, TestingModule } from "@nestjs/testing"; -import { getToken } from "next-auth/jwt"; +import { AccessScope, OAuthClientStatus, OAuthClientType } from "@calcom/prisma/enums"; +import type { INestApplication } from "@nestjs/common"; +import type { NestExpressApplication } from "@nestjs/platform-express"; +import type { TestingModule } from "@nestjs/testing"; +import { Test } from "@nestjs/testing"; import request from "supertest"; +import { ApiKeysRepositoryFixture } from "test/fixtures/repository/api-keys.repository.fixture"; import { MembershipRepositoryFixture } from "test/fixtures/repository/membership.repository.fixture"; import { OAuth2ClientRepositoryFixture } from "test/fixtures/repository/oauth2-client.repository.fixture"; import { TeamRepositoryFixture } from "test/fixtures/repository/team.repository.fixture"; @@ -21,20 +22,16 @@ import { AuthModule } from "@/modules/auth/auth.module"; import { PrismaModule } from "@/modules/prisma/prisma.module"; import { UsersModule } from "@/modules/users/users.module"; -// Mock next-auth/jwt getToken for ApiAuthStrategy NEXT_AUTH authentication +// Mock next-auth/jwt getToken so the NextAuth fallback in ApiAuthStrategy doesn't crash in tests jest.mock("next-auth/jwt", () => ({ - getToken: jest.fn(), + getToken: jest.fn().mockResolvedValue(null), })); -const mockGetToken = getToken as jest.MockedFunction; describe("OAuth2 Controller Endpoints", () => { describe("User Not Authenticated", () => { let appWithoutAuth: INestApplication; beforeAll(async () => { - // Mock getToken to return null for unauthenticated tests - mockGetToken.mockResolvedValue(null); - const moduleRef = await Test.createTestingModule({ providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter], imports: [AppModule, UsersModule, AuthModule, PrismaModule], @@ -69,20 +66,23 @@ describe("OAuth2 Controller Endpoints", () => { }); }); - describe("User Authenticated", () => { + describe("User Authenticated (non-owner)", () => { let app: INestApplication; let moduleRef: TestingModule; let userRepositoryFixture: UserRepositoryFixture; let teamRepositoryFixture: TeamRepositoryFixture; let membershipRepositoryFixture: MembershipRepositoryFixture; + let apiKeysRepositoryFixture: ApiKeysRepositoryFixture; let oAuthClientFixture: OAuth2ClientRepositoryFixture; let oAuthService: OAuthService; - let user: User; + let clientOwner: User; + let authenticatedUser: User; let team: Team; let membership: Membership; let oAuthClient: { clientId: string }; + let apiKeyString: string; let refreshToken: string; const testClientId = `test-oauth-client-${randomString()}`; @@ -96,7 +96,7 @@ describe("OAuth2 Controller Endpoints", () => { ): Promise { const result = await oAuthService.generateAuthorizationCode( testClientId, - user.id, + authenticatedUser.id, testRedirectUri, scopes, undefined, @@ -107,12 +107,6 @@ describe("OAuth2 Controller Endpoints", () => { } beforeAll(async () => { - const userEmail = `oauth2-e2e-user-${randomString()}@api.com`; - - // Mock getToken to return the user email for authenticated tests - // This is needed because ApiAuthGuard uses ApiAuthStrategy which calls getToken from next-auth/jwt - mockGetToken.mockResolvedValue({ email: userEmail }); - moduleRef = await Test.createTestingModule({ providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter], imports: [AppModule, UsersModule, AuthModule, PrismaModule], @@ -125,20 +119,28 @@ describe("OAuth2 Controller Endpoints", () => { userRepositoryFixture = new UserRepositoryFixture(moduleRef); teamRepositoryFixture = new TeamRepositoryFixture(moduleRef); membershipRepositoryFixture = new MembershipRepositoryFixture(moduleRef); + apiKeysRepositoryFixture = new ApiKeysRepositoryFixture(moduleRef); oAuthClientFixture = new OAuth2ClientRepositoryFixture(moduleRef); oAuthService = moduleRef.get(OAuthService); - user = await userRepositoryFixture.create({ - email: userEmail, + clientOwner = await userRepositoryFixture.create({ + email: `oauth2-e2e-owner-${randomString()}@api.com`, + }); + + authenticatedUser = await userRepositoryFixture.create({ + email: `oauth2-e2e-user-${randomString()}@api.com`, }); + const { keyString } = await apiKeysRepositoryFixture.createApiKey(authenticatedUser.id, null); + apiKeyString = keyString; + team = await teamRepositoryFixture.create({ name: `oauth2-e2e-team-${randomString()}`, slug: `oauth2-e2e-team-${randomString()}`, }); membership = await membershipRepositoryFixture.create({ - user: { connect: { id: user.id } }, + user: { connect: { id: authenticatedUser.id } }, team: { connect: { id: team.id } }, role: "OWNER", accepted: true, @@ -151,6 +153,7 @@ describe("OAuth2 Controller Endpoints", () => { redirectUri: testRedirectUri, clientSecret: hashedSecret, clientType: OAuthClientType.CONFIDENTIAL, + userId: clientOwner.id, }); }); @@ -159,6 +162,7 @@ describe("OAuth2 Controller Endpoints", () => { it("should return OAuth client info for valid client ID", async () => { const response = await request(app.getHttpServer()) .get(`/api/v2/auth/oauth2/clients/${testClientId}`) + .set({ Authorization: `Bearer cal_test_${apiKeyString}` }) .expect(200); expect(response.body.status).toBe("success"); @@ -174,6 +178,7 @@ describe("OAuth2 Controller Endpoints", () => { it("should return 404 for non-existent client ID", async () => { await request(app.getHttpServer()) .get("/api/v2/auth/oauth2/clients/non-existent-client-id") + .set({ Authorization: `Bearer cal_test_${apiKeyString}` }) .expect(404); }); }); @@ -434,7 +439,238 @@ describe("OAuth2 Controller Endpoints", () => { await oAuthClientFixture.delete(oAuthClient.clientId); await membershipRepositoryFixture.delete(membership.id); await teamRepositoryFixture.delete(team.id); - await userRepositoryFixture.delete(user.id); + await userRepositoryFixture.delete(authenticatedUser.id); + await userRepositoryFixture.delete(clientOwner.id); + await app.close(); + }); + }); + + describe("Owner can use non-approved OAuth client", () => { + let app: INestApplication; + let moduleRef: TestingModule; + + let userRepositoryFixture: UserRepositoryFixture; + let teamRepositoryFixture: TeamRepositoryFixture; + let membershipRepositoryFixture: MembershipRepositoryFixture; + let apiKeysRepositoryFixture: ApiKeysRepositoryFixture; + let oAuthClientFixture: OAuth2ClientRepositoryFixture; + let oAuthService: OAuthService; + + let owner: User; + let team: Team; + let membership: Membership; + let pendingClient: { clientId: string }; + let rejectedClient: { clientId: string }; + let apiKeyString: string; + + const pendingClientId = `test-pending-client-${randomString()}`; + const rejectedClientId = `test-rejected-client-${randomString()}`; + const testClientSecret = "test-secret-456"; + const testRedirectUri = "https://example.com/callback"; + + /** Generate a user-scoped authorization code (no teamSlug) so the owner's userId is in the token. */ + async function generateAuthCodeForClient( + clientId: string, + scopes: AccessScope[] = [AccessScope.READ_BOOKING] + ): Promise { + const result = await oAuthService.generateAuthorizationCode( + clientId, + owner.id, + testRedirectUri, + scopes + ); + const redirectUrl = new URL(result.redirectUrl); + return redirectUrl.searchParams.get("code") as string; + } + + beforeAll(async () => { + moduleRef = await Test.createTestingModule({ + providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter], + imports: [AppModule, UsersModule, AuthModule, PrismaModule], + }).compile(); + + app = moduleRef.createNestApplication(); + bootstrap(app as NestExpressApplication); + await app.init(); + + userRepositoryFixture = new UserRepositoryFixture(moduleRef); + teamRepositoryFixture = new TeamRepositoryFixture(moduleRef); + membershipRepositoryFixture = new MembershipRepositoryFixture(moduleRef); + apiKeysRepositoryFixture = new ApiKeysRepositoryFixture(moduleRef); + oAuthClientFixture = new OAuth2ClientRepositoryFixture(moduleRef); + oAuthService = moduleRef.get(OAuthService); + + owner = await userRepositoryFixture.create({ + email: `oauth2-owner-e2e-${randomString()}@api.com`, + }); + + const { keyString } = await apiKeysRepositoryFixture.createApiKey(owner.id, null); + apiKeyString = keyString; + + team = await teamRepositoryFixture.create({ + name: `oauth2-owner-team-${randomString()}`, + slug: `oauth2-owner-team-${randomString()}`, + }); + + membership = await membershipRepositoryFixture.create({ + user: { connect: { id: owner.id } }, + team: { connect: { id: team.id } }, + role: "OWNER", + accepted: true, + }); + + const [hashedSecret] = generateSecret(testClientSecret); + + pendingClient = await oAuthClientFixture.create({ + clientId: pendingClientId, + name: "Pending OAuth Client", + redirectUri: testRedirectUri, + clientSecret: hashedSecret, + clientType: OAuthClientType.CONFIDENTIAL, + status: OAuthClientStatus.PENDING, + userId: owner.id, + }); + + rejectedClient = await oAuthClientFixture.create({ + clientId: rejectedClientId, + name: "Rejected OAuth Client", + redirectUri: testRedirectUri, + clientSecret: hashedSecret, + clientType: OAuthClientType.CONFIDENTIAL, + status: OAuthClientStatus.REJECTED, + userId: owner.id, + }); + }); + + it("should return PENDING client info when authenticated as owner", async () => { + const response = await request(app.getHttpServer()) + .get(`/api/v2/auth/oauth2/clients/${pendingClientId}`) + .set({ Authorization: `Bearer cal_test_${apiKeyString}` }) + .expect(200); + + expect(response.body.status).toBe("success"); + expect(response.body.data.client_id).toBe(pendingClientId); + expect(response.body.data.name).toBe("Pending OAuth Client"); + }); + + it("should exchange authorization code for tokens with PENDING client owned by user", async () => { + const code = await generateAuthCodeForClient(pendingClientId); + + const response = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "authorization_code", + code, + client_secret: testClientSecret, + redirect_uri: testRedirectUri, + }) + .expect(200); + + expect(response.body.access_token).toBeDefined(); + expect(response.body.refresh_token).toBeDefined(); + expect(response.body.token_type).toBe("bearer"); + }); + + it("should refresh tokens with PENDING client owned by user", async () => { + const code = await generateAuthCodeForClient(pendingClientId); + + const tokenResponse = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "authorization_code", + code, + client_secret: testClientSecret, + redirect_uri: testRedirectUri, + }) + .expect(200); + + const response = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "refresh_token", + refresh_token: tokenResponse.body.refresh_token, + client_secret: testClientSecret, + }) + .expect(200); + + expect(response.body.access_token).toBeDefined(); + expect(response.body.refresh_token).toBeDefined(); + expect(response.body.token_type).toBe("bearer"); + }); + + it("should reject authorization code generation for REJECTED client even as owner", async () => { + await expect(generateAuthCodeForClient(rejectedClientId)).rejects.toThrow(); + }); + + it("should reject token exchange when client becomes rejected", async () => { + const code = await generateAuthCodeForClient(pendingClientId); + + await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.REJECTED); + + const response = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "authorization_code", + code, + client_secret: testClientSecret, + redirect_uri: testRedirectUri, + }) + .expect(401); + + expect(response.body.error).toBe("unauthorized_client"); + expect(response.body.error_description).toBe("client_rejected"); + + await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.PENDING); + }); + + it("should reject token refresh when client becomes rejected", async () => { + const code = await generateAuthCodeForClient(pendingClientId); + + const tokenResponse = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "authorization_code", + code, + client_secret: testClientSecret, + redirect_uri: testRedirectUri, + }) + .expect(200); + + await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.REJECTED); + + const response = await request(app.getHttpServer()) + .post("/api/v2/auth/oauth2/token") + .type("form") + .send({ + client_id: pendingClientId, + grant_type: "refresh_token", + refresh_token: tokenResponse.body.refresh_token, + client_secret: testClientSecret, + }) + .expect(401); + + expect(response.body.error).toBe("unauthorized_client"); + expect(response.body.error_description).toBe("client_rejected"); + + await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.PENDING); + }); + + afterAll(async () => { + await oAuthClientFixture.delete(pendingClient.clientId); + await oAuthClientFixture.delete(rejectedClient.clientId); + await membershipRepositoryFixture.delete(membership.id); + await teamRepositoryFixture.delete(team.id); + await userRepositoryFixture.delete(owner.id); await app.close(); }); }); diff --git a/apps/api/v2/src/modules/auth/oauth2/filters/oauth2-http-exception.filter.ts b/apps/api/v2/src/modules/auth/oauth2/filters/oauth2-http-exception.filter.ts index 49e76beee8737d..ac4155dcfe621c 100644 --- a/apps/api/v2/src/modules/auth/oauth2/filters/oauth2-http-exception.filter.ts +++ b/apps/api/v2/src/modules/auth/oauth2/filters/oauth2-http-exception.filter.ts @@ -16,15 +16,16 @@ export class OAuth2HttpExceptionFilter implements ExceptionFilter - {t("oauth_client_rejection_reason")}:{" "} - {client.rejectionReason} - + +

"{client.rejectionReason}".

+

{t("oauth_client_rejected_resubmit_info")}

+ + } + /> ) : null}
diff --git a/apps/web/playwright/oauth/oauth-authorize-approval-status.e2e.ts b/apps/web/playwright/oauth/oauth-authorize-approval-status.e2e.ts index e9d2b9ea3dc48c..84a1e386babf28 100644 --- a/apps/web/playwright/oauth/oauth-authorize-approval-status.e2e.ts +++ b/apps/web/playwright/oauth/oauth-authorize-approval-status.e2e.ts @@ -1,9 +1,7 @@ -import { expect } from "@playwright/test"; import { randomBytes } from "node:crypto"; - import { OAUTH_ERROR_REASONS } from "@calcom/features/oauth/services/OAuthService"; import type { PrismaClient } from "@calcom/prisma"; - +import { expect } from "@playwright/test"; import { test } from "../lib/fixtures"; test.describe("OAuth authorize - client approval status", () => { @@ -25,10 +23,12 @@ test.describe("OAuth authorize - client approval status", () => { prisma, name, status, + userId, }: { prisma: PrismaClient; name: string; status: "PENDING" | "APPROVED" | "REJECTED"; + userId?: number; }) { const clientId = randomBytes(32).toString("hex"); @@ -40,6 +40,7 @@ test.describe("OAuth authorize - client approval status", () => { clientSecret: null, clientType: "CONFIDENTIAL", status, + ...(userId && { user: { connect: { id: userId } } }), }, }); @@ -65,10 +66,8 @@ test.describe("OAuth authorize - client approval status", () => { `auth/oauth2/authorize?client_id=${client.clientId}&redirect_uri=${client.redirectUri}&state=1234` ); - // Should NOT redirect, should show error on page await expect(page).not.toHaveURL(/^https:\/\/example\.com/); - // Check error message is displayed await expect(page.getByText(OAUTH_ERROR_REASONS["client_not_approved"])).toBeVisible(); }); @@ -91,11 +90,9 @@ test.describe("OAuth authorize - client approval status", () => { `auth/oauth2/authorize?client_id=${client.clientId}&redirect_uri=${client.redirectUri}&state=1234` ); - // Should NOT redirect, should show error on page await expect(page).not.toHaveURL(/^https:\/\/example\.com/); - // Check error message is displayed - await expect(page.getByText(OAUTH_ERROR_REASONS["client_not_approved"])).toBeVisible(); + await expect(page.getByText(OAUTH_ERROR_REASONS["client_rejected"])).toBeVisible(); }); test("APPROVED client redirects to redirectUri with code", async ({ page, users, prisma }, testInfo) => { @@ -171,4 +168,64 @@ test.describe("OAuth authorize - client approval status", () => { await expect(page).toHaveURL(/\/auth\/oauth2\/authorize/); await expect(page.getByText(OAUTH_ERROR_REASONS["client_not_found"])).toBeVisible(); }); + + test("PENDING client owned by logged-in user allows authorization", async ({ + page, + users, + prisma, + }, testInfo) => { + const user = await users.create({ username: "oauth-authorize-pending-owner" }); + await user.apiLogin(); + + const testPrefix = `e2e-oauth-authorize-status-${testInfo.testId}-`; + const client = await createOAuthClient({ + prisma, + name: `${testPrefix}pending-owner-${Date.now()}`, + status: "PENDING", + userId: user.id, + }); + + await page.goto( + `auth/oauth2/authorize?client_id=${client.clientId}&redirect_uri=${client.redirectUri}&state=1234` + ); + + await page.waitForSelector('[data-testid="allow-button"]'); + await page.getByTestId("allow-button").click(); + + await page.waitForFunction(() => { + return window.location.href.startsWith("https://example.com"); + }); + + await expect(page).toHaveURL(/^https:\/\/example\.com/); + + const url = new URL(page.url()); + expect(url.searchParams.get("code")).toBeTruthy(); + expect(url.searchParams.get("state")).toBe("1234"); + expect(url.searchParams.get("error")).toBeNull(); + }); + + test("REJECTED client owned by logged-in user blocks authorization", async ({ + page, + users, + prisma, + }, testInfo) => { + const user = await users.create({ username: "oauth-authorize-rejected-owner" }); + await user.apiLogin(); + + const testPrefix = `e2e-oauth-authorize-status-${testInfo.testId}-`; + const client = await createOAuthClient({ + prisma, + name: `${testPrefix}rejected-owner-${Date.now()}`, + status: "REJECTED", + userId: user.id, + }); + + await page.goto( + `auth/oauth2/authorize?client_id=${client.clientId}&redirect_uri=${client.redirectUri}&state=1234` + ); + + await expect(page).not.toHaveURL(/^https:\/\/example\.com/); + + await expect(page.getByText(OAUTH_ERROR_REASONS["client_rejected"])).toBeVisible(); + }); }); diff --git a/apps/web/playwright/oauth/oauth-refresh-tokens.e2e.ts b/apps/web/playwright/oauth/oauth-refresh-tokens.e2e.ts index 35d911289344bf..e884a000d98283 100644 --- a/apps/web/playwright/oauth/oauth-refresh-tokens.e2e.ts +++ b/apps/web/playwright/oauth/oauth-refresh-tokens.e2e.ts @@ -1,5 +1,6 @@ import { expect } from "@playwright/test"; import { randomBytes } from "node:crypto"; +import jwt from "jsonwebtoken"; import { test } from "../lib/fixtures"; import type { PrismaClient } from "@calcom/prisma"; @@ -46,6 +47,25 @@ test.describe("OAuth - refresh tokens", () => { return client; } + function signRefreshToken(clientId: string, userId: number): string { + const secretKey = process.env.CALENDSO_ENCRYPTION_KEY; + if (!secretKey) { + throw new Error("CALENDSO_ENCRYPTION_KEY is not set"); + } + + return jwt.sign( + { + userId, + teamId: null, + scope: [], + token_type: "Refresh Token", + clientId, + }, + secretKey, + { expiresIn: 30 * 24 * 60 * 60 } + ); + } + test("token refresh fails if client is not approved", async ({ page, users, prisma }, testInfo) => { const user = await users.create({ username: "oauth-refresh-status-check" }); await user.apiLogin(); @@ -58,11 +78,13 @@ test.describe("OAuth - refresh tokens", () => { clientType: "PUBLIC", }); + const refreshToken = signRefreshToken(client.clientId, user.id); + const refreshResponse = await page.request.post("/api/auth/oauth/refreshToken", { form: { grant_type: "refresh_token", client_id: client.clientId, - refresh_token: "fake-refresh-token", + refresh_token: refreshToken, }, }); diff --git a/apps/web/public/static/locales/en/common.json b/apps/web/public/static/locales/en/common.json index 4b0816df03b6ab..3d5145dd933a64 100644 --- a/apps/web/public/static/locales/en/common.json +++ b/apps/web/public/static/locales/en/common.json @@ -1150,9 +1150,10 @@ "oauth_client_approved_email_footer": "You can now use your client ID and secret to integrate with Cal.com.", "oauth_client_secret_one_time_warning": "Important: This client secret is shown only once. Store it securely now - you will not be able to view it again.", "reason_for_rejection": "Reason for rejection", - "oauth_client_rejection_reason": "Rejection reason", + "oauth_client_rejection_reason": "Your OAuth client was rejected. Rejection reason:", + "oauth_client_rejected_resubmit_info": "Please address this issue and submit for re-approval. While in rejected status, this client cannot be used by anyone, including the client owner.", "oauth_client_approved_reapproval_info": "Changing the client's name, logo, website URL, or redirect URIs will trigger a re-review by our admins, and the client will be unusable until approved again.", - "oauth_client_pending_info_description": "This OAuth client is pending approval and cannot be used yet.", + "oauth_client_pending_info_description": "This OAuth client is pending approval and can only be used by the client owner for testing. It cannot be used by other users until approved.", "reject_oauth_client": "Reject OAuth client", "oauth_client_rejected_email_subject": "Your OAuth Client {{clientName}} Has Been Rejected", "oauth_client_rejected_email_title": "OAuth Client Rejected: {{clientName}}", diff --git a/packages/features/oauth/repositories/OAuthClientRepository.ts b/packages/features/oauth/repositories/OAuthClientRepository.ts index de776327f95e44..91cfde2affccce 100644 --- a/packages/features/oauth/repositories/OAuthClientRepository.ts +++ b/packages/features/oauth/repositories/OAuthClientRepository.ts @@ -37,6 +37,7 @@ export class OAuthClientRepository { clientSecret: true, clientType: true, status: true, + userId: true, }, }); } diff --git a/packages/features/oauth/services/OAuthService.ts b/packages/features/oauth/services/OAuthService.ts index f833f095df0907..a945aa76a9bb11 100644 --- a/packages/features/oauth/services/OAuthService.ts +++ b/packages/features/oauth/services/OAuthService.ts @@ -1,15 +1,15 @@ import { randomBytes } from "node:crypto"; -import jwt from "jsonwebtoken"; - -import { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository"; -import { AccessCodeRepository } from "@calcom/features/oauth/repositories/AccessCodeRepository"; -import { OAuthClientRepository } from "@calcom/features/oauth/repositories/OAuthClientRepository"; +import process from "node:process"; +import type { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository"; +import type { AccessCodeRepository } from "@calcom/features/oauth/repositories/AccessCodeRepository"; +import type { OAuthClientRepository } from "@calcom/features/oauth/repositories/OAuthClientRepository"; import { generateSecret } from "@calcom/features/oauth/utils/generateSecret"; import { ErrorCode } from "@calcom/lib/errorCodes"; import { ErrorWithCode } from "@calcom/lib/errors"; import { verifyCodeChallenge } from "@calcom/lib/pkce"; -import { OAuthClientStatus } from "@calcom/prisma/enums"; import type { AccessScope, OAuthClientType } from "@calcom/prisma/enums"; +import { OAuthClientStatus } from "@calcom/prisma/enums"; +import jwt from "jsonwebtoken"; export interface OAuth2Client { clientId: string; @@ -82,7 +82,11 @@ export class OAuthService { }; } - async getClientForAuthorization(clientId: string, redirectUri: string): Promise { + async getClientForAuthorization( + clientId: string, + redirectUri: string, + loggedInUserId?: number + ): Promise { const client = await this.oAuthClientRepository.findByClientId(clientId); if (!client) { @@ -91,7 +95,7 @@ export class OAuthService { this.validateRedirectUri(client.redirectUri, redirectUri); - this.ensureClientIsApproved(client); + this.ensureClientAccessAllowed(client, loggedInUserId); return { clientId: client.clientId, @@ -105,7 +109,7 @@ export class OAuthService { async generateAuthorizationCode( clientId: string, - userId: number, + loggedInUserId: number, redirectUri: string, scopes: AccessScope[], state?: string, @@ -119,7 +123,7 @@ export class OAuthService { throw new ErrorWithCode(ErrorCode.Unauthorized, "unauthorized_client", { reason: "client_not_found" }); } - this.ensureClientIsApproved(client); + this.ensureClientAccessAllowed(client, loggedInUserId); // RFC 6749 4.1.2.1: Redirect URI mismatch on Auth step is 'invalid_request' this.validateRedirectUri(client.redirectUri, redirectUri); @@ -143,7 +147,7 @@ export class OAuthService { let teamId: number | undefined; if (teamSlug) { - const team = await this.teamsRepository.findTeamBySlugWithAdminRole(teamSlug, userId); + const team = await this.teamsRepository.findTeamBySlugWithAdminRole(teamSlug, loggedInUserId); if (!team) { // Specific OAuth error for user denying or failing permission throw new ErrorWithCode(ErrorCode.Unauthorized, "access_denied", { @@ -158,7 +162,7 @@ export class OAuthService { await this.accessCodeRepository.create({ code: authorizationCode, clientId, - userId: teamSlug ? undefined : userId, + userId: teamSlug ? undefined : loggedInUserId, teamId, scopes, codeChallenge, @@ -173,6 +177,22 @@ export class OAuthService { return { redirectUrl, authorizationCode, client }; } + private ensureClientAccessAllowed( + client: { status: OAuthClientStatus; userId: number | null }, + loggedInUserId?: number | null + ): void { + if (client.status === OAuthClientStatus.REJECTED) { + throw new ErrorWithCode(ErrorCode.Unauthorized, "unauthorized_client", { + reason: "client_rejected", + }); + } + + const isOwner = loggedInUserId != null && loggedInUserId === client.userId; + if (!isOwner) { + this.ensureClientIsApproved(client); + } + } + private ensureClientIsApproved(client: { status: OAuthClientStatus }): void { if (client.status !== OAuthClientStatus.APPROVED) { throw new ErrorWithCode(ErrorCode.Unauthorized, "unauthorized_client", { @@ -283,6 +303,8 @@ export class OAuthService { throw new ErrorWithCode(ErrorCode.BadRequest, "invalid_grant", { reason: "code_invalid_or_expired" }); } + this.ensureClientAccessAllowed(client, accessCode.userId); + const pkceError = this.verifyPKCE(client, accessCode, codeVerifier); if (pkceError) { // RFC 7636 4.4.1: If verification fails, return 'invalid_grant' @@ -318,8 +340,6 @@ export class OAuthService { }); } - this.ensureClientIsApproved(client); - const decodedToken = this.verifyRefreshToken(refreshToken); if (!decodedToken || decodedToken.token_type !== "Refresh Token") { @@ -330,6 +350,8 @@ export class OAuthService { throw new ErrorWithCode(ErrorCode.BadRequest, "invalid_grant", { reason: "client_id_mismatch" }); } + this.ensureClientAccessAllowed(client, decodedToken.userId); + const tokens = this.createTokens({ clientId, userId: decodedToken.userId, @@ -459,6 +481,7 @@ export class OAuthService { export type OAuthErrorReason = | "client_not_found" | "client_not_approved" + | "client_rejected" | "redirect_uri_mismatch" | "pkce_required" | "invalid_code_challenge_method" @@ -476,6 +499,7 @@ export type OAuthErrorReason = export const OAUTH_ERROR_REASONS: Record = { client_not_found: "OAuth client with ID not found", client_not_approved: "OAuth client is not approved", + client_rejected: "OAuth client has been rejected", redirect_uri_mismatch: "redirect_uri does not match OAuth client's redirect URI", pkce_required: "code_challenge required for public clients", invalid_code_challenge_method: "code_challenge_method must be S256", diff --git a/packages/trpc/server/routers/viewer/oAuth/_router.tsx b/packages/trpc/server/routers/viewer/oAuth/_router.tsx index cb08001ce5c334..9797ac3eef5d74 100644 --- a/packages/trpc/server/routers/viewer/oAuth/_router.tsx +++ b/packages/trpc/server/routers/viewer/oAuth/_router.tsx @@ -1,21 +1,21 @@ import authedProcedure, { authedAdminProcedure } from "@calcom/trpc/server/procedures/authedProcedure"; - import { router } from "../../../trpc"; import { ZCreateClientInputSchema } from "./createClient.schema"; +import { ZDeleteClientInputSchema } from "./deleteClient.schema"; import { ZGenerateAuthCodeInputSchema } from "./generateAuthCode.schema"; import { ZGetClientForAuthorizationInputSchema } from "./getClientForAuthorization.schema"; import { ZListClientsInputSchema } from "./listClients.schema"; import { ZSubmitClientInputSchema, ZSubmitClientOutputSchema } from "./submitClientForReview.schema"; import { ZUpdateClientInputSchema } from "./updateClient.schema"; -import { ZDeleteClientInputSchema } from "./deleteClient.schema"; export const oAuthRouter = router({ getClientForAuthorization: authedProcedure .input(ZGetClientForAuthorizationInputSchema) - .query(async ({ input }) => { + .query(async ({ ctx, input }) => { const { getClientForAuthorizationHandler } = await import("./getClientForAuthorization.handler"); return getClientForAuthorizationHandler({ + ctx, input, }); }), diff --git a/packages/trpc/server/routers/viewer/oAuth/generateAuthCode.handler.test.ts b/packages/trpc/server/routers/viewer/oAuth/generateAuthCode.handler.test.ts index fa5f88af83aff5..757312b5048355 100644 --- a/packages/trpc/server/routers/viewer/oAuth/generateAuthCode.handler.test.ts +++ b/packages/trpc/server/routers/viewer/oAuth/generateAuthCode.handler.test.ts @@ -425,11 +425,118 @@ describe("generateAuthCodeHandler", () => { await expect(generateAuthCodeHandler({ ctx: mockCtx, input })).rejects.toThrow( new TRPCError({ code: "UNAUTHORIZED", - message: OAUTH_ERROR_REASONS.client_not_approved, + message: OAUTH_ERROR_REASONS.client_rejected, + }) + ); + + expect(prismaMock.accessCode.create).not.toHaveBeenCalled(); + }); + + it("should reject REJECTED client even when user is the owner", async () => { + const mockRejectedClientOwnedByUser = { + clientId: "rejected_client_owned", + redirectUri: "https://app.example.com/callback", + name: "Test Rejected Client Owned", + clientType: "CONFIDENTIAL" as const, + status: "REJECTED" as const, + userId: 1, + }; + + prismaMock.oAuthClient.findFirst.mockResolvedValue(mockRejectedClientOwnedByUser); + + const input = { + clientId: "rejected_client_owned", + scopes: [], + teamSlug: undefined, + codeChallenge: undefined, + codeChallengeMethod: undefined, + redirectUri: "https://app.example.com/callback", + }; + + await expect(generateAuthCodeHandler({ ctx: mockCtx, input })).rejects.toThrow( + new TRPCError({ + code: "UNAUTHORIZED", + message: OAUTH_ERROR_REASONS.client_rejected, }) ); expect(prismaMock.accessCode.create).not.toHaveBeenCalled(); }); + + it("should allow PENDING client when user is the owner", async () => { + const mockPendingClientOwnedByUser = { + clientId: "pending_client_owned", + redirectUri: "https://app.example.com/callback", + name: "Test Pending Client Owned", + clientType: "CONFIDENTIAL" as const, + status: "PENDING" as const, + userId: 1, + }; + + prismaMock.oAuthClient.findFirst.mockResolvedValue(mockPendingClientOwnedByUser); + prismaMock.accessCode.create.mockResolvedValue({ + id: 1, + code: "test_auth_code", + clientId: "pending_client_owned", + userId: 1, + teamId: null, + scopes: [], + codeChallenge: null, + codeChallengeMethod: null, + expiresAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + }); + + const input = { + clientId: "pending_client_owned", + scopes: [], + teamSlug: undefined, + codeChallenge: undefined, + codeChallengeMethod: undefined, + redirectUri: "https://app.example.com/callback", + }; + + const result = await generateAuthCodeHandler({ ctx: mockCtx, input }); + expect(result.authorizationCode).toBeDefined(); + }); + + it("should allow APPROVED client when user is the owner", async () => { + const mockApprovedClientOwnedByUser = { + clientId: "approved_client_owned", + redirectUri: "https://app.example.com/callback", + name: "Test Approved Client Owned", + clientType: "CONFIDENTIAL" as const, + status: "APPROVED" as const, + userId: 1, + }; + + prismaMock.oAuthClient.findFirst.mockResolvedValue(mockApprovedClientOwnedByUser); + prismaMock.accessCode.create.mockResolvedValue({ + id: 1, + code: "test_auth_code", + clientId: "approved_client_owned", + userId: 1, + teamId: null, + scopes: [], + codeChallenge: null, + codeChallengeMethod: null, + expiresAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + }); + + const input = { + clientId: "approved_client_owned", + scopes: [], + teamSlug: undefined, + codeChallenge: undefined, + codeChallengeMethod: undefined, + redirectUri: "https://app.example.com/callback", + }; + + const result = await generateAuthCodeHandler({ ctx: mockCtx, input }); + expect(result.authorizationCode).toBeDefined(); + }); }); }); diff --git a/packages/trpc/server/routers/viewer/oAuth/getClientForAuthorization.handler.ts b/packages/trpc/server/routers/viewer/oAuth/getClientForAuthorization.handler.ts index ff8172fb893513..2b9abb8b7bbf15 100644 --- a/packages/trpc/server/routers/viewer/oAuth/getClientForAuthorization.handler.ts +++ b/packages/trpc/server/routers/viewer/oAuth/getClientForAuthorization.handler.ts @@ -1,24 +1,27 @@ import { getOAuthService } from "@calcom/features/oauth/di/OAuthService.container"; -import { OAUTH_ERROR_REASONS, OAuthErrorReason } from "@calcom/features/oauth/services/OAuthService"; +import type { OAuthErrorReason } from "@calcom/features/oauth/services/OAuthService"; +import { OAUTH_ERROR_REASONS } from "@calcom/features/oauth/services/OAuthService"; import { ErrorWithCode } from "@calcom/lib/errors"; import { getHttpStatusCode } from "@calcom/lib/server/getServerErrorFromUnknown"; import { httpStatusToTrpcCode } from "@calcom/trpc/server/lib/toTRPCError"; - +import type { TrpcSessionUser } from "@calcom/trpc/server/types"; import { TRPCError } from "@trpc/server"; - import type { TGetClientForAuthorizationInputSchema } from "./getClientForAuthorization.schema"; type GetClientForAuthorizationOptions = { + ctx: { + user: NonNullable; + }; input: TGetClientForAuthorizationInputSchema; }; -export const getClientForAuthorizationHandler = async ({ input }: GetClientForAuthorizationOptions) => { +export const getClientForAuthorizationHandler = async ({ ctx, input }: GetClientForAuthorizationOptions) => { try { const { clientId, redirectUri } = input; const oAuthService = getOAuthService(); - const oAuthClient = await oAuthService.getClientForAuthorization(clientId, redirectUri); + const oAuthClient = await oAuthService.getClientForAuthorization(clientId, redirectUri, ctx.user.id); return { clientId: oAuthClient.clientId,