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

restrict-to-owner throws error when user id is 0 #319

Closed
cgislason opened this Issue Oct 20, 2016 · 1 comment

Comments

Projects
None yet
2 participants
@cgislason
Copy link

cgislason commented Oct 20, 2016

I was doing some testing and created a test user in feathers-memory like this:

userService.create({
  email: 'test@example.com',
  password: 'password',
}).then((value) => {
  console.log('created user:', value);
});

Then when I tried to authenticate with auth/local I got the error: The 'restrictToOwner' hook should only be used on the 'get', 'update', 'patch' and 'remove' service methods. I knew that I was only using the hook on those 4 service methods so I dug into the restrict-to-owner code and found this:

if (!hook.id) {
  throw new _feathersErrors2.default.MethodNotAllowed('The \'restrictToOwner\' hook should only be used on the \'get\', \'update\', \'patch\' and \'remove\' service methods.');
}

The !hook.id check fails when the id is 0 (or any other false value). This check should probably be more careful. (I worked around it by giving a proper id)

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Oct 25, 2016

Can you submit a PR? Basically https://github.com/feathersjs/feathers-authentication/blob/master/src/hooks/restrict-to-owner.js#L15-L17 should instead look something like:

    if (['get', 'update', 'patch', 'remove'].indexOf(hook.method) === -1) {
      throw new errors.MethodNotAllowed(`The 'restrictToOwner' hook should only be used on the 'get', 'update', 'patch' and 'remove' service methods.`);
    }

aengushearne added a commit to aengushearne/feathers-authentication that referenced this issue Oct 26, 2016

restrictToOwner -Fix check for methodNotAllowed
methodNotAllowed was being checked as "if (!hook.id)", leading to erroneous or misleading error messages.
Changed to check against hook.method 

This also fixes feathersjs#319.

@daffl daffl closed this in #335 Oct 28, 2016

daffl added a commit that referenced this issue Oct 28, 2016

restrictToOwner -Fix check for methodNotAllowed (#335)
* restrictToOwner -Fix check for methodNotAllowed

methodNotAllowed was being checked as "if (!hook.id)", leading to erroneous or misleading error messages.
Changed to check against hook.method 

This also fixes #319.

* Update restrict-to-owner.js

Fix works but integration test failure, trying alternative syntax.

* Fix restrict to owner and test

* Skip test for Node 0.10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.