Skip to content

Commit c321a6c

Browse files
feat: owner can test non accepted OAuth client (calcom#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 39cc4aa. * Revert "fix: address Cubic AI review feedback on OAuth2 endpoints" This reverts commit 97bf593. * 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 a080e93. * Revert "fix: address Cubic AI review feedback on OAuth2 endpoints" This reverts commit 04986a1. * 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 416bef9. * 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>
1 parent 0eb2c15 commit c321a6c

12 files changed

Lines changed: 532 additions & 67 deletions

File tree

apps/api/v2/src/modules/auth/oauth2/controllers/oauth2.controller.e2e-spec.ts

Lines changed: 260 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { generateSecret } from "@calcom/platform-libraries";
22
import type { Membership, Team, User } from "@calcom/prisma/client";
3-
import { AccessScope, OAuthClientType } from "@calcom/prisma/enums";
4-
import { INestApplication } from "@nestjs/common";
5-
import { NestExpressApplication } from "@nestjs/platform-express";
6-
import { Test, TestingModule } from "@nestjs/testing";
7-
import { getToken } from "next-auth/jwt";
3+
import { AccessScope, OAuthClientStatus, OAuthClientType } from "@calcom/prisma/enums";
4+
import type { INestApplication } from "@nestjs/common";
5+
import type { NestExpressApplication } from "@nestjs/platform-express";
6+
import type { TestingModule } from "@nestjs/testing";
7+
import { Test } from "@nestjs/testing";
88
import request from "supertest";
9+
import { ApiKeysRepositoryFixture } from "test/fixtures/repository/api-keys.repository.fixture";
910
import { MembershipRepositoryFixture } from "test/fixtures/repository/membership.repository.fixture";
1011
import { OAuth2ClientRepositoryFixture } from "test/fixtures/repository/oauth2-client.repository.fixture";
1112
import { TeamRepositoryFixture } from "test/fixtures/repository/team.repository.fixture";
@@ -21,20 +22,16 @@ import { AuthModule } from "@/modules/auth/auth.module";
2122
import { PrismaModule } from "@/modules/prisma/prisma.module";
2223
import { UsersModule } from "@/modules/users/users.module";
2324

24-
// Mock next-auth/jwt getToken for ApiAuthStrategy NEXT_AUTH authentication
25+
// Mock next-auth/jwt getToken so the NextAuth fallback in ApiAuthStrategy doesn't crash in tests
2526
jest.mock("next-auth/jwt", () => ({
26-
getToken: jest.fn(),
27+
getToken: jest.fn().mockResolvedValue(null),
2728
}));
28-
const mockGetToken = getToken as jest.MockedFunction<typeof getToken>;
2929

3030
describe("OAuth2 Controller Endpoints", () => {
3131
describe("User Not Authenticated", () => {
3232
let appWithoutAuth: INestApplication;
3333

3434
beforeAll(async () => {
35-
// Mock getToken to return null for unauthenticated tests
36-
mockGetToken.mockResolvedValue(null);
37-
3835
const moduleRef = await Test.createTestingModule({
3936
providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter],
4037
imports: [AppModule, UsersModule, AuthModule, PrismaModule],
@@ -69,20 +66,23 @@ describe("OAuth2 Controller Endpoints", () => {
6966
});
7067
});
7168

72-
describe("User Authenticated", () => {
69+
describe("User Authenticated (non-owner)", () => {
7370
let app: INestApplication;
7471
let moduleRef: TestingModule;
7572

7673
let userRepositoryFixture: UserRepositoryFixture;
7774
let teamRepositoryFixture: TeamRepositoryFixture;
7875
let membershipRepositoryFixture: MembershipRepositoryFixture;
76+
let apiKeysRepositoryFixture: ApiKeysRepositoryFixture;
7977
let oAuthClientFixture: OAuth2ClientRepositoryFixture;
8078
let oAuthService: OAuthService;
8179

82-
let user: User;
80+
let clientOwner: User;
81+
let authenticatedUser: User;
8382
let team: Team;
8483
let membership: Membership;
8584
let oAuthClient: { clientId: string };
85+
let apiKeyString: string;
8686
let refreshToken: string;
8787

8888
const testClientId = `test-oauth-client-${randomString()}`;
@@ -96,7 +96,7 @@ describe("OAuth2 Controller Endpoints", () => {
9696
): Promise<string> {
9797
const result = await oAuthService.generateAuthorizationCode(
9898
testClientId,
99-
user.id,
99+
authenticatedUser.id,
100100
testRedirectUri,
101101
scopes,
102102
undefined,
@@ -107,12 +107,6 @@ describe("OAuth2 Controller Endpoints", () => {
107107
}
108108

109109
beforeAll(async () => {
110-
const userEmail = `oauth2-e2e-user-${randomString()}@api.com`;
111-
112-
// Mock getToken to return the user email for authenticated tests
113-
// This is needed because ApiAuthGuard uses ApiAuthStrategy which calls getToken from next-auth/jwt
114-
mockGetToken.mockResolvedValue({ email: userEmail });
115-
116110
moduleRef = await Test.createTestingModule({
117111
providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter],
118112
imports: [AppModule, UsersModule, AuthModule, PrismaModule],
@@ -125,20 +119,28 @@ describe("OAuth2 Controller Endpoints", () => {
125119
userRepositoryFixture = new UserRepositoryFixture(moduleRef);
126120
teamRepositoryFixture = new TeamRepositoryFixture(moduleRef);
127121
membershipRepositoryFixture = new MembershipRepositoryFixture(moduleRef);
122+
apiKeysRepositoryFixture = new ApiKeysRepositoryFixture(moduleRef);
128123
oAuthClientFixture = new OAuth2ClientRepositoryFixture(moduleRef);
129124
oAuthService = moduleRef.get(OAuthService);
130125

131-
user = await userRepositoryFixture.create({
132-
email: userEmail,
126+
clientOwner = await userRepositoryFixture.create({
127+
email: `oauth2-e2e-owner-${randomString()}@api.com`,
128+
});
129+
130+
authenticatedUser = await userRepositoryFixture.create({
131+
email: `oauth2-e2e-user-${randomString()}@api.com`,
133132
});
134133

134+
const { keyString } = await apiKeysRepositoryFixture.createApiKey(authenticatedUser.id, null);
135+
apiKeyString = keyString;
136+
135137
team = await teamRepositoryFixture.create({
136138
name: `oauth2-e2e-team-${randomString()}`,
137139
slug: `oauth2-e2e-team-${randomString()}`,
138140
});
139141

140142
membership = await membershipRepositoryFixture.create({
141-
user: { connect: { id: user.id } },
143+
user: { connect: { id: authenticatedUser.id } },
142144
team: { connect: { id: team.id } },
143145
role: "OWNER",
144146
accepted: true,
@@ -151,6 +153,7 @@ describe("OAuth2 Controller Endpoints", () => {
151153
redirectUri: testRedirectUri,
152154
clientSecret: hashedSecret,
153155
clientType: OAuthClientType.CONFIDENTIAL,
156+
userId: clientOwner.id,
154157
});
155158
});
156159

@@ -159,6 +162,7 @@ describe("OAuth2 Controller Endpoints", () => {
159162
it("should return OAuth client info for valid client ID", async () => {
160163
const response = await request(app.getHttpServer())
161164
.get(`/api/v2/auth/oauth2/clients/${testClientId}`)
165+
.set({ Authorization: `Bearer cal_test_${apiKeyString}` })
162166
.expect(200);
163167

164168
expect(response.body.status).toBe("success");
@@ -174,6 +178,7 @@ describe("OAuth2 Controller Endpoints", () => {
174178
it("should return 404 for non-existent client ID", async () => {
175179
await request(app.getHttpServer())
176180
.get("/api/v2/auth/oauth2/clients/non-existent-client-id")
181+
.set({ Authorization: `Bearer cal_test_${apiKeyString}` })
177182
.expect(404);
178183
});
179184
});
@@ -434,7 +439,238 @@ describe("OAuth2 Controller Endpoints", () => {
434439
await oAuthClientFixture.delete(oAuthClient.clientId);
435440
await membershipRepositoryFixture.delete(membership.id);
436441
await teamRepositoryFixture.delete(team.id);
437-
await userRepositoryFixture.delete(user.id);
442+
await userRepositoryFixture.delete(authenticatedUser.id);
443+
await userRepositoryFixture.delete(clientOwner.id);
444+
await app.close();
445+
});
446+
});
447+
448+
describe("Owner can use non-approved OAuth client", () => {
449+
let app: INestApplication;
450+
let moduleRef: TestingModule;
451+
452+
let userRepositoryFixture: UserRepositoryFixture;
453+
let teamRepositoryFixture: TeamRepositoryFixture;
454+
let membershipRepositoryFixture: MembershipRepositoryFixture;
455+
let apiKeysRepositoryFixture: ApiKeysRepositoryFixture;
456+
let oAuthClientFixture: OAuth2ClientRepositoryFixture;
457+
let oAuthService: OAuthService;
458+
459+
let owner: User;
460+
let team: Team;
461+
let membership: Membership;
462+
let pendingClient: { clientId: string };
463+
let rejectedClient: { clientId: string };
464+
let apiKeyString: string;
465+
466+
const pendingClientId = `test-pending-client-${randomString()}`;
467+
const rejectedClientId = `test-rejected-client-${randomString()}`;
468+
const testClientSecret = "test-secret-456";
469+
const testRedirectUri = "https://example.com/callback";
470+
471+
/** Generate a user-scoped authorization code (no teamSlug) so the owner's userId is in the token. */
472+
async function generateAuthCodeForClient(
473+
clientId: string,
474+
scopes: AccessScope[] = [AccessScope.READ_BOOKING]
475+
): Promise<string> {
476+
const result = await oAuthService.generateAuthorizationCode(
477+
clientId,
478+
owner.id,
479+
testRedirectUri,
480+
scopes
481+
);
482+
const redirectUrl = new URL(result.redirectUrl);
483+
return redirectUrl.searchParams.get("code") as string;
484+
}
485+
486+
beforeAll(async () => {
487+
moduleRef = await Test.createTestingModule({
488+
providers: [PrismaExceptionFilter, HttpExceptionFilter, ZodExceptionFilter],
489+
imports: [AppModule, UsersModule, AuthModule, PrismaModule],
490+
}).compile();
491+
492+
app = moduleRef.createNestApplication();
493+
bootstrap(app as NestExpressApplication);
494+
await app.init();
495+
496+
userRepositoryFixture = new UserRepositoryFixture(moduleRef);
497+
teamRepositoryFixture = new TeamRepositoryFixture(moduleRef);
498+
membershipRepositoryFixture = new MembershipRepositoryFixture(moduleRef);
499+
apiKeysRepositoryFixture = new ApiKeysRepositoryFixture(moduleRef);
500+
oAuthClientFixture = new OAuth2ClientRepositoryFixture(moduleRef);
501+
oAuthService = moduleRef.get(OAuthService);
502+
503+
owner = await userRepositoryFixture.create({
504+
email: `oauth2-owner-e2e-${randomString()}@api.com`,
505+
});
506+
507+
const { keyString } = await apiKeysRepositoryFixture.createApiKey(owner.id, null);
508+
apiKeyString = keyString;
509+
510+
team = await teamRepositoryFixture.create({
511+
name: `oauth2-owner-team-${randomString()}`,
512+
slug: `oauth2-owner-team-${randomString()}`,
513+
});
514+
515+
membership = await membershipRepositoryFixture.create({
516+
user: { connect: { id: owner.id } },
517+
team: { connect: { id: team.id } },
518+
role: "OWNER",
519+
accepted: true,
520+
});
521+
522+
const [hashedSecret] = generateSecret(testClientSecret);
523+
524+
pendingClient = await oAuthClientFixture.create({
525+
clientId: pendingClientId,
526+
name: "Pending OAuth Client",
527+
redirectUri: testRedirectUri,
528+
clientSecret: hashedSecret,
529+
clientType: OAuthClientType.CONFIDENTIAL,
530+
status: OAuthClientStatus.PENDING,
531+
userId: owner.id,
532+
});
533+
534+
rejectedClient = await oAuthClientFixture.create({
535+
clientId: rejectedClientId,
536+
name: "Rejected OAuth Client",
537+
redirectUri: testRedirectUri,
538+
clientSecret: hashedSecret,
539+
clientType: OAuthClientType.CONFIDENTIAL,
540+
status: OAuthClientStatus.REJECTED,
541+
userId: owner.id,
542+
});
543+
});
544+
545+
it("should return PENDING client info when authenticated as owner", async () => {
546+
const response = await request(app.getHttpServer())
547+
.get(`/api/v2/auth/oauth2/clients/${pendingClientId}`)
548+
.set({ Authorization: `Bearer cal_test_${apiKeyString}` })
549+
.expect(200);
550+
551+
expect(response.body.status).toBe("success");
552+
expect(response.body.data.client_id).toBe(pendingClientId);
553+
expect(response.body.data.name).toBe("Pending OAuth Client");
554+
});
555+
556+
it("should exchange authorization code for tokens with PENDING client owned by user", async () => {
557+
const code = await generateAuthCodeForClient(pendingClientId);
558+
559+
const response = await request(app.getHttpServer())
560+
.post("/api/v2/auth/oauth2/token")
561+
.type("form")
562+
.send({
563+
client_id: pendingClientId,
564+
grant_type: "authorization_code",
565+
code,
566+
client_secret: testClientSecret,
567+
redirect_uri: testRedirectUri,
568+
})
569+
.expect(200);
570+
571+
expect(response.body.access_token).toBeDefined();
572+
expect(response.body.refresh_token).toBeDefined();
573+
expect(response.body.token_type).toBe("bearer");
574+
});
575+
576+
it("should refresh tokens with PENDING client owned by user", async () => {
577+
const code = await generateAuthCodeForClient(pendingClientId);
578+
579+
const tokenResponse = await request(app.getHttpServer())
580+
.post("/api/v2/auth/oauth2/token")
581+
.type("form")
582+
.send({
583+
client_id: pendingClientId,
584+
grant_type: "authorization_code",
585+
code,
586+
client_secret: testClientSecret,
587+
redirect_uri: testRedirectUri,
588+
})
589+
.expect(200);
590+
591+
const response = await request(app.getHttpServer())
592+
.post("/api/v2/auth/oauth2/token")
593+
.type("form")
594+
.send({
595+
client_id: pendingClientId,
596+
grant_type: "refresh_token",
597+
refresh_token: tokenResponse.body.refresh_token,
598+
client_secret: testClientSecret,
599+
})
600+
.expect(200);
601+
602+
expect(response.body.access_token).toBeDefined();
603+
expect(response.body.refresh_token).toBeDefined();
604+
expect(response.body.token_type).toBe("bearer");
605+
});
606+
607+
it("should reject authorization code generation for REJECTED client even as owner", async () => {
608+
await expect(generateAuthCodeForClient(rejectedClientId)).rejects.toThrow();
609+
});
610+
611+
it("should reject token exchange when client becomes rejected", async () => {
612+
const code = await generateAuthCodeForClient(pendingClientId);
613+
614+
await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.REJECTED);
615+
616+
const response = await request(app.getHttpServer())
617+
.post("/api/v2/auth/oauth2/token")
618+
.type("form")
619+
.send({
620+
client_id: pendingClientId,
621+
grant_type: "authorization_code",
622+
code,
623+
client_secret: testClientSecret,
624+
redirect_uri: testRedirectUri,
625+
})
626+
.expect(401);
627+
628+
expect(response.body.error).toBe("unauthorized_client");
629+
expect(response.body.error_description).toBe("client_rejected");
630+
631+
await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.PENDING);
632+
});
633+
634+
it("should reject token refresh when client becomes rejected", async () => {
635+
const code = await generateAuthCodeForClient(pendingClientId);
636+
637+
const tokenResponse = await request(app.getHttpServer())
638+
.post("/api/v2/auth/oauth2/token")
639+
.type("form")
640+
.send({
641+
client_id: pendingClientId,
642+
grant_type: "authorization_code",
643+
code,
644+
client_secret: testClientSecret,
645+
redirect_uri: testRedirectUri,
646+
})
647+
.expect(200);
648+
649+
await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.REJECTED);
650+
651+
const response = await request(app.getHttpServer())
652+
.post("/api/v2/auth/oauth2/token")
653+
.type("form")
654+
.send({
655+
client_id: pendingClientId,
656+
grant_type: "refresh_token",
657+
refresh_token: tokenResponse.body.refresh_token,
658+
client_secret: testClientSecret,
659+
})
660+
.expect(401);
661+
662+
expect(response.body.error).toBe("unauthorized_client");
663+
expect(response.body.error_description).toBe("client_rejected");
664+
665+
await oAuthClientFixture.updateStatus(pendingClientId, OAuthClientStatus.PENDING);
666+
});
667+
668+
afterAll(async () => {
669+
await oAuthClientFixture.delete(pendingClient.clientId);
670+
await oAuthClientFixture.delete(rejectedClient.clientId);
671+
await membershipRepositoryFixture.delete(membership.id);
672+
await teamRepositoryFixture.delete(team.id);
673+
await userRepositoryFixture.delete(owner.id);
438674
await app.close();
439675
});
440676
});

0 commit comments

Comments
 (0)