Skip to content

Commit b90192a

Browse files
committed
Add skipLocalPkceValidation param and create providers folder
1 parent 8d7b387 commit b90192a

File tree

5 files changed

+24
-23
lines changed

5 files changed

+24
-23
lines changed

src/server/auth/handlers/token.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import supertest from 'supertest';
77
import * as pkceChallenge from 'pkce-challenge';
88
import { InvalidGrantError, InvalidTokenError } from '../errors.js';
99
import { AuthInfo } from '../types.js';
10-
import { ProxyOAuthServerProvider } from '../proxyProvider.js';
10+
import { ProxyOAuthServerProvider } from '../providers/proxyProvider.js';
1111

1212
// Mock pkce-challenge
1313
jest.mock('pkce-challenge', () => ({

src/server/auth/handlers/token.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
TooManyRequestsError,
1515
OAuthError
1616
} from "../errors.js";
17-
import { ProxyOAuthServerProvider } from "../proxyProvider.js";
1817

1918
export type TokenHandlerOptions = {
2019
provider: OAuthServerProvider;
@@ -91,9 +90,10 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
9190

9291
const { code, code_verifier } = parseResult.data;
9392

94-
const skipLocalPkceValidation = provider instanceof ProxyOAuthServerProvider ? provider.skipLocalPkceValidation : false;
93+
const skipLocalPkceValidation = provider.skipLocalPkceValidation;
9594

96-
// Perform local PKCE validation unless explicitly delegated (e.g. by proxy provider)
95+
// Perform local PKCE validation unless explicitly skipped
96+
// (e.g. to validate code_verifier in upstream server)
9797
if (!skipLocalPkceValidation) {
9898
const codeChallenge = await provider.challengeForAuthorizationCode(client, code);
9999
if (!(await verifyChallenge(code_verifier, codeChallenge))) {

src/server/auth/provider.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,13 @@ export interface OAuthServerProvider {
5454
* If the given token is invalid or already revoked, this method should do nothing.
5555
*/
5656
revokeToken?(client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest): Promise<void>;
57+
58+
/**
59+
* Whether to skip local PKCE validation.
60+
*
61+
* If true, the server will not perform PKCE validation locally and will pass the code_verifier to the upstream server.
62+
*
63+
* NOTE: This should only be true if the upstream server is performing the actual PKCE validation.
64+
*/
65+
skipLocalPkceValidation?: boolean;
5766
}

src/server/auth/proxyProvider.test.ts renamed to src/server/auth/providers/proxyProvider.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { Response } from "express";
22
import { ProxyOAuthServerProvider, ProxyOptions } from "./proxyProvider.js";
3-
import { AuthInfo } from "./types.js";
4-
import { OAuthClientInformationFull, OAuthTokens } from "../../shared/auth.js";
5-
import { ServerError } from "./errors.js";
6-
import { InvalidTokenError } from "./errors.js";
7-
import { InsufficientScopeError } from "./errors.js";
3+
import { AuthInfo } from "../types.js";
4+
import { OAuthClientInformationFull, OAuthTokens } from "../../../shared/auth.js";
5+
import { ServerError } from "../errors.js";
6+
import { InvalidTokenError } from "../errors.js";
7+
import { InsufficientScopeError } from "../errors.js";
88

99
describe("Proxy OAuth Server Provider", () => {
1010
// Mock client data

src/server/auth/proxyProvider.ts renamed to src/server/auth/providers/proxyProvider.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { Response } from "express";
2-
import { OAuthRegisteredClientsStore } from "./clients.js";
2+
import { OAuthRegisteredClientsStore } from "../clients.js";
33
import {
44
OAuthClientInformationFull,
55
OAuthClientInformationFullSchema,
66
OAuthTokenRevocationRequest,
77
OAuthTokens,
88
OAuthTokensSchema,
9-
} from "./../../shared/auth.js";
10-
import { AuthInfo } from "./types.js";
11-
import { AuthorizationParams, OAuthServerProvider } from "./provider.js";
12-
import { ServerError } from "./errors.js";
9+
} from "../../../shared/auth.js";
10+
import { AuthInfo } from "../types.js";
11+
import { AuthorizationParams, OAuthServerProvider } from "../provider.js";
12+
import { ServerError } from "../errors.js";
1313

1414
export type ProxyEndpoints = {
1515
authorizationUrl: string;
@@ -44,15 +44,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
4444
protected readonly _verifyAccessToken: (token: string) => Promise<AuthInfo>;
4545
protected readonly _getClient: (clientId: string) => Promise<OAuthClientInformationFull | undefined>;
4646

47-
/**
48-
* Always true for proxy providers since PKCE validation happens at the upstream server.
49-
* Can consider adding to the base OAuthServerProvider interface if it becomes useful elsewhere.
50-
* This ensures that:
51-
* 1. We skip local PKCE validation (which would fail since we don't store challenges)
52-
* 2. The code_verifier is still passed through to the upstream server
53-
* 3. The upstream server performs the actual PKCE validation
54-
*/
55-
readonly skipLocalPkceValidation = true;
47+
skipLocalPkceValidation = true;
5648

5749
revokeToken?: (
5850
client: OAuthClientInformationFull,

0 commit comments

Comments
 (0)