Merge "Move unlock_override policy enforcement into V2.1 REST API layer"

etc/nova/policy.json

@@ -305,6 +305,7 @@
     "os_compute_api:os-lock-server:discoverable": "",
     "os_compute_api:os-lock-server:lock": "rule:admin_or_owner",
     "os_compute_api:os-lock-server:unlock": "rule:admin_or_owner",
+    "os_compute_api:os-lock-server:unlock:unlock_override": "rule:admin_api",
     "os_compute_api:os-migrate-server:discoverable": "",
     "os_compute_api:os-migrate-server:migrate": "rule:admin_api",
     "os_compute_api:os-migrate-server:migrate_live": "rule:admin_api",

nova/api/openstack/compute/plugins/v3/lock_server.py

@@ -46,6 +46,10 @@ class LockServerController(wsgi.Controller):
         context = req.environ['nova.context']
         authorize(context, action='unlock')
         instance = common.get_instance(self.compute_api, context, id)
+        if not self.compute_api.is_expected_locked_by(context, instance):
+            authorize(context, target=instance,
+                      action='unlock:unlock_override')
+
         self.compute_api.unlock(context, instance)
 
 

nova/compute/api.py

@@ -2948,15 +2948,21 @@ class API(base.Base):
         instance.locked_by = 'owner' if is_owner else 'admin'
         instance.save()
 
+    def is_expected_locked_by(self, context, instance):
+        is_owner = instance.project_id == context.project_id
+        expect_locked_by = 'owner' if is_owner else 'admin'
+        locked_by = instance.locked_by
+        if locked_by and locked_by != expect_locked_by:
+            return False
+        return True
+
     @wrap_check_policy
     def unlock(self, context, instance):
         """Unlock the given instance."""
         # If the instance was locked by someone else, check
         # that we're allowed to override the lock
-        is_owner = instance.project_id == context.project_id
-        expect_locked_by = 'owner' if is_owner else 'admin'
-        locked_by = instance.locked_by
-        if locked_by and locked_by != expect_locked_by:
+        if not self.skip_policy_check and not self.is_expected_locked_by(
+            context, instance):
             check_policy(context, 'unlock_override', instance)
 
         context = context.elevated()

nova/tests/unit/api/openstack/compute/contrib/test_lock_server.py

@@ -15,14 +15,19 @@
 
 import webob
 
+import mock
+
+from nova.api.openstack import common
 from nova.api.openstack.compute.contrib import (admin_actions as
                                                 lock_server_v2)
 from nova.api.openstack.compute.plugins.v3 import (lock_server as
                                                    lock_server_v21)
+from nova import context
 from nova import exception
 from nova import test
 from nova.tests.unit.api.openstack.compute import admin_only_action_common
 from nova.tests.unit.api.openstack import fakes
+from nova.tests.unit import fake_instance
 
 
 class LockServerTestsV21(admin_only_action_common.CommonTests):
@@ -101,3 +106,22 @@ class LockServerPolicyEnforcementV21(test.NoDBTestCase):
         self.assertEqual(
                       "Policy doesn't allow %s to be performed." % rule_name,
                       exc.format_message())
+
+    @mock.patch.object(common, 'get_instance')
+    def test_unlock_policy_failed_with_unlock_override(self,
+                                                       get_instance_mock):
+        ctxt = context.RequestContext('fake', 'fake')
+        instance = fake_instance.fake_instance_obj(ctxt)
+        instance.locked_by = "fake"
+        get_instance_mock.return_value = instance
+        rule_name = ("os_compute_api:os-lock-server:"
+                     "unlock:unlock_override")
+        rules = {"os_compute_api:os-lock-server:unlock": "@",
+                 rule_name: "project:non_fake"}
+        self.policy.set_rules(rules)
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized, self.controller._unlock,
+            self.req, fakes.FAKE_UUID, body={'unlock': {}})
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())

nova/tests/unit/fake_policy.py

@@ -273,6 +273,7 @@ policy_data = """
     "os_compute_api:os-keypairs:delete": "",
     "os_compute_api:os-lock-server:lock": "",
     "os_compute_api:os-lock-server:unlock": "",
+    "os_compute_api:os-lock-server:unlock:unlock_override": "",
     "os_compute_api:os-migrate-server:migrate": "",
     "os_compute_api:os-migrate-server:migrate_live": "",
     "compute_extension:multinic": "",