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

RolePolicy Cannot set property 'owner' of undefined

Open dottodot opened this issue 8 years ago • 6 comments

error: TypeError: Cannot set property 'owner' of undefined
    at module.exports (/node_modules/sails-permissions/dist/api/policies/RolePolicy.js:43:34)

This doesn't seem to work

if (!_lodash2['default'].contains(['update', 'delete'], action) && req.options.modelDefinition.attributes.owner) {
    // Some parsing must happen on the query down the line,
    // as req.query has no impact on the results from PermissionService.findTargetObjects.
    // I had to look at the actionUtil parseCriteria method to see where to augment the criteria
    req.params.all().where = req.params.all().where || {};
    req.params.all().where.owner = req.user.id;
    req.query.owner = req.user.id;
    _lodash2['default'].isObject(req.body) && (req.body.owner = req.user.id);
  }

where as this does

if (!_lodash2['default'].contains(['update', 'delete'], action) && req.options.modelDefinition.attributes.owner) {
    // Some parsing must happen on the query down the line,
    // as req.query has no impact on the results from PermissionService.findTargetObjects.
    // I had to look at the actionUtil parseCriteria method to see where to augment the criteria
    var params = req.params.all();
    params.where = params.where || {};
    params.where.owner = req.user.id;
    req.query.owner = req.user.id;
    _lodash2['default'].isObject(req.body) && (req.body.owner = req.user.id);
  }

dottodot avatar Apr 13 '16 07:04 dottodot

That seems to fix it. I wonder how/when this broke down.

asmello avatar Apr 16 '16 14:04 asmello

Didn't work for me. It seems to happen because "where" is actually a string and "owner" property is being assigned to literal value.

andriichumak avatar May 02 '16 19:05 andriichumak

If you have come here looking for a solution and you are lazy like me and don't want to fork and npm install etc., copy RolePolicy.js in your apps policies folder and apply the fix mentioned above there...

indr avatar Aug 14 '16 12:08 indr

@dottodot The provided fix isn't fixing actually, but simply disabling owner policy. The previous code is actually bad practice as it expects req.params.all() to always return the same object. This expectation isn't backed by any specification (checked docs for expressjs as well as sailsjs on this). Therefore the code is hackish and error-prone.

Try this to prove my assumption:

console.log( req.params.all() === req.params.all() );

In my case this is logging false and so documenting that invoked method is actually providing copy of req.params data rather than any actual storage on parsed parameters. By calling it three times in falsy code all writes operate on different objects. However, this code tries to modify req.params for a reason here: to use it somewhere else. Your fix seems to be working but comes with different semantics. It is still working on a copy of req.params thus not modifying the original data. Try this after your fixing code to illustrate that:

console.log( req.params );

Guess it doesn't contain where unless some parameter named where was found in actually requested URL. So, your code isn't adjusting req.params either but qualifying some local-only variable which isn't used afterwards. Your fix is thus equivalent to simply removing the two changed lines completely.

Here comes the full proof:

    console.log( req.params.all() === req.params.all(), req.params, req.params.all() );
    var params = req.params.all();
    params.where = params.where || {};
    params.where.owner = req.user.id;
    console.log( req.params, req.params.all() );

So, how to fix this? Neither way is correct. Actually sails-permissions has to support such implicit provision of where parameter using some custom data injected into req. Other files relying on parameter req.params.where need to merge that parameter with custom data injected here.

UPDATE: Another option working for now is this:

      if ( !req.params.where ) {
        req.params.where = {};
      }

      req.params.where.owner = req.user.id;

But this applies only if semantics of req.params won't change in future. A final solution might require approach described before.

soletan avatar Feb 01 '17 22:02 soletan

I also stumbled upon this issue, it actually happens for me every time i try to call the /user/me route without being admin (so you are the owner of the resource). So quite a prominent use-case I think ...

JannikStreek avatar Apr 19 '17 06:04 JannikStreek

++

pixelbacon avatar Sep 26 '17 20:09 pixelbacon