Skip to content
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

RolePolicy Cannot set property 'owner' of undefined #227

Open
dottodot opened this issue Apr 13, 2016 · 6 comments
Open

RolePolicy Cannot set property 'owner' of undefined #227

dottodot opened this issue Apr 13, 2016 · 6 comments

Comments

@dottodot
Copy link

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);
  }
@asmello
Copy link

asmello commented Apr 16, 2016

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

@andriichumak
Copy link

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

tiffanytangt added a commit to tiffanytangt/sails-permissions that referenced this issue May 27, 2016
addresses error: TypeError: Cannot set property 'owner' of undefined
    at module.exports (/node_modules/sails-permissions/dist/api/policies/RolePolicy.js:43:34)
@indr
Copy link

indr commented Aug 14, 2016

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...

@soletan
Copy link

soletan commented Feb 1, 2017

@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.

@JannikStreek
Copy link

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 ...

@pixelbacon
Copy link

++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants