Skip to content
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

AuthApiError: Invalid Refresh Token: Refresh Token Not Found when refreshing token in middleware #68

Open
2 tasks done
lourd opened this issue Sep 22, 2024 · 10 comments
Open
2 tasks done
Labels
bug Something isn't working

Comments

@lourd
Copy link

lourd commented Sep 22, 2024

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

I'm inadvertently logged out of my web application every day, with this error in the back-end logs: AuthApiError: Invalid Refresh Token: Refresh Token Not Found.

I recently set up Supabase auth on my new Next.js application. I followed the Setting up Server-Side Auth for Next.js guide, with some small tweaks. My code looks like this:

middleware.ts
import { type NextRequest } from "next/server"

import { updateSession } from "./app/supabase/middleware"

export async function middleware(request: NextRequest) {
  return await updateSession(request)
}

export const config = {
  matcher: [
    /*
     * Match all request paths except for the ones starting with:
     * - _next/static (static files)
     * - _next/image (image optimization files)
     * - favicon.ico (favicon file)
     * - @vercel/speed-insights script
     */
    "/((?!_next/static|_next/image|favicon.ico|_vercel/speed-insights|.*\\.(?:svg|png|jpg|jpeg|gif|webp)$).*)",
  ],
}
app/supabase/middleware.ts
import { CookieMethodsServer, createServerClient } from "@supabase/ssr"
import { NextResponse, type NextRequest } from "next/server"

export async function updateSession(request: NextRequest) {
  const response = NextResponse.next({ request })
  const supabase = createServerClient(
    process.env.NEXT_PUBLIC_SUPABASE_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookies: nextMiddlewareCookies(request, response),
    }
  )
  await supabase.auth.getSession()
  return response
}

function nextMiddlewareCookies(
  req: NextRequest,
  res: NextResponse
): CookieMethodsServer {
  return {
    getAll() {
      return req.cookies.getAll()
    },
    setAll(cookiesToSet) {
      cookiesToSet.forEach((cookie) => {
        req.cookies.set(cookie.name, cookie.value)
        res.cookies.set(cookie.name, cookie.value, cookie.options)
      })
    },
  }
}

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Use the snippets above to set up token refresh with middleware
  2. Log in
  3. Wait 24 hours
  4. Load any page that will hit the middleware, which for me is any page of my application.
  5. Get the AuthApiError: Invalid Refresh Token: Refresh Token Not Found error in your middleware logs

Expected behavior

The token should refresh if there is one and it's expired, without an error. If there's not a session, it should not try to refresh it, and there should be no error.

Screenshots

From my Vercel logs:
Screenshot 2024-09-22 at 12 24 16 PM

System information

  • OS: macOS and Ubuntu
  • Browser (if applies): Chrome, Safari and Firefox
  • Version of supabase-js: 2.45.4
  • Version of @supabase/ssr: 0.5.1
  • Version of Next.js: 14.2.13 and 15.0.0-canary.159
  • Version of Node.js: 20.12.1

Additional context

This appears to be the same or a similar bug as supabase/auth-helpers#436, as well as supabase/auth-js#620

I've tried forcing the issue to happen more quickly by shortening the JWT expiry to 60 seconds, but it refreshes just fine. There's something with the 24 hour window that seems to be causing the error.

@lourd lourd added the bug Something isn't working label Sep 22, 2024
@lourd
Copy link
Author

lourd commented Sep 22, 2024

I'll point out that I didn't copy the guide code 100% because it doesn't seem necessary to loop through the cookiesToSet twice and make a new response each time, seems inefficient to me. These lines:

https://github.com/supabase/supabase/blob/9e0e4f652b772d37cf5da1388f0fd3d71ed3dd39/apps/docs/content/guides/auth/server-side/nextjs.mdx?plain=1#L237-L244

Please correct me if I'm wrong!

@lourd
Copy link
Author

lourd commented Sep 22, 2024

Also, you don't need to set the updated cookies on the request and the response in the middleware. Setting them on the response makes them available to the downstream server components reading from cookies() as of this change back in April vercel/next.js#65008

@nickdobos
Copy link

I'm also running into this via the Swift SDK :(

@danielgwilson
Copy link

Also getting this

@kangmingtay
Copy link
Member

I'll point out that I didn't copy the guide code 100% because it doesn't seem necessary to loop through the cookiesToSet twice and make a new response each time, seems inefficient to me.

@lourd it's not doing the same thing twice:

  • on the first time, you're setting the cookies on the request, which is sent to the server components in nextjs
  • the second time, you're setting the "Set-Cookie" header on the response, which tells the client to set the cookie in the header when it makes the next request (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie)

have you tried following the guide 100% to see if it works for you?

@lourd
Copy link
Author

lourd commented Oct 2, 2024

Hi @kangmingtay, yes, I have tried following the guide 100% and still get the error. I made the changes after it was having errors, to make sure I understood what was going on.

it's not doing the same thing twice:

Correct, it's not doing the same thing twice, but it is looping twice. You can accomplish updating the request and response cookies in one loop. You can also update the existing response instead of creating a new response object on each call to setAll.

My code, copied from my app/supabase/middleware.ts snippet

      setAll(cookiesToSet) {
        cookiesToSet.forEach((cookie) => {
          req.cookies.set(cookie.name, cookie.value)
          res.cookies.set(cookie.name, cookie.value, cookie.options)
        })
      }

The example code:

      setAll(cookiesToSet) {
        cookiesToSet.forEach(({ name, value, options }) => request.cookies.set(name, value))
        supabaseResponse = NextResponse.next({ request })
        cookiesToSet.forEach(({ name, value, options }) => supabaseResponse.cookies.set(name, value, options))
      }

@kangmingtay
Copy link
Member

@lourd i think the problem here is because of how you are passing down the response in your middleware.ts file (i've added comments in the places where it seems problematic to me):

middleware.ts (with comments)
export async function updateSession(request: NextRequest) {
  // this response object is being passed into nextMiddlewareCookies which may
  // contain stale "Set-Cookie" headers 
  const response = NextResponse.next({ request })

  const supabase = createServerClient(
    process.env.NEXT_PUBLIC_SUPABASE_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookies: nextMiddlewareCookies(request, response),
    }
  )

  // you want to use getUser() here rather than getSession() since this is on the server-side
  await supabase.auth.getSession()
  return response
}

function nextMiddlewareCookies(
  req: NextRequest,
  res: NextResponse
): CookieMethodsServer {
  return {
    getAll() {
      return req.cookies.getAll()
    },
    setAll(cookiesToSet) {
      cookiesToSet.forEach((cookie) => {
        req.cookies.set(cookie.name, cookie.value)

        // this uses the initial NextResponse.next() defined in updateSession
        // in the e.g, we are deliberately creating a new NextResponse.next() object to remove any stale "Set-Cookie" headers
        res.cookies.set(cookie.name, cookie.value, cookie.options)
      })
    },
  }
}

yes, I have tried following the guide 100% and still get the error.

One way to debug this issue is to print out the cookie headers before returning the response (you can also check the network tab to see what the "Set-Cookie" response header contains and what cookies are set on the subsequent request. If you're still having troubles with getting this to work, can you please open a ticket with us at https://supabase.help ? We would love to help get to the bottom of this!

@lourd
Copy link
Author

lourd commented Oct 3, 2024

Hey @kangmingtay, really appreciate you investigating this!

Thankfully Next.js handles the cases of possible "staleness" or duplicate calls to set a cookie that you've illustrated. Here's a test case illustrating that behavior. I've just run it again using Next.js 14.2.14.

Create a new Next.js app, create a middleware.ts module. In the middleware, do this:

import { type NextRequest, NextResponse } from "next/server";

export async function middleware(request: NextRequest) {
  const cookies = request.cookies;
  console.log("incoming cookies", cookies.getAll());
  const res = NextResponse.next({ request });
  let i = 0;
  request.cookies.set("foo", `${i}`);
  res.cookies.set("foo", `${i}`);
  request.cookies.set("foo", `${i + 1}`);
  res.cookies.set("foo", `${i + 1}`);
  request.cookies.set("foo", `${i + 2}`);
  res.cookies.set("foo", `${i + 2}`);

  console.log("end of middleware");
  console.log("request headers", [...request.headers.entries()]);
  console.log("response headers", [...res.headers.entries()]);
  return res;
}

export const config = {
  matcher: [
    /*
     * Match all request paths except for the ones starting with:
     * - _next/static (static files)
     * - _next/image (image optimization files)
     * - favicon.ico (favicon file)
     * - @vercel/speed-insights script
     */
    "/((?!_next/static|_next/image|favicon.ico|_vercel/speed-insights|.*\\.(?:svg|png|jpg|jpeg|gif|webp|php)$).*)",
  ],
};

In app/page.tsx, log the incoming values:

export default function Home() {
  const cooks = cookies();
  const header = headers();
  console.log("Home page server component");
  console.log("incoming headers", [...header.entries()]);
  console.log("incoming cookies", cooks.getAll());
  ...
}

Load up localhost:3000 in an incognito window.

Here is what you see in the middleware logs

  • No incoming cookies
  • The request has a single cookie header: [ 'cookie', 'foo=2' ],
  • The response has a single set-cookie header: [ 'set-cookie', 'foo=2; Path=/' ],

In the server component logs:

  • The list from headers() contains:
  [ 'set-cookie', 'foo=2; Path=/' ],
  [ 'x-middleware-set-cookie', 'foo=2; Path=/' ],
  [ 'cookie', 'foo=2' ]
  • The list from cookies() contains a single element: [ { name: 'foo', value: '2' } ]

And in the browser's network response for the document, there is a single Set-Cookie header on the response, with the value foo=2; Path=/, and a X-Middleware-Set-Cookie header, with the same value as the Set-Cookie header: foo=2; Path=/

All that to say, when setting the same cookie on either the request or response, it is properly deduped. There's no need to keep making a new response.

I ran this test on both my local server and deployed on Vercel - the results are the same.


you want to use getUser() here rather than getSession() since this is on the server-side

Do I have to use getUser here? The thinking behind using getSession is that I'm not actually referencing any user data here. All I want to do is refresh the token if it's there and it's expired. I don't want to make a network request if it's not necessary. If the session is valid and non-expired, there should be no network request. IIUC that's what getSession does. It appears to work as expected, properly refreshing tokens, with the exception of this 24-hours-later bug.

Additionally, I've used getUser instead of getSession and it produced the same error.

One way to debug this issue is to print out the cookie headers before returning the response

The error occurs as part of the call to getSession, before any headers have been set on the response. Do you mean to suggest I print out the cookie headers on the request?

@shivpratik
Copy link

Even I have run into same issue. When I run npm audit, I get @supabase/ssr depends on vulnerable versions of cookie.

@lourd
Copy link
Author

lourd commented Oct 10, 2024

I've been keeping a log of how often it happens to me. It doesn't happen at a consistent interval. The longest I've gone without the problem is 3 days. The shortest I've gone without the problem is still ~24 hours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants