Fix owner policy enforcement

Previously, target was always None, and so ownership check essentially
compared the current context against itself. Under this implementation,
if the target is None, we look up the relevant object based on kwargs,
and then use this object as the target.

At the moment "lease" is the only type of object that has ownership,
and so it is the only case.

Co-Authored-By: Mark Powers <markpowers@uchicago.edu>
Closes-Bug: #2120655
Change-Id: I4bb85d650f4d7f3534206f1c4690cef8eca0a4c2
Signed-off-by: Matt Crees <mattc@stackhpc.com>

blazar/policy.py

@@ -22,6 +22,7 @@ from oslo_log import log as logging
 from oslo_policy import policy
 
 from blazar import context
+from blazar.db import api as db_api
 from blazar import exceptions
 from blazar import policies
 
@@ -101,8 +102,22 @@ def authorize(extension, action=None, api='blazar', ctx=None,
         @functools.wraps(func)
         def wrapped(self, *args, **kwargs):
             cur_ctx = ctx or context.current()
-            tgt = target or {'project_id': cur_ctx.project_id,
-                             'user_id': cur_ctx.user_id}
+            tgt = target
+            if tgt is None:
+                obj = None
+                if kwargs.get("lease_id"):
+                    obj = db_api.lease_get(kwargs.get("lease_id"))
+                if obj:
+                    tgt = {
+                        'project_id': obj.get("project_id"),
+                        'user_id': obj.get("user_id"),
+                    }
+                else:
+                    tgt = {
+                        'project_id': cur_ctx.project_id,
+                        'user_id': cur_ctx.user_id,
+                    }
+
             if action is None:
                 act = '%s:%s' % (api, extension)
             else:

blazar/tests/test_policy.py

@@ -15,6 +15,8 @@
 
 """Test of Policy Engine For Blazar."""
 
+import unittest.mock as mock
+
 from oslo_config import cfg
 
 from blazar import context
@@ -65,3 +67,24 @@ class BlazarPolicyTestCase(tests.TestCase):
         self.assertTrue(user_method_with_action(self))
         self.assertRaises(exceptions.PolicyNotAuthorized,
                           adminonly_method_with_action, self)
+
+        @mock.patch(
+            'blazar.db.api.lease_get',
+            mock.MagicMock(return_value={'id': '1', 'user_id': 'alt_fake',
+                                         'project_id': 'alt_fake'}))
+        @policy.authorize('leases', 'get', ctx=self.context)
+        def alt_user_get_lease(self, **kwargs):
+            return True
+
+        self.assertRaises(exceptions.PolicyNotAuthorized, alt_user_get_lease,
+                          self, lease_id='1')
+
+        @mock.patch(
+            'blazar.db.api.lease_get',
+            mock.MagicMock(return_value={'id': '1', 'user_id': 'fake',
+                                         'project_id': 'fake'}))
+        @policy.authorize('leases', 'get', ctx=self.context)
+        def user_get_lease(self, **kwargs):
+            return True
+
+        self.assertTrue(user_get_lease(self, lease_id='1'))

releasenotes/notes/fix-owner-policy-enforcement-57a6d71c37ffec3d.yaml

@@ -0,0 +1,10 @@
+---
+security:
+  - |
+    Fixes a bug where any user could update/delete a lease from any project,
+    provided that they had the lease ID. When using the default policy
+    configuration, there is no way for a regular user to see lease IDs of
+    another project. However, operators that are running the Resource
+    Availability Calendar may have overridden this policy, and so may be
+    vulnerable without this fix.
+    `LP#2120655 <https://bugs.launchpad.net/blazar/+bug/2120655>`__