nextauthjs / next-auth

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

signIn doesn't handle a provider returning null as expected #12037

Open zacharyblasczyk opened 1 month ago

zacharyblasczyk commented 1 month ago

Provider type

Credentials

Environment

  System:
    OS: macOS 15.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 2.51 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - ~/.nvm/versions/node/v20.12.2/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v20.12.2/bin/npm
    pnpm: 9.11.0 - ~/.local/share/pnpm/pnpm
    bun: 1.0.2 - ~/.bun/bin/bun
  Browsers:
    Chrome: 129.0.6668.100
    Safari: 18.0
  next:
    specifier: ^14.2.13
    version: 14.2.13
  next-auth:
    specifier: 5.0.0-beta.22
    version: 5.0.0-beta.22
  react:
    specifier: 18.3.1
    version: 18.3.1
  '@auth/drizzle-adapter':
    specifier: 1.4.1
    version: 1.4.1

Reproduction URL

https://github.com/ctrlplanedev/ctrlplane

Describe the issue

In the documentation it states

If you return null then an error will be displayed advising the user to check their details.

Using the credentials provider to sign in causes the page to refresh and an invalid login doesn't allow me to catch and handle any built in errors if it is returning null.

Form/hook use

  const onSubmit = form.handleSubmit(async (data, event) => {
    event?.preventDefault();
    await signIn("credentials", { ...data })
      .then(() => router.push("/"))
      .catch(() => {
        form.setError("root", {
          message: "Sign in failed. Please try again.",
        });
      });
  });

Backend Logic

const getUserByEmail = (email: string) =>
  db
    .select()
    .from(schema.user)
    .where(eq(schema.user.email, email))
    .then(takeFirstOrNull);

export const getUserByCredentials = async (email: string, password: string) => {
  const user = await getUserByEmail(email);
  if (user == null) return null;
  const { passwordHash } = user;
  if (passwordHash == null) return null;
  const isPasswordCorrect = compareSync(password, passwordHash);
  return isPasswordCorrect ? user : null;
};

Provider Logic

const providers = (): Provider[] => {
  const p: Provider[] = [];

  ...

  if (isCredentialsAuthEnabled)
    p.push(
      Credentials({
        credentials: { email: {}, password: {} },
        authorize: async (credentials) => {
          try {
            const { email, password } = signInSchema.parse(credentials);
            const user = await getUserByCredentials(email, password);
            console.log(user);
            return user;
          } catch (error) {
            console.log(error);
            // Return `null` to indicate that the credentials are invalid
            if (error instanceof ZodError) return null;
            throw error;
          }
        },
      }),
    );

  return p;
};

How to reproduce

Create a from that uses signIn and a credentials provider that returns null when a credential isn't found. Triggering that submit hook will cause the page to refresh instead of signIn throwing an error that can gracefully be handled.

Expected behavior

I expected the signIn to throw an error if it receives a null response from the provider. It isn't possible to log in if the credential is null and while I can work around this by changing the logic to the following, it leaves more room for someone to accidentally leak if a user exists, and forces errors to be thrown and caught that don't create a lot of value.

export const getUserByCredentials = async (email: string, password: string) => {
  const user = await getUserByEmail(email);
  if (user == null) return new Error("Invalid credentials");
  const { passwordHash } = user;
  if (passwordHash == null) return new Error("Invalid credentials");
  const isPasswordCorrect = compareSync(password, passwordHash);
  return isPasswordCorrect ? user : new Error("Invalid credentials");
};
Ali-Raza764 commented 1 month ago

Hi there, What I understand by your issue is that you want to throw some error when the authentication fails. We had a long discussion with a solution here https://github.com/nextauthjs/next-auth/issues/11747#issuecomment-2367785095 Check this out I have given a full example with images and two different ways to get the error on the client. If still you have issues you can ask. Peace

Ali-Raza764 commented 1 month ago

In your auth.js file you have to extend the error class like this

import NextAuth, { CredentialsSignin } from "next-auth"
import Credentials from "next-auth/providers/credentials"

class InvalidLoginError extends CredentialsSignin {
  code = "Invalid identifier or password"
}

export const { handlers, auth } = NextAuth({
  providers: [
    Credentials({
      credentials: {
        username: { label: "Username" },
        password: { label: "Password", type: "password" },
      },
      async authorize(credentials) {
        throw new InvalidLoginError()
      },
    }),
  ],
})

You can do this in a server action or so and it will throw the error Also note that if you have to handle separately when using the server signIn() and client signIn() from next-auth/react

ARiyou2000 commented 3 weeks ago

Another breaking change is that the signIn method results is something like follow:

type SignInResult = {
  ok: true;
  status: 200;
  error: string | null;
  code: number | null;
  url: string | null;
}

Node that ok is always true and status is always 200.

so if you were previously checking for result?.ok you need to change that to !result?.error in your login form on client side.

zacharyblasczyk commented 2 weeks ago

@ARiyou2000, Thank you for the update. I will take a look and try to update what we are doing here.