lazaronixon / authentication-zero

An authentication system generator for Rails applications.
MIT License
1.62k stars 53 forks source link

Password reset should confirm users email #94

Closed gobijan closed 4 weeks ago

gobijan commented 11 months ago

I had a look at:

https://github.com/lazaronixon/authentication-zero/issues/80

In case a user forgot his password but never received a confirmation mail (be it some junk filter or broken mailer) he is basically locked out. On my applications I change it to still send out a password recovery link and when the link is used the user is confirmed automatically. This makes sense in my opinion, what do you think @lazaronixon .

Let me know if I should send in a PR :)

lazaronixon commented 11 months ago

Authentication zero follows the hey.com workflow which means that:

1 - The session never expires, so you can resend the email confirmation again in the future. 2 - To verify your email you must be signed in. 3 - To receive a "password reset" you must verify your email account.

The only problem I see is that if a user signs up with the wrong email, another person can take control of your account, I don't want to change this behavior by now but it's up to you if you can live with that, no problem.

image
gobijan commented 11 months ago

Valid point. What would be the best approach to go for this deadlock situation:

  1. User signs up and uses service (usage is not prevented by default before email verification).
  2. Confirmation Mail never appeared in his inbox.
  3. Time passes.
  4. User wants to comes back on another system (or browser etc.) but forgot his password.
  5. Can't send recovery mail because he's not confirmed.

Then there is no way to retrigger the confirmation mail out of the box isn't it? Should there then be a resend confirmation mail path? What do you think?

CleanShot 2023-11-27 at 15 00 47

A resend of the confirmation email could be triggered here automatically?

lazaronixon commented 11 months ago

There's a link to resend email verifications, but the user must be signed in. In your case, the user can contact the support team and they can resend the confirmation email. Of course, there are other options:

1 - Remove the requirement to only send password resets from verified accounts. 2 - Remove the email confirmation process, it worked for Basecamp. it's so 2000's 3 - Prevent sign-in from unverified users as Devise does. 4 - A recurrent job to resend verification emails =/

kirley commented 8 months ago

I ran into this lock scenario today while testing and I feel like the current functionality basically tells hackers what email addresses exist in the system and which ones don't.

This code generated by the gem feels like a security gap to me: app/controllers/identity/password_resets_controller.rb

  def create
    if @user = User.find_by(email: params[:email], verified: true)
      send_password_reset_email
      redirect_to sign_in_path, notice: "Check your email for reset instructions"
    else
      redirect_to new_identity_password_reset_path, alert: "You can't reset your password until you verify your email"
    end
  end

My suggestion to fix this would be to allow password resets without email verification.
app/controllers/identity/password_resets_controller.rb

  def create
    if @user = User.find_by(email: params[:email])
      send_password_reset_email
    end
    redirect_to sign_in_path, notice: "Check your email for reset instructions"
  end

And then automatically re-send the email verification if a user resets their password on an account that isn't verified and remind the user to verify their email when the password reset is confirmed on the screen.

lazaronixon commented 8 months ago

@kirley You're not wrong, this is why we have a rate_limit on this endpoint.

https://github.com/lazaronixon/authentication-zero/blob/0eb2ba99c4b46b856066bcbc1d1ece42bd87887b/lib/generators/authentication/templates/controllers/html/identity/password_resets_controller.rb.tt#L4-L6