[Bug?]: Default dbAuth workflow leaks resetToken

[maddijoyce]

What's not working?

After following the tutorial to get an idea of how redwood works, I was testing out the dbAuth functionality with the console open. I was looking at the response from the forgotPassword lambda (without changing anything from the scaffolded code) and it's returning the reset token:

{
  "id": "b6e9b773-ecc1-4d27-bcdb-aa6729255bfe",
  "email": "maddi@maddijoyce.com",
  "resetToken": "ZTEyNzI3YjdjMjc1", // <-- This token allows me to reset
  "resetTokenExpiresAt": "2022-09-06T11:35:07.169Z",
  "createdAt": "2022-09-05T10:06:06.648Z"
}

How do we reproduce the bug?

Just follow the instructions here - https://redwoodjs.com/docs/auth/dbauth

At it's most basic, on a brand new redwood repo run:

• yarn rw setup auth dbAuth
• yarn rw g dbAuth

What's your environment? (If it applies)

System:
    OS: macOS 12.5
    Shell: 3.3.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.13.0 - /private/var/folders/79/3b3vgxfs4633nxwvbjs0c0y00000gn/T/xfs-09c55961/node
    Yarn: 3.2.1 - /private/var/folders/79/3b3vgxfs4633nxwvbjs0c0y00000gn/T/xfs-09c55961/yarn
  Databases:
    SQLite: 3.37.0 - /usr/bin/sqlite3
  Browsers:
    Chrome: 96.0.4664.55
    Firefox: 104.0.1
    Safari: 15.6
  npmPackages:
    @redwoodjs/core: 2.2.3 => 2.2.3

Are you interested in working on this?

• I'm interested in working on this

[maddijoyce]

My feeling is that there are 2 possible options here.

• One would be to update the documentation to make sure users understand they need to modify that handler
• The other would be to modify the generated function, possibly as simple as:

handler: (user) => {
    // Ensure the reset token is not sent to the client
    delete user.resetToken;
    return user;
  },

[jtoar]

Thanks for the report @maddijoyce! @cannikin could I hand this one off to you cause dbAuth?

[cannikin]

Yup, I’m on it!

…

On Sep 5, 2022, at 8:30 AM, Dominic Saadi ***@***.***> wrote:  Thanks for the report @maddijoyce! @cannikin could I hand this one off to you cause dbAuth? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[cannikin]

@dac09 Didn't we just update this function to only return a boolean, or throw an error? Was that in the TS strict stuff?

[dac09]

@dac09 Didn't we just update this function to only return a boolean, or throw an error? Was that in the TS strict stuff?

Yeah we changed the template - but we mainly changed the templates though, but didn't change the dbAuthHandler underneath to throw an error, so I think it still needs a looking at.

[cannikin]

I don't know that we need to throw an error, just update this function to not return the same thing that came from that forgotPassword handler: https://github.com/redwoodjs/redwood/blob/main/packages/api/src/functions/dbAuth/DbAuthHandler.ts#L500-L509 So even if someone DID return something sensitive, the actual function would return nothing. Just update it to return an empty string or something.

Then update ForgotPasswordPage to use the email from the form field on the page, instead of getting it from the response object: https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/commands/generate/dbAuth/templates/forgotPassword.tsx.template#L33

[cannikin]

Did you want to work on this one @maddijoyce? How does my suggestion above sound?

[maddijoyce]

Ah yeah that makes sense to me. Don't just limit the chance for leakage in the tutorial/generated code. Instead eliminate it entirely in the DbAuthHandler.

I'd love to have a crack at this, so I'll check the contributor guide and push a PR. Cheers!

[cannikin]

I marked it as a breaking change: if someone is doing something with the user object returned by that function then they're screwed. Unfortunately, that's everyone running dbAuth since the toast message it shows on the ForgotPasswordPage gets the email address from the response object. 😬 So we'd have to release this with 4.0 at the earliest.

Turns out it was easier to just strip resetToken and resetTokenExpiresAt from the handler, which will NOT be breaking.

[cannikin]

We do have the ability to write codemods, so that your files are automatically updated by us during the yarn rw upgrade process, but it's no picnic writing them. And depending on how much someone has customized their forgotPassword hander or page, they might be impossible to run. In which case we'd have to have release notes anyway explaining how to make the changes. Personally, I'm inclined to just have Release Notes tell you what to do, but if you want to take a stab at a codemod @maddijoyce be my guest! :)

[cannikin]

Hey @maddijoyce any chance you still want to take this one? No problem if not, just say the word and I'll add to my own queue. :)