From 44f2a47e6e6a54a5495f0e6ea49f19c7b991dce2 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sun, 18 Sep 2022 12:07:46 +0800 Subject: [PATCH] fix(middleware): use `includes()` for NextAuth pages (#5104) * fix(middleware): use `includes()` for NextAuth pages Some users could be setting their `signIn` and `error` pages option to `/` to disable the automatically generated pages, as suggested in [1]. This commit reverts the behaviour for matching `signIn` and `error` pages in `handleMiddleware` to pre-v4.10.3. ``` const signInPage = "/" const errorPage = "/" const publicPaths = [signInPage, errorPage, "/_next", "/favicon.ico"] // pathname = "/" will return true publicPaths.some((p) => pathname.startsWith(p)) ``` Fixes: aedabc8d ("fix: avoid redirect on always public paths") Reference [1]: https://github.com/nextauthjs/next-auth/discussions/2330#discussioncomment-1678298 Signed-off-by: Eng Zer Jun * test(middleware): add tests for public paths Signed-off-by: Eng Zer Jun Signed-off-by: Eng Zer Jun Co-authored-by: Thang Vu --- packages/next-auth/src/next/middleware.ts | 3 +- packages/next-auth/tests/middleware.test.ts | 40 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 packages/next-auth/tests/middleware.test.ts diff --git a/packages/next-auth/src/next/middleware.ts b/packages/next-auth/src/next/middleware.ts index a9efb3ec7e..4837ccb39d 100644 --- a/packages/next-auth/src/next/middleware.ts +++ b/packages/next-auth/src/next/middleware.ts @@ -106,12 +106,13 @@ async function handleMiddleware( const signInPage = options?.pages?.signIn ?? "/api/auth/signin" const errorPage = options?.pages?.error ?? "/api/auth/error" const basePath = parseUrl(process.env.NEXTAUTH_URL).path - const publicPaths = [signInPage, errorPage, "/_next", "/favicon.ico"] + const publicPaths = ["/_next", "/favicon.ico"] // Avoid infinite redirects/invalid response // on paths that never require authentication if ( pathname.startsWith(basePath) || + [signInPage, errorPage].includes(pathname) || publicPaths.some((p) => pathname.startsWith(p)) ) { return diff --git a/packages/next-auth/tests/middleware.test.ts b/packages/next-auth/tests/middleware.test.ts new file mode 100644 index 0000000000..e98525f10e --- /dev/null +++ b/packages/next-auth/tests/middleware.test.ts @@ -0,0 +1,40 @@ +import { NextMiddleware } from "next/server" +import { NextAuthMiddlewareOptions, withAuth } from "../next/middleware" + +it("should not match pages as public paths", async () => { + const options: NextAuthMiddlewareOptions = { + pages: { + signIn: "/", + error: "/" + }, + secret: "secret" + } + + const nextUrl: any = { + pathname: "/protected/pathA", + search: "", + origin: "http://127.0.0.1" + } + const req: any = { nextUrl, headers: { authorization: "" } } + + const handleMiddleware = withAuth(options) as NextMiddleware + const res = await handleMiddleware(req, null) + expect(res).toBeDefined() + expect(res.status).toBe(307) +}) + +it("should not redirect on public paths", async () => { + const options: NextAuthMiddlewareOptions = { + secret: "secret" + } + const nextUrl: any = { + pathname: "/_next/foo", + search: "", + origin: "http://127.0.0.1" + } + const req: any = { nextUrl, headers: { authorization: "" } } + + const handleMiddleware = withAuth(options) as NextMiddleware + const res = await handleMiddleware(req, null) + expect(res).toBeUndefined() +})