Skip to content

Commit

Permalink
fix(nextjs,shared): Allow manually passed publishable/secret key to m…
Browse files Browse the repository at this point in the history
…iddleware (#3001)

* fix(nextjs,shared): Allow manually passed publishable/secret key to middleware

* chore(nextjs): Authmiddleware cleanup

* chore(nextjs): Clerkmiddleware cleanup

* feat(nextjs): Better typing for assertKey util
  • Loading branch information
desiprisg committed Mar 29, 2024
1 parent 63ef35e commit 1fd2eff
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 76 deletions.
6 changes: 6 additions & 0 deletions .changeset/stupid-coats-smell.md
@@ -0,0 +1,6 @@
---
'@clerk/nextjs': minor
'@clerk/shared': minor
---

Allow manually passing a publishable/secret key pair to the `authMiddleware` and `clerkMiddleware` helpers.
4 changes: 1 addition & 3 deletions packages/backend/src/util/assertValidSecretKey.ts
@@ -1,8 +1,6 @@
export function assertValidSecretKey(val: unknown): asserts val is string {
if (!val || typeof val !== 'string') {
throw Error(
'Missing Clerk Secret Key or API Key. Go to https://dashboard.clerk.com and get your key for your instance.',
);
throw Error('Missing Clerk Secret Key. Go to https://dashboard.clerk.com and get your key for your instance.');
}

//TODO: Check if the key is invalid and throw error
Expand Down
44 changes: 12 additions & 32 deletions packages/nextjs/src/server/authMiddleware.test.ts
Expand Up @@ -10,6 +10,15 @@ const authenticateRequestMock = jest.fn().mockResolvedValue({
headers: new Headers(),
});

// Removing this mock will cause the authMiddleware tests to fail due to missing publishable key
// This mock SHOULD exist before the imports
jest.mock('./constants', () => {
return {
PUBLISHABLE_KEY: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
SECRET_KEY: 'sk_test_xxxxxxxxxxxxxxxxxx',
};
});

jest.mock('./clerkClient', () => {
return {
clerkClient: {
Expand All @@ -19,20 +28,7 @@ jest.mock('./clerkClient', () => {
};
});

const mockRedirectToSignIn = jest.fn().mockImplementation(() => {
const res = NextResponse.redirect(
'https://accounts.included.katydid-92.lcl.dev/sign-in?redirect_url=https%3A%2F%2Fwww.clerk.com%2Fprotected',
);
return setHeader(res, 'x-clerk-redirect-to', 'true');
});

jest.mock('./redirectHelpers', () => {
return {
redirectToSignIn: mockRedirectToSignIn,
};
});

import { paths, setHeader } from '../utils';
import { paths } from '../utils';
import { authMiddleware, DEFAULT_CONFIG_MATCHER, DEFAULT_IGNORED_ROUTES } from './authMiddleware';
// used to assert the mock
import { clerkClient } from './clerkClient';
Expand All @@ -50,15 +46,6 @@ afterAll(() => {
global.console.warn = consoleWarn;
});

// Removing this mock will cause the authMiddleware tests to fail due to missing publishable key
// This mock SHOULD exist before the imports
jest.mock('./constants', () => {
return {
PUBLISHABLE_KEY: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
SECRET_KEY: 'sk_test_xxxxxxxxxxxxxxxxxx',
};
});

type MockRequestParams = {
url: string;
appendDevBrowserCookie?: boolean;
Expand Down Expand Up @@ -449,19 +436,12 @@ describe('Dev Browser JWT when redirecting to cross origin', function () {
});

it('does NOT append the Dev Browser JWT if x-clerk-redirect-to header is not set', async () => {
mockRedirectToSignIn.mockReturnValueOnce(
NextResponse.redirect(
'https://accounts.included.katydid-92.lcl.dev/sign-in?redirect_url=https%3A%2F%2Fwww.clerk.com%2Fprotected',
),
);
const resp = await authMiddleware({
beforeAuth: () => NextResponse.next(),
beforeAuth: () => NextResponse.redirect('https://google.com/'),
})(mockRequest({ url: '/protected', appendDevBrowserCookie: true }), {} as NextFetchEvent);

expect(resp?.status).toEqual(307);
expect(resp?.headers.get('location')).toEqual(
'https://accounts.included.katydid-92.lcl.dev/sign-in?redirect_url=https%3A%2F%2Fwww.clerk.com%2Fprotected',
);
expect(resp?.headers.get('location')).toEqual('https://google.com/');
expect(clerkClient.authenticateRequest).toBeCalled();
});
});
Expand Down
75 changes: 50 additions & 25 deletions packages/nextjs/src/server/authMiddleware.ts
@@ -1,5 +1,5 @@
import type { AuthenticateRequestOptions, AuthObject, ClerkRequest } from '@clerk/backend/internal';
import { AuthStatus, constants, createClerkRequest } from '@clerk/backend/internal';
import { AuthStatus, constants, createClerkRequest, createRedirect } from '@clerk/backend/internal';
import { isDevelopmentFromSecretKey } from '@clerk/shared/keys';
import { eventMethodCalled } from '@clerk/shared/telemetry';
import type { NextFetchEvent, NextMiddleware, NextRequest } from 'next/server';
Expand All @@ -9,13 +9,19 @@ import { isRedirect, mergeResponses, serverRedirectWithAuth, setHeader, stringif
import { withLogger } from '../utils/debugLogger';
import { clerkClient } from './clerkClient';
import { createAuthenticateRequestOptions } from './clerkMiddleware';
import { SECRET_KEY } from './constants';
import { PUBLISHABLE_KEY, SECRET_KEY, SIGN_IN_URL, SIGN_UP_URL } from './constants';
import { informAboutProtectedRouteInfo, receivedRequestForIgnoredRoute } from './errors';
import { redirectToSignIn } from './redirectHelpers';
import { errorThrower } from './errorThrower';
import type { RouteMatcherParam } from './routeMatcher';
import { createRouteMatcher } from './routeMatcher';
import type { NextMiddlewareReturn } from './types';
import { apiEndpointUnauthorizedNextResponse, decorateRequest, setRequestHeadersOnNextResponse } from './utils';
import {
apiEndpointUnauthorizedNextResponse,
assertKey,
decorateRequest,
redirectAdapter,
setRequestHeadersOnNextResponse,
} from './utils';

/**
* The default ideal matcher that excludes the _next directory (internals) and all static files,
Expand Down Expand Up @@ -115,19 +121,26 @@ export interface AuthMiddleware {
*/
const authMiddleware: AuthMiddleware = (...args: unknown[]) => {
const [params = {}] = args as [AuthMiddlewareParams?];
const { beforeAuth, afterAuth, publicRoutes, ignoredRoutes, apiRoutes, ...options } = params;
const publishableKey = assertKey(params.publishableKey || PUBLISHABLE_KEY, () =>
errorThrower.throwMissingPublishableKeyError(),
);
const secretKey = assertKey(params.secretKey || SECRET_KEY, () => errorThrower.throwMissingPublishableKeyError());
const signInUrl = params.signInUrl || SIGN_IN_URL;
const signUpUrl = params.signUpUrl || SIGN_UP_URL;

const options = { ...params, publishableKey, secretKey, signInUrl, signUpUrl };

const isIgnoredRoute = createRouteMatcher(ignoredRoutes || DEFAULT_IGNORED_ROUTES);
const isPublicRoute = createRouteMatcher(withDefaultPublicRoutes(publicRoutes));
const isApiRoute = createApiRoutes(apiRoutes);
const defaultAfterAuth = createDefaultAfterAuth(isPublicRoute, isApiRoute, params);
const isIgnoredRoute = createRouteMatcher(options.ignoredRoutes || DEFAULT_IGNORED_ROUTES);
const isPublicRoute = createRouteMatcher(withDefaultPublicRoutes(options.publicRoutes));
const isApiRoute = createApiRoutes(options.apiRoutes);
const defaultAfterAuth = createDefaultAfterAuth(isPublicRoute, isApiRoute, options);

clerkClient.telemetry.record(
eventMethodCalled('authMiddleware', {
publicRoutes: Boolean(publicRoutes),
ignoredRoutes: Boolean(ignoredRoutes),
beforeAuth: Boolean(beforeAuth),
afterAuth: Boolean(afterAuth),
publicRoutes: Boolean(options.publicRoutes),
ignoredRoutes: Boolean(options.ignoredRoutes),
beforeAuth: Boolean(options.beforeAuth),
afterAuth: Boolean(options.afterAuth),
}),
);

Expand All @@ -146,11 +159,11 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => {
clerkUrl: nextRequest.experimental_clerkUrl.href,
});

logger.debug('Options debug', { ...options, beforeAuth: !!beforeAuth, afterAuth: !!afterAuth });
logger.debug('Options debug', { ...options, beforeAuth: !!options.beforeAuth, afterAuth: !!options.afterAuth });

if (isIgnoredRoute(nextRequest)) {
logger.debug({ isIgnoredRoute: true });
if (isDevelopmentFromSecretKey(options.secretKey || SECRET_KEY) && !params.ignoredRoutes) {
if (isDevelopmentFromSecretKey(options.secretKey) && !options.ignoredRoutes) {
console.warn(
receivedRequestForIgnoredRoute(
nextRequest.experimental_clerkUrl.href,
Expand All @@ -161,7 +174,7 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => {
return setHeader(NextResponse.next(), constants.Headers.AuthReason, 'ignored-route');
}

const beforeAuthRes = await (beforeAuth && beforeAuth(nextRequest, evt));
const beforeAuthRes = await (options.beforeAuth && options.beforeAuth(nextRequest, evt));

if (beforeAuthRes === false) {
logger.debug('Before auth returned false, skipping');
Expand Down Expand Up @@ -192,7 +205,7 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => {
});

logger.debug(() => ({ auth: JSON.stringify(auth), debug: auth.debug() }));
const afterAuthRes = await (afterAuth || defaultAfterAuth)(auth, nextRequest, evt);
const afterAuthRes = await (options.afterAuth || defaultAfterAuth)(auth, nextRequest, evt);
const finalRes = mergeResponses(beforeAuthRes, afterAuthRes) || NextResponse.next();
logger.debug(() => ({ mergedHeaders: stringifyHeaders(finalRes.headers) }));

Expand Down Expand Up @@ -223,17 +236,25 @@ export { authMiddleware };
const createDefaultAfterAuth = (
isPublicRoute: ReturnType<typeof createRouteMatcher>,
isApiRoute: ReturnType<typeof createApiRoutes>,
params: AuthMiddlewareParams,
options: { signInUrl: string; signUpUrl: string; publishableKey: string; secretKey: string },
) => {
return (auth: AuthObject, req: WithExperimentalClerkUrl<NextRequest>) => {
if (!auth.userId && !isPublicRoute(req)) {
if (isApiRoute(req)) {
informAboutProtectedRoute(req.experimental_clerkUrl.pathname, params, true);
informAboutProtectedRoute(req.experimental_clerkUrl.pathname, options, true);
return apiEndpointUnauthorizedNextResponse();
} else {
informAboutProtectedRoute(req.experimental_clerkUrl.pathname, params, false);
informAboutProtectedRoute(req.experimental_clerkUrl.pathname, options, false);
}
return redirectToSignIn({ returnBackUrl: req.experimental_clerkUrl.href });
return createRedirect({
redirectAdapter,
signInUrl: options.signInUrl,
signUpUrl: options.signUpUrl,
publishableKey: options.publishableKey,
// We're setting baseUrl to '' here as we want to keep the legacy behavior of
// the redirectToSignIn, redirectToSignUp helpers in the backend package.
baseUrl: '',
}).redirectToSignIn({ returnBackUrl: req.experimental_clerkUrl.href });
}
return NextResponse.next();
};
Expand Down Expand Up @@ -305,13 +326,17 @@ const withNormalizedClerkUrl = (
return Object.assign(nextRequest, { experimental_clerkUrl: res });
};

const informAboutProtectedRoute = (path: string, params: AuthMiddlewareParams, isApiRoute: boolean) => {
if (params.debug || isDevelopmentFromSecretKey(params.secretKey || SECRET_KEY)) {
const informAboutProtectedRoute = (
path: string,
options: AuthMiddlewareParams & { secretKey: string },
isApiRoute: boolean,
) => {
if (options.debug || isDevelopmentFromSecretKey(options.secretKey)) {
console.warn(
informAboutProtectedRouteInfo(
path,
!!params.publicRoutes,
!!params.ignoredRoutes,
!!options.publicRoutes,
!!options.ignoredRoutes,
isApiRoute,
DEFAULT_IGNORED_ROUTES,
),
Expand Down
15 changes: 15 additions & 0 deletions packages/nextjs/src/server/clerkMiddleware.test.ts
Expand Up @@ -5,9 +5,11 @@ import { describe, expect } from '@jest/globals';
import type { NextFetchEvent } from 'next/server';
import { NextRequest, NextResponse } from 'next/server';

const publishableKey = 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA';
const authenticateRequestMock = jest.fn().mockResolvedValue({
toAuth: () => ({}),
headers: new Headers(),
publishableKey,
});

jest.mock('./clerkClient', () => {
Expand Down Expand Up @@ -170,6 +172,7 @@ describe('authenticateRequest & handshake', () => {
it('returns 307 and starts the handshake flow for handshake requestState status', async () => {
const mockLocationUrl = 'https://example.com';
authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.Handshake,
headers: new Headers({ Location: mockLocationUrl }),
});
Expand Down Expand Up @@ -293,6 +296,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -315,6 +319,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedIn,
headers: new Headers(),
toAuth: () => ({ userId: 'user-id' }),
Expand All @@ -337,6 +342,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -359,6 +365,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedIn,
headers: new Headers(),
toAuth: () => ({ userId: 'user-id', has: () => false }),
Expand All @@ -381,6 +388,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -404,6 +412,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedIn,
headers: new Headers(),
toAuth: () => ({ userId: 'user-id', has: () => false }),
Expand Down Expand Up @@ -435,6 +444,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -457,6 +467,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -483,6 +494,7 @@ describe('clerkMiddleware(params)', () => {
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: 'userId', has: () => false }),
Expand Down Expand Up @@ -515,6 +527,7 @@ describe('Dev Browser JWT when redirecting to cross origin for page requests', f
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -539,6 +552,7 @@ describe('Dev Browser JWT when redirecting to cross origin for page requests', f
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand All @@ -562,6 +576,7 @@ describe('Dev Browser JWT when redirecting to cross origin for page requests', f
});

authenticateRequestMock.mockResolvedValueOnce({
publishableKey,
status: AuthStatus.SignedOut,
headers: new Headers(),
toAuth: () => ({ userId: null }),
Expand Down

0 comments on commit 1fd2eff

Please sign in to comment.