Skip to content

Commit 7d3fdde

Browse files
committed
Ask Claude to make redirect_uri optional on token exchanges with PKCE.
The current OAuth 2.1 draft section 10.2 says: > 10.2. Redirect URI Parameter in Token Request > > In OAuth 2.0, the request to the token endpoint in the authorization code flow (section 4.1.3 of [RFC6749]) contains an optional redirect_uri parameter. The parameter was intended to prevent an authorization code injection attack, and was required if the redirect_uri parameter was sent in the original authorization request. The authorization request only required the redirect_uri parameter if multiple redirect URIs were registered to the specific client. However, in practice, many authorization server implementations required the redirect_uri parameter in the authorization request even if only one was registered, leading the redirect_uri parameter to be required at the token endpoint as well. > > In OAuth 2.1, authorization code injection is prevented by the code_challenge and code_verifier parameters, making the inclusion of the redirect_uri parameter serve no purpose in the token request. As such, it has been removed. > > For backwards compatibility of an authorization server wishing to support both OAuth 2.0 and OAuth 2.1 clients, the authorization server MUST allow clients to send the redirect_uri parameter in the token request (Section 4.1.3), and MUST enforce the parameter as described in [RFC6749]. The authorization server can use the client_id in the request to determine whether to enforce this behavior for the specific client that it knows will be using the older OAuth 2.0 behavior. > > A client following only the OAuth 2.1 recommendations will not send the redirect_uri in the token request, and therefore will not be compatible with an authorization server that expects the parameter in the token request. I first had a conversation with Claude about this requirement. Claude first insisted that redirect_uri was still required under OAuth 2.1. I quoted the section above, and Claude then agreed it is not required. Claude said we'd need to add a way to distinguish 2.0 client from 2.1. I proposed we just base it on whether they are using PKCE, since that seems to be the reason for the requirement change, and Claude agreed.
1 parent 6e3165a commit 7d3fdde

File tree

2 files changed

+133
-17
lines changed

2 files changed

+133
-17
lines changed

__tests__/oauth-provider.test.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,119 @@ describe('OAuthProvider', () => {
646646
expect(grant.refreshTokenId).toBeDefined(); // Refresh token should be added
647647
});
648648

649+
it('should reject token exchange without redirect_uri when not using PKCE', async () => {
650+
// First get an auth code
651+
const authRequest = createMockRequest(
652+
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
653+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
654+
`&scope=read%20write&state=xyz123`
655+
);
656+
657+
const authResponse = await oauthProvider.fetch(authRequest, mockEnv, mockCtx);
658+
const location = authResponse.headers.get('Location')!;
659+
const url = new URL(location);
660+
const code = url.searchParams.get('code')!;
661+
662+
// Now exchange the code without providing redirect_uri
663+
const formData = new MockFormData();
664+
formData.append('grant_type', 'authorization_code');
665+
formData.append('code', code);
666+
// redirect_uri intentionally omitted
667+
formData.append('client_id', clientId);
668+
formData.append('client_secret', clientSecret);
669+
670+
// Mock FormData in request
671+
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
672+
673+
const tokenRequest = createMockRequest(
674+
'https://example.com/oauth/token',
675+
'POST',
676+
{ 'Content-Type': 'application/x-www-form-urlencoded' },
677+
formData as unknown as FormData
678+
);
679+
680+
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
681+
682+
// Should fail because redirect_uri is required when not using PKCE
683+
expect(tokenResponse.status).toBe(400);
684+
const error = await tokenResponse.json();
685+
expect(error.error).toBe('invalid_request');
686+
expect(error.error_description).toBe('redirect_uri is required when not using PKCE');
687+
});
688+
689+
// Helper function for PKCE tests
690+
function generateRandomString(length: number): string {
691+
const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
692+
let result = '';
693+
const values = new Uint8Array(length);
694+
crypto.getRandomValues(values);
695+
for (let i = 0; i < length; i++) {
696+
result += characters.charAt(values[i] % characters.length);
697+
}
698+
return result;
699+
}
700+
701+
// Helper function for PKCE tests
702+
function base64UrlEncode(str: string): string {
703+
return btoa(str)
704+
.replace(/\+/g, '-')
705+
.replace(/\//g, '_')
706+
.replace(/=/g, '');
707+
}
708+
709+
it('should accept token exchange without redirect_uri when using PKCE', async () => {
710+
// Generate PKCE code verifier and challenge
711+
const codeVerifier = generateRandomString(43); // Recommended length
712+
const encoder = new TextEncoder();
713+
const data = encoder.encode(codeVerifier);
714+
const hashBuffer = await crypto.subtle.digest('SHA-256', data);
715+
const hashArray = Array.from(new Uint8Array(hashBuffer));
716+
const codeChallenge = base64UrlEncode(String.fromCharCode(...hashArray));
717+
718+
// First get an auth code with PKCE
719+
const authRequest = createMockRequest(
720+
`https://example.com/authorize?response_type=code&client_id=${clientId}` +
721+
`&redirect_uri=${encodeURIComponent(redirectUri)}` +
722+
`&scope=read%20write&state=xyz123` +
723+
`&code_challenge=${codeChallenge}&code_challenge_method=S256`
724+
);
725+
726+
const authResponse = await oauthProvider.fetch(authRequest, mockEnv, mockCtx);
727+
const location = authResponse.headers.get('Location')!;
728+
const url = new URL(location);
729+
const code = url.searchParams.get('code')!;
730+
731+
// Now exchange the code without providing redirect_uri
732+
const formData = new MockFormData();
733+
formData.append('grant_type', 'authorization_code');
734+
formData.append('code', code);
735+
// redirect_uri intentionally omitted
736+
formData.append('client_id', clientId);
737+
formData.append('client_secret', clientSecret);
738+
formData.append('code_verifier', codeVerifier);
739+
740+
// Mock FormData in request
741+
vi.spyOn(Request.prototype, 'formData').mockResolvedValue(formData as unknown as FormData);
742+
743+
const tokenRequest = createMockRequest(
744+
'https://example.com/oauth/token',
745+
'POST',
746+
{ 'Content-Type': 'application/x-www-form-urlencoded' },
747+
formData as unknown as FormData
748+
);
749+
750+
const tokenResponse = await oauthProvider.fetch(tokenRequest, mockEnv, mockCtx);
751+
752+
// Should succeed because redirect_uri is optional when using PKCE
753+
expect(tokenResponse.status).toBe(200);
754+
755+
const tokens = await tokenResponse.json();
756+
expect(tokens.access_token).toBeDefined();
757+
expect(tokens.refresh_token).toBeDefined();
758+
expect(tokens.token_type).toBe('bearer');
759+
expect(tokens.expires_in).toBe(3600);
760+
});
761+
649762
it('should accept the access token for API requests', async () => {
650763
// Get an auth code
651764
const authRequest = createMockRequest(

oauth-provider.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,22 +1036,6 @@ class OAuthProviderImpl {
10361036
);
10371037
}
10381038

1039-
// OAuth 2.1 requires redirect_uri parameter
1040-
if (!redirectUri) {
1041-
return createErrorResponse(
1042-
'invalid_request',
1043-
'redirect_uri is required'
1044-
);
1045-
}
1046-
1047-
// OAuth 2.1 requires exact match for redirect URIs
1048-
if (!clientInfo.redirectUris.includes(redirectUri)) {
1049-
return createErrorResponse(
1050-
'invalid_grant',
1051-
'Invalid redirect URI'
1052-
);
1053-
}
1054-
10551039
// Parse the authorization code to extract user ID and grant ID
10561040
const codeParts = code.split(':');
10571041
if (codeParts.length !== 3) {
@@ -1099,8 +1083,27 @@ class OAuthProviderImpl {
10991083
);
11001084
}
11011085

1086+
// Check if PKCE is being used
1087+
const isPkceEnabled = !!grantData.codeChallenge;
1088+
1089+
// OAuth 2.1 requires redirect_uri parameter unless PKCE is used
1090+
if (!redirectUri && !isPkceEnabled) {
1091+
return createErrorResponse(
1092+
'invalid_request',
1093+
'redirect_uri is required when not using PKCE'
1094+
);
1095+
}
1096+
1097+
// Verify redirect URI if provided
1098+
if (redirectUri && !clientInfo.redirectUris.includes(redirectUri)) {
1099+
return createErrorResponse(
1100+
'invalid_grant',
1101+
'Invalid redirect URI'
1102+
);
1103+
}
1104+
11021105
// Verify PKCE code_verifier if code_challenge was provided during authorization
1103-
if (grantData.codeChallenge) {
1106+
if (isPkceEnabled) {
11041107
if (!codeVerifier) {
11051108
return createErrorResponse(
11061109
'invalid_request',

0 commit comments

Comments
 (0)