-
Notifications
You must be signed in to change notification settings - Fork 37
feat(auth-nextjs): add auth-nextjs package #2765
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
base: main
Are you sure you want to change the base?
Conversation
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
packages/passport/sdk-sample-app/src/pages/api/auth/sandbox/[...nextauth].api.ts
Outdated
Show resolved
Hide resolved
|
View your CI Pipeline Execution ↗ for commit 89d47d5
☁️ Nx Cloud last updated this comment at |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected -t build,lint,test |
❌ Failed | 3m 12s | View ↗ |
nx run-many -p @imtbl/sdk,@imtbl/checkout-widge... |
✅ Succeeded | 3s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-12-17 04:04:32 UTC
| if (searchParams.get('error')) { | ||
| handleOAuthError(); | ||
| return; | ||
| } |
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.
OAuth error callback lacks double-invocation guard
Medium Severity
The OAuth error handling path lacks a ref guard to prevent double invocation, while the success path properly uses callbackProcessedRef. When handling an error response, if the useEffect dependencies change (particularly the onError callback which may not be memoized by consumers), handleOAuthError() will execute multiple times, invoking onError repeatedly with the same error message. This inconsistency could cause duplicate error notifications, logging, or analytics events. The error path at lines 213-217 needs protection similar to the callbackProcessedRef guard used for the code handling path at lines 221-223.
Additional Locations (1)
| if (searchParams.get('code') && !callbackProcessedRef.current) { | ||
| callbackProcessedRef.current = true; | ||
| handleCallback(); | ||
| } |
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.
Callback page shows loading indefinitely without OAuth params
Medium Severity
The CallbackPage component's useEffect only processes requests when code or error URL parameters are present. If a user navigates directly to the callback page without OAuth parameters (e.g., typing the URL, bookmarking, or if the OAuth redirect loses its parameters), the component renders the loading state indefinitely with no timeout, error handling, or way to recover. The effect simply does nothing when neither condition is met, leaving users stuck on "Completing authentication..." forever.
|
|
||
| setAuth(newAuth); | ||
| setIsAuthReady(true); | ||
| }, [config]); |
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.
Auth instance never cleaned up on config change
Medium Severity
The useEffect that initializes the Auth instance lacks a cleanup function. When config changes (e.g., environment switching in the sample app) or the component unmounts, the old Auth instance is abandoned without cleanup. The Auth class internally holds a UserManager from oidc-client-ts which registers window event listeners and may set up timers for token refresh. Without explicit cleanup, these resources persist, causing memory leaks and potentially multiple Auth instances competing for the same localStorage keys.
| setAuth(null); | ||
| setIsAuthReady(false); | ||
| prevConfigRef.current = null; | ||
| }; |
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.
Cleanup resets ref defeating config change optimization
Medium Severity
The effect cleanup sets prevConfigRef.current = null, which defeats the optimization designed to avoid recreating the Auth instance when only the config reference changes but values remain the same. In React's effect lifecycle, cleanup runs before the new effect when dependencies change. This means when a parent re-renders with a non-memoized config object (same values, different reference), the cleanup first resets prevConfigRef.current to null, then the comparison at line 82 (prevConfigRef.current === configKey) fails because null never equals the configKey string. This causes unnecessary Auth instance recreation, potentially leading to multiple OIDC client instances competing for localStorage keys and wasted resources.
Additional Locations (1)
| setAuth(null); | ||
| setIsAuthReady(false); | ||
| }; | ||
| }, [config]); |
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.
Auth instance lost after React StrictMode double-invoke
Medium Severity
In React 18 Strict Mode, effects run twice on initial mount (setup → cleanup → setup). The optimization at line 82 that skips Auth recreation when prevConfigRef.current === configKey causes a bug: after the first setup, the ref is set; the cleanup then nullifies auth; but the second setup sees the ref matches and returns early without recreating Auth. This leaves auth permanently null, causing handleSignIn to throw "Auth not initialized" when users try to sign in during development. The ref persists across StrictMode's simulated unmount/remount, breaking the optimization's assumption.
| "dependencies": { | ||
| "@biom3/react": "^0.27.12", | ||
| "@imtbl/sdk": "workspace:*", | ||
| "@imtbl/sdk": "file:.yalc/@imtbl/sdk", |
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.
Example packages reference local .yalc paths instead of workspace
High Severity
The @imtbl/sdk dependency was changed from workspace:* to file:.yalc/@imtbl/sdk in three example packages. The .yalc directory is a local development tool artifact (already listed in the root .gitignore) and doesn't exist in the repository. This breaks the examples for anyone cloning the repo since pnpm install will fail trying to resolve the non-existent .yalc path. Other example packages in the codebase correctly use workspace:* for this dependency.
🔬 Verification Test
Test code:
The verification involved checking that .yalc is in the root .gitignore and that other example packages use workspace:*.
Command run:
grep -r "yalc" .gitignore
grep -r "workspace:\*" examples/*/package.json
Output:
.gitignore:18:.yalc
.gitignore:19:yalc.lock
examples/_deprecated/identity-with-nextjs/package.json: "@imtbl/sdk": "workspace:*",
examples/blockchain-data/package.json: "@imtbl/sdk": "workspace:*"
examples/checkout/sdk-connect-with-nextjs/package.json: "@imtbl/sdk": "workspace:*",
examples/checkout/sdk-gas-estimation-with-nextjs/package.json: "@imtbl/sdk": "workspace:*",
examples/checkout/sdk-switch-network-with-nextjs/package.json: "@imtbl/sdk": "workspace:*",
Why this proves the bug: The .yalc directory is explicitly gitignored (showing it's not meant to be committed), and all other example packages correctly use workspace:* for the @imtbl/sdk dependency. The three affected passport examples were incorrectly changed to use local .yalc paths that won't exist for other developers.
Additional Locations (2)
| ...(update.idToken ? { idToken: update.idToken } : {}), | ||
| ...(update.accessTokenExpires ? { accessTokenExpires: update.accessTokenExpires } : {}), | ||
| ...(update.zkEvm ? { zkEvm: update.zkEvm } : {}), | ||
| }; |
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.
Session update doesn't clear previous error flag
Medium Severity
When the JWT callback handles session updates (from client-side token sync via updateSession), it spreads ...token which preserves any existing error field, but never clears it. This contrasts with refreshAccessToken which explicitly sets error: undefined on success. The bug manifests when server-side token refresh fails (setting error: 'RefreshTokenError'), but client-side Auth subsequently refreshes tokens successfully and syncs them via TOKEN_REFRESHED event. The valid tokens are stored, but the stale error persists in the session, causing users to see error states even though authentication is valid.
🔬 Verification Test
Why verification test was not possible: This bug requires a running Next.js application with Auth.js integration, an OAuth provider, and the ability to simulate server-side refresh failures followed by successful client-side refreshes. The bug is in the interaction between the JWT callback's update trigger and the session state, which cannot be unit tested without mocking the entire Auth.js callback chain and session update mechanism.
| * Default session max age in seconds (30 days) | ||
| * This is how long the NextAuth session cookie will be valid | ||
| */ | ||
| export const DEFAULT_SESSION_MAX_AGE_SECONDS = 365 * 24 * 60 * 60; |
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.
Session max age set to 365 days instead of documented 30 days
Medium Severity
The DEFAULT_SESSION_MAX_AGE_SECONDS constant is set to 365 * 24 * 60 * 60 (365 days/1 year), but the comment states it should be 30 days. This same "30 days" claim appears in config.ts and the README. NextAuth's default is also 30 days. Sessions will remain valid 12x longer than expected and documented, which impacts security assumptions. The value likely should be 30 * 24 * 60 * 60 to match the documentation.
Additional Locations (1)
| ...authConfig, | ||
| ...overrides, | ||
| callbacks: composedCallbacks, | ||
| }; |
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.
Session override removes required JWT strategy setting
Medium Severity
When users provide overrides with a session property (e.g., { session: { maxAge: 3600 } }), the spread operator ...overrides completely replaces authConfig.session, removing the critical strategy: 'jwt' setting. The package relies on JWT-based sessions for token storage and refresh. While callbacks are correctly composed to preserve internal functionality, the session config is not similarly merged. A user innocently trying to change just maxAge would silently break authentication because the strategy: 'jwt' that createAuthConfig explicitly sets would be lost.
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| }); | ||
| if (!isProtected) { | ||
| return NextResponse.next(); | ||
| } |
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.
Middleware path matching ignores path boundaries
Medium Severity
The string path matching in createAuthMiddleware uses startsWith without verifying path boundaries. When a string pattern like '/api' is provided in protectedPaths or publicPaths, it matches not just /api and /api/users but also unrelated paths like /apiversion or /api-docs. This occurs because pathname.startsWith(pattern) doesn't check if the next character is / or end-of-string. This could cause routes to be unexpectedly protected or exposed, leading to potential access control issues.
|
|
||
| return handler(session, ...args); | ||
| }; | ||
| } |
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.
withAuth doesn't check for session errors
Medium Severity
The withAuth helper only checks if (!session) but doesn't check for session.error. When a refresh token is invalid (e.g., revoked or expired after 365 days), auth() returns a session with error: "RefreshTokenError". The handler would receive this invalid session and could fail when using the stale accessToken for API calls. The README explicitly documents that server code should check for session.error, but this helper doesn't implement that check.

Summary
Adding auth-nextjs for convenient integration with nextjs + SSR.
Note
Introduces a new
@imtbl/auth-nextjspackage that bridges@imtbl/authwith Auth.js v5 for Next.js, providing SSR-friendly auth and client-only token refresh.@imtbl/auth-nextjswithcreateImmutableAuth, credentials provider config, clientImmutableAuthProvider, hooks (useImmutableAuth,useAccessToken,useHydratedData),CallbackPage, server helpers (getAuthProps,getAuthenticatedData,withServerAuth,createAuthMiddleware,withAuth), and token utilities (isTokenExpired,getTokenExpiry)@imtbl/authnow emitsAuthEvents.TOKEN_REFRESHED; listeners in the provider update NextAuth session to handle refresh-token rotation; added testsImmutableAuthProvider, adds callback page and example UI; Next.js config updated to optionally enable API routes and adjust page extensions; dependencies updatedNODE_OPTIONSin pre-commit, serialize network installs in.npmrcWritten by Cursor Bugbot for commit 89d47d5. This will update automatically on new commits. Configure here.