diff --git a/.changeset/funny-pots-brush.md b/.changeset/funny-pots-brush.md new file mode 100644 index 00000000000..152336e7a46 --- /dev/null +++ b/.changeset/funny-pots-brush.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-sdk-node': patch +--- + +Integrate handshake handling into `ClerkExpressWithAuth()` and `ClerkExpressRequireWith()`. If the `authenticateRequest()` returns a redirect or is in a handshake state, the middlewares will properly handle this and respond accordingly. diff --git a/packages/sdk-node/src/__tests__/middleware.test.ts b/packages/sdk-node/src/__tests__/middleware.test.ts index 74d7f7ca6ca..38593bcbc8f 100644 --- a/packages/sdk-node/src/__tests__/middleware.test.ts +++ b/packages/sdk-node/src/__tests__/middleware.test.ts @@ -25,6 +25,7 @@ describe('ClerkExpressWithAuth', () => { const clerkClient = mockClerkClient() as any; clerkClient.authenticateRequest.mockReturnValue({ toAuth: () => ({ sessionId: null }), + headers: new Headers(), } as RequestState); await createClerkExpressWithAuth({ clerkClient })()(req, res, mockNext as NextFunction); @@ -40,12 +41,38 @@ describe('ClerkExpressWithAuth', () => { const clerkClient = mockClerkClient() as any; clerkClient.authenticateRequest.mockReturnValue({ toAuth: () => ({ sessionId: '1' }), + headers: new Headers(), } as RequestState); await createClerkExpressWithAuth({ clerkClient })()(req, res, mockNext as NextFunction); expect((req as WithAuthProp).auth.sessionId).toEqual('1'); expect(mockNext).toHaveBeenCalledWith(); }); + + it('should redirect if a Location header is returned', async () => { + const req = createRequest(); + const res = { + status: jest.fn(() => res), + appendHeader: jest.fn(), + end: jest.fn(), + getHeader: jest.fn(), + } as unknown as Response; + + const headers = new Headers({ Location: 'https://clerk.example.com/v1/handshake' }); + + const clerkClient = mockClerkClient() as any; + clerkClient.authenticateRequest.mockReturnValue({ + toAuth: () => ({ sessionId: '1' }), + headers: new Headers({ Location: 'https://clerk.example.com/v1/handshake' }), + } as RequestState); + + await createClerkExpressWithAuth({ clerkClient })()(req, res, mockNext as NextFunction); + expect(res.status).toHaveBeenCalledWith(307); + + for (const [key, value] of headers.entries()) { + expect(res.appendHeader).toHaveBeenCalledWith(key, value); + } + }); }); describe('ClerkExpressRequireAuth', () => { @@ -56,6 +83,7 @@ describe('ClerkExpressRequireAuth', () => { const clerkClient = mockClerkClient() as any; clerkClient.authenticateRequest.mockReturnValue({ toAuth: () => ({ sessionId: null }), + headers: new Headers(), } as RequestState); await createClerkExpressRequireAuth({ clerkClient })()(req, res, mockNext as NextFunction); @@ -72,10 +100,36 @@ describe('ClerkExpressRequireAuth', () => { clerkClient.authenticateRequest.mockReturnValue({ isSignedIn: true, toAuth: () => ({ sessionId: '1' }), + headers: new Headers(), } as RequestState); await createClerkExpressRequireAuth({ clerkClient })()(req, res, mockNext as NextFunction); expect((req as WithAuthProp).auth.sessionId).toEqual('1'); expect(mockNext).toHaveBeenCalledWith(); }); + + it('should redirect if a Location header is returned', async () => { + const req = createRequest(); + const res = { + status: jest.fn(() => res), + appendHeader: jest.fn(), + end: jest.fn(), + getHeader: jest.fn(), + } as unknown as Response; + + const headers = new Headers({ Location: 'https://clerk.example.com/v1/handshake' }); + + const clerkClient = mockClerkClient() as any; + clerkClient.authenticateRequest.mockReturnValue({ + toAuth: () => ({ sessionId: '1' }), + headers: new Headers({ Location: 'https://clerk.example.com/v1/handshake' }), + } as RequestState); + + await createClerkExpressRequireAuth({ clerkClient })()(req, res, mockNext as NextFunction); + expect(res.status).toHaveBeenCalledWith(307); + + for (const [key, value] of headers.entries()) { + expect(res.appendHeader).toHaveBeenCalledWith(key, value); + } + }); }); diff --git a/packages/sdk-node/src/authenticateRequest.ts b/packages/sdk-node/src/authenticateRequest.ts index 04d346fb97f..66ca17cabe3 100644 --- a/packages/sdk-node/src/authenticateRequest.ts +++ b/packages/sdk-node/src/authenticateRequest.ts @@ -1,8 +1,9 @@ import type { RequestState } from '@clerk/backend/internal'; -import { constants, createClerkRequest } from '@clerk/backend/internal'; +import { AuthStatus, constants, createClerkRequest } from '@clerk/backend/internal'; import { handleValueOrFn } from '@clerk/shared/handleValueOrFn'; import { isDevelopmentFromSecretKey } from '@clerk/shared/keys'; import { isHttpOrHttps, isProxyUrlRelative, isValidProxyUrl } from '@clerk/shared/proxy'; +import type { Response } from 'express'; import type { IncomingMessage, ServerResponse } from 'http'; import type { AuthenticateRequestParams } from './types'; @@ -56,6 +57,31 @@ const incomingMessageToRequest = (req: IncomingMessage): Request => { }); }; +/** + * Depending on the auth state of the request, handles applying redirects and validating that a handshake state was properly handled. + * + * Returns an error if state is handshake without a redirect, otherwise returns undefined. res.writableEnded should be checked after this method is called. + */ +export const setResponseForHandshake = (requestState: RequestState, res: Response) => { + const hasLocationHeader = requestState.headers.get('location'); + if (hasLocationHeader) { + requestState.headers.forEach((value, key) => { + res.appendHeader(key, value); + }); + + // triggering a handshake redirect + res.status(307).end(); + + return; + } + + if (requestState.status === AuthStatus.Handshake) { + return new Error('Clerk: unexpected handshake without redirect'); + } + + return; +}; + // TODO: Move to backend export const decorateResponseWithObservabilityHeaders = (res: ServerResponse, requestState: RequestState) => { requestState.message && res.setHeader(constants.Headers.AuthMessage, encodeURIComponent(requestState.message)); diff --git a/packages/sdk-node/src/clerkExpressRequireAuth.ts b/packages/sdk-node/src/clerkExpressRequireAuth.ts index fa9880ab199..7d65ab2707e 100644 --- a/packages/sdk-node/src/clerkExpressRequireAuth.ts +++ b/packages/sdk-node/src/clerkExpressRequireAuth.ts @@ -1,7 +1,10 @@ import type { createClerkClient } from '@clerk/backend'; -import { AuthStatus } from '@clerk/backend/internal'; -import { authenticateRequest, decorateResponseWithObservabilityHeaders } from './authenticateRequest'; +import { + authenticateRequest, + decorateResponseWithObservabilityHeaders, + setResponseForHandshake, +} from './authenticateRequest'; import type { ClerkMiddlewareOptions, MiddlewareRequireAuthProp, RequireAuthProp } from './types'; export type CreateClerkExpressMiddlewareOptions = { @@ -25,9 +28,11 @@ export const createClerkExpressRequireAuth = (createOpts: CreateClerkExpressMidd }); decorateResponseWithObservabilityHeaders(res, requestState); - if (requestState.status === AuthStatus.Handshake) { - // TODO: Handle handshake - // This needs to be refactored and reused by clerkExpressWithAuth as well + const err = setResponseForHandshake(requestState, res); + if (err || res.writableEnded) { + if (err) { + next(err); + } return; } diff --git a/packages/sdk-node/src/clerkExpressWithAuth.ts b/packages/sdk-node/src/clerkExpressWithAuth.ts index bf3ce796e99..510b292758e 100644 --- a/packages/sdk-node/src/clerkExpressWithAuth.ts +++ b/packages/sdk-node/src/clerkExpressWithAuth.ts @@ -1,6 +1,8 @@ -import { AuthStatus } from '@clerk/backend/internal'; - -import { authenticateRequest, decorateResponseWithObservabilityHeaders } from './authenticateRequest'; +import { + authenticateRequest, + decorateResponseWithObservabilityHeaders, + setResponseForHandshake, +} from './authenticateRequest'; import type { CreateClerkExpressMiddlewareOptions } from './clerkExpressRequireAuth'; import type { ClerkMiddlewareOptions, MiddlewareWithAuthProp, WithAuthProp } from './types'; @@ -17,15 +19,17 @@ export const createClerkExpressWithAuth = (createOpts: CreateClerkExpressMiddlew }); decorateResponseWithObservabilityHeaders(res, requestState); - if (requestState.status === AuthStatus.Handshake) { - // TODO: Handle handshake - // This needs to be refactored and reused by clerkExpressRequireAuth as well + const err = setResponseForHandshake(requestState, res); + if (err || res.writableEnded) { + if (err) { + next(err); + } return; } (req as WithAuthProp).auth = { ...requestState.toAuth(), - claims: requestState.toAuth().sessionClaims, + claims: requestState.toAuth()?.sessionClaims, }; next(); };