When using an authorized callback in conjunction with user-defined middleware, it does not redirect to the sign-in page

[yicrotkd]

Environment

System:
OS: Linux 5.15 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
CPU: (12) x64 12th Gen Intel(R) Core(TM) i7-1265U
Memory: 6.81 GB / 17.57 GB
Container: Yes
Shell: 3.6.4 - /usr/bin/fish
Binaries:
Node: 20.11.0 - ~/.volta/tools/image/node/20.11.0/bin/node
Yarn: 4.0.0-rc.48 - ~/.volta/tools/image/yarn/4.0.0-rc.48/bin/yarn
npm: 10.2.4 - ~/.volta/tools/image/npm/10.2.4/bin/npm
pnpm: 8.15.2 - ~/.volta/bin/pnpm
bun: 1.0.17 - ~/.volta/bin/bun
npmPackages:
next: latest => 14.1.4
next-auth: beta => 5.0.0-beta.16
react: ^18.2.0 => 18.2.0

Reproduction URL

https://github.com/yicrotkd/next-auth-example/tree/auth-callbacks-with-user-defined-middleware

Describe the issue

When accessing the path /middleware-example without being authenticated, it redirects to the sign-in page. This is a case where it behaves as intended.

However, when using user-defined middleware, it does not redirect to the sign-in page.

How to reproduce

As explained in the above.

Expected behavior

It redirects to the sign-in page even when using user-defined middleware.

[cbdabner]

We have some chained middleware that wasn't playing too nicely with this, so here's what I understood. When you define middleware or user-defined (wrapped) middleware, you end up calling the logic in handAuth(), but the combination of middleware and the authorized() callback has 2 (potentially competing) locations for middleware logic, as shown in the diagram.

You can end up, either (in order of priority):

1. Using only the NextResponse from authorized()
2. Ignoring the boolean logic from authorized() and using the NextResponse from your wrapped middleware.
3. Routing or redirecting based on the truthy response from authorizd().

Regardless of the logic (existing or in the PR), the combination is confusing. If using user-defined middleware, it makes sense to only define it using one pattern. So, our NextAuth config has a comment to NOT define an authorized() callback at all, and because we're implementing a wrapped middleware, specifically rely on our own logic in there.

It might be more simple to consider a breaking change around authorized() only defining a boolean response to remove the complexity of a NextResponse competing in the two locations.

flowchart

A{"Is authorized() defined?"};
A -- Yes --> B{"authorized() returns instanceof NextResponse?"};
B == Yes ==> C["1. Return authorized() NextResponse"];
B -- No --> D["Set authorized = authorized()"] --> F{"User-defined middleware (wrapped) exists?"};
A -- No --> E[Set authorized = true] --> F;
F == Yes ==> G["2. Return middleware() NextResponse"];
F -- No --> H{authorized is truthy?};
H == Yes ==> I["3. Continue on route"];
H == No ==> J["3. Redirect to sign-in"];

[yicrotkd]

Thanks for sharing your insights and the approach you've taken with the middleware configuration.

I prefer to delegate the redirection to the sign-in page to NextAuth. Although incorporating all the necessary logic into wrapped middleware is viable, delegating remains my preferred approach.

For now, we intend to adopt a strategy akin to yours, not using an authorized() callback but instead incorporating a redirect process similar to the one in this code segment into our wrapped middleware:

next-auth/packages/next-auth/src/lib/index.ts

Lines 266 to 273 in 37348c6

	const signInPage = config.pages?.signIn ?? `${config.basePath}/signin`
	if (request.nextUrl.pathname !== signInPage) {
	// Redirect to signin page by default if not authorized
	const signInUrl = request.nextUrl.clone()
	signInUrl.pathname = signInPage
	signInUrl.searchParams.set("callbackUrl", request.nextUrl.href)
	response = NextResponse.redirect(signInUrl)
	}

It might be more simple to consider a breaking change around authorized() only defining a boolean response to remove the complexity of a NextResponse competing in the two locations.

While I somewhat agree with the suggestion of simplifying things by making authorized() only define a boolean response to eliminate the complexity of a NextResponse competing at two locations, it's a tough call because I think it's a suitable API for specifying authentication-related NextResponses.