flask-security icon indicating copy to clipboard operation
flask-security copied to clipboard

Add dynamic permissions

Open michaelbukachi opened this issue 7 years ago • 14 comments

Hello. Would it be possible to add permissions per add endpoint? That is, a user has a role, a role has a list of permissions. An endpoint would be decorated by a @permission decorator. So two roles can have the same permission. An implementation of the decorator that I use for one of my projects is shown below:

def permission(name, role_check=True, strict=False):
    def inner(func):
        func.__permission_name = name

        @wraps(func)
        def wrapper(*args, **kwargs):
            if current_app.config['TESTING'] and not strict:
                return func(*args, **kwargs)

            if has_app_context() and role_check:
                jwt_data = _decode_jwt_from_headers()
                ctx_stack.top.jwt = jwt_data
                user = User.query.filter_by(email=jwt_data['sub']).first()
                roles = user.roles
                perm = Permission.query.filter_by(name=name).first()
                permission_found = False
                for role in roles:
                    if perm in role.permissions:
                        permission_found = True
                        break

                if not permission_found:
                    return make_response(jsonify('Unauthorized access'), 401)
            return func(*args, **kwargs)

        return wrapper

    return inner

The decorator is used as follows:

class User(Resource):

    @permission('view-user-details')
    def get(self, user_id):
        """Get a single user"""
       ....

I'm willing to work on it and create a pull request.

michaelbukachi avatar Nov 12 '18 06:11 michaelbukachi

I've started working on this feature.

michaelbukachi avatar Apr 28 '19 17:04 michaelbukachi

Looks like there is an example of something similar in the Flask-Principle docs: https://pythonhosted.org/Flask-Principal/#granular-resource-protection

what is different about your approach?

jwag956 avatar Apr 30 '19 23:04 jwag956

Hello @jwag956 I've just gone through the example and the first question that came to mind was, "What if I need to change a permission at runtime? Do I need to extend the Permission class?". The approach I'm proposing will user a PermissionMixin so that database models can be used. This will allow assigning and de-assigning of permissions (maybe using an endpoint) dynamically. Also, each endpoint would be uniquely identified by a permission (perhaps using a decorator). That is, each endpoint would be considered atomic (you either have access to it or you don't). The example you have pointed out seems to have tight coupling to model objects which I think shouldn't be the case. This might be quite opinionated, but I think database object permissions should not be done in code, but rather in the database itself using the built in Access Control.

michaelbukachi avatar May 01 '19 09:05 michaelbukachi

Opinionated is good! Couple thoughts. I don't see the point of 'in the database itself using the built in access control' - DB objects don't really have anything to do with endpoints (endpoints might well access many DB objects). I think your main point is 'in code versus in DB'. In a standard permissioning model (such as bitbucket, jenkins, etc) permissions represent actual actions (Can client delete all posts; Can client add new subject). In an RBAC model, you then assign (dynamically, possibly) permissions to roles, and assign roles to principles. Many people feel RBAC isn't quite powerful enough and, like jenkins, want to apply permissions directly to principles. I think either is fine as long as the # of permissions is manageable (10's).

So the real question is - should permissions be a DB model, or defined in code. My perspective is that by definition they are defined in code - since unless you tag an endpoint or perform some 'if' stmt inside the code that references those permissions names, nothing happens. (BTW I can make the same case about how the default use of Roles is today - there really is no reason for them to be in the DB). It can be valuable to have a definitive list of permissions - but even that for example doesn't happen when using things like Spring-Security.

when you add permissions (which I think is a great addition), then the list of permissions would be stored in the Role object (if desired), and so it would be important to have a real DB backing for that. And you could change permissions->role assignments dynamically. But I don't think it makes sense to change permissions dynamically since besides changing the name - it is code that actually enforces the permission.

If all you want is fine-grained permissions but still use Roles - then I think Flask-Principle works fine - using ItemNeed and a new decorator. If you want to decorate based on permissions, and not roles I think the same applies - use ItemNeed and a decorator to call it.

Thanks for reading this far!

jwag956 avatar May 01 '19 15:05 jwag956

Oops. I've just realized my explanation was a bit off. You are right when you point out that permissions do not change. A permission of add-user is still a permission of add-user with or without a database since its logic is tied down it's actual implementation. In fact the use a DB to keep track of permissions is quite unnecessary. What I'm proposing is simple way of bridging the gap between what one would consider a Permission and a Role. Flask-Principal is great! In fact, I'm considering it for my future projects. However, it does have a learning curve to it. Probably due to it's use of jargon. Also defining a Need, as they put it does work for small projects but large projects with 100-1000 endpoints can make it quite tedious to use. One could play around with their code and probably introduce a bit of metaprogramming to reduce the amount of code. But this would still require extensive knowledge. Having a kind of bridge, or a decorator in this case, could make work easier. For instance:

class UserResource(MethodView):
    @permission('add-user')
    def post():
        ....

then assigning the add-user permission to a role would give that role access to the endpoint. This approach would also offer other benefits such as maybe listing all permissions of a system or removing/adding permissions to roles using another endpoint. The implementation of the decorator could be made pluggable to handle different implementations.

michaelbukachi avatar May 01 '19 16:05 michaelbukachi

Ok - I think we are converging! So you are proposing adding into the Role model, a list of permissions (which are just strings really). That seems reasonable. Doing some sleuthing through the code - I think integrating with existing Flask-Principle/Identity should be fairly easy. First a key thing is that on request, and identity is created (by flask) and the 'identity-loaded' signal is sent - flask-security subscribes to this in core.py::_on_identity_loaded. You can see that this is where id, and role names are added to the identity. This could be a perfect place to add 'endpoint_permissions' to the Identity using ItemNeed(). I.e this is where you grab all the endpoint-permissions you stuck in the role and create new ItemNeed(). The reason to do this I believe is that this will make it possible to have endpoint-permissions be completely independent of roles in the future.

Your new decorator can now look pretty much like @roles_required except is uses ItemNeed rather than RoleNeed - Flask-Principle should handle the rest.

Does that make sense? (I am doing this via code inspection and some breakpoints - so I might have missed something!)

jwag956 avatar May 02 '19 00:05 jwag956

Yes I understand. I'm even trying out your implementation over the weekend :) However, wouldn't it be better if the decorator was completely independent of the implementation (Flask-Principal and what not). Yes, we could provide a default implementation which just works out of the box. However, I think users of the decorator should have an easy way of overriding it's default behaviour without having to delve into code inspection.

michaelbukachi avatar May 03 '19 12:05 michaelbukachi

Thanks - of course all this is my personal view! If you are adding a decorator to be part of Flask-Security then in general, I would follow the pattern/design/architecture (whatever word you like) that the current system uses. Of course if what you are trying to do requires a larger refactoring ... that's a different story.

Today, Flask-Security uses and is tied to Flask-Principle. The Identity model is really quite flexible - I am really impressed with Flask-Principle - it is 400 lines of python I am not certain I could have written. It nicely separates out the 2 pieces - collecting 'rights/capabilities' for the requesting identity, and separately, comparing that information at the enforcement point (the decorator).

Note that to make this quite general, as part of _on_Identity_loaded() you could simple call into the UserModel where it would be easy for anyone to decide where to load the permissions from - for your use case it would be from the roles model - but someone else could decide not to use roles at all and have a FineGrainedAccessModel and store them there.

Also - while a decorator is great and required - it is also important that there is a non-decorator method to ask the same question - there will always be cases where you need information about the resource prior to being able to resolve permissions. This is similar to today where there is a @roles_required decorator but also a UserModel.has_roles()

jwag956 avatar May 03 '19 17:05 jwag956

I see. Thanks for the insight! Back to the drawing board for me. I have to modify the pattern I was thinking about for an even better one which will make integration much easier (decorators & non-decorators)

michaelbukachi avatar May 04 '19 20:05 michaelbukachi

@michaelbukachi In my fork I went ahead and implemented this. Currently in PR form - but if you have a chance - would be great is you think this would satisfy what you are looking for... https://github.com/jwag956/flask-security/pull/120

jwag956 avatar Jul 02 '19 03:07 jwag956

@jwag956 Awesome! I hope it's merged quickly so that I can start using it.

michaelbukachi avatar Jul 02 '19 08:07 michaelbukachi

@michaelbukachi any chance you could pull that branch and give it a try? other wise I will probably get it into master in a few days - need to do some more documentation etc.

jwag956 avatar Jul 02 '19 14:07 jwag956

@jwag956 I'll try it tonight.

michaelbukachi avatar Jul 02 '19 16:07 michaelbukachi

why isnt this solution merged yet?

arsalanpdtuae avatar Jul 06 '22 10:07 arsalanpdtuae