feat: Change to using resetToken hash in DB

[jaiakt]

Addresses #6855.
This PR makes it so we don't store raw resetTokens in our database but instead, the sha256 hash of the token. This makes it so if a database is compromised, an attacker can't steal resetTokens and reset users' passwords.

One catch with this PR is that the user defined handler() takes in a User object which it relies on for fetching the resetToken to generate a password reset link. Because now the User object will instead contain a resetToken hash, we need to make the raw token also available. I solve this by mutating the User object before calling the handler and explain this in the documentation for forgotPassword.handler(). This is potentially breaking if the application refetches the User object in the handler but not sure why they'd do that 🤔

Edit by Tobbe:
I've marked this as feature-breaking, but it's not really breaking. Just wanted to make sure that we highlight that any in-transit password resets will break when users upgrade RW to whatever version this is released in.
For low- to medium-traffic sites it's probably not a problem. Maybe just have an error message that asks users to try again if the token doesn't match.
For high-traffic sites, and/or for the best end-user experience, what developers can do is temporarily disable pw resets. Query their db and wait until no more valid reset tokens exist (by looking at resetTokenExpiresAt) and then upgrade and finally re-enable pw resets

[Tobbe]

Thanks for working on this @jaiakt!

I don't fully know the dbAuth code, so maybe I'm missing something obvious here, but how does the unhashed token get to the client?

Correct me if I'm wrong, but shouldn't the flow be like this?

1. Create a pw reset token
2. Hash the token and store in the DB
3. Send the unhashed token to the client
4. Client sends new pw + token to the dbAuth code
5. token is hashed (again)
6. if hashed token matches what's in the DB the pw is updated

[jaiakt]

Ah yeah I think that basically is the flow except for step 3. The backend application has to define a handler in which it typically sends an email to the user containing the token (in a password reset link). We don't send the token back to the client.
https://redwoodjs.com/docs/auth/dbauth#forgotpasswordhandler

[Tobbe]

Thanks 🙂 But still, that password reset link needs the unhashed token, right? With the code in this PR the handler gets the user object (if I understand the code correctly), and the user object only has the hashed token

[jaiakt]

Ohhh yes you're completely right! Let me think about this a little more

[jaiakt]

@Tobbe I don't love this solution but we can update the user's resetToken to have the raw token before calling the handler. I can update the docs to explain this behavior if it's not too hacky.

[Tobbe]

I like it! 🙂 This also makes this fully backwards compatible, right?

Except for tokens already stored in the DB I guess

[jaiakt]

Yup should be backwards compatible! Yeah tokens stored in the db probably won't be but they should expire and should be easily replaceable by using the forgot password flow again. Ty for the catch 😄

[replay-io]

15 replays were recorded for 89aaa20.

0 Failed

15 Passed

• useAuth hook, auth redirects checks
• Check that a specific blog post is prerendered
• Check that about is prerendered
• Check that homepage is prerendered
• Check that meta-tags are rendering the correct dynamic data
• Check that you can navigate from home page to specific blog post
• Waterfall prerendering (nested cells)
• RBAC: Admin user should be able to delete contacts
• RBAC: Should not be able to delete contact as non-admin user
• Smoke test with dev server
• Smoke test with rw serve
• Loads Cell mocks when Cell is nested in another story
• Loads Cell Stories
• Loads MDX Stories
• Mocks current user, and updates UI while dev server is running

View test run on Replay ↗︎

[Tobbe]

@jaiakt Can you take a look at the failing test case please?

[jaiakt]

@Tobbe just learned how to run tests locally lol. Fixed!

[Tobbe]

@Tobbe just learned how to run tests locally lol. Fixed!

I shouldn't have assumed you knew how to. Sorry

[jaiakt]

No worries! Ty for pointing out the broken test. All the CI checks are still a bit confusing to me

[jaiakt]

I noticed you tagged this as a breaking feature. Is this considered breaking because it should be backwards compatible?

[Tobbe]

No worries! Ty for pointing out the broken test. All the CI checks are still a bit confusing to me

Yeah, and unfortunately they're pretty flaky at the moment 🙁 So we might have to rerun them a few time to make them all pass

I noticed you tagged this as a breaking feature. Is this considered breaking because it should be backwards compatible?

I made an edit to your PR description trying to describe why I marked it breaking. It's not really breaking, I just wanted to make sure we added some extra info to the release notes for whatever version of RW this PR is released in

[Tobbe]

This looks good to me, just want to get @cannikin's eyes on this too before merging

[cannikin]

Sorry for the wait! This look great, thanks again!