DEV Community

Cover image for 4 issues against password reset flow. Here's what I found.
Ado Daniel Nj
Ado Daniel Nj

Posted on

4 issues against password reset flow. Here's what I found.

Password reset looks like a 20-line feature. Until it has an email leak, a half-finished transaction, and more than one valid token at a time.

I was building a task manager API for a skill assessment. Auth was supposed to be the boring part.

Then I started reviewing my own password reset like someone trying to break it. I ended up with four issues and three PRs, and not one of them was a typo.

Repo if you want to follow along: acetennyson/global-tech-trial

Bug 1: forgot-password told strangers who has an account

Issue #12: registered email leak via forgot-password.

The usual version of this endpoint says "no account found" for an unknown email and "email sent" for a known one. That's friendly, and it's also a free lookup tool for anyone with a list of emails.

The fix is that it always returns 200, with the same message, every time. That includes when the email provider fails. A 500 on a failed send leaks the same fact through a different door.

Bug 2: login leaked it through timing

Same idea, different channel. A wrong password and an unknown email both return the same 401, but that's not enough. If the unknown-email path skips the password hash, it answers faster, and the difference shows up in a stopwatch.

So for an unknown email, the code compares against a dummy hash anyway. Same work, same response, same time...stupid? yeah!! but it is what it is.

Bug 3: resetting a password took three separate database steps

Issue #14: resetting a password takes three database steps.

Roughly: check the token, set the new password, mark the token used. Three queries, no transaction. If step two succeeds and step three fails, you have a new password and a token that still works. If two requests arrive with the same link at the same moment, both can pass step one.

The fix is to make "claim the token" a single atomic statement, and run the whole reset in one transaction:

-- roughly: claim the token and read the user in one shot
UPDATE reset_tokens
SET used_at = now()
WHERE token_hash = $1
  AND used_at IS NULL
  AND expires_at > now()
RETURNING user_id;
Enter fullscreen mode Exit fullscreen mode

If it returns a row, you won. If it returns nothing, the token was wrong, expired, or already used, and you can't tell which from the outside. Two simultaneous requests can't both win, because the database picks one.

Bug 4: more than one valid token at once

PR #11, branch name: multiple concurrent valid reset password tokens.

Every "forgot password" clicked mints a new token and left the old ones alive, meaning each reset email sitting in an inbox was a live key. Now the reset links in someone's inbox were a pile of keys instead of one.

Now using any token closes all the others for that user, inside the same transaction as the password change. One reset, every other link dead.

Tokens themselves: random, stored hashed (so a database leak isn't a pile of working reset links), single-use, and they expire in an hour.

And one I haven't fixed

Idempotency on POST creates (resend the same Idempotency-Key and you get the original task back) works, with one known gap: two simultaneous requests with the same brand-new key can both pass the lookup. The second hits the primary key and returns a 500. Nothing gets duplicated, and a retry returns the stored task. It's still a 500.

Also the reset email links to a /reset-password page that doesn't exist yet, so it 404s. You can finish a reset from the playground or with POST /api/auth/reset-password and { token, password }.

What I'm asking you

  1. What did I miss in the reset flow? I'm sure there's a more bugs.
  2. How would you close the idempotency race? I have a plan, but I want to see yours first.
  3. Want an easy first PR? The /reset-password page reads a token from the URL and POSTs it. That's it.

What's the worst thing you've ever found in your own auth code?

Top comments (0)