Separated role-checking from enforcing those roles.

This allows other functions to use the same role-checking behaviour, and
separates policy from mechanism.

pillar/application/utils/authorization.py

@@ -255,24 +255,15 @@ def require_login(require_roles=set(),
     def decorator(func):
         @functools.wraps(func)
         def wrapper(*args, **kwargs):
-            current_user = g.get('current_user')
+            if not user_matches_roles(require_roles, require_all):
-
+                if g.current_user is None:
-            if current_user is None:
                     # We don't need to log at a higher level, as this is very common.
                     # Many browsers first try to see whether authentication is needed
                     # at all, before sending the password.
                     log.debug('Unauthenticated acces to %s attempted.', func)
-                abort(403)
+                else:
-
+                    log.warning('User %s is authenticated, but does not have required roles %s to '
-            intersection = require_roles.intersection(set(current_user['roles']))
+                                'access %s', g.current_user['user_id'], require_roles, func)
-            if require_all:
-                if intersection != require_roles:
-                    log.warning('User %s does not have ALL required roles %s to access %s',
-                                current_user['user_id'], require_roles, func)
-                    abort(403)
-            elif require_roles and not intersection:
-                log.warning('User %s is authenticated, but does not have any required role %s to '
-                            'access %s', current_user['user_id'], require_roles, func)
                 abort(403)
 
             return func(*args, **kwargs)
@@ -294,6 +285,36 @@ def user_has_role(role, user=None):
     return role in user['roles']
 
 
+def user_matches_roles(require_roles=set(),
+                       require_all=False):
+    """Returns True iff the user's roles matches the query.
+
+    :param require_roles: set of roles.
+    :param require_all:
+        When False (the default): if the user's roles have a
+        non-empty intersection with the given roles, returns True.
+        When True: require the user to have all given roles before
+        returning True.
+    """
+
+    if not isinstance(require_roles, set):
+        raise TypeError('require_roles param should be a set, but is a %r' % type(require_roles))
+
+    if require_all and not require_roles:
+        raise ValueError('require_login(require_all=True) cannot be used with empty require_roles.')
+
+    current_user = g.get('current_user')
+
+    if current_user is None:
+        return False
+
+    intersection = require_roles.intersection(current_user['roles'])
+    if require_all:
+        return len(intersection) == len(require_roles)
+
+    return not bool(require_roles) or bool(intersection)
+
+
 def is_admin(user):
     """Returns True iff the given user has the admin role."""