nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.75k stars 3.48k forks source link

Subsequent sign ins do not update account #3599

Closed mattgogerly closed 2 years ago

mattgogerly commented 2 years ago

Description 🐜

On first sign in, an account is "linked" (created) for a user. On subsequent sign ins, the account is not updated.

Since the access_token for a provider is stored as part of an account, these subsequent sign ins will never update the access_token.

Is this a bug in your own project?

No

How to reproduce ☕️

Using the example with a database adapter, sign in. Observe that the account row is created. Log out and then log back in. Observe that the access_token and expiry in the account row does not change.

Screenshots / Logs 📽

No response

Environment 🖥

Using Prisma as an adapter, but this should affect any adapter.

System: OS: macOS 12.1 CPU: (8) x64 Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz Memory: 481.00 MB / 8.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 16.13.1 - /usr/local/bin/node Yarn: 1.22.17 - ~/.yarn/bin/yarn Browsers: Chrome: 96.0.4664.110 Firefox: 67.0.4 Safari: 15.2 npmPackages: next: 12.0.7 => 12.0.7 next-auth: ^4.0.6 => 4.0.6 react: 17.0.2 => 17.0.2

Contributing 🙌🏽

Yes, I am willing to help solve this bug in a PR

balazsorban44 commented 2 years ago

This is not a bug, only something that isn't currently implemented. Token rotation is a planned feature though, I don't have an ETA, likely this year.

TheColorman commented 2 years ago

This seems flawed. If a user were to revoke their access and refresh token, subsequent authorizations would still use the old access token, completely locking a user out of an app, unless I've misunderstood how this works.

My current solution to this is deleting a user from the database and redirecting them to login whenever I run into errors when using APIs that are dependent on access tokens.

balazsorban44 commented 2 years ago

This isn't a flaw, simply something that isn't currently implemented.

Currently, when a user signs out, you could use the signOut event to clean up the database. https://next-auth.js.org/configuration/events#signout

You don't need to delete the user though, only remove their tokens from Account

https://next-auth.js.org/adapters/models

If you are interested in this feature, we are open to taking PRs! A prior attempt can be found here: #951

hparra commented 2 years ago

I agree this is a bug because the behavior was expected.

This hacky thing works. Since I'm using an adapter, I call the adapter's linkAccount in signIn callback:

async signIn({ user, account, profile, email, credentials }) {      
  if (user && user.email) {
    // If you're using db sessions and not logging in for first time then id = userId (UUID),
    // else id = providerAccountId (string).
    // With OAuth, account will (always?) be account from provider, not db.
    console.debug(`signIn by user.id: ${user.id}`);
    // If you are already a user and have linked your account,
    // then logging out will only destroy cookie and delete session.
    // It will *not* update your account with updated info from callback.
    // Since we don't know which id we have it's best to do lookup again.
    if (adapter) {
      // If signing in first time this will return null
      // and user creation and account linking will occur normally.
      const dbUser = await adapter.getUserByEmail(user.email);
      if (dbUser) {
        // WARNING: Google always returns a refresh_token but some OAuth providers may not.
        await adapter.linkAccount({        
          ...account,
          userId: dbUser.id,
        })
        console.debug(`signIn callback updated account for user.id: ${dbUser?.id}`);
      } else {
        console.debug(`signIn callback confirms new user.id: ${user.id}`);
      }
    }
  }
  return true;
},
herdma commented 2 years ago

I had some issues with the code above using the Prisma Adapter and AzureAD Provider:

→ 19 linkAccount: (data) => p.account.create(
  Unique constraint failed on the fields: (`provider`,`providerAccountId`)

For me, the following seems to work:

    async signIn({ user, account, profile }) {
      if (user && adapter) {
        try {
          const userFromDatabase = await adapter.getUser(user.id);
          if (userFromDatabase) {
            await prisma.account.update({
              where: {
                provider_providerAccountId: {
                  provider: account.provider,
                  providerAccountId: account.providerAccountId,
                },
              },
              data: {
                access_token: account.access_token,
                expires_at: account.expires_at,
                id_token: account.id_token,
                refresh_token: account.refresh_token,
                session_state: account.session_state,
                scope: account.scope,
              },
            });
          }
        } catch (err) {
          if (err instanceof Error) {
            console.error(err.message);
          }
        }
      }
    },

I also set the session maxAge to 86000 so hopefully I will always have a valid access token and included offline_access in the scopes to get a refresh token from AzureAD:

  providers: [
    AzureADProvider({
      authorization: {
        params: {
          /* 
            AzureAD requires the scope offline_access to issue a refresh_token.
            This scope is not included in the AzureAD provider by default, so we're adding it here.
            See also:
            - https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow#successful-response-2
            - https://github.com/nextauthjs/next-auth/blob/main/packages/next-auth/src/providers/azure-ad.ts#L31
          */
          scope: 'openid profile email offline_access',
        },
      },
    }),
  ],
  session: {
    maxAge: 86000,
  },

Maybe it helps someone.

SteveChurch commented 1 year ago

This isn't a flaw, simply something that isn't currently implemented.

Currently, when a user signs out, you could use the signOut event to clean up the database. https://next-auth.js.org/configuration/events#signout

You don't need to delete the user though, only remove their tokens from Account

https://next-auth.js.org/adapters/models

If you are interested in this feature, we are open to taking PRs! A prior attempt can be found here: #951

You say removing their tokens from Account but when this has been done, lets say we null the values, when the user logs out and back in again they are not re-populated.

I can't seem to figure out how to have these tokens refresh in any scenario.

TheCarioca commented 1 year ago

This isn't a flaw, simply something that isn't currently implemented. Currently, when a user signs out, you could use the signOut event to clean up the database. https://next-auth.js.org/configuration/events#signout You don't need to delete the user though, only remove their tokens from Account https://next-auth.js.org/adapters/models If you are interested in this feature, we are open to taking PRs! A prior attempt can be found here: #951

You say removing their tokens from Account but when this has been done, lets say we null the values, when the user logs out and back in again they are not re-populated.

I can't seem to figure out how to have these tokens refresh in any scenario.

Exact! I have been trying to remove it, but it is not re-populated. What I am trying to do is to check it manually, changing the session callback. First I verify if the expires_at > Date.now() from the account stored in my mongodb and fetching to https://discord.com/api/v10/oauth2/token. However I am not able to get the token to use the grant_type: "authorization_code". Here is my attempt so far:

import NextAuth from "next-auth";
import DiscordProvider from "next-auth/providers/discord";
import clientPromise from "@lib/mongodb";
import { MongoDBAdapter } from "@next-auth/mongodb-adapter";
import { ObjectId } from "mongodb";

export const authOptions = {
  providers: [
    DiscordProvider({
      clientId: process.env.DISCORD_CLIENT_ID,
      clientSecret: process.env.DISCORD_CLIENT_SECRET,
      authorization: { params: { scope: "identify email guilds" } },
    }),
  ],
  adapter: MongoDBAdapter(clientPromise),
  secret: process.env.NEXTAUTH_SECRET,
  callbacks: {
    session: async ({ session, user }) => {
      const client = await clientPromise;
      const db = client.db();
      const { expires_at, refresh_token } = await db
        .collection("accounts")
        .findOne({
          userId: new ObjectId(user.id),
        });

      // if (expires_at > Date.now()) {
      if (expires_at < Date.now()) {
        try {
          const response = await fetch(
            "https://discord.com/api/v10/oauth2/token",
            {
              method: "POST",
              headers: {
                "Content-Type": "application/x-www-form-urlencoded",
              },
              body: new URLSearchParams({
                client_id: process.env.DISCORD_CLIENT_ID,
                client_secret: process.env.DISCORD_CLIENT_SECRET,
                grant_type: "authorization_code",
                refresh_token: refresh_token,
                redirect_uri: "http://localhost:3000/",
                scope: "identify email guilds",
              }),
            }
          );

          const data = await response.json();
          console.log(data);

          if (!response.ok) throw data;

          await db.collection("accounts").updateOne(
            { userId: new ObjectId(user.id) },
            {
              $set: {
                access_token: data.access_token,
                expires_at: Date.now() + data.expires_in * 1000,
                refresh_token: data.refresh_token || user.refreshToken,
              },
            },
            { upsert: false }
          );
        } catch (error) {
          console.error("Error refreshing access token", error);
        }
      }

      session.user.id = user.id;
      return session;
    },
  },
};
export default NextAuth(authOptions);