Merge pull request #630 from danielballan/acp-fix

Fix Access Policy + Catalog

Author: dylanmcreynolds

tiled/_tests/test_catalog.py

@@ -21,9 +21,10 @@
 from ..client import Context, from_context
 from ..client.xarray import write_xarray_dataset
 from ..queries import Eq, Key
-from ..server.app import build_app
+from ..server.app import build_app, build_app_from_config
 from ..server.schemas import Asset, DataSource
 from ..structures.core import StructureFamily
+from .utils import enter_password
 
 
 @pytest_asyncio.fixture
@@ -342,3 +343,71 @@ async def test_delete_tree(tmpdir):
         assert len(data_sources_after_delete) == 0
         assets_after_delete = (await tree.context.execute("SELECT * from assets")).all()
         assert len(assets_after_delete) == 0
+
+
+@pytest.mark.asyncio
+async def test_access_control(tmpdir):
+    config = {
+        "authentication": {
+            "allow_anonymous_access": True,
+            "secret_keys": ["SECRET"],
+            "providers": [
+                {
+                    "provider": "toy",
+                    "authenticator": "tiled.authenticators:DictionaryAuthenticator",
+                    "args": {
+                        "users_to_passwords": {
+                            "alice": "secret1",
+                            "bob": "secret2",
+                            "admin": "admin",
+                        }
+                    },
+                }
+            ],
+        },
+        "database": {
+            "uri": "sqlite+aiosqlite://",  # in-memory
+        },
+        "trees": [
+            {
+                "tree": "catalog",
+                "path": "/",
+                "args": {
+                    "uri": f"sqlite+aiosqlite:///{tmpdir}/catalog.db",
+                    "writable_storage": str(tmpdir / "data"),
+                    "init_if_not_exists": True,
+                },
+                "access_control": {
+                    "access_policy": "tiled.access_policies:SimpleAccessPolicy",
+                    "args": {
+                        "provider": "toy",
+                        "access_lists": {
+                            "alice": ["outer_x"],
+                            "bob": ["outer_y"],
+                        },
+                        "admins": ["admin"],
+                        "public": ["outer_z"],
+                    },
+                },
+            },
+        ],
+    }
+
+    app = build_app_from_config(config)
+    with Context.from_app(app) as context:
+        with enter_password("admin"):
+            admin_client = from_context(context, username="admin")
+            for key in ["outer_x", "outer_y", "outer_z"]:
+                container = admin_client.create_container(key)
+                container.write_array([1, 2, 3], key="inner")
+            admin_client.logout()
+        with enter_password("secret1"):
+            alice_client = from_context(context, username="alice")
+            alice_client["outer_x"]["inner"].read()
+            with pytest.raises(KeyError):
+                alice_client["outer_y"]
+            alice_client.logout()
+        public_client = from_context(context)
+        public_client["outer_z"]["inner"].read()
+        with pytest.raises(KeyError):
+            public_client["outer_x"]

tiled/access_policies.py

@@ -64,13 +64,15 @@ def _get_id(self, principal):
     def allowed_scopes(self, node, principal):
         # If this is being called, filter_access has let us get this far.
         if principal is SpecialUsers.public:
-            return PUBLIC_SCOPES
-        if self._get_id(principal) in self.admins:
-            return ALL_SCOPES
+            allowed = PUBLIC_SCOPES
+        elif self._get_id(principal) in self.admins:
+            allowed = ALL_SCOPES
         # The simple policy does not provide for different Principals to
         # have different scopes on different Nodes. If the Principal has access,
         # they have the same hard-coded access everywhere.
-        return self.scopes
+        else:
+            allowed = self.scopes
+        return allowed
 
     def filters(self, node, principal, scopes):
         queries = []

tiled/catalog/adapter.py

@@ -294,7 +294,7 @@ def writable(self):
         return bool(self.context.writable_storage)
 
     def __repr__(self):
-        return f"<{type(self).__name__} {self.segments}>"
+        return f"<{type(self).__name__} /{'/'.join(self.segments)}>"
 
     async def __aiter__(self):
         statement = select(orm.Node.key).filter(orm.Node.ancestors == self.segments)
@@ -335,6 +335,18 @@ async def lookup_adapter(
         if not segments:
             return self
         *ancestors, key = segments
+        if self.conditions and len(segments) > 1:
+            # There are some conditions (i.e. WHERE clauses) applied to
+            # this node, either via user search queries or via access
+            # control policy queries. Look up first the _direct_ child of this
+            # node, if it exists within the filtered results.
+            first_level = await self.lookup_adapter(segments[:1])
+            if first_level is None:
+                return None
+            # Now proceed to traverse further down the tree, if needed.
+            # Search queries and access controls apply only at the top level.
+            assert not first_level.conditions
+            return await first_level.lookup_adapter(segments[1:])
         statement = select(orm.Node).filter(
             orm.Node.ancestors == self.segments + ancestors
         )