fix the an Unexpected API Error issue in flavor API

The flavors.add_flavor_access and flavors.remove_flavor_access
require admin privilege in DB level but the policy doesn't
require it and the exception isn't catched in the api. So the
bug arise. The privilege check in DB level is not necessary,
but it need more tests and can be remove in other patch or
blue-print.

This use extension_authorizer instead of soft_extension_authorizer
in the action of flavor_access, in order to raise exception.

Closes-Bug: #1217679
DocImpact

Change-Id: I0b1231f47fffaf99f330de02956bfe8d7cd4b920

etc/nova/policy.json

@@ -111,8 +111,12 @@
     "compute_extension:v3:os-extended-volumes:detach": "",
     "compute_extension:fixed_ips": "rule:admin_api",
     "compute_extension:flavor_access": "",
+    "compute_extension:flavor_access:addTenantAccess": "rule:admin_api",
+    "compute_extension:flavor_access:removeTenantAccess": "rule:admin_api",
     "compute_extension:v3:os-flavor-access": "",
     "compute_extension:v3:os-flavor-access:discoverable": "",
+    "compute_extension:v3:os-flavor-access:remove_tenant_access": "rule:admin_api",
+    "compute_extension:v3:os-flavor-access:add_tenant_access": "rule:admin_api",
     "compute_extension:flavor_disabled": "",
     "compute_extension:v3:os-flavor-disabled": "",
     "compute_extension:v3:os-flavor-disabled:discoverable": "",

nova/api/openstack/compute/contrib/flavor_access.py

@@ -27,7 +27,9 @@
 from nova.openstack.common.gettextutils import _
 
 
-authorize = extensions.soft_extension_authorizer('compute', 'flavor_access')
+soft_authorize = extensions.soft_extension_authorizer('compute',
+                                                      'flavor_access')
+authorize = extensions.extension_authorizer('compute', 'flavor_access')
 
 
 def make_flavor(elem):
@@ -133,7 +135,7 @@ def _extend_flavor(self, flavor_rval, flavor_ref):
     @wsgi.extends
     def show(self, req, resp_obj, id):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorTemplate())
             db_flavor = req.get_db_flavor(id)
@@ -143,7 +145,7 @@ def show(self, req, resp_obj, id):
     @wsgi.extends
     def detail(self, req, resp_obj):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorsTemplate())
 
@@ -155,7 +157,7 @@ def detail(self, req, resp_obj):
     @wsgi.extends(action='create')
     def create(self, req, body, resp_obj):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorTemplate())
 
@@ -167,7 +169,8 @@ def create(self, req, body, resp_obj):
     @wsgi.action("addTenantAccess")
     def _addTenantAccess(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context)
+        authorize(context, action="addTenantAccess")
+
         self._check_body(body)
 
         vals = body['addTenantAccess']
@@ -184,7 +187,8 @@ def _addTenantAccess(self, req, id, body):
     @wsgi.action("removeTenantAccess")
     def _removeTenantAccess(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context)
+        authorize(context, action="removeTenantAccess")
+
         self._check_body(body)
 
         vals = body['removeTenantAccess']

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

@@ -27,7 +27,9 @@
 from nova.openstack.common.gettextutils import _
 
 ALIAS = 'os-flavor-access'
-authorize = extensions.soft_extension_authorizer('compute', 'v3:' + ALIAS)
+soft_authorize = extensions.soft_extension_authorizer('compute',
+                                                      'v3:' + ALIAS)
+authorize = extensions.extension_authorizer('compute', 'v3:%s' % ALIAS)
 
 
 def make_flavor(elem):
@@ -126,7 +128,7 @@ def _extend_flavor(self, flavor_rval, flavor_ref):
     @wsgi.extends
     def show(self, req, resp_obj, id):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorTemplate())
             db_flavor = req.get_db_flavor(id)
@@ -136,7 +138,7 @@ def show(self, req, resp_obj, id):
     @wsgi.extends
     def detail(self, req, resp_obj):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorsTemplate())
 
@@ -148,20 +150,21 @@ def detail(self, req, resp_obj):
     @wsgi.extends(action='create')
     def create(self, req, body, resp_obj):
         context = req.environ['nova.context']
-        if authorize(context):
+        if soft_authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=FlavorTemplate())
 
             db_flavor = req.get_db_flavor(resp_obj.obj['flavor']['id'])
 
             self._extend_flavor(resp_obj.obj['flavor'], db_flavor)
 
-    @extensions.expected_errors((400, 404, 409))
+    @extensions.expected_errors((400, 403, 404, 409))
     @wsgi.serializers(xml=FlavorAccessTemplate)
     @wsgi.action("add_tenant_access")
     def _add_tenant_access(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context)
+        authorize(context, action="add_tenant_access")
+
         if not self.is_valid_body(body, 'add_tenant_access'):
             raise webob.exc.HTTPBadRequest(explanation=_("Invalid request"))
 
@@ -178,15 +181,17 @@ def _add_tenant_access(self, req, id, body):
             raise webob.exc.HTTPConflict(explanation=err.format_message())
         except exception.FlavorNotFound as e:
             raise webob.exc.HTTPNotFound(explanation=e.format_message())
-
+        except exception.AdminRequired as e:
+            raise webob.exc.HTTPForbidden(explanation=e.format_message())
         return _marshall_flavor_access(id)
 
-    @extensions.expected_errors((400, 404))
+    @extensions.expected_errors((400, 403, 404))
     @wsgi.serializers(xml=FlavorAccessTemplate)
     @wsgi.action("remove_tenant_access")
     def _remove_tenant_access(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context)
+        authorize(context, action="remove_tenant_access")
+
         if not self.is_valid_body(body, 'remove_tenant_access'):
             raise webob.exc.HTTPBadRequest(explanation=_("Invalid request"))
 
@@ -202,7 +207,8 @@ def _remove_tenant_access(self, req, id, body):
         except (exception.FlavorAccessNotFound,
                 exception.FlavorNotFound) as e:
             raise webob.exc.HTTPNotFound(explanation=e.format_message())
-
+        except exception.AdminRequired as e:
+            raise webob.exc.HTTPForbidden(explanation=e.format_message())
         return _marshall_flavor_access(id)
 
 

nova/tests/api/openstack/compute/contrib/test_flavor_access.py

@@ -153,6 +153,20 @@ def test_list_flavor_access_private(self):
         result = self.flavor_access_controller.index(self.req, '2')
         self.assertEqual(result, expected)
 
+    def test_list_with_no_context(self):
+        req = fakes.HTTPRequest.blank('/v2/flavors/fake/flavors')
+
+        def fake_authorize(context, target=None, action=None):
+            raise exception.PolicyNotAuthorized(action='index')
+
+        self.stubs.Set(flavor_access,
+                       'authorize',
+                       fake_authorize)
+
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_access_controller.index,
+                          req, 'fake')
+
     def test_list_flavor_with_admin_default_proj1(self):
         expected = {'flavors': [{'id': '0'}, {'id': '1'}]}
         req = fakes.HTTPRequest.blank('/v2/fake/flavors',
@@ -264,6 +278,14 @@ def stub_add_flavor_access(flavorid, projectid, ctxt=None):
             _addTenantAccess(req, '3', body)
         self.assertEqual(result, expected)
 
+    def test_add_tenant_access_with_no_admin_user(self):
+        req = fakes.HTTPRequest.blank('/v2/fake/flavors/2/action',
+                                      use_admin_context=False)
+        body = {'addTenantAccess': {'tenant': 'proj2'}}
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_action_controller._addTenantAccess,
+                          req, '2', body)
+
     def test_add_tenant_access_with_already_added_access(self):
         def stub_add_flavor_access(flavorid, projectid, ctxt=None):
             raise exception.FlavorAccessExists(flavor_id=flavorid,
@@ -290,6 +312,14 @@ def stub_remove_flavor_access(flavorid, projectid, ctxt=None):
                           self.flavor_action_controller._removeTenantAccess,
                           self.req, '3', body)
 
+    def test_remove_tenant_access_with_no_admin_user(self):
+        req = fakes.HTTPRequest.blank('/v2/fake/flavors/2/action',
+                                      use_admin_context=False)
+        body = {'removeTenantAccess': {'tenant': 'proj2'}}
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_action_controller._removeTenantAccess,
+                          req, '2', body)
+
 
 class FlavorAccessSerializerTest(test.TestCase):
     def test_serializer_empty(self):

nova/tests/api/openstack/compute/plugins/v3/test_flavor_access.py

@@ -140,7 +140,7 @@ def _verify_flavor_list(self, result, expected):
 
     def test_list_flavor_access_public(self):
         # query os-flavor-access on public flavor should return 404
-        req = fakes.HTTPRequestV3.blank('/flavors/os-flavor-access',
+        req = fakes.HTTPRequestV3.blank('/flavors/fake/os-flavor-access',
                                         use_admin_context=True)
         self.assertRaises(exc.HTTPNotFound,
                           self.flavor_access_controller.index,
@@ -153,6 +153,18 @@ def test_list_flavor_access_private(self):
         result = self.flavor_access_controller.index(self.req, '2')
         self.assertEqual(result, expected)
 
+    def test_list_with_no_context(self):
+        req = fakes.HTTPRequestV3.blank('/flavors/2/os-flavor-access')
+
+        def fake_authorize(context, target=None, action=None):
+            raise exception.PolicyNotAuthorized(action='index')
+
+        self.stubs.Set(flavor_access, 'authorize', fake_authorize)
+
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_access_controller.index,
+                          req, '2')
+
     def test_list_flavor_with_admin_default_proj1(self):
         expected = {'flavors': [{'id': '0'}, {'id': '1'}]}
         req = fakes.HTTPRequestV3.blank('/flavors',
@@ -276,6 +288,25 @@ def stub_add_flavor_access(flavorid, projectid, ctxt=None):
                           self.flavor_action_controller._add_tenant_access,
                           req, '3', body)
 
+    def test_add_tenant_access_with_no_admin_user(self):
+        req = fakes.HTTPRequestV3.blank('/flavors/2/action')
+        body = {'add_tenant_access': {'tenant_id': 'proj2'}}
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_action_controller._add_tenant_access,
+                          req, '2', body)
+
+    def test_add_tenant_access_without_policy_check(self):
+        req = fakes.HTTPRequestV3.blank('/flavors/2/action')
+        body = {'add_tenant_access': {'tenant_id': 'proj2'}}
+
+        def fake_authorize(context, target=None, action=None):
+            pass
+
+        self.stubs.Set(flavor_access, 'authorize', fake_authorize)
+        self.assertRaises(exc.HTTPForbidden,
+                          self.flavor_action_controller._add_tenant_access,
+                          req, '2', body)
+
     def test_add_tenant_access_without_tenant_id(self):
         def stub_add_flavor_access(flavorid, projectid, ctxt=None):
             raise exception.FlavorNotFound(flavor_id=flavorid)
@@ -362,6 +393,26 @@ def stub_remove_flavor_access(flavorid, projectid, ctxt=None):
                           self.flavor_action_controller._remove_tenant_access,
                           req, '3', body)
 
+    def test_remove_tenant_access_with_no_admin_user(self):
+        req = fakes.HTTPRequestV3.blank('flavors/2/action',
+                                        use_admin_context=False)
+        body = {'remove_tenant_access': {'tenant_id': 'proj2'}}
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.flavor_action_controller._remove_tenant_access,
+                          req, '2', body)
+
+    def test_remove_tenant_access_without_policy_check(self):
+        req = fakes.HTTPRequestV3.blank('/flavors/2/action')
+        body = {'remove_tenant_access': {'tenant_id': 'proj2'}}
+
+        def fake_authorize(context, target=None, action=None):
+            pass
+
+        self.stubs.Set(flavor_access, 'authorize', fake_authorize)
+        self.assertRaises(exc.HTTPForbidden,
+                          self.flavor_action_controller._remove_tenant_access,
+                          req, '2', body)
+
 
 class FlavorAccessSerializerTest(test.TestCase):
     def test_serializer_empty(self):

nova/tests/fake_policy.py

@@ -173,7 +173,13 @@
     "compute_extension:v3:os-extended-volumes:detach": "",
     "compute_extension:fixed_ips": "",
     "compute_extension:flavor_access": "",
+    "compute_extension:flavor_access:addTenantAccess": "rule:admin_api",
+    "compute_extension:flavor_access:removeTenantAccess": "rule:admin_api",
     "compute_extension:v3:os-flavor-access": "",
+    "compute_extension:v3:os-flavor-access:remove_tenant_access":
+    "rule:admin_api",
+    "compute_extension:v3:os-flavor-access:add_tenant_access":
+    "rule:admin_api",
     "compute_extension:flavor_disabled": "",
     "compute_extension:v3:os-flavor-disabled": "",
     "compute_extension:flavor_rxtx": "",