feathers-starter-react-redux-login-roles icon indicating copy to clipboard operation
feathers-starter-react-redux-login-roles copied to clipboard

User list gettable, users modifiable (!) by non-admin via REST API

Open IBwWG opened this issue 8 years ago • 6 comments

  1. create your admin user normally, so that the admin user already exists and the next one will be a regular user.
  2. curl -X POST -H "Content-Type: application/x-www-form-urlencoded" -H "Cache-Control: no-cache" -d 'name=i have a name&username=nammmmmmmmmmme&password=what the hey&confirmPassword=what the hey&[email protected]' "http://localhost:3030/users"
  3. paste verification link into browser
  4. curl -X POST -H "Content-Type: application/x-www-form-urlencoded" -H "Accept: application/json" -H "Cache-Control: no-cache" -d '[email protected]&password=what the hey' "http://localhost:3030/auth/local"
  5. copy the token from the previous step's response
  6. paste it appropriately in curl -X GET -H "Accept: application/json" -H "Content-Type: application/x-www-form-urlencoded" -H "Authorization: Bearer INSERT-YOUR-TOKEN-HERE" -H "Cache-Control: no-cache" "http://localhost:3030/users"

Result: list of all users, even though it was requested as a non-admin user.

Strange, because there is:

auth.restrictToOwner({ ownerField: idName }),

...in \server\services\user\hooks\index.js. That idName does actually become _id as the config files suggest.

So...why the authorization breach?

IBwWG avatar Jan 04 '17 13:01 IBwWG

This also applies to PATCH--as a non-admin you can mess with the admin's (or anyone's) name, roles, etc.

IBwWG avatar Jan 04 '17 13:01 IBwWG

Interestingly, if I add a hook function to users/hooks:

  const myHook = () => { return (hook) => { if (hook.params.provider) throw new Error("uhoh") } };
 
  return {
    all: [],
    find: [
      auth.verifyToken(),
      auth.populateUser(),
      auth.restrictToAuthenticated(),
    ],
    get: [
      auth.verifyToken(),
      auth.populateUser(),
      auth.restrictToAuthenticated(),
      auth.restrictToOwner({ ownerField: idName }),
	  myHook(),
    ],
...

...no error is thrown on step 4 in the original here. In that code I was banking on the comment at https://github.com/feathersjs/feathers-legacy-authentication-hooks/blob/master/src/restrict-to-owner.js#L21 that implies that if (hook.params.provider) will only run on external calls.

IBwWG avatar Jan 04 '17 14:01 IBwWG

This happens on both Win7x64sp1 with Node 7.3.0 and Linux Mint 17.3 with Node 6.9.2.

IBwWG avatar Jan 04 '17 14:01 IBwWG

I also tried this, because I noticed that there was a hook function in feathers-service-verify-reset that didn't seem to be getting used here. This would also apply to #9 I guess. Anyway it didn't work, for some reason this hook always fails because .user seems to not be populated. But here was my attempt anyway:

    get: [
      auth.verifyToken(),
      auth.populateUser(),
      auth.restrictToAuthenticated(),
      verifyHooks.restrictToVerified(),
      auth.restrictToOwner({ ownerField: idName }),
    ],

IBwWG avatar Jan 04 '17 17:01 IBwWG

I reproduced your bug. Were you perhaps trying to fix the wrong service method? Step 6 is actually the find method. So a fix might be

    find: [
      auth.verifyToken(),
      auth.populateUser(),
      auth.restrictToAuthenticated(),
      auth.restrictToRoles({
        roles: ['superAdmin', 'admin'],
      }),
    ],

colinduwe avatar Dec 01 '17 22:12 colinduwe

This should be fixed in this commit https://github.com/eddyystop/feathers-starter-react-redux-login-roles/commit/0084662266c1100e8a028aaf7e0eab2d4f43fe2c

colinduwe avatar Dec 04 '17 19:12 colinduwe