Skip to content

Commit 00a82e7

Browse files
kentonvkentonv
authored
Merge pull request #4 from cloudflare/kenton/redirect-uri-requirement
Ask Claude to make redirect_uri optional on token exchanges with PKCE.
2 parents 6e3165a + baf42dc commit 00a82e7

File tree

2 files changed

+176
-135
lines changed

2 files changed

+176
-135
lines changed

‎__tests__/oauth-provider.test.ts

Lines changed: 156 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -91,62 +91,6 @@ class MockExecutionContext implements ExecutionContext {
9191
}
9292
}
9393

94-
// Mock FormData to enable testing token endpoint
95-
class MockFormData implements FormData {
96-
private data: Map<string, string> = new Map();
97-
98-
append(name: string, value: string | Blob, filename?: string): void {
99-
this.data.set(name, value.toString());
100-
}
101-
102-
delete(name: string): void {
103-
this.data.delete(name);
104-
}
105-
106-
get(name: string): FormDataEntryValue | null {
107-
return this.data.get(name) || null;
108-
}
109-
110-
getAll(name: string): FormDataEntryValue[] {
111-
const value = this.data.get(name);
112-
return value ? [value] : [];
113-
}
114-
115-
has(name: string): boolean {
116-
return this.data.has(name);
117-
}
118-
119-
set(name: string, value: string | Blob, filename?: string): void {
120-
this.data.set(name, value.toString());
121-
}
122-
123-
forEach(callbackfn: (value: FormDataEntryValue, key: string, parent: FormData) => void): void {
124-
this.data.forEach((value, key) => callbackfn(value, key, this));
125-
}
126-
127-
*entries(): IterableIterator<[string, FormDataEntryValue]> {
128-
for (const [key, value] of this.data.entries()) {
129-
yield [key, value];
130-
}
131-
}
132-
133-
*keys(): IterableIterator<string> {
134-
for (const key of this.data.keys()) {
135-
yield key;
136-
}
137-
}
138-
139-
*values(): IterableIterator<FormDataEntryValue> {
140-
for (const value of this.data.values()) {
141-
yield value;
142-
}
143-
}
144-
145-
[Symbol.iterator](): IterableIterator<[string, FormDataEntryValue]> {
146-
return this.entries();
147-
}
148-
}
149-
15094
// Simple API handler for testing
15195
class TestApiHandler extends WorkerEntrypoint {
15296
fetch(request: Request) {
@@ -606,21 +550,20 @@ describe('OAuthProvider', () => {
606550
const code = url.searchParams.get('code')!;
607551

608552
// Now exchange the code for tokens
609-
const formData = new MockFormData();
610-
formData.append('grant_type', 'authorization_code');
611-
formData.append('code', code);
612-
formData.append('redirect_uri', redirectUri);
613-
formData.append('client_id', clientId);
614-
formData.append('client_secret', clientSecret);
615-
616-
// Mock FormData in request
617-
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
618-
553+
// Use URLSearchParams which is proper for application/x-www-form-urlencoded
554+
const params = new URLSearchParams();
555+
params.append('grant_type', 'authorization_code');
556+
params.append('code', code);
557+
params.append('redirect_uri', redirectUri);
558+
params.append('client_id', clientId);
559+
params.append('client_secret', clientSecret);
560+
561+
// Use the URLSearchParams object as the body - correctly encoded for Content-Type: application/x-www-form-urlencoded
619562
const tokenRequest = createMockRequest(
620563
'https://example.com/oauth/token',
621564
'POST',
622565
{ 'Content-Type': 'application/x-www-form-urlencoded' },
623-
formData as unknown as FormData
566+
params.toString()
624567
);
625568

626569
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
@@ -646,6 +589,113 @@ describe('OAuthProvider', () => {
646589
expect(grant.refreshTokenId).toBeDefined(); // Refresh token should be added
647590
});
648591

592+
it('should reject token exchange without redirect_uri when not using PKCE', async () => {
593+
// First get an auth code
594+
const authRequest = createMockRequest(
595+
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
596+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
597+
`&scope=read%20write&state=xyz123`
598+
);
599+
600+
const authResponse = await oauthProvider.fetch(authRequest, mockEnv, mockCtx);
601+
const location = authResponse.headers.get('Location')!;
602+
const url = new URL(location);
603+
const code = url.searchParams.get('code')!;
604+
605+
// Now exchange the code without providing redirect_uri
606+
const params = new URLSearchParams();
607+
params.append('grant_type', 'authorization_code');
608+
params.append('code', code);
609+
// redirect_uri intentionally omitted
610+
params.append('client_id', clientId);
611+
params.append('client_secret', clientSecret);
612+
613+
const tokenRequest = createMockRequest(
614+
'https://example.com/oauth/token',
615+
'POST',
616+
{ 'Content-Type': 'application/x-www-form-urlencoded' },
617+
params.toString()
618+
);
619+
620+
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
621+
622+
// Should fail because redirect_uri is required when not using PKCE
623+
expect(tokenResponse.status).toBe(400);
624+
const error = await tokenResponse.json();
625+
expect(error.error).toBe('invalid_request');
626+
expect(error.error_description).toBe('redirect_uri is required when not using PKCE');
627+
});
628+
629+
// Helper function for PKCE tests
630+
function generateRandomString(length: number): string {
631+
const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
632+
let result = '';
633+
const values = new Uint8Array(length);
634+
crypto.getRandomValues(values);
635+
for (let i = 0; i < length; i++) {
636+
result += characters.charAt(values[i] % characters.length);
637+
}
638+
return result;
639+
}
640+
641+
// Helper function for PKCE tests
642+
function base64UrlEncode(str: string): string {
643+
return btoa(str)
644+
.replace(/\+/g, '-')
645+
.replace(/\//g, '_')
646+
.replace(/=/g, '');
647+
}
648+
649+
it('should accept token exchange without redirect_uri when using PKCE', async () => {
650+
// Generate PKCE code verifier and challenge
651+
const codeVerifier = generateRandomString(43); // Recommended length
652+
const encoder = new TextEncoder();
653+
const data = encoder.encode(codeVerifier);
654+
const hashBuffer = await crypto.subtle.digest('SHA-256', data);
655+
const hashArray = Array.from(new Uint8Array(hashBuffer));
656+
const codeChallenge = base64UrlEncode(String.fromCharCode(...hashArray));
657+
658+
// First get an auth code with PKCE
659+
const authRequest = createMockRequest(
660+
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
661+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
662+
`&scope=read%20write&state=xyz123` +
663+
`&code_challenge=${codeChallenge}&code_challenge_method=S256`
664+
);
665+
666+
const authResponse = await oauthProvider.fetch(authRequest, mockEnv, mockCtx);
667+
const location = authResponse.headers.get('Location')!;
668+
const url = new URL(location);
669+
const code = url.searchParams.get('code')!;
670+
671+
// Now exchange the code without providing redirect_uri
672+
const params = new URLSearchParams();
673+
params.append('grant_type', 'authorization_code');
674+
params.append('code', code);
675+
// redirect_uri intentionally omitted
676+
params.append('client_id', clientId);
677+
params.append('client_secret', clientSecret);
678+
params.append('code_verifier', codeVerifier);
679+
680+
const tokenRequest = createMockRequest(
681+
'https://example.com/oauth/token',
682+
'POST',
683+
{ 'Content-Type': 'application/x-www-form-urlencoded' },
684+
params.toString()
685+
);
686+
687+
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
688+
689+
// Should succeed because redirect_uri is optional when using PKCE
690+
expect(tokenResponse.status).toBe(200);
691+
692+
const tokens = await tokenResponse.json();
693+
expect(tokens.access_token).toBeDefined();
694+
expect(tokens.refresh_token).toBeDefined();
695+
expect(tokens.token_type).toBe('bearer');
696+
expect(tokens.expires_in).toBe(3600);
697+
});
698+
649699
it('should accept the access token for API requests', async () => {
650700
// Get an auth code
651701
const authRequest = createMockRequest(
@@ -659,20 +709,18 @@ describe('OAuthProvider', () => {
659709
const code = new URL(location).searchParams.get('code')!;
660710

661711
// Exchange for tokens
662-
const formData = new MockFormData();
663-
formData.append('grant_type', 'authorization_code');
664-
formData.append('code', code);
665-
formData.append('redirect_uri', redirectUri);
666-
formData.append('client_id', clientId);
667-
formData.append('client_secret', clientSecret);
668-
669-
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
712+
const params = new URLSearchParams();
713+
params.append('grant_type', 'authorization_code');
714+
params.append('code', code);
715+
params.append('redirect_uri', redirectUri);
716+
params.append('client_id', clientId);
717+
params.append('client_secret', clientSecret);
670718

671719
const tokenRequest = createMockRequest(
672720
'https://example.com/oauth/token',
673721
'POST',
674722
{ 'Content-Type': 'application/x-www-form-urlencoded' },
675-
formData as unknown as FormData
723+
params.toString()
676724
);
677725

678726
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
@@ -734,20 +782,18 @@ describe('OAuthProvider', () => {
734782
const code = new URL(location).searchParams.get('code')!;
735783

736784
// Exchange for tokens
737-
const formData = new MockFormData();
738-
formData.append('grant_type', 'authorization_code');
739-
formData.append('code', code);
740-
formData.append('redirect_uri', redirectUri);
741-
formData.append('client_id', clientId);
742-
formData.append('client_secret', clientSecret);
743-
744-
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
785+
const params = new URLSearchParams();
786+
params.append('grant_type', 'authorization_code');
787+
params.append('code', code);
788+
params.append('redirect_uri', redirectUri);
789+
params.append('client_id', clientId);
790+
params.append('client_secret', clientSecret);
745791

746792
const tokenRequest = createMockRequest(
747793
'https://example.com/oauth/token',
748794
'POST',
749795
{ 'Content-Type': 'application/x-www-form-urlencoded' },
750-
formData as unknown as FormData
796+
params.toString()
751797
);
752798

753799
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
@@ -761,19 +807,17 @@ describe('OAuthProvider', () => {
761807

762808
it('should issue new tokens with refresh token', async () => {
763809
// Use the refresh token to get a new access token
764-
const formData = new MockFormData();
765-
formData.append('grant_type', 'refresh_token');
766-
formData.append('refresh_token', refreshToken);
767-
formData.append('client_id', clientId);
768-
formData.append('client_secret', clientSecret);
769-
770-
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
810+
const params = new URLSearchParams();
811+
params.append('grant_type', 'refresh_token');
812+
params.append('refresh_token', refreshToken);
813+
params.append('client_id', clientId);
814+
params.append('client_secret', clientSecret);
771815

772816
const refreshRequest = createMockRequest(
773817
'https://example.com/oauth/token',
774818
'POST',
775819
{ 'Content-Type': 'application/x-www-form-urlencoded' },
776-
formData as unknown as FormData
820+
params.toString()
777821
);
778822

779823
const refreshResponse = await oauthProvider.fetch(refreshRequest, mockEnv, mockCtx);
@@ -800,39 +844,35 @@ describe('OAuthProvider', () => {
800844

801845
it('should allow using the previous refresh token once', async () => {
802846
// Use the refresh token to get a new access token (first refresh)
803-
const formData1 = new MockFormData();
804-
formData1.append('grant_type', 'refresh_token');
805-
formData1.append('refresh_token', refreshToken);
806-
formData1.append('client_id', clientId);
807-
formData1.append('client_secret', clientSecret);
808-
809-
vi.spyOn(Request.prototype, 'formData').mockResolvedValueOnce(formData1 as unknown as FormData);
847+
const params1 = new URLSearchParams();
848+
params1.append('grant_type', 'refresh_token');
849+
params1.append('refresh_token', refreshToken);
850+
params1.append('client_id', clientId);
851+
params1.append('client_secret', clientSecret);
810852

811853
const refreshRequest1 = createMockRequest(
812854
'https://example.com/oauth/token',
813855
'POST',
814856
{ 'Content-Type': 'application/x-www-form-urlencoded' },
815-
formData1 as unknown as FormData
857+
params1.toString()
816858
);
817859

818860
const refreshResponse1 = await oauthProvider.fetch(refreshRequest1, mockEnv, mockCtx);
819861
const newTokens1 = await refreshResponse1.json();
820862
const newRefreshToken = newTokens1.refresh_token;
821863

822864
// Now try to use the original refresh token again (simulating a retry after failure)
823-
const formData2 = new MockFormData();
824-
formData2.append('grant_type', 'refresh_token');
825-
formData2.append('refresh_token', refreshToken); // Original token
826-
formData2.append('client_id', clientId);
827-
formData2.append('client_secret', clientSecret);
828-
829-
vi.spyOn(Request.prototype, 'formData').mockResolvedValueOnce(formData2 as unknown as FormData);
865+
const params2 = new URLSearchParams();
866+
params2.append('grant_type', 'refresh_token');
867+
params2.append('refresh_token', refreshToken); // Original token
868+
params2.append('client_id', clientId);
869+
params2.append('client_secret', clientSecret);
830870

831871
const refreshRequest2 = createMockRequest(
832872
'https://example.com/oauth/token',
833873
'POST',
834874
{ 'Content-Type': 'application/x-www-form-urlencoded' },
835-
formData2 as unknown as FormData
875+
params2.toString()
836876
);
837877

838878
const refreshResponse2 = await oauthProvider.fetch(refreshRequest2, mockEnv, mockCtx);
@@ -892,20 +932,18 @@ describe('OAuthProvider', () => {
892932
const code = new URL(location).searchParams.get('code')!;
893933

894934
// Exchange for tokens
895-
const formData = new MockFormData();
896-
formData.append('grant_type', 'authorization_code');
897-
formData.append('code', code);
898-
formData.append('redirect_uri', redirectUri);
899-
formData.append('client_id', clientId);
900-
formData.append('client_secret', clientSecret);
901-
902-
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
935+
const params = new URLSearchParams();
936+
params.append('grant_type', 'authorization_code');
937+
params.append('code', code);
938+
params.append('redirect_uri', redirectUri);
939+
params.append('client_id', clientId);
940+
params.append('client_secret', clientSecret);
903941

904942
const tokenRequest = createMockRequest(
905943
'https://example.com/oauth/token',
906944
'POST',
907945
{ 'Content-Type': 'application/x-www-form-urlencoded' },
908-
formData as unknown as FormData
946+
params.toString()
909947
);
910948

911949
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);

0 commit comments

Comments
 (0)