6
\$\begingroup\$

After writing far too many if has_permission(user): statements at the head of my methods, I figured I would try my hand at writing a generic enough decorator to do it for me. This is what I created:

from inspect import signature

def permissions(callback, **perm_kwargs):
    '''This class decorator is used to define permission levels for individual
    methods of the decorated class.

    Attempting to call a method named by this decorator will first invoke the
    given callback. If the callback returns True, the user is authorized and
    the method is called normally. If it returns False, a permission error is
    raised instead and the method is not called.'''

    def wrap(wrapped_class):
        class PermissionsWrapper:
            def __init__(self, *args, **kwargs):
                self.wrapped = wrapped_class(*args, **kwargs)
                self.permission_levels = {}

                for name, level in perm_kwargs.items():
                    self.wrap_method(name, level)

            def __getattr__(self, attr_name):
                attr = getattr(self.wrapped, attr_name)
                if attr_name not in self.permission_levels:
                    return attr

                def wrapper_func(*args, **kwargs):
                    user = PermissionsWrapper.unpack_user(attr, *args, **kwargs)

                    if callback(user, self.permission_levels[attr_name]):
                        attr(*args, **kwargs)
                    else:
                        raise RuntimeError('Permission Denied.')
                return wrapper_func

            @staticmethod
            def unpack_user(method, *args, **kwargs):
                try:
                    # Verify that the arguments are lexically valid.
                    bindings = signature(method).bind(*args, **kwargs)
                    return bindings.arguments['user']
                except TypeError:
                    # The arguments are invalid. Call the method with the bad
                    # arguments so that a more descriptive exception is raised.
                    method(*args, **kwargs)

            def wrap_method(self, method_name, perm_level):
                if not hasattr(wrapped_class, method_name):
                    error = 'No such method: {}'.format(method_name)
                    raise RuntimeError(error)

                method = getattr(wrapped_class, method_name)
                if not hasattr(method, '__call__'):
                    error = 'Attribute {} is not a method!'.format(method_name)
                    raise RuntimeError(error)

                method_sig = signature(method)
                if 'user' not in method_sig.parameters:
                    error = ('Method signature does not have a "user" argument!'
                         ' The user argument will have its permission level'
                         ' verified and either be allowed to use the method'
                         ', or a permission error will be thrown.')
                    raise RuntimeError(error)

                self.permission_levels[method_name] = perm_level

        return PermissionsWrapper
    return wrap

You could then use the decorator like this:

def has_permission(user, required_level):
    if the_user_meets_the_required_level_of_permissions():
        return True
    else:
        return False

@permissions(callback=has_permission,
             destroy_the_company='ADMIN_ONLY', view_profile='GUEST',
             edit_profile='LOGGED_IN', post_comment='GUEST')
class ProfileManager:
    def edit_profile(self, user, form_data):
        pass

    def view_profile(self, user):
        pass

    def post_comment(self, user, comment_text):
        pass

    def destroy_the_company(self, user):
        drop_all_database_tables()
        overwrite_backups()

At the time of writing, it seemed obvious to me that using a decorator on the whole class would be more self-documenting and require less typing than using a separate decorator on each method. Now, I'm not so sure.

  • Does this offer any advantages/disadvantages over a simpler per-method decorator?
  • Are there any edge cases where this decorator will fail?

EDIT: Fixed the decorator by adding the unpack_user method and calling it in wrapper_func.

\$\endgroup\$
  • \$\begingroup\$ David Beazley had an interesting talk about the different options to tackle this. Especially slides 52 to 60 go about decorating whole classes, and using meta-programming to decorate a whole inheritance tree \$\endgroup\$ – Maarten Fabré Feb 2 '18 at 9:55
2
\$\begingroup\$

For a class more complex than this, I would think that per-method decorators are more readable and provide better documentation:

class ProfileManager:

    @permissions("LOGGED_IN")
    def edit_profile(self, user, form_data):
        pass

    @permissions("GUEST")
    def view_profile(self, user):
        pass

    @permissions("GUEST")
    def post_comment(self, user, comment_text):
        pass


    @permissions("ADMIN")
    def destroy_the_company(self, user):
        drop_all_database_tables()
        overwrite_backups()

Implementation of that new permissions decorator could be as simple as:

from functools import wraps
import logging

class AuthenticationErrror(RuntimeError):
    pass

def permission(permission_level):
    def decorator(f):
        @wraps(f)
        def wrapper(self, user, *args, **kwargs):
            if user.has_permission(permission_level):
                return f(self, user, *args, **kwargs)
            else:
                logging.critical("User %s tried accessing function %s without permission (needed: %s)", self.user.name, f.__name__, permission_level)
                raise AuthenticationError("403: Not authorized")
        return wrapper
    return decorator

(untested)


A class decorator would make sense IMO only for setting a default permission level for all methods.

\$\endgroup\$
  • \$\begingroup\$ That's a good point. The vast majority of my class methods use a single basic permission level (similar to "LOGGED_IN"), which would make sense as a default. Pros: no method could accidentally be left 'permission-less'. Cons: seems even less self-documenting and might surprise another developer. \$\endgroup\$ – Christopher Sheaf Feb 2 '18 at 23:07
  • \$\begingroup\$ @ChristopherSheaf Yes, the most explicit and self-documenting way is what I recommended in the beginning, explicitly decorating every method. It is not even more writing than your decorator (you need to explicitly add the name of all methods to the decorator call, this is actually quite error prone, since it is easy to miss or misspell a method). \$\endgroup\$ – Graipher Feb 3 '18 at 6:59
4
\$\begingroup\$

The mechanism for getting at the user argument does not work:

@permissions(callback=print, test='ADMIN')
class Test:
    def test(self, user):
        pass

>>> Test().test('user')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cr186560.py", line 31, in wrapper_func
    if callback(user, self.permission_levels[attr_name]):
NameError: name 'user' is not defined

Have you tested this code?

\$\endgroup\$
  • 1
    \$\begingroup\$ Apparently I had a user variable in my test file that I didn't realize was in the global scope. Looks like I need to find an elegant way to find whether user is in *args or **kwargs and get the value. A second pair of eyes always seems to find the embarrassing bugs. \$\endgroup\$ – Christopher Sheaf Feb 2 '18 at 22:57
  • \$\begingroup\$ The code should be fixed now, as best as I can tell. \$\endgroup\$ – Christopher Sheaf Feb 3 '18 at 22:27
  • \$\begingroup\$ There's still some indentation that isn't quite right. \$\endgroup\$ – Gareth Rees Feb 3 '18 at 22:29
  • \$\begingroup\$ Apparently your lightning response skills are faster than my edit button. The copy-paste error is gone. \$\endgroup\$ – Christopher Sheaf Feb 3 '18 at 22:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.