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 in documentation #9109

Closed
ipko1996 opened this issue Nov 10, 2023 · 8 comments
Closed

Refresh token rotation in documentation #9109

ipko1996 opened this issue Nov 10, 2023 · 8 comments
Labels
documentation Relates to documentation triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime.

Comments

@ipko1996
Copy link

ipko1996 commented Nov 10, 2023

What is the improvement or update you wish to see?

On the new website there is an example of renewing access token for google.

const tokens: TokenSet = await response.json()

line will not be good because Google response is like in this format:

interface GoogleAccessToken {
  refresh_token: string;
  access_token: string;
  expires_in: number; // 3599 (sec)
  scope: string;
  token_type: string;
  id_token: string;
}

but TokenSet has expires_at not expires_in.

tho here:

expires_at: Math.floor(Date.now() / 1000 + account.expires_in),

if i'm not wrong this is the first time google issues a token, so we won't have expires_in but rather expires_at

Furthermore

in this example jwt function complains about a ts errro because it should return JWT from next-auth/jwt

To resolve this, we should create a token at the beginning of the jwt function and return that at the end.

    jwt: async ({ token, account }) => {
      let updatedToken: JWT;
      if (account) {
        updatedToken = {
          ...token,
          id_token: account.id_token!,
          access_token: account.access_token!,
          expires_at: account.expires_at!, //Math.floor(Date.now() / 1000 + (account.expires_at ?? 0)),
          refresh_token: account.refresh_token!,
        };
      } else if (Date.now() < token.expires_at * 1000) {
        updatedToken = token;
      } else {
        try {
          const response = await fetch("https://oauth2.googleapis.com/token", {
            headers: { "Content-Type": "application/x-www-form-urlencoded" },
            body: new URLSearchParams({
              client_id: env.GOOGLE_CLIENT_ID,
              client_secret: env.GOOGLE_CLIENT_SECRET,
              grant_type: "refresh_token",
              refresh_token: token.refresh_token,
            }),
            method: "POST",
          });

          const tokens = (await response.json()) as GoogleAccessToken;
          if (!response.ok) throw tokens;

          updatedToken = {
            ...token, // Keep the previous token properties
            access_token: tokens.access_token,
            id_token: tokens.id_token,
            expires_at: Math.floor(Date.now() / 1000 + tokens.expires_in),
            // Fall back to old refresh token, but note that
            // many providers may only allow using a refresh token once.
            refresh_token: tokens.refresh_token ?? token.refresh_token,
          };
        } catch (error) {
          console.error("Error refreshing access token", error);
          // The error property will be used client-side to handle the refresh token error
          updatedToken = {
            ...token,
            error: "RefreshAccessTokenError" as const,
          };
        }
      }
      return updatedToken;
    },

Is there any context that might help us understand?

No.

Does the docs page already exist? Please link to it.

https://authjs.dev/guides/basics/refresh-token-rotation?frameworks=core

@ipko1996 ipko1996 added documentation Relates to documentation triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime. labels Nov 10, 2023
@ipko1996 ipko1996 changed the title Refresh token rotation Refresh token rotation in documentation Nov 10, 2023
@ralphilius
Copy link

Thanks for this. I also bumped into this problem.

@ndom91
Copy link
Member

ndom91 commented Apr 28, 2024

Hey thanks for pointing this out, we've put up a refresh token guide on the new website as well and it basically looks like your recommendation.

Does this cover what you were looking for? https://authjs.dev/guides/refresh-token-rotation

@ipko1996
Copy link
Author

no, there is still const tokens: TokenSet = await response.json() which is not GoogleAccessToken as I can recall. Unless you did change the TokenSet to google's intreface, this code is still not correct for typescript (it may work, but eslint will show you the errors)

also expires_at: Math.floor(Date.now() / 1000 + account.expires_in), is still there, which in my code is just expires_at: account.expires_at! because I used account's form previous I think, but it was 6 month ago, so I dont know for sure

@ndom91
Copy link
Member

ndom91 commented Apr 28, 2024

Okay gothca, I'll double check the TokenSet type and google response shape.

Regarding the expires_at value, that first if clause is for the first login, right. And that response contains 2 values, expires_in which is a count in seconds from now() and expires_at which is basically now() + expires_in in seconds. So both would work, no?

image

@ipko1996
Copy link
Author

Yeah, I guess it should work, but since google tell you the exact number (as I can remember, and you can see from the response), you don't have to calculate by yourself. And since this whole process is async it is better to set the values google sent for you.

@ndom91
Copy link
Member

ndom91 commented Apr 28, 2024

Yeah good point. I've cleaned up the jwt guide a bit now, check it out - https://authjs.dev/guides/refresh-token-rotation

@ipko1996
Copy link
Author

Yes, it's better, thanks.

I think this is just an example, and the people who will implement refresh token rotation will figure out from this what to do, even if the example code is not type perfect especially if later this will be implemented by Auth.js.

Last thing I would do is to put this link somewhere in the description.

@ndom91
Copy link
Member

ndom91 commented Apr 28, 2024

Yeah we're not really happy with it etiher, but its such a highly requested thing we just decided to put it back up for now.

Regarding the link, its obviously google specific, so I'll put it on the Google provider docs page (https://authjs.dev/getting-started/providers/google).

I think we've covered everythign then, I'll close the issue, thanks again 🙏

@ndom91 ndom91 closed this as completed Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relates to documentation triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime.
Projects
None yet
Development

No branches or pull requests

3 participants