-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSRF error status code #116
Comments
Ok, I just show that the error code is passed to Example: var createError = require('http-errors');
...
next(new createError.Forbidden(errmsg)); |
I would also like to get this change. We usually define application-wide error handlers, which rely on the error "shape" to determine how to respond. In case a generic error is returned, like the one provided by lusca, then a default response with 500 is returned. What I mean by this, is that at the end, the statusCode is easily overridable without knowing about it. This is something I just realised when I saw this issue. Having the status error available at the error instance would help on this scenario. Another option would be to attach another kind of "code" or property to the error instance. I have seen other libraries like boom or celebrate do just that. EDIT |
The status code of the response of a server (and generally all the content of a server response) should be the decision of the server itself and it's not the job of a middleware library. In my opinion the best would be not to throw an error at all but just inform the server that lusca csrf validation didn't pass. This could be done for example by passing an option at const errors = require('http-errors');
app.use('/api', (req, res, next) => {
if (!res.locals.csrf_pass) {
throw new errors.Forbidden('CSRF validation failed.');
// or do whatever you want
} else {
next();
}
}); The request/response lifecycle is now handled totally by the server and its not restricted by an Error thrown by the library. I know this is a breaking change and that could only be implemented at a major release, but I think this is the correct strategy. |
Documentation says:
But the error that is thrown from lusca doesn't have a status code. Does the documentation implies that the users should finally send a 403 error? And how do the users know that the error that was thrown, came from lusca CSRF? Just by checking the error message?
Example error stack print:
The text was updated successfully, but these errors were encountered: