Idea: custom required permissions about feathers-permissions

[daffl]

Could the syntax be the same as a normal JavaScript Array every and some?

service.hooks({
  before: {
    // All permissions
    all: isPermitted.every(
      hasPermissions({ entity: 'user', field: 'permissions', service: 'users' }), // similar to checkPermissions. All these options are optional and would default to these values
      (hook) => hook.params.user.role === 'admin' && hook.data.plan === 'pro', // just returning a boolean
      (hook) => { // returning a promise that eventually resolves a boolean
        const query = { type: 'pro', $limit: 1 };
        return hook.app
          .service('plans')
          .find({ query }, { paginate: false })
          .then(results => hook.params.user.uploads < plans[0].allowedUploads)
        });
      })
    )
  }
});

service.hooks({
  before: {
    // At least one
    all: isPermitted.some(
      hasPermissions({ entity: 'user', field: 'permissions', service: 'users' }), // similar to checkPermissions. All these options are optional and would default to these values
      (hook) => hook.params.user.role === 'admin' && hook.data.plan === 'pro', // just returning a boolean
      (hook) => { // returning a promise that eventually resolves a boolean
        const query = { type: 'pro', $limit: 1 };
        return hook.app
          .service('plans')
          .find({ query }, { paginate: false })
          .then(results => hook.params.user.uploads < plans[0].allowedUploads)
        });
      })
    )
  }
});

[ekryski]

Then I imagine it'll be fairly trivial to include general hooks to chuck in there e.g. that hasPermissions one, or isOwner.

Ya that's what I was thinking. That those would come bundled with feathers-permissions as nice to have hooks to make your life easier but you really could use whatever you want.

Ya I think splitting those would work really well! I think we could keep the same syntax @daffl suggested where we don't have the extra [] wrap. So we can just do:

service.hooks({
  before: {
    // At least one
    all: isPermitted(some(hook1, hook2, hook3))
  }
});

Which would generally make your permissions hooks/filters super tight. 😄

[ekryski]

Actually it's looking more like it could just be this:

// At least one
service.hooks({
  before: {
    all: unless(some(hook1, hook2, hook3), () => Promise.reject(new errors.Forbidden())
  }
});

// All must be true
service.hooks({
  before: {
    all: unless(every(hook1, hook2, hook3), () => Promise.reject(new errors.Forbidden())
  }
});

and then we don't even have isPermitted. Just any helper hooks for checking permissions/ownership.

@daffl I think that is the opposite of your when hook you were hoping for. So we can have:

• when (synonymous with iff)
• unless (synonymous with iff(isNot()))
• iffElse

[bedeoverend]

I like it! Then I imagine it'll be fairly trivial to include general hooks to chuck in there e.g. that hasPermissions one, or isOwner.

Another syntax option, not sure if it's worth it, but just accepting one hook, and writing utilities (perhaps over in feathers-hooks-common) for running multiple hooks?

e.g.

import { every, some } from 'feathers-hooks-common/lib/utils'

service.hooks({
  before: {
    // At least one
    all: isPermitted(
      some([
        hasPermissions({ entity: 'user', field: 'permissions', service: 'users' }), // similar to checkPermissions. All these options are optional and would default to these values
        (hook) => hook.params.user.role === 'admin' && hook.data.plan === 'pro', // just returning a boolean
        (hook) => { // returning a promise that eventually resolves a boolean
          const query = { type: 'pro', $limit: 1 };
          return hook.app
            .service('plans')
            .find({ query }, { paginate: false })
            .then(results => hook.params.user.uploads < plans[0].allowedUploads)
          });
        }
      ])
    )
  }
});

could then reuse them for for iff hook e.g. iff(some([ ... ])) or perhaps with ... spread style usage, rather than an array.

[jamesvillarrubia]

I haven't seen much movement on this library and would love to dig in a bit more. The problems, I've seen are basically: 1) can't control user ownership, 2) can't distinguish between role and service, and 3) can't negate against a permission.

I would love to see something on the format of:

(role:) (service:) (method:) (id:, own:) (not:)(field)

EXAMPLE Give admin roles rights to create on any service

admin:*:create:*

EXAMPLE Give a user rights to edit their own profile, but not certain fields

user:users:*:own //this default gives users permissions to do any action on their own 
user:users:create:* //this would allow a user to create a user profile (their own)
user:users:*:not:roles //this would reject any updates to the roles
user:users:*:not:permissions  //this would reject any updates to the permissions field

I'm imagining that the "own" field essentially applies a filter to all queries such that an owner id field is supplied inside the service ownerField: 'ownerId' or in the case of a user service ownerField:'_id' which I'm borrowing heavily from the restrictToRoles hook.

The not: filter essentially inverts the permissions. Where the existing permissions are structured as "must have one of these," the not field would be "can't have any of these." I'm worried about the expansive nature of this but I think as long as we keep the not: restricted to the field level, it is manageable.

Thoughts? Anyone done anything at the field level or resolved the ownership issue yet?

[ekryski]

@bedeoverend finally looking at this. Ya I think that might make sense. My only concern is this is sort of custom to your application. I mean checking a field is fine but really this should probably just be your own hook. So you might register it like this:

myservice.hooks({
  before: {
    patch: [
      checkPermissions({ service: 'users' }),
      userCanPromote(), // this hook either sets hook.params.permitted to true or false
      isPermitted()
    ]
  }
});

This way you have any of your custom logic, but by the time you get to userCanPromote you already know that they have the ability to edit the entity in question.

Do you think that would work @bedeoverend? I'm not opposed to what you are suggesting just wondering if it should be a part of these hooks or not as it seems like what you were proposing is pretty much just a different way of doing what I posted above.

[bedeoverend]

@ekryski yeah that'll definitely work, though if I did that, I don't think I'd bother to use feathers-permissions, given that userCanPromote() would have to know the internal logic of checkPermissions in order to work with isPermitted.

My other concern, and I'm not sure if this makes sense, but basically the request comes, we assume not permitted', the first hook checkPermissions says yes permitted, and we rely on the second hook to reverse the first decision - that feels a little more fragile, though it might be me just overthinking things.

@ekryski perhaps another option would be to expose multiple hooks for permissions? i.e. make it more modular, but keep checkPermissions which bundles them together for the general case?

e.g.

import { setAction, checkCan, serviceLevelActions } from 'feathers-permissions'
myservice.hooks({
  before: {
    all: [ setAction() ]
    patch: [
      checkCan('users:patch'), // Check specific method
      isPermitted()
    ],
     find: [
      checkCan((hook) => true), // Allow potentially async checks
      isPermitted()
    ],
    delete: [
      checkCan(serviceLevelActions('users')), // Essentially same as users:*
      isPermitted()
    ]
  }
});

myfancyservice.hooks({
  before: {
    patch: [
      // Custom classification of action
      (hook) => hook.params.action = hook.data.role === 'admin' ? 'promote' : 'unknown',
      checkCan('some_patch'),
      isPermitted()
    ]
});

Though I realise at this point it may be just getting too complex and convoluted...thoughts?

[ekryski]

🤔 hmmm... this might be a decent solution to the conundrum I outlined in this comment. I'm going to play with a couple things this evening.

[bedeoverend]

@ekryski yeah the last line in your comment:

I was hoping to avoid pre-fetching the resources in order to check ownership but it's looking like that's not really possible....

was the problem I was facing when building a similar permissions system. Essentially with simple permissions I could just use the request and incoming user information, but as things got more complex, realised I needed to perform DB lookups. The solution I ended up with looks somewhat similar to the above proposal, though far less generalised.

Look forward to seeing what you find out :) Let me know if I can help with anything

[ekryski]

I mean we did have some old hooks that did do this (pre-fetch data). We could just bring them back in. It's really not a huge hit for 1 record. It gets a bit more complex and potentially a lot more expensive to pre-fetch multiple entities in the case of patching/removing many or doing a find.

Most permissions systems simply utilize roles and then leave anything more fine grained up to the developer. I think the rails gems CanCan and Pundit are probably the best things out there that I've seen so I'm digging a bit deeper into them for some inspiration, they are kind of similar to what you proposed.

[bedeoverend]

My ruby knowledge is pretty limited so I'm not sure if I understand them properly, but CanCan looks a lot like node_acl, which might also be useful. I'm using node_acl just to store what actions are allowed for X role, then attach the action being taken and the role for every request, and check it against node_acl.

[ekryski]

Yeah pretty much the same concept. Honestly, I had been thinking that if we kept the same permissions scheme I had where you have resource ids then it would need to be a separate table/collection. The problem is still keeping them up to date.

For example,

I can edit and remove my own messages, but admins can also do the same. If an admin removes one of my messages it should remove my messages:*:1234 record.

Keeping those up to date would be a nightmare. So I don't think the node_acl way will really work that great for us. It's pretty much what we're already doing but we're forcing the permissions to be on the entity.

So I think the best bet is to maybe do something "automagically" down to the service + method level and then if you need to customize it further you either:

a) write your own hook and modify hook.params.__permitted
b) or the isPermitted hook takes an optional when option that is function that takes a hook or something and has to return true or false??

isPermitted({
  when: (hook) => hook.params.user.role === 'admin' && hook.data.plan === 'pro'
});

This hook could also return a Promise so that you can do async things.

[ekryski]

Alright so here's what I'm thinking...

service.hooks({
  before: {
    all: [
      isPermitted({
        all: true, // all "when" functions must return true. This is the default. If false, at least one must be true
        when: [
          hasPermissions({ entity: 'user', field: 'permissions', service: 'users' }), // similar to checkPermissions. All these options are optional and would default to these values,
          isOwner({ entity: 'user', ownerField: 'userId', foreignField: 'id' }),
          (hook) => hook.params.user.role === 'admin' && hook.data.plan === 'pro', // just returning a boolean
          (hook) => { // returning a promise that eventually resolves a boolean
            const query = { type: 'pro', $limit: 1 };
            return hook.app
              .service('plans')
              .find({ query }, { paginate: false })
              .then(results => hook.params.user.uploads < plans[0].allowedUploads)
            });
          },
        ]
      });
    ]
  }
});

You can then set all to false if you only want one of those to return true to allow access, or you could take it a step further and chain multiple isPermitted hooks. Thoughts @bedeoverend?

[ekryski]

Ha ha. I was thinking the same thing @daffl. So we would reject with a Forbidden error if none of those return true, and in the case of some early exit the chain as soon as one passes. Does that make sense?

Should you be able to manipulate the hook object inside of those? My gut says no, and that each one should get a copy of the hook object so that you don't have weird side effects.

[bedeoverend]

Looks great!

So am I right in thinking essentially all isPermitted would be doing is:

function isPermitted(checks) {
  return (hook) => {
    return runHooks(checks)
      .then(passed => {
        if (!passed) {
          throw new Forbidden();
        }

        return hook
      });
}

and the rest of the functionality would be in those helper hooks e.g. hasPermissions?

[ekryski]

Yup but instead of throwing a Forbidden we'd probably do Promise.reject and instead of just returning the hook we'd Promise.resolve(hook). Minor details though...

Sweet! I'm stoked.

[bedeoverend]

@ekryski yeah, was just checking the general logic and responsibilities of it - nice!

[eddyystop]

Conditional hooks features are starting to look rather impressive.

Is there anything more to document other than

• iffElse (in repo but undoc'ed)
• when, some, every (have PRs)
• unless (to do)?

[ekryski]

I don't think so. Unless you wanted to get super fancy and support atLeast(2, hook1, hook2, hook3) but I honestly can't really think of a scenario where you'd actually want that.

[beeplin]

I like the some, every, unless idea~ really cool~

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.

[danikane]

I haven't seen much movement on this library and would love to dig in a bit more. The problems, I've seen are basically: 1) can't control user ownership
Thoughts? Anyone done anything at the field level or resolved the ownership issue yet?

Hi, I was looking for the same recently so I thought it might be OK to leave this here for reference:
https://github.com/feathersjs-ecosystem/feathers-authentication-hooks#examples