0

I currently have a middleware method setup on certain routes to make sure the user is allowed to modify a specific document. I's using it in conjunction with angular $resource.

exports.hasAuthority = function hasAuthorization(req, res, next) {
    if (req.body.creator.toString() !== req.user._id.toString()) {
        return res.send(403);
    }
    next();
};

The above has been working so far when the document is on the request body so i can just grab the creator id and check if it matches.

The problem is the DELETE method in the Angular $resource does not post the object to the request body so creator is undefined. I've done some searching and came across angular $resource delete won't send body

There is a solution to override this behavior but it seems like it should be left as it is.

I have been looking at this example angular-passport and scratching my head as to how he has managed to get req.blog populated with the blog data which is then used by all methods to modify the data on the server.

I thought maybe in the middleware I could do a quick findOne query with an id passed in on the query string and checking it against the creator that way but wasn't sure if this is bad practice..

Any ideas?

Thanks, James

Community
  • 1
  • 1
jameslouiz
  • 396
  • 4
  • 12

1 Answers1

0

Thinking about it further, it seems a little flawed to pass the object data along with the user data up to the server in the same request. I guess it could allow the user to potentially change the creator id to that of the current user and grant them selves the privileged to modify the object.

In order to avoid this I have changed my middleware: 

var events = require('../app/models/events');
exports.hasAuthority = function hasAuthorization(req, res, next) {

    var id = req.params.id,
        user = req.user._id.toString();

    events.findOne({_id: id }, function (err, event) {
        var creator = event.creator;

        if ( creator != user) {
            return res.send(403);
        }

        next();
    });
};

With my route being 'DELETE' /api/events/:id

I know I'm querying the database twice but its the most secure method I could think of.

jameslouiz
  • 396
  • 4
  • 12
  • Yes, you should not trust any ownership information received from user. As a side note, `req.user` is not coming from the user and is safe to use in queries as well, ie. you could safely do `events.remove({_id: id, creator: req.user._id})` – vesse Dec 10 '14 at 15:06
  • Ah i see what you mean, essentially handling the auth in the model handler, not the middleware! I'll look into this. Thanks! – jameslouiz Dec 10 '14 at 22:42
  • I would still prefer middleware since it is very general and easy to use in all needed routes. Just thought to bring that up if you want to avoid two queries. – vesse Dec 11 '14 at 02:16
  • 1
    No its an excellent point which I hadn't thought about. I guess 2 query ain't so bad for now. Maybe there is an even better solution in the future.. thanks – jameslouiz Dec 11 '14 at 09:22