Unified user representation for web and API calls

Both approaches now use a pillar.auth.UserClass instance. g.current_user
is now always set to that instance, even for web entry points.

This UserClass instance can still be keyed like the old dict, but this is
for temporary compatibility and shouldn't be relied on in new or touched
code.

pillar/api/projects/hooks.py

@@ -110,6 +110,8 @@ def after_inserting_projects(projects):
 
 
 def after_inserting_project(project, db_user):
+    from pillar.auth import UserClass
+
     project_id = project['_id']
     user_id = db_user['_id']
 
@@ -135,7 +137,8 @@ def after_inserting_project(project, db_user):
     log.debug('Made user %s member of group %s', user_id, admin_group_id)
 
     # Assign the group to the project with admin rights
-    is_admin = authorization.is_admin(db_user)
+    owner_user = UserClass.construct('', db_user)
+    is_admin = authorization.is_admin(owner_user)
     world_permissions = ['GET'] if is_admin else []
     permissions = {
         'world': world_permissions,

pillar/api/utils/authentication.py

@@ -15,13 +15,17 @@ from flask import g
 from flask import request
 from flask import current_app
 
+from pillar.auth import UserClass
+
 log = logging.getLogger(__name__)
 
-CLI_USER = {
+CLI_USER = UserClass.construct('CLI', {
-    'user_id': 'CLI',
+    '_id': 'CLI',
     'groups': [],
     'roles': {'admin'},
-}
+    'email': 'local@nowhere',
+    'username': 'CLI',
+})
 
 
 def force_cli_user():
@@ -74,6 +78,8 @@ def validate_this_token(token, oauth_subclient=None):
     :rtype: dict
     """
 
+    from pillar.auth import UserClass
+
     g.current_user = None
     _delete_expired_tokens()
 
@@ -98,9 +104,7 @@ def validate_this_token(token, oauth_subclient=None):
         log.debug('Validation failed, user not logged in')
         return None
 
-    g.current_user = {'user_id': db_user['_id'],
+    g.current_user = UserClass.construct(token, db_user)
-                      'groups': db_user['groups'],
-                      'roles': set(db_user.get('roles', []))}
 
     return db_user
 

pillar/api/utils/authorization.py

@@ -7,6 +7,8 @@ from flask import abort
 from flask import current_app
 from werkzeug.exceptions import Forbidden
 
+from pillar.auth import UserClass
+
 CHECK_PERMISSIONS_IMPLEMENTED_FOR = {'projects', 'nodes', 'flamenco_jobs'}
 
 log = logging.getLogger(__name__)
@@ -334,17 +336,20 @@ def ab_testing(require_roles=set(),
     return decorator
 
 
-def user_has_role(role, user=None):
+def user_has_role(role, user: UserClass=None):
     """Returns True iff the user is logged in and has the given role."""
 
     if user is None:
         user = g.get('current_user')
+        if user is not None and not isinstance(user, UserClass):
+            raise TypeError(f'g.current_user should be instance of UserClass, not {type(user)}')
+    elif not isinstance(user, UserClass):
+        raise TypeError(f'user should be instance of UserClass, not {type(user)}')
 
     if user is None:
         return False
 
-    roles = user.get('roles') or ()
+    return user.has_role(role)
-    return role in roles
 
 
 def user_matches_roles(require_roles=set(),
@@ -359,22 +364,14 @@ def user_matches_roles(require_roles=set(),
         returning True.
     """
 
-    if not isinstance(require_roles, set):
+    current_user: UserClass = g.get('current_user')
-        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 not isinstance(current_user, UserClass):
-    if require_all:
+        raise TypeError(f'g.current_user should be instance of UserClass, not {type(current_user)}')
-        return len(intersection) == len(require_roles)
 
-    return not bool(require_roles) or bool(intersection)
+    return current_user.matches_roles(require_roles, require_all)
 
 
 def is_admin(user):

pillar/auth/__init__.py

@@ -1,5 +1,6 @@
 """Authentication code common to the web and api modules."""
 
+import collections
 import logging
 import typing
 
@@ -8,6 +9,8 @@ import flask_login
 import flask_oauthlib.client
 from werkzeug.local import LocalProxy
 
+import bson
+
 from ..api import utils
 from ..api.utils import authentication
 
@@ -20,11 +23,53 @@ class UserClass(flask_login.UserMixin):
         self.id = token
         self.username: str = None
         self.full_name: str = None
+        self.user_id: bson.ObjectId = None
         self.objectid: str = None
         self.gravatar: str = None
         self.email: str = None
         self.roles: typing.List[str] = []
-        self.groups: typing.List[str] = []
+        self.groups: typing.List[str] = []  # NOTE: these are stringified object IDs.
+        self.group_ids: typing.List[bson.ObjectId] = []
+
+    @classmethod
+    def construct(cls, token: str, db_user: dict) -> 'UserClass':
+        """Constructs a new UserClass instance from a Mongo user document."""
+
+        user = UserClass(token)
+
+        user.user_id = db_user['_id']
+        user.roles = db_user.get('roles') or []
+        user.group_ids = db_user.get('groups') or []
+        user.email = db_user.get('email') or ''
+        user.username = db_user['username']
+        user.full_name = db_user.get('full_name') or ''
+
+        # Derived properties
+        user.objectid = str(db_user['_id'])
+        user.gravatar = utils.gravatar(user.email)
+        user.groups = [str(g) for g in user.group_ids]
+
+        return user
+
+    def __getitem__(self, item):
+        """Compatibility layer with old dict-based g.current_user object."""
+
+        if item == 'user_id':
+            return self.user_id
+        if item == 'groups':
+            return self.group_ids
+        if item == 'roles':
+            return set(self.roles)
+
+        raise KeyError(f'No such key {item!r}')
+
+    def get(self, key, default=None):
+        """Compatibility layer with old dict-based g.current_user object."""
+
+        try:
+            return self[key]
+        except KeyError:
+            return default
 
     def has_role(self, *roles):
         """Returns True iff the user has one or more of the given roles."""
@@ -34,6 +79,32 @@ class UserClass(flask_login.UserMixin):
 
         return bool(set(self.roles).intersection(set(roles)))
 
+    def matches_roles(self,
+                      require_roles=set(),
+                      require_all=False) -> bool:
+        """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(f'require_roles param should be a set, but is {type(require_roles)!r}')
+
+        if require_all and not require_roles:
+            raise ValueError('require_login(require_all=True) cannot be used with '
+                             'empty require_roles.')
+
+        intersection = require_roles.intersection(self.roles)
+        if require_all:
+            return len(intersection) == len(require_roles)
+
+        return not bool(require_roles) or bool(intersection)
+
 
 class AnonymousUser(flask_login.AnonymousUserMixin, UserClass):
     def __init__(self):
@@ -43,27 +114,20 @@ class AnonymousUser(flask_login.AnonymousUserMixin, UserClass):
         return False
 
 
-def _load_user(token):
+
+def _load_user(token) -> typing.Union[UserClass, AnonymousUser]:
     """Loads a user by their token.
 
     :returns: returns a UserClass instance if logged in, or an AnonymousUser() if not.
-    :rtype: UserClass
     """
 
     db_user = authentication.validate_this_token(token)
     if not db_user:
         return AnonymousUser()
 
-    login_user = UserClass(token)
+    user = UserClass.construct(token, db_user)
-    login_user.email = db_user['email']
-    login_user.objectid = str(db_user['_id'])
-    login_user.username = db_user['username']
-    login_user.gravatar = utils.gravatar(db_user['email'])
-    login_user.roles = db_user.get('roles', [])
-    login_user.groups = [str(g) for g in db_user['groups'] or ()]
-    login_user.full_name = db_user.get('full_name', '')
 
-    return login_user
+    return user
 
 
 def config_login_manager(app):
@@ -83,11 +147,14 @@ def config_login_manager(app):
 def login_user(oauth_token: str, *, load_from_db=False):
     """Log in the user identified by the given token."""
 
+    from flask import g
+
     if load_from_db:
         user = _load_user(oauth_token)
     else:
         user = UserClass(oauth_token)
     flask_login.login_user(user)
+    g.current_user = user
 
 
 def get_blender_id_oauth_token():

pillar/tests/__init__.py

@@ -278,6 +278,38 @@ class AbstractPillarTest(TestMinimal):
 
         return user_id
 
+    def create_user_object(self, user_id=ObjectId(), roles=frozenset(), group_ids=None):
+        """Creates a pillar.auth.UserClass object.
+
+        :rtype: pillar.auth.UserClass
+        """
+
+        from pillar.auth import UserClass
+
+        db_user = copy.deepcopy(ctd.EXAMPLE_USER)
+        db_user['_id'] = user_id
+        db_user['roles'] = list(roles) if roles is not None else None
+
+        if group_ids is not None:
+            db_user['groups'] = list(group_ids)
+
+        return UserClass.construct('', db_user)
+
+    def login_api_as(self, user_id=ObjectId(), roles=frozenset(), group_ids=None):
+        """Creates a pillar.auth.UserClass object and sets it as g.current_user
+
+        Requires an active request context!
+
+        :rtype: pillar.auth.UserClass
+        """
+
+        from flask import g
+
+        user = self.create_user_object(user_id, roles, group_ids)
+        g.current_user = user
+
+        return user
+
     def create_valid_auth_token(self, user_id, token='token'):
         now = datetime.datetime.now(tz_util.utc)
         future = now + datetime.timedelta(days=1)

tests/test_api/test_auth.py

@@ -536,14 +536,12 @@ class RequireRolesTest(AbstractPillarTest):
             called[0] = True
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), roles=['succubus'])
-                              'roles': ['succubus']}
             call_me()
 
         self.assertTrue(called[0])
 
     def test_some_roles_required(self):
-        from flask import g
         from pillar.api.utils.authorization import require_login
 
         called = [False]
@@ -553,19 +551,16 @@ class RequireRolesTest(AbstractPillarTest):
             called[0] = True
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['succubus'])
-                              'roles': ['succubus']}
             self.assertRaises(Forbidden, call_me)
         self.assertFalse(called[0])
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['admin'])
-                              'roles': ['admin']}
             call_me()
         self.assertTrue(called[0])
 
     def test_all_roles_required(self):
-        from flask import g
         from pillar.api.utils.authorization import require_login
 
         called = [False]
@@ -576,39 +571,38 @@ class RequireRolesTest(AbstractPillarTest):
             called[0] = True
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['admin'])
-                              'roles': ['admin']}
             self.assertRaises(Forbidden, call_me)
         self.assertFalse(called[0])
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['service'])
-                              'roles': ['service']}
             self.assertRaises(Forbidden, call_me)
         self.assertFalse(called[0])
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['badger'])
-                              'roles': ['badger']}
             self.assertRaises(Forbidden, call_me)
         self.assertFalse(called[0])
 
         with self.app.test_request_context():
-            g.current_user = {'user_id': ObjectId(24 * 'a'),
+            self.login_api_as(ObjectId(24 * 'a'), ['service', 'badger'])
-                              'roles': ['service', 'badger']}
             call_me()
         self.assertTrue(called[0])
 
     def test_user_has_role(self):
         from pillar.api.utils.authorization import user_has_role
 
+        def make_user(roles):
+            return self.create_user_object(ObjectId(), roles=roles)
+
         with self.app.test_request_context():
-            self.assertTrue(user_has_role('subscriber', {'roles': ['aap', 'noot', 'subscriber']}))
+            self.assertTrue(user_has_role('subscriber', make_user(['aap', 'noot', 'subscriber'])))
-            self.assertTrue(user_has_role('subscriber', {'roles': ['aap', 'subscriber']}))
+            self.assertTrue(user_has_role('subscriber', make_user(['aap', 'subscriber'])))
-            self.assertFalse(user_has_role('admin', {'roles': ['aap', 'noot', 'subscriber']}))
+            self.assertFalse(user_has_role('admin', make_user(['aap', 'noot', 'subscriber'])))
-            self.assertFalse(user_has_role('admin', {'roles': []}))
+            self.assertFalse(user_has_role('admin', make_user([])))
-            self.assertFalse(user_has_role('admin', {'roles': None}))
+            self.assertFalse(user_has_role('admin', make_user(None)))
-            self.assertFalse(user_has_role('admin', {}))
+            self.assertFalse(user_has_role('admin', None))
 
 
 class UserCreationTest(AbstractPillarTest):

tests/test_api/test_blender_id_subclient.py

@@ -71,7 +71,7 @@ class BlenderIdSubclientTest(AbstractPillarTest):
         with self.app.test_request_context(headers={'Authorization': auth_header}):
             self.assertTrue(auth.validate_token())
             self.assertIsNotNone(g.current_user)
-            self.assertEqual(db_user['_id'], g.current_user['user_id'])
+            self.assertEqual(db_user['_id'], g.current_user.user_id)
 
     def _common_user_test(self, expected_status_code, scst=TEST_SUBCLIENT_TOKEN,
                           expected_full_name=TEST_FULL_NAME,

tests/test_api/test_nodes.py

@@ -38,10 +38,9 @@ class NodeContentTypeTest(AbstractPillarTest):
                         'name': 'My first test node'}
 
             with self.app.test_request_context():
-                g.current_user = {'user_id': user_id,
+                self.login_api_as(user_id, roles={'subscriber', 'admin'},
                                   # This group is hardcoded in the EXAMPLE_PROJECT.
-                                  'groups': [ObjectId('5596e975ea893b269af85c0e')],
+                                  group_ids=[ObjectId('5596e975ea893b269af85c0e')])
-                                  'roles': {'subscriber', 'admin'}}
                 nodes = self.app.data.driver.db['nodes']
 
                 # Create the node.