Fix password recovery token not expiring after password reset

[inkiltie]

It would be nice to prevent user from reseting its password by clicking on reset link (2 or more times) in email after password was already reset. Like in Meteor Accounts if you try to reset password second time "Token Expired" error pops up.

[inkiltie]

There are 2 ways of solving this, either token specific (e.g. only remove 1 token that been used already, do not touch others if any exist), or remove passwordResetTokens entirely after password reset, no matter how many reset tokens are there.

I solved it second way by adding this function to api/users/resetPassword:

const removeResetTokensFromUser = (emailAddress = "") => {
  try {
    return Users.updateOne(
      { emailAddress },
      { $unset: { passwordResetTokens: 1 } }
    );
  } catch (exception) {
    throw new Error(`[removeTokensFromUser] ${exception.message}`);
  }
};

then by calling in this way inside resetPassword function:

await addSessionToUser(user?._id, login).then(
  removeResetTokensFromUser(user?.emailAddress)
);

Also I noticed this session is not triggered right away. I was always moving to /login page instead of my default authenticated page. For now, I just force window to reload to automatically appear in a logged in state.

I added window.location.reload(true) to next.js frontend:

client
      .mutate({
        mutation: resetPasswordMutation,
        variables: {
          token: Router.query.token,
          newPassword,
          repeatNewPassword,
        },
      })
      .then((response) => {
        return handleApolloResponse({
          queryName: "resetPassword",
          response,
          onSuccess: () => {
            pong.success(
              "Password reset! You can log back in with your new password."
            );
            Router.push(`/login`);
            window.location.reload(true);
          },
          onError: (error) => {
            pong.danger(error);
          },
        });
      })

[inkiltie]

Another thing to think about is to make reset tokens expire before they've been executed, let's say a month after requestedAt. Is it even a good practice?

[rglover]

@inkiltie thanks for sharing your notes on this. Re: expiring after some amount of time, yes, I'd agree that's a good idea.