Replies: 86 comments 43 replies
-
If true, that would be a very nice fix! So basically there is a race condition here, where two calls made to the /ap/auth/session endpoint will both try to refresh the token, and the second one finishing fails. Right? |
Beta Was this translation helpful? Give feedback.
-
Yes correct, let me explain it better with a practical example. Imagine you issue The way it works is that when you exchange the Now imagine a timeline where you have 3 tabs open with the same app 00:00 the user opens { access_token: "at1", refresh_token: "rt1", expire_in: "15min" } 00:01 the user opens another tab with { access_token: "at1", refresh_token: "rt1", expire_in: "15min" } 05:02 the user opens another tab and keeps browsing 1st request is successful: exchange the { access_token: "at2", refresh_token: "rt2", expire_in: "15min" } 2nd and 3rd request fail with 400 because they are trying to use the { access_token: "at1", refresh_token: "rt1", expire_in: "15min", error: "RefreshAccessTokenError" } Now we have 3 big problems:
|
Beta Was this translation helpful? Give feedback.
-
Love the details, make sense! I'll come back to this, thank you! At the end of the day, I think the main problem here is not the broadcasting, but the race condition. Similar to #1455 |
Beta Was this translation helpful? Give feedback.
-
Yes but unfortunately the race condition is not solvable in a stateless serverless environment that’s why I think the only way to solve it is to prevent this from happening due to the broadcasting. To solve this at the server level we would need a persisted state of the refresh token requests that are in flight in order to stop the subsequent request and instead wait but it starts to become very complex :) |
Beta Was this translation helpful? Give feedback.
-
I see, and I was thinking the same. fixing it by making it possible to disable broadcasting is half way, but what about clientside code where after some stale time, let's say two requests are going to an external api and they use the access token from session? I know react-query does fetch deduplication, so maybe we need to do something similar in our client code? |
Beta Was this translation helpful? Give feedback.
-
I’m using SWR for the requests which works similarly to react-query for the deduplication and that’s fine. The problem imho is not deduplicating on the client side for a single page but trying to deduplicate for all the pages. I’m trying to think at the problem from a different perspective: how about the broadcast is not saying “hey all tabs please refresh the session” but rather “hey I’m |
Beta Was this translation helpful? Give feedback.
-
Don't know if SWR does that, but react-query deduplicates site-wise, using a React Context. (Or by pages I guess you meant tabs?) What you are saying how to go about broadcasting does sound interesting, and I would rather not introduce new options, if we can work something out transparently for this. |
Beta Was this translation helpful? Give feedback.
-
Going to bed now, I’ll write down a proposal tomorrow with some scenario |
Beta Was this translation helpful? Give feedback.
-
On my side the main question for you @balazsorban44 is: What problems are we trying to solve broadcasting the
|
Beta Was this translation helpful? Give feedback.
-
@balazsorban44 sorry to bother you, did you have a chance to read my last comment? |
Beta Was this translation helpful? Give feedback.
-
Will have a proper read of this in the next few days, I hope. I actually just had to deal with this problem at work, came to a conclusion and used some different approaches, but ultimately I think supporting token rotation will be something that will need some more thinking. This has certainly become rather a feature request than a bug. |
Beta Was this translation helpful? Give feedback.
-
Thanks for the update, I see your point, on my side I was more inclined to define this a bug since refreshing a token feels to me like a basic necessity rather than an "additional feature" for modern web apps 😄 |
Beta Was this translation helpful? Give feedback.
-
I would only call it a bug, if the functionality is already built-in, but do not deliver what it is meant to. A feature request is when we want to introduce a functionality that hasn't existed yet in the library. We still do not have built-in token rotation, and partially from the above mentioned issues, it is not trivial at the moment. I have looked around the web to see how others are doing it, and honestly, I haven't found a go-to solution. The main problem here be it tab sync or other reasons of race conditions, when a token refresh is in flight and a new request wants an updated token, the old refresh token will be used again, which will lead to a problem. According to my findings, the only way to avoid this is orchestrating (read deduplication) the client-side requests to To answer the question
I have to say I am not 100% sure what real-life use cases @iaincollins have looked at when he originally implemented it, but he probably had a reason. Unfortunately, I wasn't around then yet, so I cannot give an honest answer, maybe if Iain sees this thread one day, he can explain it. Until then, if an option to disable this behavior helps us here in any way, I am all for it, but I don't think it will be enough. I think if we want to move forward with built-in token rotation, we will need to complicate our clilent-side code slightly, so it handles deduplicated requests to |
Beta Was this translation helpful? Give feedback.
-
Sorry but unless there is something that I didn’t understand, I don’t think what you are saying is correct and you might not be looking at the problem from the correct angle. EDIT: I saw your edit after and the reply to my answer, thank you 😄 Thanks for your time and patience |
Beta Was this translation helpful? Give feedback.
-
OK, so we might have diverged at to So in theory, a single boolean flag to disable broadcasting would do it then? Unless there is a more complicated way that would make it possible to de-duplicate even across open tabs? 😄 |
Beta Was this translation helpful? Give feedback.
-
Is there any update to this, apart from #3940 (comment) ? |
Beta Was this translation helpful? Give feedback.
-
Tried to implement #3940 (comment) Here are some of the issues that came up
To quote
It was working fine for a few days, until redlock started to fail to achieve quorum on multiple occasions. ( not sure why...I am no expert in redlock). Also we started getting hit with multiple All of this resulted in a lot of stale data , failing refresh requests and invalid token responses from the backend, resulting in users getting logged out left right and center. Is the use rotating refresh tokens and a custom back-end API which generates access and refresh token pairs an anti-pattern with NextAuth? I would love to hear what others have to say about this. Did I steer to a completely wrong direction? Or did I make any wrong assumptions? |
Beta Was this translation helpful? Give feedback.
-
I'm also affected with this issue and I only use one tab. What I do though to test is I've set throttle to slow the connection [i've set it to either GRPS, 2G or Good 2G] to my authentication server to renew the token - so when the tab regains focus, it triggers the JWT callback... The kicker now would be is since it's throttled connection, I have enough wait time that I can hit the browser's refresh button which actually sends another request to trigger the JWT call back (and this is not an edge case behaviour for users since users can be impatient and will be glad to refresh the browser). So now comes the issue, since the tab refocus has already caused the script to send a refresh request, and the server has already received it (over slow connection) the refresh token that came earlier has actually been invalidated, but the browser refresh will still use the old token, therefore the entire refresh cycle is broken though this side effect. This also means that the "new access token" in transit (from the request caused by the tab/browser refocus) is lost because the browser refresh is in progress (while it sends the same token) This leads to three major draw backs
The only remedy I agree would be to have a configurable flag to prevent a refocus to trigger a JWT request/refresh thus giving priority for manual refresh - and I think this is recommended behaviour (if a possible fix is in place) since the refresh is ought to be directly-triggered actions particularly a click to a link or button, or navigation to a route akin to how mobile apps also trigger refreshes. |
Beta Was this translation helpful? Give feedback.
-
Has anything changed in the last two years, or is the go-to solution still #2071 (comment) ? @balazsorban44 Since #7056 got released, maybe there's a plan for the built-in refresh token strategy? |
Beta Was this translation helpful? Give feedback.
-
Is there any update on this? I'm able to replicate this issue on a single tab wherein I have two components on a page that both call What's I'm able to replicate is that the initial sign-on works, the first refresh also works. I've modified this call to always call the refresh, so that I don't have to wait X minutes, but I'm observing the behaviour even if I do wait. I.E
Combined with the refresh code (which has been taken from the OAuth rotation guide), ends up with logs like so:
|
Beta Was this translation helpful? Give feedback.
-
I also wished to disable the broadcast event (for reasons other than token refresh issues) and found a way to do it. If you ensure this code runs before the nextauthjs package, it will prevent the page receiving and broadcasting auth change events.
|
Beta Was this translation helpful? Give feedback.
-
Would a potential solution (instead of using Redis) be to use locks with a key corresponding to the user? Some pseudo-code would be:
Essentially, you acquire a lock and release it after you've gotten your token. Would a library like async-mutex work well for this or would the application get bogged down? I'm used to C# so using semaphores is pretty standard, but get kind of lost with this JS/TS stuff. |
Beta Was this translation helpful? Give feedback.
-
I found a solution that is provider agnostic using async-lock, where the finished product would look something like this:
The only thing to implement there is the cache provider, which it seems needs to be somewhere other than in here so that the cache is not destroyed when in dev mode. I believe something like this would be agnostic enough to be included in the standard library without causing too much of an issue. I.E using a form of queue-system to implement the locking and then insert a provider for the cache which implements an interface for getting / setting keys. Also not agaisnt having this stay in user land, but to document it properly as this is a major PIA |
Beta Was this translation helpful? Give feedback.
-
If you're using Next.js 13 and the App Router, you can add time-based revalidation with some arbitrary amount to your refresh access token https://nextjs.org/docs/app/building-your-application/caching#time-based-revalidation |
Beta Was this translation helpful? Give feedback.
-
For me the issue is purely on refreshing the page, or opening a new tab after expiration. if the all gives the previous token (expired). so the first one successfully refreshes the token, then the second one (it is also the expired one), so it will try for a The solution I did i used a map, if a Please note that I only need the access_token in the server so no need to pass it to the session. It is working.But needed others thoughts on this |
Beta Was this translation helpful? Give feedback.
-
My solution: import mem from 'mem'
export const refreshTokenAPI = mem(
async (tokenType: string, refreshToken: string) => {
return publicRequest<TAuthToken>('/user/auth/refresh', {
method: 'POST',
headers: {
Authorization: `${tokenType} ${refreshToken}`,
},
})
},
{
maxAge: 1_000,
cacheKey: (arguments_) => arguments_.join(','),
}
)
callbacks: {
async jwt({ token, user }) {
if (user) {
token.user = user
return token
}
const isTokenStillValid = token.user.expiredAt - getCurrentTimeStamp() > 0
if (isTokenStillValid) return token
try {
const { accessToken, refreshToken, expiredAt, tokenType } =
await refreshTokenAPI(token.user.tokenType, token.user.refreshToken)
return {
user: {
...token.user,
accessToken,
refreshToken,
tokenType,
expiredAt: expiredAt - REFRESH_TOKEN_OFFSET,
},
}
} catch {
return {
...token,
error: REFRESH_TOKEN_EXPIRED_ERROR,
}
}
},
}, |
Beta Was this translation helpful? Give feedback.
-
@nguyenhuugiatri @VGontier-cmd @ARJohnsonKwik @mohammedsafvan A cleaner solution is to use middleware.ts, as it's more inline with the official next.js docs: https://nextjs.org/docs/pages/building-your-application/authentication Solution available here (hats off to the guy Rinvii who first came up with it): #9715 In short, you implement the refresh token rotation logic in middleware, and force getServerSession() to read the new session and send it back to the browser by setting the cookies. You do something like this: import { NextResponse, type NextMiddleware, type NextRequest } from "next/server";
import { encode, getToken, type JWT } from "next-auth/jwt";
import {
admins,
SESSION_COOKIE,
SESSION_SECURE,
SESSION_TIMEOUT,
SIGNIN_SUB_URL,
TOKEN_REFRESH_BUFFER_SECONDS
} from "./config/data/internalData";
let isRefreshing = false;
export function shouldUpdateToken(token: JWT): boolean {
const timeInSeconds = Math.floor(Date.now() / 1000);
return timeInSeconds >= token?.expires_at - TOKEN_REFRESH_BUFFER_SECONDS;
}
export async function refreshAccessToken(token: JWT): Promise<JWT> {
if (isRefreshing) {
return token;
}
const timeInSeconds = Math.floor(Date.now() / 1000);
isRefreshing = true;
try {
const response = await fetch(process.env.AUTH_ENDPOINT + "/o/token/", {
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({
client_id: process.env.CLIENT_ID,
client_secret: process.env.CLIENT_SECRET,
grant_type: "refresh_token",
refresh_token: token?.refresh_token
}),
credentials: "include",
method: "POST"
});
const newTokens = await response.json();
if (!response.ok) {
throw new Error(`Token refresh failed with status: ${response.status}`);
}
return {
...token,
access_token: newTokens?.access_token ?? token?.access_token,
expires_at: newTokens?.expires_in + timeInSeconds,
refresh_token: newTokens?.refresh_token ?? token?.refresh_token
};
} catch (e) {
console.error(e);
} finally {
isRefreshing = false;
}
return token;
}
export function updateCookie(
sessionToken: string | null,
request: NextRequest,
response: NextResponse
): NextResponse<unknown> {
/*
* BASIC IDEA:
*
* 1. Set request cookies for the incoming getServerSession to read new session
* 2. Updated request cookie can only be passed to server if it's passed down here after setting its updates
* 3. Set response cookies to send back to browser
*/
if (sessionToken) {
// Set the session token in the request and response cookies for a valid session
request.cookies.set(SESSION_COOKIE, sessionToken);
response = NextResponse.next({
request: {
headers: request.headers
}
});
response.cookies.set(SESSION_COOKIE, sessionToken, {
httpOnly: true,
maxAge: SESSION_TIMEOUT,
secure: SESSION_SECURE,
sameSite: "lax"
});
} else {
request.cookies.delete(SESSION_COOKIE);
return NextResponse.redirect(new URL(SIGNIN_SUB_URL, request.url));
}
return response;
}
export const middleware: NextMiddleware = async (request: NextRequest) => {
const token = await getToken({ req: request });
const isAdminPage = request.nextUrl.pathname.startsWith("/epa");
const isAuthenticated = !!token;
let response = NextResponse.next();
if (!token) {
return NextResponse.redirect(new URL(SIGNIN_SUB_URL, request.url));
}
if (shouldUpdateToken(token)) {
try {
const newSessionToken = await encode({
secret: process.env.NEXTAUTH_SECRET,
token: await refreshAccessToken(token),
maxAge: SESSION_TIMEOUT
});
response = updateCookie(newSessionToken, request, response);
} catch (error) {
console.log("Error refreshing token: ", error);
return updateCookie(null, request, response);
}
}
if (isAdminPage && isAuthenticated && !admins.includes(token.email!)) {
return NextResponse.redirect(new URL("/forbidden", request.url));
}
return response;
};
export const config = {
matcher: ["/dashboard/:path*", "/epa/:path*"]
}; Make sure you read all the comments on the references page to apply this to your situation |
Beta Was this translation helpful? Give feedback.
-
pleasee help me, i can't refresh my token my middleware :
my main middleware :
my auth :
|
Beta Was this translation helpful? Give feedback.
-
Same here. Token doesn't update. |
Beta Was this translation helpful? Give feedback.
-
Description 🐜
Took me a bit of time but I think I finally understood why we were seeing so many errors related to refreshing the token in our app.
I'm following the suggested setup to refresh tokens https://next-auth.js.org/tutorials/refresh-token-rotation which works fine.
The issue is the broadcasting that if you have more than 1 tab open with your web app, it will trigger a session check for EVERY tab and then you end up triggering the
refreshAccessToken()
function multiple times simultaneously with the same set of parameters (samerefresh_token
)This creates an issue because the first request will be successful and obtain a new
access_token
paired with arefresh_token
while all the other calls that were made will get a 400 since the oldrefresh_token
has just been invalidated by the first successful call.Simplest way to fix this would be to have the option to disable broadcasting and let each tab try to refresh the session only when focused.
EDIT: 3 scenarios where this is a problem
getSession
event when a session refresh is already in flightCheck here for a lock mechanism proposed solution:
#2071 (comment)
How to reproduce ☕️
In the app try to set a
clientMaxAge
of 10 secondsThen add a log in
[...nextauth].js
jwt
callbackOpen 2 tabs and move between them, you will notice that every 10 seconds there are 2 calls being made to refresh the session and 2 console logs
I can create a codesandbox later after I finish work if needed but it's pretty simple to verify I believe 😄
I'm using
"next-auth": "3.23.3"
Beta Was this translation helpful? Give feedback.
All reactions