Uh oh!
There was an error while loading. Please reload this page.
fix(nextjs,shared): Allow manually passed publishable/secret key to middleware#3001
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@clerk/nextjs': minor | ||
| '@clerk/shared': minor | ||
| --- | ||
| Allow manually passing a publishable/secret key pair to the `authMiddleware` and `clerkMiddleware` helpers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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: { | ||
| @@ -19,20 +28,7 @@ jest.mock('./clerkClient', () => { | ||
| }; | ||
| }); | ||
desiprisg marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| import { paths } from '../utils'; | ||
| import { authMiddleware, DEFAULT_CONFIG_MATCHER, DEFAULT_IGNORED_ROUTES } from './authMiddleware'; | ||
| // used to assert the mock | ||
| import { clerkClient } from './clerkClient'; | ||
| @@ -50,15 +46,6 @@ afterAll(() => { | ||
| global.console.warn = consoleWarn; | ||
| }); | ||
| type MockRequestParams = { | ||
| url: string; | ||
| appendDevBrowserCookie?: boolean; | ||
| @@ -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 () => { | ||
| const resp = await authMiddleware({ | ||
| 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://google.com/'); | ||
Contributor There was a problem hiding this comment. 🙃 Could you also assert that the | ||
| expect(clerkClient.authenticateRequest).toBeCalled(); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type { AuthenticateRequestOptions, AuthObject, ClerkRequest } 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'; | ||
| @@ -9,13 +9,19 @@ import { isRedirect, mergeResponses, serverRedirectWithAuth, setHeader, stringif | ||
| import { withLogger } from '../utils/debugLogger'; | ||
| import { clerkClient } from './clerkClient'; | ||
| import { createAuthenticateRequestOptions } from './clerkMiddleware'; | ||
| import { PUBLISHABLE_KEY, SECRET_KEY, SIGN_IN_URL, SIGN_UP_URL } from './constants'; | ||
| import { informAboutProtectedRouteInfo, receivedRequestForIgnoredRoute } from './errors'; | ||
| import { errorThrower } from './errorThrower'; | ||
| import type { RouteMatcherParam } from './routeMatcher'; | ||
| import { createRouteMatcher } from './routeMatcher'; | ||
| import type { NextMiddlewareReturn } from './types'; | ||
| import { | ||
| apiEndpointUnauthorizedNextResponse, | ||
| assertKey, | ||
| decorateRequest, | ||
| redirectAdapter, | ||
| setRequestHeadersOnNextResponse, | ||
| } from './utils'; | ||
| /** | ||
| * The default ideal matcher that excludes the _next directory (internals) and all static files, | ||
| @@ -115,19 +121,26 @@ export interface AuthMiddleware { | ||
| */ | ||
| const authMiddleware: AuthMiddleware = (...args: unknown[]) => { | ||
| const [params = {}] = args as [AuthMiddlewareParams?]; | ||
| 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(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(options.publicRoutes), | ||
| ignoredRoutes: Boolean(options.ignoredRoutes), | ||
| beforeAuth: Boolean(options.beforeAuth), | ||
| afterAuth: Boolean(options.afterAuth), | ||
| }), | ||
| ); | ||
| @@ -146,11 +159,11 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => { | ||
| clerkUrl: nextRequest.experimental_clerkUrl.href, | ||
| }); | ||
| logger.debug('Options debug', { ...options, beforeAuth: !!options.beforeAuth, afterAuth: !!options.afterAuth }); | ||
| if (isIgnoredRoute(nextRequest)) { | ||
| logger.debug({ isIgnoredRoute: true }); | ||
| if (isDevelopmentFromSecretKey(options.secretKey) && !options.ignoredRoutes) { | ||
| console.warn( | ||
| receivedRequestForIgnoredRoute( | ||
| nextRequest.experimental_clerkUrl.href, | ||
| @@ -161,7 +174,7 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => { | ||
| return setHeader(NextResponse.next(), constants.Headers.AuthReason, 'ignored-route'); | ||
| } | ||
| const beforeAuthRes = await (options.beforeAuth && options.beforeAuth(nextRequest, evt)); | ||
| if (beforeAuthRes === false) { | ||
| logger.debug('Before auth returned false, skipping'); | ||
| @@ -192,7 +205,7 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => { | ||
| }); | ||
| logger.debug(() => ({ auth: JSON.stringify(auth), debug: auth.debug() })); | ||
| const afterAuthRes = await (options.afterAuth || defaultAfterAuth)(auth, nextRequest, evt); | ||
| const finalRes = mergeResponses(beforeAuthRes, afterAuthRes) || NextResponse.next(); | ||
| logger.debug(() => ({ mergedHeaders: stringifyHeaders(finalRes.headers) })); | ||
| @@ -223,17 +236,25 @@ export { authMiddleware }; | ||
| const createDefaultAfterAuth = ( | ||
| isPublicRoute: ReturnType<typeof createRouteMatcher>, | ||
| isApiRoute: ReturnType<typeof createApiRoutes>, | ||
| options: { signInUrl: string; signUpUrl: string; publishableKey: string; secretKey: string }, | ||
Member There was a problem hiding this comment. ❓ why are we defining the options here explicitly? I think you're trying to minimize what is visible to this function, which is great, but in this case I think it's better to pass the options through so we don't have to worry about maintaining an extra inline type :) ContributorAuthor There was a problem hiding this comment. If we were to pass the params type, we would have to assert the necessary keys again to be safe, and that is just not the purpose of this function. | ||
| ) => { | ||
| return (auth: AuthObject, req: WithExperimentalClerkUrl<NextRequest>) => { | ||
| if (!auth.userId && !isPublicRoute(req)) { | ||
| if (isApiRoute(req)) { | ||
| informAboutProtectedRoute(req.experimental_clerkUrl.pathname, options, true); | ||
| return apiEndpointUnauthorizedNextResponse(); | ||
| } else { | ||
| informAboutProtectedRoute(req.experimental_clerkUrl.pathname, options, false); | ||
| } | ||
| return createRedirect({ | ||
desiprisg marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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(); | ||
| }; | ||
| @@ -305,13 +326,17 @@ const withNormalizedClerkUrl = ( | ||
| return Object.assign(nextRequest, { experimental_clerkUrl: res }); | ||
| }; | ||
| const informAboutProtectedRoute = ( | ||
| path: string, | ||
| options: AuthMiddlewareParams & { secretKey: string }, | ||
| isApiRoute: boolean, | ||
| ) => { | ||
| if (options.debug || isDevelopmentFromSecretKey(options.secretKey)) { | ||
| console.warn( | ||
| informAboutProtectedRouteInfo( | ||
| path, | ||
| !!options.publicRoutes, | ||
| !!options.ignoredRoutes, | ||
| isApiRoute, | ||
| DEFAULT_IGNORED_ROUTES, | ||
| ), | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.