Dont allow reusing password reset token, use normal rate limit

[Nutomic]

With the current code it is possible to reuse the same reset token multiple times.

[dullbananas]

Use delete instead of update

[Nutomic]

We need to keep the column so that reset count works. That one is used to limit users to 3 password resets per day. However we could get rid of that and use a normal rate limit instead, eg auth rate limit which applies to login, register and password reset.

[dullbananas]

Per-user rate limits are better

[dessalines]

This seems kind of extreme to me. As the code exists, the reset token is only valid for a day anyway. Why not just decrease that time to like an hour, or 15 minutes?

These single-use tokens are annoying even when I get them as texts, and they're too quickly invalidated.

[Nutomic]

This is not about valid time, but about using the same reset token multiple times. Right now you can do "forgot password" once, then use the reset token to set a new password, and afterwards use the same reset token to change your password again. In fact you can do that an unlimited amount of times which seems completely wrong.

[dessalines]

This is basically the difference between "one-time-use" tokens, HOTP, vs time-based tokens, TOTP. TOTP is much more forgiving and reliable IMO.

IMO HOTP are way too pointlessly complicated for our use here, it should be enough to shorten our TOTP time to like 10-15 minutes.

[Nothing4You]

OTP is literally only to be used once.

Quoting Wikipedia:

A one-time password (OTP), also known as a one-time PIN, one-time authorization code (OTAC) or dynamic password, is a password that is valid for only one login session or transaction, on a computer system or other digital device.

A TOTP server side implementation that allows token reuse is vulnerable to reply attacks, the very thing that OTPs are designed to prevent.

I'm assuming you only brought up TOTP/HOTP for the user experience part, as this is not using that technology, but I don't agree with your stance on that regardless.
In a TOTP implementation you have a new code every e.g. 30 seconds. Depending on when you start reading it, it might be rotating before you're done entering it, or your time window may have passed. For the time window part (after rotation), and to account for clock drift, many providers are already tolerating multiple tokens from that time frame, such as the codes directly before and after the current time. The user experience is degraded as they may need to wait for a rotation to have enough time to read and type it.
In an HOTP implementation however, the token does not rotate on its own, but instead it rotates following user interaction. This has a few drawbacks, such as synchronization issues when generating a lot of new tokens without using them, or if someone can intercept a token, then have the user submit another one and replace it with the first one, allowing the attacker to still have a valid token. This is probably mostly theoretical, as the attacker could then probably just capture a session cookie. The general user experience is improved by not putting time pressure on them.
There is probably no harm in allowing a password reset token to be valid for e.g. a day.

Both of these implementations however are only secure if token reuse is prevented.

The first potential attack that comes to my mind for reusable reset tokens is shared computers, when someone resets their lemmy password, then logs out of all of their sessions, but doesn't clear their browsing history. The next person using that computer will be able to find the reset url in the browser history and compromise the account.

[dessalines]

This is the only line you need to change. Change this to a shorter time span, and none of this is necessary.

[Nutomic]

Thats not it, having the reset token valid for a day is fine in case you dont receive it immediately. But it should be possible to reuse the same one multiple times.

[dessalines]

Alright, seems a bit overkill to me still, but its fine.

[Nutomic]

This is not about TOTP (2 facor auth), but about the password reset at https://lemmy.ml/login_reset. After you reset the password, it should not be possible to reuse the same reset link again.

[Nutomic]

Ive changed it now to use the normal rate limit, so get_recent_password_resets_count() and valid field arent necessary anymore.

[dessalines]

Surprised that delete works with .get_result, but you verified it below in the tests anyway.