Re-use a non-expired password reset token about ideas

[judgej]

I have fixed this in my L4.2 site by overriding DatabaseReminderRepository::create(). From L5.0 that is renamed to DatabaseTokenRepository::create() but is largely the same.

Instead of blindly deleting and recreating the token:

$this->deleteExisting($user);

We delete it only if expired:

        // Delete any current token if it has expired.
        $expired = Carbon::now()->subSeconds($this->expires);

        // Maybe expand deleteExpired() to take an optional user like deleteExisting()
        $this->getTable()
            ->where('email', $user->getReminderEmail())
            ->where('created_at', '<', $expired)
            ->delete();

Then see if there is an existing token still there, and if there is, use it:

        $existing_token = $this
            ->getTable()
            ->where('email', $user->getReminderEmail())
            ->first();

        if ($existing_token) {
            return $existing_token->token;
        }

Otherwise just carry on as normal to create a brand new token.

Does that seem on the face of it a reasonable approach?

[judgej]

First questions are:

• Is there a security issue with sending non-expired password reset tokens again? I don't think so.
• Is the edge case when someone requests a password reset just moments before the current token is about to expire going to just move the problem? Again, I don't think so, but it needs to be considered.
• Are we the only people having this problem? We do have a large number of password resets going on, as everyone must do that at least once. However, I do suspect others are having this issue too, but don't realise it. No evidence for that I'm afraid.

[judgej]

We have this monitored on a site that went live with this fix yesterday. Counting the number of times the passwords.token error happens now compared to before, will give us a better idea of the number of potential support calls it avoids.

We have had ~1300 logged failures to reset a password last year due to an invalid token (NOT expired, but deleted). 182 failures so far this year. Some users resolve the problem themselves, but many can't - they just say, "it doesn't work".

[judgej]

Not sure if this is the right repo to be raising this for discussion? I can move it if not.

[themsaid]

Closing this for lack of activity, the proposal doesn't seem to be found interesting by members of the community.