Fix policy check for distil

To keep consistent with other projects and enable 'admin_or_owner' rule
in policy.json, this patch adds neccessary fields for policy check
target and use correct method to get context dictionary.

To be clear, the rule in policy.json 'project_id:%(project_id)s' comes
from Nova originally, the policy module will get project_id from
request URL to Nova service andn compare with project_id in the user
credential. For other projects which don't contain project_id in URL,
just get it from user credential to make the rule work properly.

Change-Id: I5979f2c5204e373cc4a84b1f6997845aabc891cb

distil/api/acl.py

@@ -44,10 +44,16 @@ def enforce(rule):
             ctx = context.ctx()
             ctx.is_admin = check_is_admin(ctx)
 
-            ENFORCER.enforce(rule, {}, ctx.to_dict(), do_raise=True,
-                             exc=exceptions.Forbidden)
+            target = {
+                'project_id': ctx.project_id,
+                'user_id': ctx.user_id,
+            }
+
+            ENFORCER.enforce(rule, target, ctx.to_policy_values(),
+                             do_raise=True, exc=exceptions.Forbidden)
 
             return func(*args, **kwargs)
+
         return handler
 
     return decorator

distil/tests/unit/api/test_api.py

@@ -31,6 +31,9 @@ class TestAPI(base.APITest):
         acl.setup_policy()
 
     def _setup_policy(self, policy):
+        policy.update(
+            {"admin_or_owner": "is_admin:True or project_id:%(project_id)s"}
+        )
         rules = cpolicy.Rules.from_dict(policy)
         acl.ENFORCER.set_rules(rules, use_conf=False)
 
@@ -113,7 +116,7 @@ class TestAPI(base.APITest):
                 end=end
             )
 
-        self._setup_policy({"rating:measurements:get": ""})
+        self._setup_policy({"rating:measurements:get": "rule:admin_or_owner"})
         ret = self.client.get(url, headers={'X-Tenant-Id': default_project})
 
         self.assertEqual(
@@ -163,7 +166,7 @@ class TestAPI(base.APITest):
                 end=end
             )
 
-        self._setup_policy({"rating:invoices:get": ""})
+        self._setup_policy({"rating:invoices:get": "rule:admin_or_owner"})
         ret = self.client.get(url, headers={'X-Tenant-Id': default_project})
 
         self.assertEqual(
@@ -177,6 +180,24 @@ class TestAPI(base.APITest):
             json.loads(ret.data)
         )
 
+    def test_get_other_project_invoice_not_admin(self):
+        default_project = 'tenant_1'
+        start = '2014-06-01T00:00:00'
+        end = '2014-07-01T00:00:00'
+
+        with self.app.test_request_context():
+            url = url_for(
+                'v2.invoices_get',
+                project_id='other_tenant',
+                start=start,
+                end=end
+            )
+
+        self._setup_policy({"rating:invoices:get": "rule:admin_or_owner"})
+        ret = self.client.get(url, headers={'X-Tenant-Id': default_project})
+
+        self.assertEqual(403, json.loads(ret.data).get('error_code'))
+
     @mock.patch('distil.erp.drivers.odoo.OdooDriver.get_quotations')
     @mock.patch('odoorpc.ODOO')
     def test_quotations_get(self, mock_odoo, mock_get_quotations):
@@ -211,7 +232,7 @@ class TestAPI(base.APITest):
                 project_id=default_project,
             )
 
-        self._setup_policy({"rating:quotations:get": ""})
+        self._setup_policy({"rating:quotations:get": "rule:admin_or_owner"})
         ret = self.client.get(url, headers={'X-Tenant-Id': default_project})
 
         self.assertEqual(

etc/policy.json.sample

@@ -3,9 +3,9 @@
     "admin_or_owner": "is_admin:True or project_id:%(project_id)s",
     "default": "rule:admin_or_owner",
 
-    "rating:credits:get": "",
-    "rating:measurements:get": "",
-    "rating:invoices:get": "",
-    "rating:quotations:get": "",
+    "rating:credits:get": "rule:admin_or_owner",
+    "rating:measurements:get": "rule:admin_or_owner",
+    "rating:invoices:get": "rule:admin_or_owner",
+    "rating:quotations:get": "rule:admin_or_owner",
     "health:get": "rule:context_is_admin",
 }