supabase / auth-helpers

A collection of framework specific Auth utilities for working with Supabase.
https://supabase.github.io/auth-helpers/
MIT License
908 stars 232 forks source link

feat: auth-helpers-nextjs total fix with createMiddlewareClientV2 #769

Open hf opened 7 months ago

hf commented 7 months ago

createMiddlewareClient was basically completely broken for correctly syncing the session state between the server and browser parts of a NextJS application.

It works in most situations where the user is actively interacting with the site, but as soon as they leave the site for an extended period of time and come back, it's very likely that after the JWT expiry time they will receive an Invalid Refresh Token error from the server. Because these events are not time-correlated, it's impossible to debug what is causing them.

Why does this happen:

  1. NextResponse.next() is not enough. Pages, server-rendered components and route handlers do not see the Set-Cookie header set on the response.
  2. Because this runs in the middleware.ts file, the session is correctly refreshed in the middleware client and the Set-Cookie headers are set on the response so the browser syncs up with the state of the middleware client.
  3. However, the page or component client rendered on the server still only sees the cookie header from the request which is the old session and any use of getSession() or getUser() will refresh the session every time. If you have, say, 3 Data API calls to PostgREST, it's very likely that the session would be refreshed 3 times.
  4. Because the page or component cannot set headers on the response, these refreshed sessions just remain unused, but the Auth server already tracks them as the "later" refresh tokens for the user session and is expecting (for security reasons) the browser to know about them too.
  5. One hour after this has happened, the Supabase client on browser or server will try to refresh the session from step 2 (coming from middleware.ts) and be greeted with the "Invalid Refresh Token" error.

How is this being fixed:

The design of createMiddlewareClient does not care at all about what happens to the request, but this is incredibly important so the new refreshed session is passed down to the page or server-rendered component. Furthermore, this cannot "just be set" as for the propagation to take place, NextResponse.next({ request }) needs to be called.

Therefore, createMiddlewareClient is fully deprecated and replaced with its new counterpart createMiddlewareClient2. This works a bit differently (without reinventing the simplistic beauty of the @supabase/ssr package):

export function middleware(request: NextRequest) {
  const [supabaseClient, nextResponse] = createMiddlewareClient2(request, { /* standard client options */ })

  const { data: { user } } = await supabaseClient.auth.getUser()

  if (!user) {
    // there's no user session, ask them to log in
    return NextResponse.redirect(new URL('/sign-in-page-in-your-app'))
  }

  return nextResponse()
}

Firstly it returns two arguments, the Supabase client and a nextResponse function. It should be called towards the bottom of the middleware.ts file, ideally in the return statement.

The client is setup such that, it initially reads from the request's Cookie header, but writes the changes to local memory. It tracks what items (not cookies!) are being set or deleted.

When you call nextResponse() the requests's Cookie header is reset to reflect the new state of the Supabase client. Because this header will just be propagated down to the page or server-rendered component, no cookie chunking is necessary.

Then, it calls:

NextResponse.next({
  request
})

Which ensures that the request's changes will be actually propagated down to the page or component. This enables the clients there to pick up the Cookie header which now includes the refreshed session.

Finally, it sets the Set-Cookie header on the response, which eventually reaches the browser and the session state is finally synchronized between them.

hf commented 7 months ago

pnpm build:nextjs fails - also, if createMiddlewareClient is broken, why not just modify the implementation to call createMiddlewareClient2 under the hood instead?

As described in the PR -- to forward the headers NextResponse.next() should be called after the client has processed the session.

kangmingtay commented 7 months ago

@hf you need to export the new client in the index.ts file too

kangmingtay commented 7 months ago

@hf the return types don't show up correctly when the client is instantiated, which isn't great DX-wise

const [supabaseClient, nextResponse] = createMiddlewareClient2(req);

it seems to think that createMiddlewareClient2 returns (SupabaseClient<any, "public", any> | (() => NextResponse<...>))[] (note the |)