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

feat: distinct error for unverified email login #11647

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akhrarovsaid
Copy link
Contributor

@akhrarovsaid akhrarovsaid commented Mar 12, 2025

What?

This PR adds a new error to be thrown when logging in while having verify: true set but no email has been verified for the user yet.

Why?

To have a more descriptive, actionable error thrown in this case as opposed to the generic "Invalid email or password." This gives users more insight into why the login failed.

How?

Introducing a new error: UnverifiedEmail and adjusting the check to be separate from if (!user) { ... }.

Fixes #11358

Notes:

  • In terms of account enumeration: this should not be a concern here as the check for throwing this error comes after the check for valid args as well as the find for the user. This means that credentials must be on hand, both an email and password, before seeing this error.
  • I have an int test written in /test/auth/int.spec.ts for this, however whenever I try to commit it I get an error stating that the eslint@9.14.0 module was not found during lint-staged.
Int test
it('should respond with unverifiedEmail if email is unverified on login', async () => {
  await payload.create({
    collection: publicUsersSlug,
    data: {
      email: 'user@example.com',
      password: 'test',
    },
  })

  const response = await restClient.POST(`/${publicUsersSlug}/login`, {
    body: JSON.stringify({
      email: 'user@example.com',
      password: 'test',
    }),
  })

  expect(response.status).toBe(403)

  const responseData = await response.json()
  expect(responseData.errors[0].message).toBe('Please verify your email before logging in.')
})

Demo of toast:
Login-Payload-03-11-2025_11_52_PM-unverified-after

@slavanossar
Copy link
Contributor

Nice, I think I reported this way back in v2! Thanks for putting together the PR, quite an essential feature IMO.

@GermanJablo GermanJablo self-assigned this Mar 12, 2025
Copy link
Contributor

@GermanJablo GermanJablo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @akhrarovsaid, this PR is definitely an improvement over the current behavior!

There are a couple of things I think we should also improve later:

  1. Offer the option to resend the verification email. This may be necessary if for some reason it didn't arrive or the email was deleted.
  2. Display form error messages in the form itself, and not rely on the toast for this. I think the toast is fine where there's no obvious place in the UI to display the error. This isn't an issue with this PR, though. I see that for other errors we're already doing this, or sometimes showing errors in both places.

I'm merging this PR because as it stands, it already solves the proposed problem, and the points I mentioned can be addressed later. I'll merge it today afternoon or tomorrow in case anyone wants to add comments.

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

Successfully merging this pull request may close these issues.

Unverified user login attempts still getting generic error on V3
3 participants