restrictToOwner hook needs to throw an error and not scope the query

[beeplin]

when owner is set to true, the source code uses the following logic:

    if (options.owner && !authorized) {
      // NOTE (EK): This just scopes the query for the resource requested to the
      // current user, which will result in a 404 if they are not the owner.
      hook.params.query[options.ownerField] = id;
      authorized = true;

which is fine for find method, coz when finding, if the query.userId is wrong, feathers will return 404.
But at least in mongoose, update/patch/remove/get don't rely on query to execute, and the behavior is like this:
update: the modification is executed despite it's neither admin or owner
patch and remove: the modification is not executed, and no error thrown. the unchanged data is returned to client
get: the data is returned to client despite that it's neither admin or owner.

so I think the right way to do this is just like what EK wrote in TODO:

      // TODO (EK): Maybe look up the actual document in this hook and throw a Forbidden error
      // if (field && id && field.toString() !== id.toString()) {
      //   throw new errors.Forbidden('You do not have valid permissions to access this.');
      // }

and the code could simply be like this (untested):

this.get(hook.id).then(data => {
  if (data[options.ownerField].toString() !== id.toString()) {
     throw new errors.Forbidden('You do not have valid permissions to access this.');
  }
  return hook;
});

(restrictToOwner has the same issue.)

[ekryski]

@beeplin you are absolutely correct. I had thought about this but I think was too tired to really grok my error. I'm going to be doing a patch tomorrow for this so that it will throw a legit Forbidden error if you are not authorized.

[beeplin]

looking forward to v0.7 :))