Skip to content

Conversation

@shineli1984
Copy link
Collaborator

@shineli1984 shineli1984 commented Dec 17, 2025

Summary

Adding auth-nextjs for convenient integration with nextjs + SSR.


Note

Adds a production-ready Next.js App Router integration for Immutable Auth with Auth.js v5, plus SDK and sample app wiring.

  • New @imtbl/auth-nextjs package: createImmutableAuth, server middleware/HOCs, React provider/hooks, CallbackPage, token refresh utilities, and types/constants
  • Updates @imtbl/auth to emit TOKEN_REFRESHED and tests refreshTokenAndUpdatePromise for session sync
  • SDK re-exports: new auth_nextjs (root/client/server) and auth/wallet entrypoints; updated exports map
  • Passport sample app: adds NextAuth routes per env, wraps app with ImmutableAuthProvider, adds callback page and demo component, toggles API routes via config
  • Workspace/setup tweaks: include new package, lockfile updates; increase pre-commit Node heap; serialize npm network concurrency in .npmrc

Written by Cursor Bugbot for commit 3cdbada. This will update automatically on new commits. Configure here.

@shineli1984 shineli1984 requested a review from a team as a code owner December 17, 2025 01:09
@socket-security
Copy link

socket-security bot commented Dec 17, 2025

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.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
HTTP dependency: npm @imtbl/contracts depends on https://github.com/immutable/seaport.git#1.6.0+im4

Dependency: seaport-16@https://github.com/immutable/seaport.git#1.6.0+im4

Location: Package overview

From: examples/contracts/package.jsonnpm/@imtbl/[email protected]

ℹ Read more on: This package | This alert | What are http dependencies?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Publish the HTTP URL dependency to a public or private package repository and consume it from there.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@imtbl/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
HTTP dependency: npm @imtbl/contracts depends on https://github.com/immutable/seaport-core.git#1.6.0+im2

Dependency: seaport-core-16@https://github.com/immutable/seaport-core.git#1.6.0+im2

Location: Package overview

From: examples/contracts/package.jsonnpm/@imtbl/[email protected]

ℹ Read more on: This package | This alert | What are http dependencies?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Publish the HTTP URL dependency to a public or private package repository and consume it from there.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@imtbl/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

Copy link
Contributor

@cursor cursor bot left a 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.

@nx-cloud
Copy link

nx-cloud bot commented Dec 17, 2025

View your CI Pipeline Execution ↗ for commit 3cdbada

Command Status Duration Result
nx affected -t build,lint,test ✅ Succeeded 1m 9s View ↗
nx run-many -p @imtbl/sdk,@imtbl/checkout-widge... ✅ Succeeded 2s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-09 05:17:59 UTC

@nx-cloud
Copy link

nx-cloud bot commented Dec 17, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit e9cfdc4

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 (publicPaths) {
const isPublic = publicPaths.some((pattern) => {
if (typeof pattern === 'string') {
return pathname === pattern || pathname.startsWith(pattern);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path matching incorrectly matches unrelated routes with same prefix

The string pattern matching in createAuthMiddleware uses pathname.startsWith(pattern) without checking for path boundaries. This causes /dashboardxyz to incorrectly match a pattern of /dashboard, since "/dashboardxyz".startsWith("/dashboard") is true. The fix would be to check for exact match or ensure the next character is a path separator: pathname === pattern || pathname.startsWith(pattern + '/').

Additional Locations (1)

Fix in Cursor Fix in Web

return pattern.test(pathname);
});
if (isPublic) {
return NextResponse.next();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Middleware path matching allows unintended prefix matches

The string pattern matching in createAuthMiddleware uses pathname.startsWith(pattern) which incorrectly matches any path that begins with the pattern string, not just paths that are children of that route. For example, a publicPaths entry of '/api' would match /api, /api/foo, but also unintended paths like /apikey or /api-private. This is particularly dangerous for publicPaths as it could inadvertently bypass authentication for routes that weren't intended to be public. The fix would be to check pathname === pattern || pathname.startsWith(pattern + '/') to ensure proper path segment matching.

Additional Locations (1)

Fix in Cursor Fix in Web

if (overrides.callbacks.redirect) {
composedCallbacks.redirect = overrides.callbacks.redirect;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User-provided authorized callback is silently dropped

The callback composition logic handles jwt, session, signIn, and redirect callbacks, but the Auth.js v5 authorized callback is not handled. When composedCallbacks is assigned to the final config at line 139, it completely overwrites overrides.callbacks, causing any user-provided authorized callback to be silently dropped. Users following Auth.js documentation who try to use the authorized callback for middleware authorization will find it has no effect, with no warning or error.

Additional Locations (1)

Fix in Cursor Fix in Web

if (searchParams.get('error')) {
handleOAuthError();
return;
}
Copy link
Contributor

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)

Fix in Cursor Fix in Web

if (searchParams.get('code') && !callbackProcessedRef.current) {
callbackProcessedRef.current = true;
handleCallback();
}
Copy link
Contributor

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.

Fix in Cursor Fix in Web


setAuth(newAuth);
setIsAuthReady(true);
}, [config]);
Copy link
Contributor

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.

Fix in Cursor Fix in Web

setAuth(null);
setIsAuthReady(false);
prevConfigRef.current = null;
};
Copy link
Contributor

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)

Fix in Cursor Fix in Web

setAuth(null);
setIsAuthReady(false);
};
}, [config]);
Copy link
Contributor

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.

Fix in Cursor Fix in Web

"dependencies": {
"@biom3/react": "^0.27.12",
"@imtbl/sdk": "workspace:*",
"@imtbl/sdk": "file:.yalc/@imtbl/sdk",
Copy link
Contributor

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)

Fix in Cursor Fix in Web

...(update.idToken ? { idToken: update.idToken } : {}),
...(update.accessTokenExpires ? { accessTokenExpires: update.accessTokenExpires } : {}),
...(update.zkEvm ? { zkEvm: update.zkEvm } : {}),
};
Copy link
Contributor

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.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants