From d6d2f958ddbcdbea7e62478173b05dbb101af18c Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Jun 2024 14:21:54 +0300 Subject: [PATCH 1/4] fix(nextjs): Use nextjs fetcher to identify internal page navigation --- .changeset/small-ears-appear.md | 2 ++ packages/nextjs/src/server/nextFetcher.ts | 22 ++++++++++++++++++++++ packages/nextjs/src/server/protect.ts | 18 ++++++++++++------ 3 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 .changeset/small-ears-appear.md create mode 100644 packages/nextjs/src/server/nextFetcher.ts diff --git a/.changeset/small-ears-appear.md b/.changeset/small-ears-appear.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/small-ears-appear.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/nextjs/src/server/nextFetcher.ts b/packages/nextjs/src/server/nextFetcher.ts new file mode 100644 index 00000000000..dc74668df65 --- /dev/null +++ b/packages/nextjs/src/server/nextFetcher.ts @@ -0,0 +1,22 @@ +type Fetcher = typeof globalThis.fetch; + +/** + * Based on nextjs internal implementation https://github.com/vercel/next.js/blob/6185444e0a944a82e7719ac37dad8becfed86acd/packages/next/src/server/lib/patch-fetch.ts#L23 + */ +type NextFetcher = Fetcher & { + readonly __nextPatched: true; + readonly __nextGetStaticStore: () => { getStore: () => StaticGenerationAsyncStorage }; +}; + +/** + * Full type can be found https://github.com/vercel/next.js/blob/6185444e0a944a82e7719ac37dad8becfed86acd/packages/next/src/client/components/static-generation-async-storage.external.ts#L4 + */ +interface StaticGenerationAsyncStorage { + readonly pagePath?: string; +} + +function isNextFetcher(fetch: Fetcher | NextFetcher): fetch is NextFetcher { + return '__nextPatched' in fetch && fetch.__nextPatched === true; +} + +export { isNextFetcher }; diff --git a/packages/nextjs/src/server/protect.ts b/packages/nextjs/src/server/protect.ts index 4cadbb859af..a912c1f1d18 100644 --- a/packages/nextjs/src/server/protect.ts +++ b/packages/nextjs/src/server/protect.ts @@ -6,6 +6,7 @@ import type { } from '@clerk/types'; import { constants as nextConstants } from '../constants'; +import { isNextFetcher } from './nextFetcher'; type AuthProtectOptions = { unauthorizedUrl?: string; unauthenticatedUrl?: string }; @@ -121,12 +122,17 @@ const isPageRequest = (req: Request): boolean => { return ( req.headers.get(constants.Headers.SecFetchDest) === 'document' || req.headers.get(constants.Headers.Accept)?.includes('text/html') || - (!!req.headers.get(nextConstants.Headers.NextUrl) && !isServerActionRequest(req)) || - !!req.headers.get(nextConstants.Headers.NextjsData) + isAppRouterInternalNavigation(req) || + isPagesRouterInternalNavigation(req) ); }; -// In case we want to handle router handlers and server actions differently in the future -// const isRouteHandler = (req: Request) => { -// return !isPageRequest(req) && !isServerAction(req); -// }; +const isAppRouterInternalNavigation = (req: Request) => + (!!req.headers.get(nextConstants.Headers.NextUrl) && !isServerActionRequest(req)) || isPagePathAvailable(); + +const isPagePathAvailable = () => { + const __fetch = globalThis.fetch; + return Boolean(isNextFetcher(__fetch) ? __fetch.__nextGetStaticStore().getStore().pagePath : false); +}; + +const isPagesRouterInternalNavigation = (req: Request) => !!req.headers.get(nextConstants.Headers.NextjsData); From 2b085fbe425f3c816340d1339c20b7a6c9ab9de7 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Jun 2024 15:48:13 +0300 Subject: [PATCH 2/4] test(nextjs): Remove conditional text --- integration/tests/protect.test.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/integration/tests/protect.test.ts b/integration/tests/protect.test.ts index 3c86aace334..040ab3610ef 100644 --- a/integration/tests/protect.test.ts +++ b/integration/tests/protect.test.ts @@ -67,16 +67,13 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withCustomRoles] })('authoriz test('Protect in RSCs and RCCs as `signed-out user`', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); - // Do not run this part for nextjs v14, the flow is broken in 14.2.3 because vercel removed a header that our page detection logic depends on - if (!u.nexJsVersion.startsWith('14')) { - /** - * Soft navigations - */ - await u.page.goToRelative('/'); - await page.getByText('Page Protected').click(); - await page.waitForURL('**/sign-in?**'); - await u.po.signIn.waitForMounted(); - } + /** + * Soft navigations + */ + await u.page.goToRelative('/'); + await page.getByText('Page Protected').click(); + await page.waitForURL('**/sign-in?**'); + await u.po.signIn.waitForMounted(); /** * Hard navigations From 2fe98e62701d1a811d667057d7a8aad747e15969 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Jun 2024 18:57:45 +0300 Subject: [PATCH 3/4] chore(nextjs): Add changeset --- .changeset/late-falcons-sell.md | 5 +++++ .changeset/small-ears-appear.md | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 .changeset/late-falcons-sell.md delete mode 100644 .changeset/small-ears-appear.md diff --git a/.changeset/late-falcons-sell.md b/.changeset/late-falcons-sell.md new file mode 100644 index 00000000000..8a03a3494ae --- /dev/null +++ b/.changeset/late-falcons-sell.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Enhance page detection by utilizing the patched fetch from nextjs. diff --git a/.changeset/small-ears-appear.md b/.changeset/small-ears-appear.md deleted file mode 100644 index a845151cc84..00000000000 --- a/.changeset/small-ears-appear.md +++ /dev/null @@ -1,2 +0,0 @@ ---- ---- From 9e43d82bd13cac423ba1ed67c62c4c178ead0dd0 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Jun 2024 19:24:32 +0300 Subject: [PATCH 4/4] chore(nextjs): Bring back comment --- packages/nextjs/src/server/protect.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/nextjs/src/server/protect.ts b/packages/nextjs/src/server/protect.ts index a912c1f1d18..f2d957e7463 100644 --- a/packages/nextjs/src/server/protect.ts +++ b/packages/nextjs/src/server/protect.ts @@ -136,3 +136,10 @@ const isPagePathAvailable = () => { }; const isPagesRouterInternalNavigation = (req: Request) => !!req.headers.get(nextConstants.Headers.NextjsData); + +// /** +// * In case we want to handle router handlers and server actions differently in the future +// */ +// const isApiRouteRequest = (req: Request) => { +// return !isPageRequest(req) && !isServerActionRequest(req); +// };