Repository: hasgeek/coaster

Issue:
Data export profiles
`RoleMixin` and `RoleAccessProxy` from #220 were introduced for GraphQL as a replacement for the request-level permission system. They have a secondary use in templates where it is too easy to leak data the user is not supposed to have access to.

Since GraphQL is JSON-based, `RoleAccessProxy` also behaves like a JSON-compatible dictionary. Unfortunately, SQLAlchemy models can have mutual references to each other and while Python dictionaries can handle such recursion, JSON can't. This has blown up with #220 (via #231) now that the proxy offers access to relationships (it previously didn't).

JSON renders also include fields that are unnecessary for the request, causing unnecessary database loads. We need a mechanism to define a limited number of fields.

1. Limited fields in JSON as a _subset_ of what role access allows, and
2. Limited fields in what is loaded from the database.

These two are not the same as the JSON can include properties that are not from the database, and the database will need to load essential columns such as `id` even if they are not included in the JSON.

## Proposal

SQLAlchemy already supports deferred columns and deferred column groups. We can build on that. The JSON part is entirely up to us, so maybe it can work like this:

```python
class MyModel(...):
    ...
    __data_profiles__ = {
        'search': {'col1', 'col2', 'col3'},
        'related': {'col1', 'col2'}
    }

proxy = MyModel.access_for(roles={'all'}, profiles=('search', 'related', ...))
```

The RoleMixin methods will accept a new parameter named `profiles`, which accepts an iterator of profile names and passes it in to the proxy. The proxy in turn limits access to attributes that are present in both the access type (read, write or call) _and_ the first profile. When a relationship is encountered, a new proxy is constructed (as at present since #231) but is given a list starting from the second profile. This can be used to name a truncated profile in the second level model that excludes back-references to the first level.

`profiles` has a a default value of `None` meaning no profiling is done. If passed an empty iterator, `ValueError` is raised. This can help trap accidents where the access tree has greater depth than specified in the list or profiles.

Discussion:
jace: "Dataset" may be a better term for this feature than "data profile".

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index f7bc1d6..5689092 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -42,6 +42,8 @@
 * Added lazy role grant evaluation and declarative role grants
 * New: ``nary_op`` decorator to turn a binary operator into a chained n-ary
   operator
+* Added datasets for limited enumeration in role access, as a stopgap until
+  migration to GraphQL
 
 
 0.6.0
diff --git a/coaster/sqlalchemy/roles.py b/coaster/sqlalchemy/roles.py
index e65480b..519140a 100644
--- a/coaster/sqlalchemy/roles.py
+++ b/coaster/sqlalchemy/roles.py
@@ -293,13 +293,30 @@ class RoleAccessProxy(collections.Mapping):
 
     :param obj: The object that should be wrapped with the proxy
     :param roles: A set of roles to determine what attributes are accessible
+    :param actor: The actor this proxy has been constructed for
+    :param anchors: The anchors this proxy has been constructed with
+    :param datasets: Datasets to limit attribute enumeration to
+
+    The `actor` and `anchors` parameters are not used by the proxy, but are used to
+    construct proxies for objects accessed via relationships.
     """
 
-    def __init__(self, obj, roles, actor, anchors):
+    def __init__(self, obj, roles, actor, anchors, datasets):
         object.__setattr__(self, '_obj', obj)
         object.__setattr__(self, 'current_roles', InspectableSet(roles))
         object.__setattr__(self, '_actor', actor)
         object.__setattr__(self, '_anchors', anchors)
+        if datasets is None:
+            dataset_attrs = None
+            object.__setattr__(self, '_datasets', None)
+        else:
+            if datasets:
+                dataset_attrs = set(obj.__datasets__[datasets[0]])
+            else:
+                # Got an empty list, so turn off enumeration
+                dataset_attrs = set()
+            object.__setattr__(self, '_datasets', datasets[1:])
+        object.__setattr__(self, '_dataset_attrs', dataset_attrs)
 
         # Call, read and write access attributes for the given roles
         call = set()
@@ -326,17 +343,25 @@ class RoleAccessProxy(collections.Mapping):
         # A proper take will require custom dict and list subclasses, similar to the
         # role access proxy itself.
         if isinstance(attr, RoleMixin):
-            return attr.access_for(actor=self._actor, anchors=self._anchors)
+            return attr.access_for(
+                actor=self._actor, anchors=self._anchors, datasets=self._datasets
+            )
         elif isinstance(attr, (InstrumentedDict, MappedCollection)):
             return {
-                k: v.access_for(actor=self._actor, anchors=self._anchors)
+                k: v.access_for(
+                    actor=self._actor, anchors=self._anchors, datasets=self._datasets
+                )
                 for k, v in attr.items()
             }
         elif isinstance(attr, (InstrumentedList, InstrumentedSet, AppenderMixin)):
             # InstrumentedSet is converted into a tuple because the role access proxy
-            # isn't hashable
+            # isn't hashable and can't be placed in a set. This is a side-effect of
+            # subclassing collections.Mapping: dicts are also not hashable.
             return tuple(
-                m.access_for(actor=self._actor, anchors=self._anchors) for m in attr
+                m.access_for(
+                    actor=self._actor, anchors=self._anchors, datasets=self._datasets
+                )
+                for m in attr
             )
         else:
             return attr
@@ -363,7 +388,10 @@ class RoleAccessProxy(collections.Mapping):
             raise KeyError(key)
 
     def __len__(self):
-        return len(self._read)
+        if self._dataset_attrs is not None:
+            return len(self._read & self._dataset_attrs)
+        else:
+            return len(self._read)
 
     def __contains__(self, key):
         return key in self._read or key in self._call
@@ -376,7 +404,11 @@ class RoleAccessProxy(collections.Mapping):
             raise KeyError(key)
 
     def __iter__(self):
-        for key in self._read:
+        if self._dataset_attrs is not None:
+            source = self._read & self._dataset_attrs
+        else:
+            source = self._read
+        for key in source:
             yield key
 
 
@@ -520,6 +552,8 @@ class RoleMixin(object):
 
     # This empty dictionary is necessary for the configure step below to work
     __roles__ = {}
+    # Datasets for limited access to attributes
+    __datasets__ = {}
 
     def roles_for(self, actor=None, anchors=()):
         """
@@ -599,16 +633,43 @@ class RoleMixin(object):
                     actors.add(attr)
         return actors
 
-    def access_for(self, roles=None, actor=None, anchors=()):
+    def access_for(self, roles=None, actor=None, anchors=(), datasets=None):
         """
         Return a proxy object that limits read and write access to attributes
-        based on the actor's roles. If the ``roles`` parameter isn't
-        provided, :meth:`roles_for` is called with the other parameters::
+        based on the actor's roles.
 
-            # This typical call:
-            obj.access_for(actor=current_auth.actor)
-            # Is shorthand for:
-            obj.access_for(roles=obj.roles_for(actor=current_auth.actor))
+        .. warning::
+            If the `roles` parameter is provided, it overrides discovery of the actor's
+            roles in both the current object and related objects. It should only be
+            used when roles are pre-determined and related objects are not required.
+
+        :param set roles: Roles to limit access to (not recommended)
+        :param actor: Limit access to this actor's roles
+        :param anchors: Retrieve additional roles from anchors
+        :param tuple datasets: Limit enumeration to the attributes in the dataset
+
+        If a `datasets` sequence is provided, the first dataset is applied to the
+        current object and subsequent datasets are applied to objects accessed via
+        relationships. Datasets limit the attributes available via enumeration when the
+        proxy is cast into a dict or JSON. This can be used to remove unnecessary data
+        or bi-directional relationships, which JSON can't handle.
+
+        Attributes must be specified in a ``__datasets__`` dictionary on the object::
+
+            __datasets__ = {
+                'primary': {'uuid', 'name', 'title', 'children', 'parent'},
+                'related': {'uuid', 'name', 'title'}
+            }
+
+        Objects and related objects can be safely enumerated like this::
+
+            proxy = obj.access_for(user, datasets=('primary', 'related'))
+            proxydict = dict(proxy)
+            proxyjson = json.dumps(proxy)  # This needs a custom JSON encoder
+
+        If a dataset includes an attribute the role doesn't have access to, it will be
+        skipped. If it includes a relationship for which no dataset is specified, it
+        will be rendered as an empty object.
         """
         if roles is None:
             roles = self.roles_for(actor=actor, anchors=anchors)
@@ -616,14 +677,20 @@ class RoleMixin(object):
             raise TypeError(
                 'If roles are specified, actor/anchors must not be specified'
             )
-        return RoleAccessProxy(self, roles=roles, actor=actor, anchors=anchors)
+        return RoleAccessProxy(
+            self, roles=roles, actor=actor, anchors=anchors, datasets=datasets
+        )
 
-    def current_access(self):
+    def current_access(self, datasets=None):
         """
         Wraps :meth:`access_for` with :obj:`~coaster.auth.current_auth` to
         return a proxy for the currently authenticated user.
+
+        :param tuple datasets: Datasets to limit enumeration to
         """
-        return self.access_for(actor=current_auth.actor, anchors=current_auth.anchors)
+        return self.access_for(
+            actor=current_auth.actor, anchors=current_auth.anchors, datasets=datasets
+        )
 
 
 @event.listens_for(RoleMixin, 'mapper_configured', propagate=True)
diff --git a/coaster/sqlalchemy/statemanager.py b/coaster/sqlalchemy/statemanager.py
index c211910..ee48574 100644
--- a/coaster/sqlalchemy/statemanager.py
+++ b/coaster/sqlalchemy/statemanager.py
@@ -999,7 +999,7 @@ class StateManagerWrapper(object):
         returns currently available transitions for the specified
         roles or actor as a dictionary of name: :class:`StateTransitionWrapper`.
         """
-        proxy = self.obj.access_for(roles, actor, anchors)
+        proxy = self.obj.access_for(roles=roles, actor=actor, anchors=anchors)
         return {
             name: transition
             for name, transition in self.transitions(current=False).items()


Test patch:
diff --git a/tests/test_roles.py b/tests/test_roles.py
index d6f0363..6adaf0e 100644
--- a/tests/test_roles.py
+++ b/tests/test_roles.py
@@ -2,6 +2,7 @@
 
 from __future__ import unicode_literals
 
+import json
 import unittest
 
 from sqlalchemy.ext.declarative import declared_attr
@@ -66,6 +67,11 @@ class RoleModel(DeclaredAttrMixin, RoleMixin, db.Model):
 
     __roles__ = {'all': {'read': {'id', 'name', 'title'}}}
 
+    __datasets__ = {
+        'minimal': {'id', 'name', 'title'},
+        'extra': {'id', 'name', 'title', 'mixed_in1'},
+    }
+
     # Approach two, annotate roles on the attributes.
     # These annotations always add to anything specified in __roles__
 
@@ -84,8 +90,11 @@ class RoleModel(DeclaredAttrMixin, RoleMixin, db.Model):
     def hello(self):
         return "Hello!"
 
-    # Your model is responsible for granting roles given an actor or anchors
-    # (an iterable). The format for anchors is not specified by RoleMixin.
+    # RoleMixin provides a `roles_for` that automatically grants roles from
+    # `granted_by` declarations. See the RoleGrant models below for examples.
+    # This is optional however, and your model can take independent responsibility
+    # for granting roles given an actor and anchors (an iterable). The format for
+    # anchors is not specified by RoleMixin.
 
     def roles_for(self, actor=None, anchors=()):
         # Calling super gives us a set with the standard roles
@@ -121,6 +130,10 @@ class RelationshipChild(BaseNameMixin, db.Model):
     parent_id = db.Column(None, db.ForeignKey('relationship_parent.id'), nullable=False)
 
     __roles__ = {'all': {'read': {'name', 'title', 'parent'}}}
+    __datasets__ = {
+        'primary': {'name', 'title', 'parent'},
+        'related': {'name', 'title'},
+    }
 
 
 class RelationshipParent(BaseNameMixin, db.Model):
@@ -148,6 +161,17 @@ class RelationshipParent(BaseNameMixin, db.Model):
             }
         }
     }
+    __datasets__ = {
+        'primary': {
+            'name',
+            'title',
+            'children_list',
+            'children_set',
+            'children_dict_attr',
+            'children_dict_column',
+        },
+        'related': {'name', 'title'},
+    }
 
 
 granted_users = db.Table(
@@ -194,6 +218,16 @@ class RoleGrantOne(BaseMixin, db.Model):
     user = with_roles(db.relationship(RoleUser), grants={'creator'})
 
 
+# --- Utilities ---------------------------------------------------------------
+
+
+class JsonEncoder(json.JSONEncoder):
+    def default(self, o):
+        if isinstance(o, RoleAccessProxy):
+            return dict(o)
+        return super(JsonEncoder, self).default(o)
+
+
 # --- Tests -------------------------------------------------------------------
 
 
@@ -361,6 +395,29 @@ class TestCoasterRoles(unittest.TestCase):
             },
         )
 
+    def test_diff_roles_single_model_dataset(self):
+        """Data profiles constrain the attributes available via enumeration"""
+        rm = RoleModel(name='test', title='Test')
+        proxy1a = rm.access_for(roles={'all'}, datasets=('minimal',))
+        proxy2a = rm.access_for(roles={'owner'}, datasets=('minimal',))
+        proxy3a = rm.access_for(roles={'all', 'owner'}, datasets=('minimal',))
+        assert set(proxy1a) == {'id', 'name', 'title'}
+        assert len(proxy1a) == 3
+        assert set(proxy2a) == {'name'}
+        assert len(proxy2a) == 1
+        assert set(proxy3a) == {'id', 'name', 'title'}
+        assert len(proxy3a) == 3
+
+        proxy1b = rm.access_for(roles={'all'}, datasets=('extra',))
+        proxy2b = rm.access_for(roles={'owner'}, datasets=('extra',))
+        proxy3b = rm.access_for(roles={'all', 'owner'}, datasets=('extra',))
+        assert set(proxy1b) == {'id', 'name', 'title'}
+        assert len(proxy1b) == 3
+        assert set(proxy2b) == {'name', 'mixed_in1'}
+        assert len(proxy2b) == 2
+        assert set(proxy3b) == {'id', 'name', 'title', 'mixed_in1'}
+        assert len(proxy3b) == 4
+
     def test_write_without_read(self):
         """A proxy may allow writes without allowing reads"""
         rm = RoleModel(name='test', title='Test')
@@ -449,6 +506,11 @@ class TestCoasterRoles(unittest.TestCase):
         self.session.add_all([parent, child])
         self.session.commit()
 
+        # These tests use ``access_for(roles={'all'})`` even though the `roles`
+        # parameter is not suitable for accessing relationships, as roles are specific
+        # to an object. This is okay here only because the 'all' role is automatically
+        # granted on all objects. Production use should be with `actor` and `anchors`.
+
         proxy = parent.access_for(roles={'all'})
         assert proxy.title == parent.title
 
@@ -468,6 +530,65 @@ class TestCoasterRoles(unittest.TestCase):
         assert isinstance(proxy.children_dict_column['child'], RoleAccessProxy)
         assert proxy.children_dict_column['child'].title == child.title
 
+    def test_cascading_datasets(self):
+        """Test data profile cascades"""
+        parent = RelationshipParent(title="Parent")
+        child = RelationshipChild(title="Child", parent=parent)
+        self.session.add_all([parent, child])
+        self.session.commit()
+
+        # These tests use ``access_for(roles={'all'})`` even though the `roles`
+        # parameter is not suitable for accessing relationships, as roles are specific
+        # to an object. This is okay here only because the 'all' role is automatically
+        # granted on all objects. Production use should be with `actor` and `anchors`.
+
+        pchild = child.access_for(roles={'all'}, datasets=('primary',))
+        assert set(pchild) == {'name', 'title', 'parent'}
+
+        # pchild's 'primary' profile includes 'parent', but we haven't specified a
+        # profile for the parent so it will be empty
+        assert pchild.parent == {}
+
+        pchild = child.access_for(roles={'all'}, datasets=('primary', 'primary'))
+        pparent = pchild.parent
+        assert set(pparent) == {
+            'name',
+            'title',
+            'children_list',
+            'children_set',
+            'children_dict_attr',
+            'children_dict_column',
+        }
+        # Same blank object when we recursively access the child
+        assert pparent.children_list[0] == {}
+
+        # Using a well crafted set of profiles will result in a clean containment
+        pchild = child.access_for(roles={'all'}, datasets=('primary', 'related'))
+        assert json.loads(json.dumps(pchild, cls=JsonEncoder)) == {
+            'name': "child",
+            'title': "Child",
+            'parent': {'name': "parent", 'title': "Parent"},
+        }
+
+        pparent = parent.access_for(roles={'all'}, datasets=('primary', 'related'))
+        assert json.loads(json.dumps(pparent, cls=JsonEncoder)) == {
+            'name': "parent",
+            'title': "Parent",
+            'children_list': [{'name': "child", 'title': "Child"}],
+            'children_set': [{'name': "child", 'title': "Child"}],
+            'children_dict_attr': {'child': {'name': "child", 'title': "Child"}},
+            'children_dict_column': {'child': {'name': "child", 'title': "Child"}},
+        }
+
+        # Data profiles only affect enumeration
+        # Actual availability is determined by role access
+
+        pchild = child.access_for(roles={'all'}, datasets=('related', 'related'))
+        assert 'parent' not in set(pchild)  # Enumerate and test for containment
+        assert 'parent' in pchild  # Test for containment directly
+        assert pchild['parent'] is not None
+        assert pchild.parent is not None
+
     def test_role_grant(self):
         m1 = RoleGrantMany()
         m2 = RoleGrantMany()
diff --git a/tests/test_statemanager.py b/tests/test_statemanager.py
index 89bc75d..5da2d15 100644
--- a/tests/test_statemanager.py
+++ b/tests/test_statemanager.py
@@ -690,8 +690,8 @@ class TestStateManager(unittest.TestCase):
         """with_roles works on the transition decorator"""
         self.assertTrue(self.post.state.DRAFT)
         # Create access proxies for each of these roles
-        author = self.post.access_for({'author'})
-        reviewer = self.post.access_for({'reviewer'})
+        author = self.post.access_for(roles={'author'})
+        reviewer = self.post.access_for(roles={'reviewer'})
 
         # Transitions are listed in the proxy even if not callable
         self.assertIn('submit', author)