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.

asked Feb 1, 2018 at 22:28
\$\endgroup\$
1
  • \$\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\$ Commented Feb 2, 2018 at 9:55

2 Answers 2

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.

answered Feb 2, 2018 at 10:19
\$\endgroup\$
2
  • \$\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\$ Commented Feb 2, 2018 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\$ Commented Feb 3, 2018 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?

community wiki

\$\endgroup\$
4
  • 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\$ Commented Feb 2, 2018 at 22:57
  • \$\begingroup\$ The code should be fixed now, as best as I can tell. \$\endgroup\$ Commented Feb 3, 2018 at 22:27
  • \$\begingroup\$ There's still some indentation that isn't quite right. \$\endgroup\$ Commented Feb 3, 2018 at 22:29
  • \$\begingroup\$ Apparently your lightning response skills are faster than my edit button. The copy-paste error is gone. \$\endgroup\$ Commented Feb 3, 2018 at 22:45

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.