Skip to content

Commit 73b2760

Browse files
CarinaWolliCarinaWolli
CarinaWolli
and
CarinaWolli
authored
fix: remove pkce check for refreshToken endpoint (calcom#26050)
* remove pkce check for refreshToken endpoint * adjust e2e tests --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
1 parent 9c223fc commit 73b2760

2 files changed

Lines changed: 4 additions & 225 deletions

File tree

‎apps/web/app/api/auth/oauth/refreshToken/route.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import prisma from "@calcom/prisma";
1010
import type { OAuthTokenPayload } from "@calcom/types/oauth";
1111

1212
async function handler(req: NextRequest) {
13-
const { client_id, client_secret, grant_type, refresh_token, code_verifier } = await parseUrlFormData(req);
13+
const { client_id, client_secret, grant_type, refresh_token } = await parseUrlFormData(req);
1414

1515
if (!process.env.CALENDSO_ENCRYPTION_KEY) {
1616
return NextResponse.json({ message: "CALENDSO_ENCRYPTION_KEY is not set" }, { status: 500 });
@@ -62,11 +62,6 @@ async function handler(req: NextRequest) {
6262
return NextResponse.json({ error: "invalid_grant" }, { status: 400 });
6363
}
6464

65-
const pkceError = OAuthService.verifyPKCE(client, decodedRefreshToken, code_verifier);
66-
if (pkceError) {
67-
return NextResponse.json({ error: pkceError.error }, { status: pkceError.status });
68-
}
69-
7065
const accessTokenExpiresIn = 1800; // 30 minutes
7166

7267
const payloadAuthToken: OAuthTokenPayload = {
@@ -83,11 +78,6 @@ async function handler(req: NextRequest) {
8378
scope: decodedRefreshToken.scope,
8479
token_type: "Refresh Token",
8580
clientId: client_id,
86-
// Preserve PKCE information for any client that used PKCE originally
87-
...(decodedRefreshToken.codeChallenge && {
88-
codeChallenge: decodedRefreshToken.codeChallenge,
89-
codeChallengeMethod: decodedRefreshToken.codeChallengeMethod,
90-
}),
9181
};
9282

9383
const access_token = jwt.sign(payloadAuthToken, secretKey, {

‎apps/web/playwright/oauth-provider.e2e.ts‎

Lines changed: 3 additions & 214 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ test.describe("OAuth Provider - PKCE (Public Clients)", () => {
423423
expect(url.searchParams.get("code")).toBeNull();
424424
});
425425

426-
test("should refresh tokens for PUBLIC client with valid PKCE", async ({ page, users }) => {
426+
test("should refresh tokens for PUBLIC client", async ({ page, users }) => {
427427
const user = await users.create({ username: "test user refresh", name: "test user refresh" });
428428
await user.apiLogin();
429429

@@ -470,12 +470,11 @@ test.describe("OAuth Provider - PKCE (Public Clients)", () => {
470470
expect(tokenData.access_token).toBeDefined();
471471
expect(tokenData.refresh_token).toBeDefined();
472472

473-
// Now test refresh token with PKCE
473+
// Refresh token - NO PKCE needed
474474
const refreshTokenForm = new URLSearchParams();
475475
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
476476
refreshTokenForm.append("client_id", publicClient.clientId);
477477
refreshTokenForm.append("grant_type", "refresh_token");
478-
refreshTokenForm.append("code_verifier", pkce.codeVerifier); // PUBLIC clients need code_verifier
479478

480479
const refreshTokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/refreshToken`, {
481480
body: refreshTokenForm.toString(),
@@ -505,141 +504,6 @@ test.describe("OAuth Provider - PKCE (Public Clients)", () => {
505504
const meData = await meResponse.json();
506505
expect(meData.username.startsWith("test user refresh")).toBe(true);
507506
});
508-
509-
test("should reject PUBLIC client refresh token without code_verifier", async ({ page, users }) => {
510-
const user = await users.create({
511-
username: "test user refresh no pkce",
512-
name: "test user refresh no pkce",
513-
});
514-
await user.apiLogin();
515-
516-
// Generate PKCE values
517-
const pkce = generatePKCE();
518-
519-
// Get tokens first
520-
await page.goto(
521-
`auth/oauth2/authorize?client_id=${publicClient.clientId}&redirect_uri=${publicClient.redirectUri}&response_type=code&scope=READ_PROFILE&state=1234&code_challenge=${pkce.codeChallenge}&code_challenge_method=${pkce.codeChallengeMethod}`
522-
);
523-
await page.getByTestId("allow-button").click();
524-
525-
await page.waitForFunction(() => {
526-
return window.location.href.startsWith("https://example.com");
527-
});
528-
529-
// Assert URL to catch unexpected redirects
530-
await expect(page).toHaveURL(/^https:\/\/example\.com/);
531-
expect(page.url()).toContain("code=");
532-
expect(page.url()).toContain("state=1234");
533-
534-
const url = new URL(page.url());
535-
const code = url.searchParams.get("code");
536-
537-
const tokenForm = new URLSearchParams();
538-
tokenForm.append("code", code ?? "");
539-
tokenForm.append("client_id", publicClient.clientId);
540-
tokenForm.append("grant_type", "authorization_code");
541-
tokenForm.append("redirect_uri", publicClient.redirectUri);
542-
tokenForm.append("code_verifier", pkce.codeVerifier);
543-
544-
const tokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/token`, {
545-
body: tokenForm.toString(),
546-
method: "POST",
547-
headers: {
548-
"Content-Type": "application/x-www-form-urlencoded",
549-
},
550-
});
551-
552-
const tokenData = await tokenResponse.json();
553-
expect(tokenResponse.status).toBe(200);
554-
555-
// Now try refresh WITHOUT code_verifier (should fail)
556-
const refreshTokenForm = new URLSearchParams();
557-
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
558-
refreshTokenForm.append("client_id", publicClient.clientId);
559-
refreshTokenForm.append("grant_type", "refresh_token");
560-
// Missing code_verifier!
561-
562-
const refreshTokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/refreshToken`, {
563-
body: refreshTokenForm.toString(),
564-
method: "POST",
565-
headers: {
566-
"Content-Type": "application/x-www-form-urlencoded",
567-
},
568-
});
569-
570-
const refreshTokenData = await refreshTokenResponse.json();
571-
572-
expect(refreshTokenResponse.status).toBe(400);
573-
expect(refreshTokenData.error).toBe("invalid_request");
574-
});
575-
576-
test("should reject PUBLIC client refresh token with invalid code_verifier", async ({ page, users }) => {
577-
const user = await users.create({
578-
username: "test user refresh wrong pkce",
579-
name: "test user refresh wrong pkce",
580-
});
581-
await user.apiLogin();
582-
583-
// Generate PKCE values
584-
const pkce = generatePKCE();
585-
const wrongVerifier = randomBytes(32).toString("base64url");
586-
587-
// Get tokens first
588-
await page.goto(
589-
`auth/oauth2/authorize?client_id=${publicClient.clientId}&redirect_uri=${publicClient.redirectUri}&response_type=code&scope=READ_PROFILE&state=1234&code_challenge=${pkce.codeChallenge}&code_challenge_method=${pkce.codeChallengeMethod}`
590-
);
591-
await page.getByTestId("allow-button").click();
592-
593-
await page.waitForFunction(() => {
594-
return window.location.href.startsWith("https://example.com");
595-
});
596-
597-
// Assert URL to catch unexpected redirects
598-
await expect(page).toHaveURL(/^https:\/\/example\.com/);
599-
expect(page.url()).toContain("code=");
600-
expect(page.url()).toContain("state=1234");
601-
602-
const url = new URL(page.url());
603-
const code = url.searchParams.get("code");
604-
605-
const tokenForm = new URLSearchParams();
606-
tokenForm.append("code", code ?? "");
607-
tokenForm.append("client_id", publicClient.clientId);
608-
tokenForm.append("grant_type", "authorization_code");
609-
tokenForm.append("redirect_uri", publicClient.redirectUri);
610-
tokenForm.append("code_verifier", pkce.codeVerifier);
611-
612-
const tokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/token`, {
613-
body: tokenForm.toString(),
614-
method: "POST",
615-
headers: {
616-
"Content-Type": "application/x-www-form-urlencoded",
617-
},
618-
});
619-
620-
const tokenData = await tokenResponse.json();
621-
expect(tokenResponse.status).toBe(200);
622-
623-
// Now try refresh with WRONG code_verifier (should fail)
624-
const refreshTokenForm = new URLSearchParams();
625-
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
626-
refreshTokenForm.append("client_id", publicClient.clientId);
627-
refreshTokenForm.append("grant_type", "refresh_token");
628-
refreshTokenForm.append("code_verifier", wrongVerifier); // Wrong verifier!
629-
630-
const refreshTokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/refreshToken`, {
631-
body: refreshTokenForm.toString(),
632-
method: "POST",
633-
headers: {
634-
"Content-Type": "application/x-www-form-urlencoded",
635-
},
636-
});
637-
638-
const refreshTokenData = await refreshTokenResponse.json();
639-
640-
expect(refreshTokenResponse.status).toBe(400);
641-
expect(refreshTokenData.error).toBe("invalid_grant");
642-
});
643507
});
644508

645509
test.describe("OAuth Provider - PKCE with CONFIDENTIAL Clients (Enhanced Security)", () => {
@@ -710,13 +574,11 @@ test.describe("OAuth Provider - PKCE with CONFIDENTIAL Clients (Enhanced Securit
710574
const meData = await meResponse.json();
711575
expect(meData.username.startsWith("test user conf pkce")).toBe(true);
712576

713-
// Test refresh with both client_secret and code_verifier (enhanced security)
714577
const refreshTokenForm = new URLSearchParams();
715578
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
716579
refreshTokenForm.append("client_id", client.clientId);
717580
refreshTokenForm.append("client_secret", client.orginalSecret);
718581
refreshTokenForm.append("grant_type", "refresh_token");
719-
refreshTokenForm.append("code_verifier", pkce.codeVerifier);
720582

721583
const refreshTokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/refreshToken`, {
722584
body: refreshTokenForm.toString(),
@@ -733,79 +595,6 @@ test.describe("OAuth Provider - PKCE with CONFIDENTIAL Clients (Enhanced Securit
733595
expect(refreshTokenData.refresh_token).toBeDefined();
734596
});
735597

736-
test("should reject CONFIDENTIAL client refresh without code_verifier when PKCE was used", async ({
737-
page,
738-
users,
739-
}) => {
740-
const user = await users.create({
741-
username: "test user conf no verifier",
742-
name: "test user conf no verifier",
743-
});
744-
await user.apiLogin();
745-
746-
// Generate PKCE values for initial authorization
747-
const pkce = generatePKCE();
748-
749-
// Authorization with PKCE
750-
await page.goto(
751-
`auth/oauth2/authorize?client_id=${client.clientId}&redirect_uri=${client.redirectUri}&response_type=code&scope=READ_PROFILE&state=1234&code_challenge=${pkce.codeChallenge}&code_challenge_method=${pkce.codeChallengeMethod}`
752-
);
753-
await page.getByTestId("allow-button").click();
754-
755-
await page.waitForFunction(() => {
756-
return window.location.href.startsWith("https://example.com");
757-
});
758-
759-
// Assert URL to catch unexpected redirects
760-
await expect(page).toHaveURL(/^https:\/\/example\.com/);
761-
expect(page.url()).toContain("code=");
762-
expect(page.url()).toContain("state=1234");
763-
764-
const url = new URL(page.url());
765-
const code = url.searchParams.get("code");
766-
767-
// Token exchange with both credentials
768-
const tokenForm = new URLSearchParams();
769-
tokenForm.append("code", code ?? "");
770-
tokenForm.append("client_id", client.clientId);
771-
tokenForm.append("client_secret", client.orginalSecret);
772-
tokenForm.append("grant_type", "authorization_code");
773-
tokenForm.append("redirect_uri", client.redirectUri);
774-
tokenForm.append("code_verifier", pkce.codeVerifier);
775-
776-
const tokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/token`, {
777-
body: tokenForm.toString(),
778-
method: "POST",
779-
headers: {
780-
"Content-Type": "application/x-www-form-urlencoded",
781-
},
782-
});
783-
784-
const tokenData = await tokenResponse.json();
785-
expect(tokenResponse.status).toBe(200);
786-
787-
// Test refresh with ONLY client_secret (no code_verifier) - should FAIL since PKCE was used originally
788-
const refreshTokenForm = new URLSearchParams();
789-
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
790-
refreshTokenForm.append("client_id", client.clientId);
791-
refreshTokenForm.append("client_secret", client.orginalSecret);
792-
refreshTokenForm.append("grant_type", "refresh_token");
793-
// Intentionally not providing code_verifier
794-
795-
const refreshTokenResponse = await fetch(`${WEBAPP_URL}/api/auth/oauth/refreshToken`, {
796-
body: refreshTokenForm.toString(),
797-
method: "POST",
798-
headers: {
799-
"Content-Type": "application/x-www-form-urlencoded",
800-
},
801-
});
802-
803-
const refreshTokenData = await refreshTokenResponse.json();
804-
805-
expect(refreshTokenResponse.status).toBe(400);
806-
expect(refreshTokenData.error).toBe("invalid_request");
807-
});
808-
809598
test("should allow CONFIDENTIAL client refresh with only client_secret when PKCE was NOT used", async ({
810599
page,
811600
users,
@@ -850,7 +639,7 @@ test.describe("OAuth Provider - PKCE with CONFIDENTIAL Clients (Enhanced Securit
850639
const tokenData = await tokenResponse.json();
851640
expect(tokenResponse.status).toBe(200);
852641

853-
// Refresh with ONLY client_secret - should work since PKCE was never used
642+
// Refresh with client_secret
854643
const refreshTokenForm = new URLSearchParams();
855644
refreshTokenForm.append("refresh_token", tokenData.refresh_token);
856645
refreshTokenForm.append("client_id", client.clientId);

0 commit comments

Comments
 (0)