digitalsadhu/admittance

Role based access control implementation for node

admittance's Issues

Expected object to always be accessible by owner

From the documentation on expressions I would expect the following to work:

var check = admittance({}, {1: 'edit'})
check(2).can('edit', 2)

Since that second expression validates as 2 === 2. (Passing in 2 === 2 also fails).

Regardless of the group to which a member belongs, they should be admitted to content to which they're the owner it feels.

Shouldn't success default to false

https://github.com/digitalsadhu/admittance/blob/master/admittance.js#L58

Shouldn't success default to false so that if any oddity is passed in other than a valid number or a valid boolean the value returned would be false instead of true?

Include example usage

Be great if an example project was included. This would help discussions around things like api design

Documentation for version 4 is still only relevant to version 3

Why do we need assignments

Hi,
Why do we need to assign user IDs to roles? Typically, I don't need it because role(s) of my users are stored in a JWT. I just need to test if an "admin" can "write comment".

I don't need to test if "user24" which "is an admin" can "write comment". From my point of view this mapping is useless.

If you have to manage 1 millions users, and only 2 roles (for example "admin" and "user"), you have to map this millions users (where almost 99% of them will be simple "users"). It's a lot of redundant information to just finally rely on a role.

I think this assignment step is not the responsability of your library.

(it's just my opinion. Thank you for sharing your work!)

Improve API section of documentation

This section of the docs is not quite as clear as it could be. Consider improving this when possible

Allow multiple instances of admittance

You can currently only have one instance of admittance loaded in a process. It would be nice to be able to have multiple, either:

var Admittance = require('admittance');
var user = new Admittance({ roles: require('./roles.json') });
user(13).is('admin');

or

var admittance = require('admittance');
var user = admittance({ roles: require('./roles.json') });
user(13).is('admin');

where the return value of admittance() is an instance.

Testing private methods

Rather then using rewire to reach inside modules for testing purposes it is probably better to extract the private functions into a separate module and then test that module's interface.

Thoughts?

Concider separating permissions and roles

I personally feel that splitting permissions and roles into separate options would be wise.

As it is now, I will have to get the user => role mappings from my database, then fetch my roles => permissions mappings and merge them into one object which is passed in to admittance.

In addition, this might cause unexpected behavior when people are not using integers for userIds, but instead just use usernames. If someone has admin as a username, it might wreak havoc on the system.

Pass in objects to be authorised

Maybe something like

var a = new Admittance({
  getRoles: function(user){ return user.roles;}
});
a(user).can('viewPosts');

Business rules

Main thing missing from Admittance now is the ability to add business rule checking to permissions.

A good example of where this is useful is where you want to check if a user can edit a post but you also need to restrict editing to only posts owned by that user. You need some kind of business rule checking to ensure that a user has permission to edit and that the posts id is correct for the user

Business rules section contains invalid objects

var permissions = {
  'editPosts'
}

Is not a valid javascript object.