Auth: resetPassword invalid token response status code from 500 to 400 or 403?

[edtorba]

When resetting the password via resetPassword, if an incorrect token is provided, an error is received from the API. While receiving a 500 status code is acceptable (ish?), it seems more appropriate in this instance to use a 400 (Bad Request) or 403 (Forbidden) status code.

The definition for the 500 status code is as follows:

The HyperText Transfer Protocol (HTTP) 500 Internal Server Error server error response code indicates that the server encountered an unexpected condition that prevented it from fulfilling the request.

However, in our case, the server is aware of the situation, and I believe a 400 (Bad Request) status code, indicating that the server cannot or will not process the request due to something perceived as a client error, or a 403 (Forbidden) status code, indicating that the server understands the request but refuses to authorise it, would be more appropriate in this instance.

In terms of effort, the required change is straightforward; it simply involves adding an extra argument to set the appropriate status code.

resetPassword.ts

// ...

 const user = await payload.db.findOne<any>({
      collection: collectionConfig.slug,
      req,
      where: {
        resetPasswordExpiration: { greater_than: new Date() },
        resetPasswordToken: { equals: data.token },
      },
    })

    if (!user) throw new APIError('Token is either invalid or has expired.')

// ...

payload/packages/payload/src/auth/operations/resetPassword.ts

Line 70 in b40e9f8

if (!user) throw new APIError('Token is either invalid or has expired.')

APIError.ts

class APIError<
  TData extends null | object = { [key: string]: unknown } | null,
> extends ExtendableError<TData> {
  /**
   * Creates an API error.
   * @param {string} message - Error message.
   * @param {number} status - HTTP status code of error.
   * @param {object} data - response data to be returned.
   * @param {boolean} isPublic - Whether the message should be visible to user or not.
   */
  constructor(
    message: string,
    status: number = httpStatus.INTERNAL_SERVER_ERROR,
    data: TData = null,
    isPublic = false,
  ) {
    super(message, status, data, isPublic)
  }
}

payload/packages/payload/src/errors/APIError.ts

Line 41 in b40e9f8

status: number = httpStatus.INTERNAL_SERVER_ERROR,

[DanRibbens]

I agree with returning 403 status instead of 500. Because it is a breaking change though, I would open this as a PR against what will be the 3.0 branch when that is ready to contribute to.

Are you interested in making a PR?

[nunoci]

How to avoid the server stop responding when the auth fails? ** on login, or on reset pass** I have to restart the server 4 times a day...

[edtorba]

@nunoci I'd try setting up a custom global error handler in your Express server configuration (server.ts file).

const app = express();

// ... your stuff.

// Error handling middleware
app.use((err, req, res, next) => {
  console.error(err.stack); // Log the error stack
  res.status(500).send('Internal Server Error'); // Send a generic response
});

// "app..listen(..." stuff

[edtorba]

@nunoci did that work? 👀

[nuno]

@nunoci did that work? 👀

It is just to paste that code before app.listen(3001); that is it?

[nunoci]

Does not work at all.