fix(clerk-sdk-node): Correctly forward errors to next() if request URL is invalid

.changeset/fast-boxes-knock.md

@@ -0,0 +1,5 @@
+---
+"@clerk/clerk-sdk-node": patch
+---
+
+Correctly forward errors to `next()` if the request URL is invalid

packages/express/src/__tests__/clerkMiddleware.test.ts

@@ -1,4 +1,4 @@
-import type { RequestHandler } from 'express';
+import type { Request, RequestHandler, Response } from 'express';
 
 import { clerkMiddleware } from '../clerkMiddleware';
 import { getAuth } from '../getAuth';
@@ -152,4 +152,20 @@ describe('clerkMiddleware', () => {
     expect(response.header).toHaveProperty('x-clerk-auth-status', 'handshake');
     expect(response.header).toHaveProperty('location', expect.stringContaining('/v1/client/handshake?redirect_url='));
   });
+
+  it('it calls next with an error when request URL is invalid', async () => {
+    const req = {
+      url: '//',
+      cookies: {},
+      headers: { host: 'example.com' },
+    } as Request;
+    const res = {} as Response;
+    const mockNext = jest.fn();
+
+    await clerkMiddleware()[0](req, res, mockNext);
+
+    expect(mockNext.mock.calls[0][0].message).toBe('Invalid URL');
+
+    mockNext.mockReset();
+  });
 });

packages/express/src/__tests__/requireAuth.test.ts

@@ -14,8 +14,8 @@ describe('requireAuth', () => {
 
     requireAuth(mockRequestWithAuth(), response, nextFn);
 
-    expect(response.status).toBeCalledWith(401);
-    expect(nextFn).not.toBeCalled();
+    expect(response.status).toHaveBeenCalledWith(401);
+    expect(nextFn).not.toHaveBeenCalled();
   });
 
   it('make application require auth - proceed with next middlewares for signed-in', async () => {
@@ -25,7 +25,7 @@ describe('requireAuth', () => {
 
     requireAuth(request, response, nextFn);
 
-    expect(response.status).not.toBeCalled();
-    expect(nextFn).toBeCalled();
+    expect(response.status).not.toHaveBeenCalled();
+    expect(nextFn).toHaveBeenCalled();
   });
 });

packages/sdk-node/src/__tests__/middleware.test.ts

@@ -18,6 +18,26 @@ const mockClerkClient = () => ({
 });
 
 describe('ClerkExpressWithAuth', () => {
+  it('should halt middleware execution by calling next with an error when request URL is invalid', async () => {
+    const req = {
+      url: '//',
+      cookies: {},
+      headers: { host: 'example.com' },
+    } as Request;
+    const res = {} as Response;
+
+    const clerkClient = mockClerkClient() as any;
+    clerkClient.authenticateRequest.mockReturnValue({
+      toAuth: () => ({ sessionId: null }),
+      headers: new Headers(),
+    } as RequestState);
+
+    await createClerkExpressWithAuth({ clerkClient })()(req, res, mockNext as NextFunction);
+    expect((req as WithAuthProp<Request>).auth).toBe(undefined);
+    expect(mockNext.mock.calls[0][0]).toBeInstanceOf(Error);
+    expect(mockNext.mock.calls[0][0].message).toBe('Invalid request URL: //');
+  });
+
   it('should decorate request with auth and move on to the next middleware when no session token exists', async () => {
     const req = createRequest();
     const res = {} as Response;
@@ -92,6 +112,26 @@ describe('ClerkExpressRequireAuth', () => {
     expect(mockNext.mock.calls[0][0].message).toBe('Unauthenticated');
   });
 
+  it('should halt middleware execution by calling next with an error when request URL is invalid', async () => {
+    const req = {
+      url: '//',
+      cookies: {},
+      headers: { host: 'example.com' },
+    } as Request;
+    const res = {} as Response;
+
+    const clerkClient = mockClerkClient() as any;
+    clerkClient.authenticateRequest.mockReturnValue({
+      toAuth: () => ({ sessionId: null }),
+      headers: new Headers(),
+    } as RequestState);
+
+    await createClerkExpressRequireAuth({ clerkClient })()(req, res, mockNext as NextFunction);
+    expect((req as WithAuthProp<Request>).auth).toBe(undefined);
+    expect(mockNext.mock.calls[0][0]).toBeInstanceOf(Error);
+    expect(mockNext.mock.calls[0][0].message).toBe('Invalid request URL: //');
+  });
+
   it('should decorate request with auth and move on to the next middleware when a session token does exist', async () => {
     const req = createRequest();
     const res = {} as Response;

packages/sdk-node/src/authenticateRequest.ts

@@ -50,7 +50,14 @@ const incomingMessageToRequest = (req: IncomingMessage): Request => {
   // req extends IncomingMessage in a useful way. No guarantee
   // it'll work.
   const protocol = req.connection?.encrypted ? 'https' : 'http';
-  const dummyOriginReqUrl = new URL(req.url || '', `${protocol}://clerk-dummy`);
+  let dummyOriginReqUrl: URL;
+
+  try {
+    dummyOriginReqUrl = new URL(req.url || '', `${protocol}://clerk-dummy`);
+  } catch (e) {
+    throw new Error(`Invalid request URL: ${req.url}`);
+  }
+
   return new Request(dummyOriginReqUrl, {
     method: req.method,
     headers: new Headers(headers),

packages/sdk-node/src/clerkExpressRequireAuth.ts

@@ -1,4 +1,5 @@
 import type { createClerkClient } from '@clerk/backend';
+import type { RequestState } from '@clerk/backend/internal';
 
 import { authenticateRequest, setResponseHeaders } from './authenticateRequest';
 import type { ClerkMiddlewareOptions, MiddlewareRequireAuthProp, RequireAuthProp } from './types';
@@ -15,13 +16,20 @@ export const createClerkExpressRequireAuth = (createOpts: CreateClerkExpressMidd
 
   return (options: ClerkMiddlewareOptions = {}): MiddlewareRequireAuthProp => {
     return async (req, res, next) => {
-      const requestState = await authenticateRequest({
-        clerkClient,
-        secretKey,
-        publishableKey,
-        req,
-        options,
-      });
+      let requestState: RequestState;
+
+      try {
+        requestState = await authenticateRequest({
+          clerkClient,
+          secretKey,
+          publishableKey,
+          req,
+          options,
+        });
+      } catch (e) {
+        next(e);
+        return;
+      }
 
       const err = setResponseHeaders(requestState, res);
       if (err || res.writableEnded) {

packages/sdk-node/src/clerkExpressWithAuth.ts

@@ -1,3 +1,5 @@
+import type { RequestState } from '@clerk/backend/internal';
+
 import { authenticateRequest, setResponseHeaders } from './authenticateRequest';
 import type { CreateClerkExpressMiddlewareOptions } from './clerkExpressRequireAuth';
 import type { ClerkMiddlewareOptions, MiddlewareWithAuthProp, WithAuthProp } from './types';
@@ -6,13 +8,20 @@ export const createClerkExpressWithAuth = (createOpts: CreateClerkExpressMiddlew
   const { clerkClient, secretKey = '', publishableKey = '' } = createOpts;
   return (options: ClerkMiddlewareOptions = {}): MiddlewareWithAuthProp => {
     return async (req, res, next) => {
-      const requestState = await authenticateRequest({
-        clerkClient,
-        secretKey,
-        publishableKey,
-        req,
-        options,
-      });
+      let requestState: RequestState;
+
+      try {
+        requestState = await authenticateRequest({
+          clerkClient,
+          secretKey,
+          publishableKey,
+          req,
+          options,
+        });
+      } catch (e) {
+        next(e);
+        return;
+      }
 
       const err = setResponseHeaders(requestState, res);
       if (err || res.writableEnded) {