Question: Should we send password reset instructions to unconfirmed emails?

[nickL]

Question: Should we send password reset instructions to unconfirmed emails?

Background:

It looks like in the event theres a pending_reconfirmation? for a record we send the password reset email to the unconfirmed email instead of the confirmed. Just noticed this and thought I'd ask before submitting a PR. Is this expected? Thoughts?

[booleanbetrayal]

I'm honestly not entirely sure what the best thing to do in this situation is. I think if you require it to be the previously confirmed email, you could get into the following situation:

1. User migrates email address from dormant or inaccessible address.
2. User does not accept confirmation.
3. User loses session, but can't recall password.
4. User effectively locked out until they eventually confirm email.

In the other case, as it stands, if a user erroneously updates their email by accident, then can't get reset instructions if needed, as the emails are going to the invalid address.

Additionally, these issues could be exacerbated or mitigated by confirmation windows and when devise cleans up unconfirmed_email (if at all). Would definitely love to get some additional feedback.

[coryschires]

@booleanbetrayal Thanks for working on this library!

I think the situation you outlined is actually the correct / intended behavior. IMO, devise should never send email to an unconfirmed email address (except, of course, the confirmation email itself). This assumption is based on two thoughts:

1. Sending token authorized emails (like password reset) to an unconfirmed email address is a security liability. If the unconfirmed email was mistyped, you could be sending the password reset email to a complete stranger. Bad news.
2. As far as I can tell looking through the source, Devise does not send reset instructions to unconfirmed email addresses. So, I'd argue, that you're gem (which strictly speaking has nothing to do with password reset behavior) should not override the default devise behavior. Chances are this question has been debated by a bunch of people smarter than us. So I'd just roll with their assumption unless of course you think it's blatantly wrong. But, even if you do think it's wrong, that's probably an issue best debated in devise itself rather than devise_token_auth.

In the example you outlined, I'd say the user should:

1. Request confirmation instructions
2. Confirm their email address
3. Request password reset instructions
4. Reset their password
5. Login

As for code changes, I think you could simply delete these lines of code. Lemme know if this makes sense, I'd be happy to make a PR.

[booleanbetrayal]

I think that sounds sane. If anyone wants to put in a PR and ensures tests still pass, I'll approve / merge it ASAP. Thanks for the catch and sanity check, guys!

[coryschires]

@booleanbetrayal Thanks for following up! Here's a PR: #288

[nickL]

LOL, @coryschires we both added PR's for this seconds apart (quick fix). I'll remove mine (#289) since the commits are identical.

@booleanbetrayal: I couldn't find a good place to add a test for this, and I'm not convinced it's needed considering the fallback to devise. If all looks well, let's merge in #288.

[coryschires]

@nickL LOL! Damn, we're too eager!

[booleanbetrayal]

#288 is in ... thanks guys!