Validating redirect_uri on authorize

Author: geelen

__tests__/oauth-provider.test.ts

@@ -206,13 +206,13 @@ describe('OAuthProvider', () => {
           return new Response('Users API response', { status: 200 });
         }
       }
-      
+
       class DocumentsApiHandler extends WorkerEntrypoint {
         fetch(request: Request) {
           return new Response('Documents API response', { status: 200 });
         }
       }
-      
+
       // Create provider with multi-handler configuration
       const providerWithMultiHandler = new OAuthProvider({
         apiHandlers: {
@@ -225,96 +225,96 @@ describe('OAuthProvider', () => {
         clientRegistrationEndpoint: '/oauth/register', // Important for registering clients in the test
         scopesSupported: ['read', 'write'],
       });
-      
+
       // Create a client and get an access token
       const clientData = {
         redirect_uris: ['https://client.example.com/callback'],
         client_name: 'Test Client',
         token_endpoint_auth_method: 'client_secret_basic',
       };
-      
+
       const registerRequest = createMockRequest(
         'https://example.com/oauth/register',
         'POST',
         { 'Content-Type': 'application/json' },
         JSON.stringify(clientData)
       );
-      
+
       const registerResponse = await providerWithMultiHandler.fetch(registerRequest, mockEnv, mockCtx);
       const client = await registerResponse.json();
       const clientId = client.client_id;
       const clientSecret = client.client_secret;
       const redirectUri = 'https://client.example.com/callback';
-      
+
       // Get an auth code
       const authRequest = createMockRequest(
         `https://example.com/authorize?response_type=code&client_id=${clientId}` +
-        `&redirect_uri=${encodeURIComponent(redirectUri)}` +
-        `&scope=read%20write&state=xyz123`
+          `&redirect_uri=${encodeURIComponent(redirectUri)}` +
+          `&scope=read%20write&state=xyz123`
       );
-      
+
       const authResponse = await providerWithMultiHandler.fetch(authRequest, mockEnv, mockCtx);
       const location = authResponse.headers.get('Location')!;
       const code = new URL(location).searchParams.get('code')!;
-      
+
       // Exchange for tokens
       const params = new URLSearchParams();
       params.append('grant_type', 'authorization_code');
       params.append('code', code);
       params.append('redirect_uri', redirectUri);
       params.append('client_id', clientId);
       params.append('client_secret', clientSecret);
-      
+
       const tokenRequest = createMockRequest(
         'https://example.com/oauth/token',
         'POST',
         { 'Content-Type': 'application/x-www-form-urlencoded' },
         params.toString()
       );
-      
+
       const tokenResponse = await providerWithMultiHandler.fetch(tokenRequest, mockEnv, mockCtx);
       const tokens = await tokenResponse.json();
       const accessToken = tokens.access_token;
-      
+
       // Make requests to different API routes
       const usersApiRequest = createMockRequest('https://example.com/api/users/profile', 'GET', {
         Authorization: `Bearer ${accessToken}`,
       });
-      
+
       const documentsApiRequest = createMockRequest('https://example.com/api/documents/list', 'GET', {
         Authorization: `Bearer ${accessToken}`,
       });
-      
+
       // Request to Users API should be handled by UsersApiHandler
       const usersResponse = await providerWithMultiHandler.fetch(usersApiRequest, mockEnv, mockCtx);
       expect(usersResponse.status).toBe(200);
       expect(await usersResponse.text()).toBe('Users API response');
-      
+
       // Request to Documents API should be handled by DocumentsApiHandler
       const documentsResponse = await providerWithMultiHandler.fetch(documentsApiRequest, mockEnv, mockCtx);
       expect(documentsResponse.status).toBe(200);
       expect(await documentsResponse.text()).toBe('Documents API response');
     });
-    
+
     it('should throw an error when both single-handler and multi-handler configs are provided', () => {
       expect(() => {
         new OAuthProvider({
           apiRoute: '/api/',
           apiHandler: {
-            fetch: () => Promise.resolve(new Response())
+            fetch: () => Promise.resolve(new Response()),
           },
           apiHandlers: {
             '/api/users/': {
-              fetch: () => Promise.resolve(new Response())
-            }
+              fetch: () => Promise.resolve(new Response()),
+            },
           },
           defaultHandler: testDefaultHandler,
           authorizeEndpoint: '/authorize',
           tokenEndpoint: '/oauth/token',
         });
       }).toThrow('Cannot use both apiRoute/apiHandler and apiHandlers');
     });
-    
+
     it('should throw an error when neither single-handler nor multi-handler config is provided', () => {
       expect(() => {
         new OAuthProvider({
@@ -494,6 +494,23 @@ describe('OAuthProvider', () => {
       expect(grants.keys.length).toBe(1);
     });
 
+    it('should reject authorization request with invalid redirect URI', async () => {
+      // Create an authorization request with an invalid redirect URI
+      const invalidRedirectUri = 'https://attacker.example.com/callback';
+      const authRequest = createMockRequest(
+        `https://example.com/authorize?response_type=code&client_id=${clientId}` +
+          `&redirect_uri=${encodeURIComponent(invalidRedirectUri)}` +
+          `&scope=read%20write&state=xyz123`
+      );
+
+      // Expect the request to be rejected
+      await expect(oauthProvider.fetch(authRequest, mockEnv, mockCtx)).rejects.toThrow('Invalid redirect URI');
+
+      // Verify no grant was created
+      const grants = await mockEnv.OAUTH_KV.list({ prefix: 'grant:' });
+      expect(grants.keys.length).toBe(0);
+    });
+
     // Add more tests for auth code flow...
   });
 

src/oauth-provider.ts

@@ -99,7 +99,7 @@ export interface OAuthProviderOptions {
    * URL(s) for API routes. Requests with URLs starting with any of these prefixes
    * will be treated as API requests and require a valid access token.
    * Can be a single route or an array of routes. Each route can be a full URL or just a path.
-   * 
+   *
    * Used with `apiHandler` for the single-handler configuration. This is incompatible with
    * the `apiHandlers` property. You must use either `apiRoute` + `apiHandler` OR `apiHandlers`, not both.
    */
@@ -109,7 +109,7 @@ export interface OAuthProviderOptions {
    * Handler for API requests that have a valid access token.
    * This handler will receive the authenticated user properties in ctx.props.
    * Can be either an ExportedHandler object with a fetch method or a class extending WorkerEntrypoint.
-   * 
+   *
    * Used with `apiRoute` for the single-handler configuration. This is incompatible with
    * the `apiHandlers` property. You must use either `apiRoute` + `apiHandler` OR `apiHandlers`, not both.
    */
@@ -120,12 +120,15 @@ export interface OAuthProviderOptions {
    * The keys are the API routes (strings only, not arrays), and the values are the handlers.
    * Each route can be a full URL or just a path, and each handler can be either an ExportedHandler
    * object with a fetch method or a class extending WorkerEntrypoint.
-   * 
+   *
    * This is incompatible with the `apiRoute` and `apiHandler` properties. You must use either
-   * `apiRoute` + `apiHandler` (single-handler configuration) OR `apiHandlers` (multi-handler 
+   * `apiRoute` + `apiHandler` (single-handler configuration) OR `apiHandlers` (multi-handler
    * configuration), not both.
    */
-  apiHandlers?: Record<string, ExportedHandlerWithFetch | (new (ctx: ExecutionContext, env: any) => WorkerEntrypointWithFetch)>;
+  apiHandlers?: Record<
+    string,
+    ExportedHandlerWithFetch | (new (ctx: ExecutionContext, env: any) => WorkerEntrypointWithFetch)
+  >;
 
   /**
    * Handler for all non-API requests or API requests without a valid token.
@@ -690,7 +693,7 @@ class OAuthProviderImpl {
    * Represents the validated type of a handler (ExportedHandler or WorkerEntrypoint)
    */
   private typedDefaultHandler: TypedHandler;
-  
+
   /**
    * Array of tuples of API routes and their validated handlers
    * In the simple case, this will be a single entry with the route and handler from options.apiRoute/apiHandler
@@ -705,33 +708,32 @@ class OAuthProviderImpl {
   constructor(options: OAuthProviderOptions) {
     // Initialize typedApiHandlers as an array
     this.typedApiHandlers = [];
-    
+
     // Check if we have incompatible configuration
     const hasSingleHandlerConfig = !!(options.apiRoute && options.apiHandler);
     const hasMultiHandlerConfig = !!options.apiHandlers;
-    
+
     if (hasSingleHandlerConfig && hasMultiHandlerConfig) {
       throw new TypeError(
         'Cannot use both apiRoute/apiHandler and apiHandlers. ' +
-        'Use either apiRoute + apiHandler OR apiHandlers, not both.'
+          'Use either apiRoute + apiHandler OR apiHandlers, not both.'
       );
     }
-    
+
     if (!hasSingleHandlerConfig && !hasMultiHandlerConfig) {
       throw new TypeError(
-        'Must provide either apiRoute + apiHandler OR apiHandlers. ' +
-        'No API route configuration provided.'
+        'Must provide either apiRoute + apiHandler OR apiHandlers. ' + 'No API route configuration provided.'
       );
     }
-    
+
     // Validate default handler
     this.typedDefaultHandler = this.validateHandler(options.defaultHandler, 'defaultHandler');
 
     // Process and validate the API handlers
     if (hasSingleHandlerConfig) {
       // Single handler mode with apiRoute + apiHandler
       const apiHandler = this.validateHandler(options.apiHandler!, 'apiHandler');
-      
+
       // For single handler mode, process the apiRoute(s) and map them all to the single apiHandler
       if (Array.isArray(options.apiRoute)) {
         options.apiRoute.forEach((route, index) => {
@@ -964,7 +966,7 @@ class OAuthProviderImpl {
     }
     return false;
   }
-  
+
   /**
    * Finds the appropriate API handler for a URL
    * @param url - The URL to find a handler for
@@ -1837,13 +1839,13 @@ class OAuthProviderImpl {
     // Find the appropriate API handler for this URL
     const url = new URL(request.url);
     const apiHandler = this.findApiHandlerForUrl(url);
-    
+
     if (!apiHandler) {
       // This shouldn't happen since we already checked with isApiRequest,
       // but handle it gracefully just in case
       return this.createErrorResponse('invalid_request', 'No handler found for API route', 404);
     }
-    
+
     // Call the API handler based on its type
     if (apiHandler.type === HandlerType.EXPORTED_HANDLER) {
       // It's an object with a fetch method
@@ -2192,6 +2194,20 @@ class OAuthHelpersImpl implements OAuthHelpers {
       throw new Error('The implicit grant flow is not enabled for this provider');
     }
 
+    // Validate the client ID and redirect URI
+    if (clientId) {
+      const clientInfo = await this.lookupClient(clientId);
+
+      // If client exists, validate the redirect URI against registered URIs
+      if (clientInfo && redirectUri) {
+        if (!clientInfo.redirectUris.includes(redirectUri)) {
+          throw new Error(
+            `Invalid redirect URI. The redirect URI provided does not match any registered URI for this client.`
+          );
+        }
+      }
+    }
+
     return {
       responseType,
       clientId,