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

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

Closed
ayalon opened this issue Jan 31, 2023 · 12 comments · Fixed by #215
Closed

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

ayalon opened this issue Jan 31, 2023 · 12 comments · Fixed by #215

Comments

@ayalon
Copy link
Contributor

ayalon commented Jan 31, 2023

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:

return _fetch<SessionData>(nuxt, 'session', {

  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

@ayalon ayalon added the bug label Jan 31, 2023
@vanling
Copy link
Contributor

vanling commented Feb 5, 2023

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

vanling added a commit to vanling/nuxt-auth that referenced this issue Feb 6, 2023
…server side.

Copied the small proposal for a fix for sidebase#200 from @ayalon
@BracketJohn
Copy link
Contributor

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
Copy link
Contributor

vanling commented Feb 6, 2023

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

@BracketJohn
Copy link
Contributor

: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 🎊

@BracketJohn BracketJohn mentioned this issue Feb 6, 2023
3 tasks
BracketJohn added a commit that referenced this issue Feb 8, 2023
* #200 Refresh Token Rotation does not update cookie if called server side.

Copied the small proposal for a fix for #200 from @ayalon

* Pass SSR set-cookie header along with the client request.

* Add import for appendHeader from h3

* Update src/runtime/composables/useSession.ts

Co-authored-by: Nils <njonalik@protonmail.com>

* Move import

---------

Co-authored-by: Nils <njonalik@protonmail.com>
Co-authored-by: ayalon <info@ayalon.ch>
@vanling
Copy link
Contributor

vanling commented Feb 8, 2023

🎉

@BracketJohn
Copy link
Contributor

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

@BracketJohn
Copy link
Contributor

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

@vanling
Copy link
Contributor

vanling commented Feb 8, 2023

  • Changing version ✅
  • Setting Directus token length to 2 min ✅
  • Opening my test project✅
  • Creating small tears of joy ✅
    It works 🎉

Thanks @BracketJohn and @ayalon !

@BracketJohn
Copy link
Contributor

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
Copy link
Contributor

vanling commented Feb 8, 2023

@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
Copy link

@ayalon @BracketJohn @vanling Thank you so much for fixing this!! 🎉

@allcho
Copy link

allcho commented Nov 16, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants