riverrun / phauxth

Not actively maintained - Authentication library for Phoenix, and other Plug-based, web applications
409 stars 20 forks source link

Check confirmed_at when resetting the password #82

Closed riverrun closed 5 years ago

riverrun commented 5 years ago

I would like feedback on this issue - about the default behavior of the Confirm.PassReset module. Should it also check the confirmed_at status as well?

riverrun commented 5 years ago

Fixed

carterbryden commented 5 years ago

Is there a way to reset the password before the account is confirmed? Or even confirm the account when the reset key is submitted for the reset (after receiving the email and following the link)?

riverrun commented 5 years ago

@carterbryden this is possible, but it is not really recommended. Here are a couple of links providing you with more information about this - https://ux.stackexchange.com/questions/30249/how-to-handle-forgot-password-when-user-has-not-confirmed-email and https://ux.stackexchange.com/questions/30249/how-to-handle-forgot-password-when-user-has-not-confirmed-email

But if you still want to do this, one way is to create a custom PassReset module:

defmodule MyApp.Confirm.PassReset do

  use Phauxth.Confirm.Base

  @impl true
  def report({:ok, user}, meta), do: log_output(user, meta)
  def report(result, meta), do: super(result, meta)

  defp log_output(%{reset_sent_at: nil}, meta) do
    Log.warn(%Log{message: "no reset token found", meta: meta})
    {:error, Config.user_messages().invalid_token()}
  end

  defp log_output(user, meta) do
    Log.info(%Log{user: user.id, message: "user confirmed for password reset", meta: meta})
    {:ok, Map.drop(user, Config.drop_user_keys())}
  end
end

The above module is just an example - the important thing is that you create a module without the confirmed_at: nil check.

If you have any further comments / questions, just let me know.

krystofbe commented 5 years ago

I just stumbled over this and asked myself the question why you wouldn't allow to reset an unconfirmed user account password. As someone wrote in the mentioned discussion on SO

If user forgetting his password:

User automatically confirms, that the mail address is his own mail address, when he request for the "Reset Password Link", since he can only activate the reset password link through his email, and he won't get the key otherwise, and as i said before if the mail or mail server are compromised, then it's over for him either way.

So are there any other security concerns on allowing this?

riverrun commented 5 years ago

I spent some time looking into this in the past, and it was difficult to get a definitive answer. However, it does seem that the general advice is to not allow an unverified email to be used for password reset.

There was also a question about this on quora, and it seems that Instagram also do not allow this. One of the answers there mentions that this helps prevent accounts being stolen, but unfortunately, it does not go into details about how an account can be stolen.

In situations like this, I think it's better to be safe than sorry.