sidebase / nuxt-auth

Authentication built for Nuxt 3! Easily add authentication via OAuth providers, credentials or Email Magic URLs!
https://auth.sidebase.io
MIT License
1.31k stars 164 forks source link

Refresh Token Rotation does not update cookie if called server side. #200

Closed ayalon closed 1 year ago

ayalon commented 1 year ago

Environment

No response

Reproduction

No response

Describe the bug

I have spent hours on making the Refresh Token Rotation work based on the existing example. https://sidebase.io/nuxt-auth/recipes/directus and https://next-auth.js.org/tutorials/refresh-token-rotation

As a test scenario, I have integrated an OAuth server and was able to authenticate against it. I set the AccessToken lifetime to 10 seconds. After 10 seconds I did a hard refresh in the browser to simulate a server side request on Nuxt. This correctly fetches a new AccessToken and a RefreshToken.

Unfortunatly this does not update the JWT cookie in the browser, the old cookie is sent along.

After 10 seconds I did another hard refresh. This results in an error, because another refresh happens but based on an invalidated refresh token from the previous cookie.

After debugging I figured out, that the token is refreshed correctly. This happens in the route middleware where useSession() is called. There a fetch to the session server endpoint happens and the token is updated correctly.

To make it work Iextracted the the setCookie header from the request and passed it along with the server side request.

In order to support refresh token rotation I propose the following fix:

File: https://github.com/sidebase/nuxt-auth/blob/6696abdaed350b066ab74c9541e88c774743255a/src/runtime/composables/useSession.ts#L203

  const headers = await getRequestCookies(nuxt)

  return _fetch<SessionData>(nuxt, 'session', {
    onResponse: ({ response }) => {
      const sessionData = response._data

      // Pass the new cookie to the server side request   
      if (process.server) {
        if (response.headers.get('set-cookie')) {
          const setCookieValue = response.headers.get('set-cookie')
          appendHeader(nuxt.ssrContext.event, 'set-cookie', setCookieValue)
        }
      }

      data.value = isNonEmptyObject(sessionData) ? sessionData : null
      loading.value = false

      if (required && status.value === 'unauthenticated') {
        return onUnauthenticated()
      }

      return sessionData
    },

Basically I'm just appending the updated setCookie header to the existing SSR request. This will updated the cookie correctly.

I can also make a pull request as soon as someone validated my proposal.

Additional context

No response

Logs

No response

vanling commented 1 year ago

@ayalon I thought I was going crazy, thank you for getting my sanity back. Debugging refresh tokens was no fun πŸ˜‚

BracketJohn commented 1 year ago

Hey @vanling and @ayalon 〰️

I've reviewed #215 and it is basically ready to go.

I can also make a pull request as soon as someone validated my proposal.

Thanks for the offer, I din't get around to this yet - sorry for that. But I'm even happier that you still took up the PR @vanling (: Let's get this merged and hopefully help a lot of other people.

vanling commented 1 year ago

yeah sorry for my impatience ;) I am just way too excited that the Directus login finally worked using this :D

BracketJohn commented 1 year ago

:D no worries at all - thanks for the initiative (: Actually I just saw that there's also TS-errors in #215, should still be a quick fix. And I can do a quick release after we get that done + merged 🎊

vanling commented 1 year ago

πŸŽ‰

BracketJohn commented 1 year ago

Will do a release in a second so that you can verify the fix!

BracketJohn commented 1 year ago

Release is out, please verify with @sidebase/nuxt-auth@0.4.0-alpha.6 and let me know if it resolves your problem (:

vanling commented 1 year ago

Thanks @BracketJohn and @ayalon !

BracketJohn commented 1 year ago

Yaaaay - this is awesome ❀️ Is anything else in the directus-recipe misleading / not working for you? (reference: https://sidebase.io/nuxt-auth/recipes/directus)

Other than that: Also thank you @vanling for taking the initiative here - this is what was needed in the end (:

vanling commented 1 year ago

@BracketJohn I will have another look at it in the coming days, for now this is a simplefied (and JS) version of the existing one

https://gist.github.com/vanling/b10ebe14ee90ff249bea28bf94abb745

corbinday commented 1 year ago

@ayalon @BracketJohn @vanling Thank you so much for fixing this!! πŸŽ‰

allcho commented 1 year ago

Release is out, please verify with @sidebase/nuxt-auth@0.4.0-alpha.6 and let me know if it resolves your problem (:

Hello @BracketJohn I have some problem. I using this recipe https://sidebase.io/nuxt-auth/recipes/directus and this composable https://nuxt.com/docs/examples/advanced/use-custom-fetch-composable Inside composable I tried get accessToken, for put it to header Authorization

const { data } = useAuth()

 data?.value?.user?.accessToken

When I'm walking through the pages useAuth() not to call to callbak(https://nuxt.com/docs/examples/advanced/use-custom-fetch-composable) for checking expires time from AccessToken and refresh it if token not valid, just getting accessToken. But if I hard reload page it's call to callbak.

I tried getting token with const { getSession } = useAuth() and when I'm walking through the pages useAuth() calling to callbak, but if I hard reset page I get error [nuxt] A composable that requires access to the Nuxt instance was called outside of a plugin, Nuxt hook, Nuxt middleware, or Vue setup function. This is probably not a Nuxt bug. How to do this correctly so that before each accessToken is received there is a check expires time?