Skip to content

Commit d954473

Browse files
committed
Mitigated PKCE Downgrade Attack
See https://datatracker.ietf.org/doc/html/rfc9700#name-pkce-downgrade-attack for more details
1 parent 3e3d0bf commit d954473

File tree

2 files changed

+64
-21
lines changed

2 files changed

+64
-21
lines changed

__tests__/oauth-provider.test.ts

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,13 @@ describe('OAuthProvider', () => {
206206
return new Response('Users API response', { status: 200 });
207207
}
208208
}
209-
209+
210210
class DocumentsApiHandler extends WorkerEntrypoint {
211211
fetch(request: Request) {
212212
return new Response('Documents API response', { status: 200 });
213213
}
214214
}
215-
215+
216216
// Create provider with multi-handler configuration
217217
const providerWithMultiHandler = new OAuthProvider({
218218
apiHandlers: {
@@ -225,96 +225,96 @@ describe('OAuthProvider', () => {
225225
clientRegistrationEndpoint: '/oauth/register', // Important for registering clients in the test
226226
scopesSupported: ['read', 'write'],
227227
});
228-
228+
229229
// Create a client and get an access token
230230
const clientData = {
231231
redirect_uris: ['https://client.example.com/callback'],
232232
client_name: 'Test Client',
233233
token_endpoint_auth_method: 'client_secret_basic',
234234
};
235-
235+
236236
const registerRequest = createMockRequest(
237237
'https://example.com/oauth/register',
238238
'POST',
239239
{ 'Content-Type': 'application/json' },
240240
JSON.stringify(clientData)
241241
);
242-
242+
243243
const registerResponse = await providerWithMultiHandler.fetch(registerRequest, mockEnv, mockCtx);
244244
const client = await registerResponse.json();
245245
const clientId = client.client_id;
246246
const clientSecret = client.client_secret;
247247
const redirectUri = 'https://client.example.com/callback';
248-
248+
249249
// Get an auth code
250250
const authRequest = createMockRequest(
251251
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
252-
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
253-
`&scope=read%20write&state=xyz123`
252+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
253+
`&scope=read%20write&state=xyz123`
254254
);
255-
255+
256256
const authResponse = await providerWithMultiHandler.fetch(authRequest, mockEnv, mockCtx);
257257
const location = authResponse.headers.get('Location')!;
258258
const code = new URL(location).searchParams.get('code')!;
259-
259+
260260
// Exchange for tokens
261261
const params = new URLSearchParams();
262262
params.append('grant_type', 'authorization_code');
263263
params.append('code', code);
264264
params.append('redirect_uri', redirectUri);
265265
params.append('client_id', clientId);
266266
params.append('client_secret', clientSecret);
267-
267+
268268
const tokenRequest = createMockRequest(
269269
'https://example.com/oauth/token',
270270
'POST',
271271
{ 'Content-Type': 'application/x-www-form-urlencoded' },
272272
params.toString()
273273
);
274-
274+
275275
const tokenResponse = await providerWithMultiHandler.fetch(tokenRequest, mockEnv, mockCtx);
276276
const tokens = await tokenResponse.json();
277277
const accessToken = tokens.access_token;
278-
278+
279279
// Make requests to different API routes
280280
const usersApiRequest = createMockRequest('https://example.com/api/users/profile', 'GET', {
281281
Authorization: `Bearer ${accessToken}`,
282282
});
283-
283+
284284
const documentsApiRequest = createMockRequest('https://example.com/api/documents/list', 'GET', {
285285
Authorization: `Bearer ${accessToken}`,
286286
});
287-
287+
288288
// Request to Users API should be handled by UsersApiHandler
289289
const usersResponse = await providerWithMultiHandler.fetch(usersApiRequest, mockEnv, mockCtx);
290290
expect(usersResponse.status).toBe(200);
291291
expect(await usersResponse.text()).toBe('Users API response');
292-
292+
293293
// Request to Documents API should be handled by DocumentsApiHandler
294294
const documentsResponse = await providerWithMultiHandler.fetch(documentsApiRequest, mockEnv, mockCtx);
295295
expect(documentsResponse.status).toBe(200);
296296
expect(await documentsResponse.text()).toBe('Documents API response');
297297
});
298-
298+
299299
it('should throw an error when both single-handler and multi-handler configs are provided', () => {
300300
expect(() => {
301301
new OAuthProvider({
302302
apiRoute: '/api/',
303303
apiHandler: {
304-
fetch: () => Promise.resolve(new Response())
304+
fetch: () => Promise.resolve(new Response()),
305305
},
306306
apiHandlers: {
307307
'/api/users/': {
308-
fetch: () => Promise.resolve(new Response())
309-
}
308+
fetch: () => Promise.resolve(new Response()),
309+
},
310310
},
311311
defaultHandler: testDefaultHandler,
312312
authorizeEndpoint: '/authorize',
313313
tokenEndpoint: '/oauth/token',
314314
});
315315
}).toThrow('Cannot use both apiRoute/apiHandler and apiHandlers');
316316
});
317-
317+
318318
it('should throw an error when neither single-handler nor multi-handler config is provided', () => {
319319
expect(() => {
320320
new OAuthProvider({
@@ -756,6 +756,44 @@ describe('OAuthProvider', () => {
756756
expect(error.error_description).toBe('redirect_uri is required when not using PKCE');
757757
});
758758

759+
it('should reject token exchange with code_verifier when PKCE was not used in authorization', async () => {
760+
// First get an auth code WITHOUT using PKCE
761+
const authRequest = createMockRequest(
762+
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
763+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
764+
`&scope=read%20write&state=xyz123`
765+
);
766+
767+
const authResponse = await oauthProvider.fetch(authRequest, mockEnv, mockCtx);
768+
const location = authResponse.headers.get('Location')!;
769+
const url = new URL(location);
770+
const code = url.searchParams.get('code')!;
771+
772+
// Now exchange the code and incorrectly provide a code_verifier
773+
const params = new URLSearchParams();
774+
params.append('grant_type', 'authorization_code');
775+
params.append('code', code);
776+
params.append('redirect_uri', redirectUri);
777+
params.append('client_id', clientId);
778+
params.append('client_secret', clientSecret);
779+
params.append('code_verifier', 'some_random_verifier_that_wasnt_used_in_auth');
780+
781+
const tokenRequest = createMockRequest(
782+
'https://example.com/oauth/token',
783+
'POST',
784+
{ 'Content-Type': 'application/x-www-form-urlencoded' },
785+
params.toString()
786+
);
787+
788+
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
789+
790+
// Should fail because code_verifier is provided but PKCE wasn't used in authorization
791+
expect(tokenResponse.status).toBe(400);
792+
const error = await tokenResponse.json();
793+
expect(error.error).toBe('invalid_request');
794+
expect(error.error_description).toBe('code_verifier provided for a flow that did not use PKCE');
795+
});
796+
759797
// Helper function for PKCE tests
760798
function generateRandomString(length: number): string {
761799
const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';

src/oauth-provider.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,11 @@ class OAuthProviderImpl {
12351235
return this.createErrorResponse('invalid_grant', 'Invalid redirect URI');
12361236
}
12371237

1238+
// Reject if code_verifier is provided but PKCE wasn't used in authorization
1239+
if (!isPkceEnabled && codeVerifier) {
1240+
return this.createErrorResponse('invalid_request', 'code_verifier provided for a flow that did not use PKCE');
1241+
}
1242+
12381243
// Verify PKCE code_verifier if code_challenge was provided during authorization
12391244
if (isPkceEnabled) {
12401245
if (!codeVerifier) {

0 commit comments

Comments
 (0)