From 2964f8a47e473fa8457a27104adb4d008613a0e3 Mon Sep 17 00:00:00 2001 From: Dimitris Klouvas Date: Mon, 11 Mar 2024 13:25:32 +0200 Subject: [PATCH] chore(*): Expose requestState.headers to response and populate debug headers only in backend (#2898) --- .changeset/lemon-starfishes-buy.md | 9 +++++ .../src/tokens/__tests__/authStatus.test.ts | 40 +++++++++++++++++++ packages/backend/src/tokens/authStatus.ts | 27 +++++++++++-- packages/backend/tests/suites.ts | 2 + .../fastify/src/withClerkMiddleware.test.ts | 7 +++- packages/fastify/src/withClerkMiddleware.ts | 11 +---- packages/nextjs/src/server/authMiddleware.ts | 12 +----- packages/nextjs/src/server/clerkMiddleware.ts | 10 +---- packages/nextjs/src/server/utils.ts | 7 ---- packages/remix/src/ssr/index.ts | 1 - packages/remix/src/ssr/utils.ts | 32 +-------------- packages/sdk-node/src/authenticateRequest.ts | 25 +++++------- .../sdk-node/src/clerkExpressRequireAuth.ts | 9 +---- packages/sdk-node/src/clerkExpressWithAuth.ts | 9 +---- 14 files changed, 101 insertions(+), 100 deletions(-) create mode 100644 .changeset/lemon-starfishes-buy.md create mode 100644 packages/backend/src/tokens/__tests__/authStatus.test.ts diff --git a/.changeset/lemon-starfishes-buy.md b/.changeset/lemon-starfishes-buy.md new file mode 100644 index 0000000000..45c55241e3 --- /dev/null +++ b/.changeset/lemon-starfishes-buy.md @@ -0,0 +1,9 @@ +--- +'@clerk/clerk-sdk-node': minor +'@clerk/backend': minor +'@clerk/fastify': minor +'@clerk/nextjs': minor +'@clerk/remix': minor +--- + +Expose debug headers in response for handshake / signed-out states from SDKs using headers returned from `authenticateRequest()` diff --git a/packages/backend/src/tokens/__tests__/authStatus.test.ts b/packages/backend/src/tokens/__tests__/authStatus.test.ts new file mode 100644 index 0000000000..92c8bfaf39 --- /dev/null +++ b/packages/backend/src/tokens/__tests__/authStatus.test.ts @@ -0,0 +1,40 @@ +import type QUnit from 'qunit'; + +import { handshake, signedIn, signedOut } from '../authStatus'; + +export default (QUnit: QUnit) => { + const { module, test } = QUnit; + + module('signed-in', () => { + test('does not include debug headers', assert => { + const authObject = signedIn({} as any, {} as any, undefined, 'token'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-status'), null); + assert.strictEqual(authObject.headers.get('x-clerk-auth-reason'), null); + assert.strictEqual(authObject.headers.get('x-clerk-auth-message'), null); + }); + }); + + module('signed-out', () => { + test('includes debug headers', assert => { + const headers = new Headers({ 'custom-header': 'value' }); + const authObject = signedOut({} as any, 'auth-reason', 'auth-message', headers); + + assert.strictEqual(authObject.headers.get('custom-header'), 'value'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-status'), 'signed-out'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-reason'), 'auth-reason'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-message'), 'auth-message'); + }); + }); + + module('handshake', () => { + test('includes debug headers', assert => { + const headers = new Headers({ location: '/' }); + const authObject = handshake({} as any, 'auth-reason', 'auth-message', headers); + + assert.strictEqual(authObject.headers.get('location'), '/'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-status'), 'handshake'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-reason'), 'auth-reason'); + assert.strictEqual(authObject.headers.get('x-clerk-auth-message'), 'auth-message'); + }); + }); +}; diff --git a/packages/backend/src/tokens/authStatus.ts b/packages/backend/src/tokens/authStatus.ts index e4a00d4e50..8405ab17a6 100644 --- a/packages/backend/src/tokens/authStatus.ts +++ b/packages/backend/src/tokens/authStatus.ts @@ -1,5 +1,6 @@ import type { JwtPayload } from '@clerk/types'; +import { constants } from '../constants'; import type { TokenVerificationErrorReason } from '../errors'; import type { AuthenticateContext } from './authenticateContext'; import type { SignedInAuthObject, SignedOutAuthObject } from './authObjects'; @@ -105,7 +106,7 @@ export function signedOut( message = '', headers: Headers = new Headers(), ): SignedOutState { - return { + return withDebugHeaders({ status: AuthStatus.SignedOut, reason, message, @@ -121,7 +122,7 @@ export function signedOut( headers, toAuth: () => signedOutAuthObject({ ...authenticateContext, status: AuthStatus.SignedOut, reason, message }), token: null, - }; + }); } export function handshake( @@ -130,7 +131,7 @@ export function handshake( message = '', headers: Headers, ): HandshakeState { - return { + return withDebugHeaders({ status: AuthStatus.Handshake, reason, message, @@ -146,5 +147,23 @@ export function handshake( headers, toAuth: () => null, token: null, - }; + }); } + +const withDebugHeaders = (requestState: T): T => { + const headers = new Headers(requestState.headers || {}); + + if (requestState.message) { + headers.set(constants.Headers.AuthMessage, requestState.message); + } + if (requestState.reason) { + headers.set(constants.Headers.AuthReason, requestState.reason); + } + if (requestState.status) { + headers.set(constants.Headers.AuthStatus, requestState.status); + } + + requestState.headers = headers; + + return requestState; +}; diff --git a/packages/backend/tests/suites.ts b/packages/backend/tests/suites.ts index 3e44425946..4a4700f7bc 100644 --- a/packages/backend/tests/suites.ts +++ b/packages/backend/tests/suites.ts @@ -9,6 +9,7 @@ import cryptoKeysTest from './dist/jwt/__tests__/cryptoKeys.test.js'; import signJwtTest from './dist/jwt/__tests__/signJwt.test.js'; import verifyJwtTest from './dist/jwt/__tests__/verifyJwt.test.js'; import authObjectsTest from './dist/tokens/__tests__/authObjects.test.js'; +import authStatusTest from './dist/tokens/__tests__/authStatus.test.js'; import clerkRequestTest from './dist/tokens/__tests__/clerkRequest.test.js'; import tokenFactoryTest from './dist/tokens/__tests__/factory.test.js'; import keysTest from './dist/tokens/__tests__/keys.test.js'; @@ -19,6 +20,7 @@ import pathTest from './dist/util/__tests__/path.test.js'; // Add them to the suite array const suites = [ authObjectsTest, + authStatusTest, cryptoKeysTest, exportsTest, factoryTest, diff --git a/packages/fastify/src/withClerkMiddleware.test.ts b/packages/fastify/src/withClerkMiddleware.test.ts index eec86cacd6..b0439df519 100644 --- a/packages/fastify/src/withClerkMiddleware.test.ts +++ b/packages/fastify/src/withClerkMiddleware.test.ts @@ -101,7 +101,12 @@ describe('withClerkMiddleware(options)', () => { status: 'handshake', reason: 'auth-reason', message: 'auth-message', - headers: new Headers({ location: 'https://fapi.example.com/v1/clients/handshake' }), + headers: new Headers({ + location: 'https://fapi.example.com/v1/clients/handshake', + 'x-clerk-auth-message': 'auth-message', + 'x-clerk-auth-reason': 'auth-reason', + 'x-clerk-auth-status': 'handshake', + }), toAuth: () => 'mockedAuth', }); const fastify = Fastify(); diff --git a/packages/fastify/src/withClerkMiddleware.ts b/packages/fastify/src/withClerkMiddleware.ts index 8325796715..b9c2abf6ab 100644 --- a/packages/fastify/src/withClerkMiddleware.ts +++ b/packages/fastify/src/withClerkMiddleware.ts @@ -1,4 +1,3 @@ -import type { RequestState } from '@clerk/backend/internal'; import { AuthStatus } from '@clerk/backend/internal'; import type { FastifyReply, FastifyRequest } from 'fastify'; @@ -7,13 +6,6 @@ import * as constants from './constants'; import type { ClerkFastifyOptions } from './types'; import { fastifyRequestToRequest } from './utils'; -const decorateResponseWithObservabilityHeaders = (reply: FastifyReply, requestState: RequestState): FastifyReply => { - return reply - .header(constants.Headers.AuthStatus, requestState.status) - .header(constants.Headers.AuthReason, requestState.reason) - .header(constants.Headers.AuthMessage, requestState.message); -}; - export const withClerkMiddleware = (options: ClerkFastifyOptions) => { return async (fastifyRequest: FastifyRequest, reply: FastifyReply) => { const req = fastifyRequestToRequest(fastifyRequest); @@ -23,11 +15,12 @@ export const withClerkMiddleware = (options: ClerkFastifyOptions) => { secretKey: options.secretKey || constants.SECRET_KEY, publishableKey: options.publishableKey || constants.PUBLISHABLE_KEY, }); + requestState.headers.forEach((value, key) => reply.header(key, value)); const locationHeader = requestState.headers.get(constants.Headers.Location); if (locationHeader) { - return decorateResponseWithObservabilityHeaders(reply, requestState).code(307).send(); + return reply.code(307).send(); } else if (requestState.status === AuthStatus.Handshake) { throw new Error('Clerk: handshake status without redirect'); } diff --git a/packages/nextjs/src/server/authMiddleware.ts b/packages/nextjs/src/server/authMiddleware.ts index db91cf228f..43bf64ab5b 100644 --- a/packages/nextjs/src/server/authMiddleware.ts +++ b/packages/nextjs/src/server/authMiddleware.ts @@ -15,12 +15,7 @@ import { redirectToSignIn } from './redirectHelpers'; import type { RouteMatcherParam } from './routeMatcher'; import { createRouteMatcher } from './routeMatcher'; import type { NextMiddlewareReturn } from './types'; -import { - apiEndpointUnauthorizedNextResponse, - decorateRequest, - decorateResponseWithObservabilityHeaders, - setRequestHeadersOnNextResponse, -} from './utils'; +import { apiEndpointUnauthorizedNextResponse, decorateRequest, setRequestHeadersOnNextResponse } from './utils'; /** * The default ideal matcher that excludes the _next directory (internals) and all static files, @@ -184,10 +179,7 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => { const locationHeader = requestState.headers.get('location'); if (locationHeader) { // triggering a handshake redirect - return decorateResponseWithObservabilityHeaders( - new Response(null, { status: 307, headers: requestState.headers }), - requestState, - ); + return new Response(null, { status: 307, headers: requestState.headers }); } if (requestState.status === AuthStatus.Handshake) { diff --git a/packages/nextjs/src/server/clerkMiddleware.ts b/packages/nextjs/src/server/clerkMiddleware.ts index 4d40357d78..202771ec03 100644 --- a/packages/nextjs/src/server/clerkMiddleware.ts +++ b/packages/nextjs/src/server/clerkMiddleware.ts @@ -15,12 +15,7 @@ import { PUBLISHABLE_KEY, SECRET_KEY } from './constants'; import type { AuthProtect } from './protect'; import { createProtect } from './protect'; import type { NextMiddlewareEvtParam, NextMiddlewareRequestParam, NextMiddlewareReturn } from './types'; -import { - decorateRequest, - decorateResponseWithObservabilityHeaders, - handleMultiDomainAndProxy, - setRequestHeadersOnNextResponse, -} from './utils'; +import { decorateRequest, handleMultiDomainAndProxy, setRequestHeadersOnNextResponse } from './utils'; const CONTROL_FLOW_ERROR = { FORCE_NOT_FOUND: 'CLERK_PROTECT_REWRITE', @@ -79,8 +74,7 @@ export const clerkMiddleware: ClerkMiddleware = (...args: unknown[]): any => { const locationHeader = requestState.headers.get(constants.Headers.Location); if (locationHeader) { - const res = new Response(null, { status: 307, headers: requestState.headers }); - return decorateResponseWithObservabilityHeaders(res, requestState); + return new Response(null, { status: 307, headers: requestState.headers }); } else if (requestState.status === AuthStatus.Handshake) { throw new Error('Clerk: handshake status without redirect'); } diff --git a/packages/nextjs/src/server/utils.ts b/packages/nextjs/src/server/utils.ts index 985a2214f2..3e1cc8d1d1 100644 --- a/packages/nextjs/src/server/utils.ts +++ b/packages/nextjs/src/server/utils.ts @@ -194,10 +194,3 @@ export const handleMultiDomainAndProxy = (clerkRequest: ClerkRequest, opts: Auth signInUrl, }; }; - -export const decorateResponseWithObservabilityHeaders = (res: Response, requestState: RequestState): Response => { - requestState.message && res.headers.set(constants.Headers.AuthMessage, encodeURIComponent(requestState.message)); - requestState.reason && res.headers.set(constants.Headers.AuthReason, encodeURIComponent(requestState.reason)); - requestState.status && res.headers.set(constants.Headers.AuthStatus, encodeURIComponent(requestState.status)); - return res; -}; diff --git a/packages/remix/src/ssr/index.ts b/packages/remix/src/ssr/index.ts index 66ec349e98..ab4d72778d 100644 --- a/packages/remix/src/ssr/index.ts +++ b/packages/remix/src/ssr/index.ts @@ -1,6 +1,5 @@ export * from './rootAuthLoader'; export * from './getAuth'; -export { getClerkDebugHeaders } from './utils'; /** * Re-export resource types from @clerk/backend diff --git a/packages/remix/src/ssr/utils.ts b/packages/remix/src/ssr/utils.ts index ef8355168d..90a40f49ad 100644 --- a/packages/remix/src/ssr/utils.ts +++ b/packages/remix/src/ssr/utils.ts @@ -31,34 +31,6 @@ export function assertValidHandlerResult(val: any, error?: string): asserts val } } -const observabilityHeadersFromRequestState = (requestState: RequestState): Headers => { - const headers = {} as Record; - - if (requestState.message) { - headers[constants.Headers.AuthMessage] = requestState.message; - } - if (requestState.reason) { - headers[constants.Headers.AuthReason] = requestState.reason; - } - if (requestState.status) { - headers[constants.Headers.AuthStatus] = requestState.status; - } - - return new Headers(headers); -}; - -/** - * Retrieve Clerk auth headers. Should be used only for debugging and not in production. - * @internal - */ -export const getClerkDebugHeaders = (headers: Headers) => { - return { - [constants.Headers.AuthMessage]: headers.get(constants.Headers.AuthMessage), - [constants.Headers.AuthReason]: headers.get(constants.Headers.AuthReason), - [constants.Headers.AuthStatus]: headers.get(constants.Headers.AuthStatus), - }; -}; - export const injectRequestStateIntoResponse = async ( response: Response, requestState: RequestState, @@ -125,11 +97,9 @@ export function getResponseClerkState(requestState: RequestState, context: AppLo __telemetryDebug: isTruthy(getEnvVariable('CLERK_TELEMETRY_DEBUG', context)), }); - const headers = observabilityHeadersFromRequestState(requestState); - return { clerkState, - headers, + headers: requestState.headers, }; } diff --git a/packages/sdk-node/src/authenticateRequest.ts b/packages/sdk-node/src/authenticateRequest.ts index 66ca17cabe..4fcc4d7cc1 100644 --- a/packages/sdk-node/src/authenticateRequest.ts +++ b/packages/sdk-node/src/authenticateRequest.ts @@ -1,10 +1,10 @@ import type { RequestState } from '@clerk/backend/internal'; -import { AuthStatus, constants, createClerkRequest } from '@clerk/backend/internal'; +import { AuthStatus, 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 { IncomingMessage } from 'http'; import type { AuthenticateRequestParams } from './types'; import { loadApiEnv, loadClientEnv } from './utils'; @@ -57,21 +57,23 @@ const incomingMessageToRequest = (req: IncomingMessage): Request => { }); }; +export const setResponseHeaders = (requestState: RequestState, res: Response): Error | undefined => { + if (requestState.headers) { + requestState.headers.forEach((value, key) => res.appendHeader(key, value)); + } + return setResponseForHandshake(requestState, res); +}; + /** * 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 setResponseForHandshake = (requestState: RequestState, res: Response): Error | undefined => { 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; } @@ -82,13 +84,6 @@ export const setResponseForHandshake = (requestState: RequestState, res: Respons return; }; -// TODO: Move to backend -export const decorateResponseWithObservabilityHeaders = (res: ServerResponse, requestState: RequestState) => { - requestState.message && res.setHeader(constants.Headers.AuthMessage, encodeURIComponent(requestState.message)); - requestState.reason && res.setHeader(constants.Headers.AuthReason, encodeURIComponent(requestState.reason)); - requestState.status && res.setHeader(constants.Headers.AuthStatus, encodeURIComponent(requestState.status)); -}; - const absoluteProxyUrl = (relativeOrAbsoluteUrl: string, baseUrl: string): string => { if (!relativeOrAbsoluteUrl || !isValidProxyUrl(relativeOrAbsoluteUrl) || !isProxyUrlRelative(relativeOrAbsoluteUrl)) { return relativeOrAbsoluteUrl; diff --git a/packages/sdk-node/src/clerkExpressRequireAuth.ts b/packages/sdk-node/src/clerkExpressRequireAuth.ts index 7d65ab2707..63ae910acf 100644 --- a/packages/sdk-node/src/clerkExpressRequireAuth.ts +++ b/packages/sdk-node/src/clerkExpressRequireAuth.ts @@ -1,10 +1,6 @@ import type { createClerkClient } from '@clerk/backend'; -import { - authenticateRequest, - decorateResponseWithObservabilityHeaders, - setResponseForHandshake, -} from './authenticateRequest'; +import { authenticateRequest, setResponseHeaders } from './authenticateRequest'; import type { ClerkMiddlewareOptions, MiddlewareRequireAuthProp, RequireAuthProp } from './types'; export type CreateClerkExpressMiddlewareOptions = { @@ -26,9 +22,8 @@ export const createClerkExpressRequireAuth = (createOpts: CreateClerkExpressMidd req, options, }); - decorateResponseWithObservabilityHeaders(res, requestState); - const err = setResponseForHandshake(requestState, res); + const err = setResponseHeaders(requestState, res); if (err || res.writableEnded) { if (err) { next(err); diff --git a/packages/sdk-node/src/clerkExpressWithAuth.ts b/packages/sdk-node/src/clerkExpressWithAuth.ts index 510b292758..32301fabc8 100644 --- a/packages/sdk-node/src/clerkExpressWithAuth.ts +++ b/packages/sdk-node/src/clerkExpressWithAuth.ts @@ -1,8 +1,4 @@ -import { - authenticateRequest, - decorateResponseWithObservabilityHeaders, - setResponseForHandshake, -} from './authenticateRequest'; +import { authenticateRequest, setResponseHeaders } from './authenticateRequest'; import type { CreateClerkExpressMiddlewareOptions } from './clerkExpressRequireAuth'; import type { ClerkMiddlewareOptions, MiddlewareWithAuthProp, WithAuthProp } from './types'; @@ -17,9 +13,8 @@ export const createClerkExpressWithAuth = (createOpts: CreateClerkExpressMiddlew req, options, }); - decorateResponseWithObservabilityHeaders(res, requestState); - const err = setResponseForHandshake(requestState, res); + const err = setResponseHeaders(requestState, res); if (err || res.writableEnded) { if (err) { next(err);