-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.5.0 - Next 13 compatibility #257
2.5.0 - Next 13 compatibility #257
Conversation
prevents certain edge cases (primarily in Next13) that can cause redirect loops
The current logic assumes headers is a map (which is true for Next >14), but in Next 13 the request headers are an object. This causes errors on Next 13 like so: TypeError: e.headers.get is not a function Given the headers function was added in 13, we should resort to using that instead as it provides a stable and consistent API for accessing headers in all versions greater than Next 13.
WalkthroughThe pull request introduces changes to authentication middleware and request handling across multiple files. The modifications primarily focus on refining route handling, pre-fetch request detection, and authentication flow. Key changes include adding new constants for callback and registration pages in the authentication middleware, updating how pre-fetch checks are performed in login, logout, and register handlers, and simplifying the Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthMiddleware
participant Handler
participant Headers
Client->>AuthMiddleware: Request
AuthMiddleware->>AuthMiddleware: Check public paths
alt Is public path
AuthMiddleware-->>Client: Allow access
else Not public path
AuthMiddleware->>Handler: Validate authentication
Handler->>Headers: Get headers
Headers->>Handler: Check pre-fetch
alt Is pre-fetch
Handler-->>Client: Return null
else Not pre-fetch
Handler->>Client: Process request
end
end
Possibly related PRs
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/utils/isPreFetch.ts (1)
1-1
: Consider using the public Next.js types packageInstead of importing from internal Next.js paths, consider using the public
@types/next
package for better stability across Next.js versions.-import { ReadonlyHeaders } from "next/dist/server/web/spec-extension/adapters/headers"; +import { ReadonlyHeaders } from '@types/next';src/handlers/logout.ts (1)
1-1
: Add missing semicolon for consistencyWhile the implementation correctly mirrors other handlers, there's a minor inconsistency in the return statement.
if (isPreFetch(heads)) { - return null + return null; }Also applies to: 11-12
src/authMiddleware/authMiddleware.ts (1)
22-24
: LGTM! Early bailout implementation.The early bailout for internal Kinde paths effectively prevents redirect loops in Next.js 13.
However, consider adding debug logging for consistency with the rest of the middleware:
if(loginPage == pathname || callbackPage == pathname || registerPage == pathname) { + if(config.isDebugMode) { + console.log('authMiddleware: internal Kinde path, skipping auth check') + } return NextResponse.next(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/authMiddleware/authMiddleware.ts
(2 hunks)src/handlers/login.ts
(2 hunks)src/handlers/logout.ts
(2 hunks)src/handlers/register.ts
(2 hunks)src/utils/isPreFetch.test.ts
(1 hunks)src/utils/isPreFetch.ts
(1 hunks)
🔇 Additional comments (7)
src/utils/isPreFetch.ts (1)
3-8
: LGTM! Clean and robust prefetch detectionThe implementation correctly handles all common prefetch header variations and the type change to
ReadonlyHeaders
makes the function more precise.src/handlers/login.ts (1)
1-1
: LGTM! Robust header handling for Next.js compatibilityThe change to use
headers()
function instead of accessing headers directly from the request object ensures compatibility with both Next.js 13 and 14.Also applies to: 10-12
src/handlers/register.ts (1)
1-1
: LGTM! Consistent implementation with other handlersThe changes mirror the login handler implementation, maintaining consistency across the codebase.
Also applies to: 10-12
src/utils/isPreFetch.test.ts (2)
8-10
: LGTM! Clean helper function implementation.The
mockHeaders
helper function correctly creates aReadonlyHeaders
object, which aligns with Next.js 13's header handling.
13-14
: LGTM! Comprehensive test coverage.The test cases thoroughly cover all prefetch scenarios:
- Purpose header
- X-Purpose header
- X-Moz header
- No prefetch headers
- Different header values
Also applies to: 18-19, 23-24, 28-29, 33-38
src/authMiddleware/authMiddleware.ts (2)
19-21
: LGTM! Clear constant definitions.The new constants for callback and register pages improve code maintainability.
37-37
: Verify the impact of simplified public path check.The simplified public path check might affect routes that were previously handled differently.
Let's verify if any routes are impacted:
✅ Verification successful
Simplified public path check is safe to use
The change to use
startsWith
for public paths is appropriate as it only affects completely public resources (like/_next
andfavicon.ico
). Protected routes use separate protection mechanisms throughprotectPage
andprotectApi
functions that handle authentication and authorization independently.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for route definitions and middleware usage rg -A 3 'publicPaths.*=' || \ rg -A 3 'withAuth\(' || \ rg -A 3 'NextResponse\.redirect.*loginPage'Length of output: 796
Script:
#!/bin/bash # Check git history for the file git log -L :publicPaths:src/authMiddleware/authMiddleware.ts # Search for route handlers fd -e ts -e js -e tsx -e jsx --exec rg -l 'export.*handler|createHandler|route\.ts$|route\.js$|api.*handler'Length of output: 158
Script:
#!/bin/bash # Check handler files content echo "=== src/server/index.js ===" cat src/server/index.js echo -e "\n=== src/handlers/protect.js ===" cat src/handlers/protect.js # Search for path-related configurations rg -g '!*.{lock,md}' -A 3 'path.*=.*["\']/'Length of output: 4684
Explain your changes
This PR contains some minor fixes for the upcoming 2.5.0 release to improve compatibility with Next 13.
login
) has been moved to the very top of the middleware. It has also been expanded to include other paths likekinde_callback
andregister
. This was the source of a redirect loop in Next 13.login
,logout
andregister
links has been reworked to use theheaders()
function instead of directly accessing headers on aNextRequest
object. In Next 13, theheaders
on aNextRequest
are an object, but in 14 onwards they're a map. The existing logic treated it like a map and thus caused breakages in Next 13 (TypeError: e.headers.get is not a function
). Instead, the prefetch check now expects the result ofheaders()
as it's a stable and consistent API for working with headers in all versions of Next from 13 on.Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.