New version = no global middleware?

[viki53]

Hi,

I just switched to the new version of multer on an existing project and now my upload forms won't work because csurf doesn't have the necessary headers (a bit like issue #71) because the middleware is used on a case-by-case basis instead of being global.

Is there any way to make multer parse the form data (if provided) for every requests (or at least the POST ones)?

[LinusU]

That's an interesting one!

At the first glance I'm not entirely sure on how to solve this best. How are you constructing your routes? My first instinct is that the csurf middleware should be ran after the middleware from multer have. This seems quite straight forward if you are using anything other than standard express functions for defining routes.

Otherwise I could think of letting csurf only validate requests that isn't multpart/form-data, and than always to another call to csurf after multer has finished parsing. But that seems a bit complicated...

[viki53]

I basically do this:

app.use(csrf({ cookie: true }));

app.use(function (err, req, res, next) {
    if (!err || !err.code || err.code !== 'EBADCSRFTOKEN') {
        res.locals.csrfToken = req.csrfToken();
        return next(err);
    }

    res.status(403);
    res.render('error403', { error: { code: err.code, message: 'Formulaire invalide : le jeton est expiré ou non reconnu' }});
    res.end();
});

And then my routes (at least those that use multer) are like this:

app.route(['/book/:userpseudo/:albumid', '/book/:userpseudo/:albumid/-:albumname']).all(function (req, res, next) {
    // Some treatments…
    next();
}).post(upload.single('image[src]'), function (req, res, next) {
    // Some treatments on post…
    res.render('view_name');
    res.end();
}).get(function (req, res, next) {
    // Some treatments on get…
});

Everything worked fine before I updated to multer 1.0 because I just did app.use(multer()); before everything else and that was it.

---

I thought about creating a middleware that would apply multer on the appropriate routes before csurf is loaded, but that would quickly become hard to maintain as the project will evolve…

[gabeio]

In essence you don't need csurf for anything except things like post/put/patch/delete routes which "change" things. So as @LinusU suggested you would want to do something like this:

app.route(['/book/:userpseudo/:albumid', '/book/:userpseudo/:albumid/-:albumname']).all(function (req, res, next) {
    // Some treatments…
    next();
}).post(upload.single('image[src]'), csurf({ cookie:true }), function (req, res, next) {
    // Some treatments on post…
    res.render('view_name');
    res.end();
}).get(function (req, res, next) {
    // Some treatments on get…
});

(note csurf after multer)

alternatively you can attach the csurf token to the url as a query variable so the csurf is protecting multer and csurf can stay global...

req.query._csrf - a built-in from Express.js to read from the URL query string.

---

csurf also allows headers ['csrf-token','xsrf-token','x-csrf-token','x-xsrf-token'] but this would require either a javascript multipart upload or a client application as I am not aware of any way to add a header into a normal html form

[viki53]

I would have tried that way, but I do I manage the CSRF errors then (see middleware in my previous)?

I already have some forms that need CSRF protection and the project is only at the beginning. Ultimately I will have lots of them.

I'm not sure about putting the token in the URL as it's really easy to tamper with it. But as a last resort that would do it…

Why is there no way anymore to use mutler as a global middleware as it was before (and other npm modules do, but not the express 4.x way unfortunately)?

[gabeio]

I'm not sure about putting the token in the URL as it's really easy to tamper with it. But as a last resort that would do it…

csurf is not a session. it's literally meant only to protect from cross site post/put/delete requests that are aimed to maliciously(or otherwise) change a user's account on a different website, see here for more details. csurf basically is a one time good random string that needs to be verified with a cookie that allows a post/put/delete request to be processed. so the query variable makes no difference (though it may look ugly) you can redirect removing the query variable afterward.

Why is there no way anymore to use mutler as a global middleware as it was before (and other npm modules do, but not the express 4.x way unfortunately)?

Previously multer had a bug that would allow users to upload post/put/delete files to any url valid or invalid and the files would never be processed (filling hard drive space) see #131 for more details. csurf would not protect you from this attack as csurf only verifies user X is doing something they want to be doing. if the csurf token and cookie verify the request multer is still vulnerable.

---

csurf protects against cross site requests by giving a user a cookie and token that verify a request. a site that makes a form for you to just click a submit button and contains a token from your site will not be enough as they can not set cookies within your domain and the request will fail.

[gabeio]

I would have tried that way, but I do I manage the CSRF errors then (see middleware in my previous)?

I believe the error catching middleware for csurf if it was catching errors before will be fine.

[LinusU]

Error catching middleware should work exactly as before, no need to change that. I think that this might be the best way of solving it.

app.post('/path', upload.single('image[src]'), csurf({ cookie:true }), function (req, res, next) {
  // File was uploaded and cross site token passed, hurrah!
})

app.use(function (err, req, res, next) {
  switch (err.code) {
    case 'EBADCSRFTOKEN':
      // Bad cross site script token.
      break
    case 'LIMIT_FILE_SIZE':
      // The file uploaded was too large.
      break
  }

  // Return error page to user
})

---

A bit of topic but your error handlers first check could be simplified. That route will only be called when there is an error so you don't need to check for err to be present. Also, since err.code will evaluate to undefined when there wasn't a code specified, you don't need to check for the presence of that either. You can just use err.code straight away, e.g. if (err.code === 'EBADCSRFTOKEN').

[viki53]

Ok, I edited my code and now I can get the token to show up in the form.

But when I send the form I get an 404 error on the same URL.

The route is handled as follows:

app.route(['/book/:userpseudo/:albumid', '/book/:userpseudo/:albumid/-:albumname']).all(function (req, res, next) {
    // Check URL parameters (id's) and load some data (profile, album…)
    // …
}).post(upload.single('image[src]'), csurf({ cookie: true }), function (req, res, next) {
    // Need the csurf middleware to validate the token (or create a new one if the form is incorrect)
    // …
}).get(csurf({ cookie: true }), function (req, res, next) {
    // Need the csurf middleware to create a token and send it with the form
    // …
});

So basically I can get into the get method but not the post one. And the the error handler (which is now below, at the end of the routes, just before the 404 — as a default — one) doesn't seem to be called, although the 404 route is:

app.use(function (err, req, res, next) {
    switch (err.code) {
        case 'EBADCSRFTOKEN':
            res.status(403);
            res.render('error403', { error: { code: err.code, message: 'Formulaire invalide : le jeton est expiré ou non reconnu' }});
            res.end();
            return;
        break;
    }

    next();
})

app.route('*').all(function (req, res, next) {
    res.status(404);
    res.render('error404');
    res.end();
});

[gabeio]

I can't say why that's happening though I think the err.code should have been fine where it was if it was working before. I do suggest you try using csurf globally and just add the csrf token as a query variable and see that you literally don't have to change anything but where the csrf is located.

[viki53]

I just did that (csurf as global middleware & token as query variable) but I still have the same behavior : 404 error, no log in the csurf error handling middleware (doesn't matter if I put before or after the route)…

[gabeio]

I just tried moving the action="?_csrf={{token}}" to the query variable it worked fine for me... csurf was mounted globally to the app near the top of my main js file and the error catcher was at the very bottom after all the routes and routers were attached and listened for the "EBADCSRFTOKEN".

[LinusU]

@viki53 It's very weird that it's giving you a 404 for that route. Could you try and remove the csurf middleware from both the get and post request, and remote multer from the post request. Does it than still give you a 404?

[viki53]

Nevermind!

I just had a condition (if (!req.files || !req.files['image[src]'])) that used to work fine but it seems like I need to update it for the new multer version (if(!req.file)). I didn't see that this part changed too before…

This condition simply did a next(); return; so that's why I ended up in the 404 route. I also changed that code to show the current page with an error. That's much better!

Thanks guy, sorry for the trouble!

[LinusU]

No problem, glad that you got it resolved!

Are you satisfied with how you are using the library now? Feel free to close if you feel that its working now

[gabeio]

@LinusU we probably should show two examples of csurf with multer on the readme!
(csurf globally with query [probably more secure] && csurf in route middleware after multer)

[viki53]

Well I don't have a file path anymore in the file object, but maybe I need to specify a folder?

My code actually reads the uploaded temporary file (with a promised version of fs), then loads its data into GraphicsMagick to read some more info and then writes to a private folder once I checked it has the proper dimensions.

[LinusU]

req.file.path is empty for you? That's strange.

How do you instantiate the variable upload? Note that currently multer() will default to storing the file in memory. Use either multer({ dest: 'path/here/' }) for a specific directory, or multer({ storage: multer.diskStorage({}) }) to store them in the os temporary folder.

[viki53]

I only did upload = multer. Your last solution (OS temp folder) works fine. Thanks!

BTW, maybe this should be the default option (instead of getting a buffer)?

[LinusU]

Yes that might be a good idea, discussion in #174

[gabooh]

I have this problem too and can't manage to make it work. I've tried the adding the csurf middleware after the upload one on the post route, as advised above, but it still says mising csrf token. Any complete example somewhere I could use ?

Here is my relevant code if you notice anything wrong ...

var csrf = require('csurf');
var multer = require('multer');

var csrfProtection = csrf({ cookie: true });
app.use(csrfProtection);

var upload = multer({dest: './uploads'});
app.post('/question/:id', upload.single('image[src]'), csrf({ cookie:true }), questionController.saveQuestion);

[LinusU]

That's because you are also adding it before the route with .use.

 var csrf = require('csurf');
 var multer = require('multer');

 var csrfProtection = csrf({ cookie: true });
-app.use(csrfProtection);

 var upload = multer({dest: './uploads'});
-app.post('/question/:id', upload.single('image[src]'), csrf({ cookie:true }), questionController.saveQuestion);
+app.post('/question/:id', upload.single('image[src]'), csrfProtection, questionController.saveQuestion);

[trogne]

Maybe my answer here is relevant :
#755