sails-permissions icon indicating copy to clipboard operation
sails-permissions copied to clipboard

Permissions ignored in custom User.find

Open dottodot opened this issue 9 years ago • 15 comments

If I put the following in my own UserController permissions seem to be ignored and a user making a request to /user can see all users

module.exports = {
  find: function(req, res) {
    User.find().then(function (users) {
      res.ok(users);
    });
  }
}

If I remove this they only can view their own.

dottodot avatar Feb 29 '16 15:02 dottodot

Any chance of some help with this. It's a pretty critical issue if you can't use the UserController without exposing all users to everyone.

For reference I'm using v2.2.0

dottodot avatar Mar 03 '16 14:03 dottodot

Can u paste me your config/policies.js?

I am guessing that u did not apply a policies to your UserController and actions.

ghost avatar Mar 11 '16 10:03 ghost

This is what I have at the moment

module.exports.policies = {

  '*': [
    'basicAuth',
    'passport',
    'sessionAuth',
    'ModelPolicy',
    'AuditPolicy',
    'OwnerPolicy',
    'PermissionPolicy',
    'RolePolicy',
    'CriteriaPolicy'
  ],

  UserController: {
    'create': true,
    'signup': true,
    'forgot': true,
    'reset': true
  }

};

dottodot avatar Mar 11 '16 10:03 dottodot

  1. As I thought the following lines are not restricting access to UserController.find.
 UserController: {
    'create': true,
    'signup': true,
    'forgot': true,
    'reset': true
  }

Moreso no UserController action is restricted as every action is set to 'true', meaning anybody may hit this controller's actions.

  1. As far as i understand sails-permissions this module restricts access to your models ONLY IF u access them via RESTful routes. Specifically sails-permissions uses policies and policies only restrict Controllers and their actions, nothing else. If someone permittedly hits one of ur controller actions and you use Waterline models to alter ur database, sails-permissions will not restrict anything as it protects the REST-API of sails, as mentioned above. So you either got to write ur own policy or handle this within your find action.

You could be doing something like:

UserController: {
    '*': [   // restrict any action of the UserController with the following policies. But if u do this there is imo no need for the find action on your UserController, as you could query your users via REST which would be authorized (or not) as intended by sails-permissions. 
    'basicAuth',
    'passport',
    'sessionAuth',
    'ModelPolicy',
    'AuditPolicy',
    'OwnerPolicy',
    'PermissionPolicy',
    'RolePolicy',
    'CriteriaPolicy'
  ]
    'find': [ // exclude the find action from the above listing and explicitly name the policies to be checked on this controller's action.
         'basicAuth', // if basic authorization is present on the http header this policy passes if auth succeeds
         'passport', // init passport and session
         'sessionAuth', // check if req.session.authenticated ==  true
         'AuditPolicy',  // write logs
     ] // now check in the find action if everything is met to authorize whatever you want to be doing
  }

Please have a read of Sails Policies and Sails Blueprint API

If I am wrong please correct me :) Improvements and/or new knowledge are always welcome!

Hoping to be helpful..

Regards, René

ghost avatar Mar 11 '16 12:03 ghost

This reason I have the find action on my controller is so I can customise the response. My original post just shows basic example as it doesn't make any difference to the problem.

So am I right in thinking Sail permissions only works if you only use the default blueprint routes?

dottodot avatar Mar 11 '16 14:03 dottodot

Not exactly but yes, kind of :)

Sails policies only work on controllers and their actions. So u can protect ur find action. But the model logic will not be touched. It basically applies the policies to the blueprint controllers (more correctly ALL controller u mention and configure in policies.js).

ghost avatar Mar 11 '16 14:03 ghost

I still a bit confused. If I have the following policies

UserController: {
    '*': [
      'basicAuth',
      'passport',
      'sessionAuth',
      'ModelPolicy',
      'AuditPolicy',
      'OwnerPolicy',
      'PermissionPolicy',
      'RolePolicy',
      'CriteriaPolicy'
    ],
    'find': [
      'basicAuth',
      'passport',
      'sessionAuth',
      'ModelPolicy',
      'AuditPolicy',
      'OwnerPolicy',
      'PermissionPolicy',
      'RolePolicy',
      'CriteriaPolicy'
    ],
    'create': true,
    'signup': true
  }

I can see that the Polices are triggered but just don't have any effect on the results returned. Just seem bizarre that it can go through all the checks only to not have any outcome on the result. Surely that defeats the purpose.

dottodot avatar Mar 11 '16 15:03 dottodot

Also as an experiment I've tried with the Role controller setting policy for read to owner returns no results, and setting it to role does, so policies seem to work sometimes.

dottodot avatar Mar 11 '16 16:03 dottodot

Also my custom findOne for users also returns the correct response i.e users can only view themselves, so it seems it's an issue with just custom find.

dottodot avatar Mar 11 '16 16:03 dottodot

Think this issue might be related to https://github.com/tjwebb/sails-permissions/issues/212

findTargetObjects: function findTargetObjects(req) {

    // handle add/remove routes that have :parentid as the primary key field
    var originalId;
    if (req.params.parentid) {
      originalId = req.params.id;
      req.params.id = req.params.parentid;
    }

    return new Promise(function (resolve, reject) {
      sails.hooks.blueprints.middleware.find(req, {
        ok: resolve,
        serverError: reject,
        // this isn't perfect, since it returns a 500 error instead of a 404 error
        // but it is better than crashing the app when a record doesn't exist
        notFound: reject
      });
    }).then(function (result) {
      console.log(result);
      if (originalId !== undefined) {
        req.params.id = originalId;
      }
      return result;
    });
  }

Here the result is actually correct

@tjwebb are you able to provide any help with this as little unsure where to look next to find out where it's going wrong.

dottodot avatar Mar 11 '16 16:03 dottodot

see https://github.com/tjwebb/sails-permissions/issues/200

tjwebb avatar Mar 12 '16 23:03 tjwebb

OK thanks for letting me know, massively disappointing though.

dottodot avatar Mar 13 '16 10:03 dottodot

We're disappointed as well: https://github.com/balderdashy/sails/issues/3429#issuecomment-165004024

We are looking for active users/contributes to assist in transferring the long-term maintenance responsibility of this project to someone else.

tjwebb avatar Mar 13 '16 15:03 tjwebb

It is weird. Yesterday I took more than 5 hours reaching for the bug. Permissions seems to be working OK, because until the last policy it reaches only the correct results. But at the end the find request returns everything. I think this could be a problem between sails-permissions and the ORM.

I would like to maintain this module, as I use in many projects.

medisoft avatar Apr 07 '17 19:04 medisoft

@medisoft if you're interested in maintaining, can you send me an email? [email protected]. Thanks

tjwebb avatar Apr 08 '17 01:04 tjwebb