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

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

Open
zacharyblasczyk opened this issue Oct 13, 2024 · 4 comments
Open

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

zacharyblasczyk opened this issue Oct 13, 2024 · 4 comments
Labels
bug Something isn't working providers triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime.

Comments

@zacharyblasczyk
Copy link

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");
};
@zacharyblasczyk zacharyblasczyk added bug Something isn't working providers triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime. labels Oct 13, 2024
@Ali-Raza764
Copy link

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 #11747 (comment) 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
Copy link

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working providers 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