Remove db layer hard-code permission checks for quota_get_all_*

This patch removes the hard-code permission checks for db call
quota_get_all_*

For v2 API, there is same project owner hard-code permission checks
at REST API layer, so nothing we need to for it.

For v2.1, adds the 'target' for policy enforcement that used to
enforce owner checks. The default method use same rule with show
method before. After removed hard-code permission checks for show
method, this patch adds new rule for distinguish different permission
of defaults and show.

Partially implements bp nova-api-policy-final-part

UpgradeImpact: Due to the db layer permission checks was deleted. The
default rule is added to "os_compute_api:os-quota-sets:show" for
matching the same permission as before. Show and default action are
shared same rule before. But after the cleanup, those two methods have
different permission. So add new rule for default action
"os_compute_api:os-quota-sets:defaults".

Change-Id: I63f180770a9199046452be7ad6e031a142d3c79d

6 files changed, 134 insertions, 115 deletions

diff --git a/etc/nova/policy.json b/etc/nova/policy.json
index c23839394d..300d040c3c 100644
--- a/etc/nova/policy.json
+++ b/etc/nova/policy.json
@@ -324,7 +324,8 @@
     "os_compute_api:os-personality:discoverable": "",
     "os_compute_api:os-preserve-ephemeral-rebuild:discoverable": "",
     "os_compute_api:os-quota-sets:discoverable": "",
-    "os_compute_api:os-quota-sets:show": "",
+    "os_compute_api:os-quota-sets:show": "rule:admin_or_owner",
+    "os_compute_api:os-quota-sets:defaults": "",
     "os_compute_api:os-quota-sets:update": "rule:admin_api",
     "os_compute_api:os-quota-sets:delete": "rule:admin_api",
     "os_compute_api:os-quota-sets:detail": "rule:admin_api",
diff --git a/nova/api/openstack/compute/contrib/quotas.py b/nova/api/openstack/compute/contrib/quotas.py
index 0c74c98b1d..670a3e7a98 100644
--- a/nova/api/openstack/compute/contrib/quotas.py
+++ b/nova/api/openstack/compute/contrib/quotas.py
@@ -116,6 +116,15 @@ class QuotaSetsController(wsgi.Controller):
     def update(self, req, id, body):
         context = req.environ['nova.context']
         authorize_update(context)
+        try:
+            # NOTE(alex_xu): back-compatible with db layer hard-code admin
+            # permission checks. This has to be left only for API v2.0 because
+            # this version has to be stable even if it means that only admins
+            # can call this method while the policy could be changed.
+            nova.context.require_admin_context(context)
+        except exception.AdminRequired:
+            raise webob.exc.HTTPForbidden()
+
         project_id = id
 
         bad_keys = []
@@ -137,6 +146,9 @@ class QuotaSetsController(wsgi.Controller):
             user_id = params.get('user_id', [None])[0]
 
         try:
+            # NOTE(alex_xu): back-compatible with db layer hard-code admin
+            # permission checks.
+            nova.context.authorize_project_context(context, id)
             settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
                                                          user_id=user_id)
         except exception.Forbidden:
@@ -199,8 +211,6 @@ class QuotaSetsController(wsgi.Controller):
             except exception.QuotaExists:
                 objects.Quotas.update_limit(context, project_id,
                                             key, value, user_id=user_id)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
         values = self._get_quotas(context, id, user_id=user_id)
         return self._format_quota_set(None, values)
 
diff --git a/nova/api/openstack/compute/plugins/v3/quota_sets.py b/nova/api/openstack/compute/plugins/v3/quota_sets.py
index 2222e7ad5f..f75bf63f46 100644
--- a/nova/api/openstack/compute/plugins/v3/quota_sets.py
+++ b/nova/api/openstack/compute/plugins/v3/quota_sets.py
@@ -21,7 +21,6 @@ from nova.api.openstack.compute.schemas.v3 import quota_sets
 from nova.api.openstack import extensions
 from nova.api.openstack import wsgi
 from nova.api import validation
-import nova.context
 from nova import exception
 from nova.i18n import _
 from nova import objects
@@ -82,37 +81,29 @@ class QuotaSetsController(wsgi.Controller):
         else:
             return {k: v['limit'] for k, v in values.items()}
 
-    @extensions.expected_errors(403)
+    @extensions.expected_errors(())
     def show(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='show')
+        authorize(context, action='show', target={'project_id': id})
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
-        try:
-            nova.context.authorize_project_context(context, id)
-            return self._format_quota_set(id,
-                    self._get_quotas(context, id, user_id=user_id))
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
-
-    @extensions.expected_errors(403)
+        return self._format_quota_set(id,
+            self._get_quotas(context, id, user_id=user_id))
+
+    @extensions.expected_errors(())
     def detail(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='detail')
+        authorize(context, action='detail', target={'project_id': id})
         user_id = req.GET.get('user_id', None)
-        try:
-            nova.context.authorize_project_context(context, id)
-            return self._format_quota_set(id, self._get_quotas(context, id,
-                                                               user_id=user_id,
-                                                               usages=True))
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
-
-    @extensions.expected_errors((400, 403))
+        return self._format_quota_set(id, self._get_quotas(context, id,
+                                                           user_id=user_id,
+                                                           usages=True))
+
+    @extensions.expected_errors(400)
     @validation.schema(quota_sets.update)
     def update(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context, action='update')
+        authorize(context, action='update', target={'project_id': id})
         project_id = id
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
@@ -120,12 +111,8 @@ class QuotaSetsController(wsgi.Controller):
         quota_set = body['quota_set']
         force_update = strutils.bool_from_string(quota_set.get('force',
                                                                'False'))
-
-        try:
-            settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
-                                                         user_id=user_id)
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
+        settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
+                                                     user_id=user_id)
 
         # NOTE(dims): Pass #1 - In this loop for quota_set.items(), we validate
         # min/max values and bail out if any of the items in the set is bad.
@@ -154,8 +141,6 @@ class QuotaSetsController(wsgi.Controller):
             except exception.QuotaExists:
                 objects.Quotas.update_limit(context, project_id,
                                             key, value, user_id=user_id)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
         # Note(gmann): Removed 'id' from update's response to make it same
         # as V2. If needed it can be added with microversion.
         return self._format_quota_set(None, self._get_quotas(context, id,
@@ -164,7 +149,7 @@ class QuotaSetsController(wsgi.Controller):
     @extensions.expected_errors(())
     def defaults(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='show')
+        authorize(context, action='defaults', target={'project_id': id})
         values = QUOTAS.get_defaults(context)
         return self._format_quota_set(id, values)
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 32beceb907..34fe51e6f8 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -3055,8 +3055,6 @@ def quota_get(context, project_id, resource, user_id=None):
 
 @require_context
 def quota_get_all_by_project_and_user(context, project_id, user_id):
-    nova.context.authorize_project_context(context, project_id)
-
     user_quotas = model_query(context, models.ProjectUserQuota,
                               (models.ProjectUserQuota.resource,
                                models.ProjectUserQuota.hard_limit)).\
@@ -3073,8 +3071,6 @@ def quota_get_all_by_project_and_user(context, project_id, user_id):
 
 @require_context
 def quota_get_all_by_project(context, project_id):
-    nova.context.authorize_project_context(context, project_id)
-
     rows = model_query(context, models.Quota, read_deleted="no").\
                    filter_by(project_id=project_id).\
                    all()
@@ -3088,8 +3084,6 @@ def quota_get_all_by_project(context, project_id):
 
 @require_context
 def quota_get_all(context, project_id):
-    nova.context.authorize_project_context(context, project_id)
-
     result = model_query(context, models.ProjectUserQuota).\
                    filter_by(project_id=project_id).\
                    all()
@@ -3097,7 +3091,6 @@ def quota_get_all(context, project_id):
     return result
 
 
-@require_admin_context
 def quota_create(context, project_id, resource, limit, user_id=None):
     per_user = user_id and resource not in PER_PROJECT_QUOTAS
     quota_ref = models.ProjectUserQuota() if per_user else models.Quota()
@@ -3113,7 +3106,6 @@ def quota_create(context, project_id, resource, limit, user_id=None):
     return quota_ref
 
 
-@require_admin_context
 def quota_update(context, project_id, resource, limit, user_id=None):
     per_user = user_id and resource not in PER_PROJECT_QUOTAS
     model = models.ProjectUserQuota if per_user else models.Quota
@@ -3219,7 +3211,6 @@ def quota_usage_get(context, project_id, resource, user_id=None):
 
 
 def _quota_usage_get_all(context, project_id, user_id=None):
-    nova.context.authorize_project_context(context, project_id)
     query = model_query(context, models.QuotaUsage, read_deleted="no").\
                    filter_by(project_id=project_id)
     result = {'project_id': project_id}
diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py b/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py
index 877d91725b..e5cdb02e94 100644
--- a/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py
+++ b/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py
@@ -177,49 +177,38 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
 
         self.assertEqual(res_dict, expected)
 
-    def test_quotas_show_as_admin(self):
+    def test_quotas_show(self):
         self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         res_dict = self.controller.show(req, 1234)
 
         ref_quota_set = quota_set('1234', self.include_server_group_quotas)
         self.assertEqual(res_dict, ref_quota_set)
 
-    def test_quotas_show_as_unauthorized_user(self):
-        self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
-                          req, 1234)
-
-    def test_quotas_update_as_admin(self):
+    def test_quotas_update(self):
         self.setup_mock_for_update()
         self.default_quotas.update({
             'instances': 50,
             'cores': 50
         })
         body = {'quota_set': self.default_quotas}
-
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         res_dict = self.controller.update(req, 'update_me', body=body)
         self.assertEqual(body, res_dict)
 
     @mock.patch('nova.objects.Quotas.create_limit')
-    def test_quotas_update_with_good_data_as_admin(self, mock_createlimit):
+    def test_quotas_update_with_good_data(self, mock_createlimit):
         self.setup_mock_for_update()
         self.default_quotas.update({})
         body = {'quota_set': self.default_quotas}
-
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         self.controller.update(req, 'update_me', body=body)
         self.assertEqual(len(self.default_quotas),
                          len(mock_createlimit.mock_calls))
 
     @mock.patch('nova.api.validation.validators._SchemaValidator.validate')
     @mock.patch('nova.objects.Quotas.create_limit')
-    def test_quotas_update_with_bad_data_as_admin(self, mock_createlimit,
+    def test_quotas_update_with_bad_data(self, mock_createlimit,
                                                   mock_validate):
         self.setup_mock_for_update()
         self.default_quotas.update({
@@ -227,15 +216,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
             'cores': -50
         })
         body = {'quota_set': self.default_quotas}
-
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
         self.assertEqual(0,
                          len(mock_createlimit.mock_calls))
 
-    def test_quotas_update_zero_value_as_admin(self):
+    def test_quotas_update_zero_value(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 0, 'cores': 0,
                               'ram': 0, 'floating_ips': 0,
@@ -250,27 +237,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
             body['quota_set']['server_groups'] = 10
             body['quota_set']['server_group_members'] = 10
 
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         res_dict = self.controller.update(req, 'update_me', body=body)
         self.assertEqual(body, res_dict)
 
-    def test_quotas_update_as_user(self):
-        self.setup_mock_for_update()
-        self.default_quotas.update({
-            'instances': 50,
-            'cores': 50
-        })
-        body = {'quota_set': self.default_quotas}
-
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
-                          req, 'update_me', body=body)
-
     def _quotas_update_bad_request_case(self, body):
         self.setup_mock_for_update()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         self.assertRaises(self.validation_error, self.controller.update,
                           req, 'update_me', body=body)
 
@@ -371,6 +344,9 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
                           'maximum': -1},
         }
 
+    def _get_http_request(self, url=''):
+        return fakes.HTTPRequest.blank(url)
+
     def test_quotas_update_exceed_in_used(self):
         patcher = mock.patch.object(quota.QUOTAS, 'get_settable_quotas')
         get_settable_quotas = patcher.start()
@@ -378,8 +354,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
         body = {'quota_set': {'cores': 10}}
 
         get_settable_quotas.side_effect = self.fake_get_settable_quotas
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
         mock.patch.stopall()
@@ -395,8 +370,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
 
         get_settable_quotas.side_effect = self.fake_get_settable_quotas
         _get_quotas.side_effect = self.fake_get_quotas
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
+        req = self._get_http_request()
         self.controller.update(req, 'update_me', body=body)
         mock.patch.stopall()
 
@@ -443,21 +417,14 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
         self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager)
         self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
 
-    def test_user_quotas_show_as_admin(self):
+    def test_user_quotas_show(self):
         self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1',
-                                      use_admin_context=True)
+        req = self._get_http_request('/v2/fake4/os-quota-sets/1234?user_id=1')
         res_dict = self.controller.show(req, 1234)
         ref_quota_set = quota_set('1234', self.include_server_group_quotas)
         self.assertEqual(res_dict, ref_quota_set)
 
-    def test_user_quotas_show_as_unauthorized_user(self):
-        self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
-                          req, 1234)
-
-    def test_user_quotas_update_as_admin(self):
+    def test_user_quotas_update(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 10, 'cores': 20,
                               'ram': 51200, 'floating_ips': 10,
@@ -473,35 +440,17 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
             body['quota_set']['server_group_members'] = 10
 
         url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url, use_admin_context=True)
+        req = self._get_http_request(url)
         res_dict = self.controller.update(req, 'update_me', body=body)
 
         self.assertEqual(body, res_dict)
 
-    def test_user_quotas_update_as_user(self):
-        self.setup_mock_for_update()
-        body = {'quota_set': {'instances': 10, 'cores': 20,
-                              'ram': 51200, 'floating_ips': 10,
-                              'fixed_ips': -1, 'metadata_items': 128,
-                              'injected_files': 5,
-                              'injected_file_content_bytes': 10240,
-                              'security_groups': 10,
-                              'security_group_rules': 20,
-                              'key_pairs': 100,
-                              'server_groups': 10,
-                              'server_group_members': 10}}
-
-        url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url)
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
-                          req, 'update_me', body=body)
-
     def test_user_quotas_update_exceed_project(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 20}}
 
         url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url, use_admin_context=True)
+        req = self._get_http_request(url)
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
 
@@ -622,6 +571,23 @@ class QuotaSetsTestV2(QuotaSetsTestV21):
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
                           req, 1234)
 
+    def test_quotas_show_as_unauthorized_user(self):
+        self.setup_mock_for_show()
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
+                          req, 1234)
+
+    def test_quotas_update_as_user(self):
+        self.default_quotas.update({
+            'instances': 50,
+            'cores': 50
+        })
+        body = {'quota_set': self.default_quotas}
+
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          req, 'update_me', body=body)
+
 
 class QuotaSetsTestV2WithoutServerGroupQuotas(QuotaSetsTestV2):
     include_server_group_quotas = False
@@ -653,6 +619,9 @@ class ExtendedQuotasTestV2(ExtendedQuotasTestV21):
         self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
         self.mox.ResetAll()
 
+    def _get_http_request(self, url=''):
+        return fakes.HTTPRequest.blank(url, use_admin_context=True)
+
 
 class UserQuotasTestV2(UserQuotasTestV21):
     plugin = quotas_v2
@@ -674,6 +643,27 @@ class UserQuotasTestV2(UserQuotasTestV21):
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
                           req, '1234')
 
+    def test_user_quotas_show_as_unauthorized_user(self):
+        self.setup_mock_for_show()
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
+                          req, 1234)
+
+    def test_user_quotas_update_as_user(self):
+        body = {'quota_set': {'instances': 10, 'cores': 20,
+                              'ram': 51200, 'floating_ips': 10,
+                              'fixed_ips': -1, 'metadata_items': 128,
+                              'injected_files': 5,
+                              'injected_file_content_bytes': 10240,
+                              'key_pairs': 100,
+                              'security_groups': 10,
+                              'security_group_rules': 20}}
+
+        url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
+        req = fakes.HTTPRequest.blank(url)
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          req, 'update_me', body=body)
+
 
 class UserQuotasTestV2WithoutServerGroupQuotas(UserQuotasTestV2):
     include_server_group_quotas = False
@@ -716,3 +706,44 @@ class QuotaSetsPolicyEnforcementV21(test.NoDBTestCase):
         self.assertEqual(
             "Policy doesn't allow %s to be performed." % rule_name,
             exc.format_message())
+
+    def test_defaults_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:defaults"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.defaults, self.req, fakes.FAKE_UUID)
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
+
+    def test_show_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:show"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.show, self.req, fakes.FAKE_UUID)
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
+
+    def test_detail_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:detail"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.detail, self.req, fakes.FAKE_UUID)
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
+
+    def test_update_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:update"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.update, self.req, fakes.FAKE_UUID,
+            body={'quota_set': {}})
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py
index 34d023f327..832b682e0b 100644
--- a/nova/tests/unit/fake_policy.py
+++ b/nova/tests/unit/fake_policy.py
@@ -296,6 +296,7 @@ policy_data = """
     "os_compute_api:os-quota-sets:update": "",
     "os_compute_api:os-quota-sets:delete": "",
     "os_compute_api:os-quota-sets:detail": "",
+    "os_compute_api:os-quota-sets:defaults": "",
     "compute_extension:quota_classes": "",
     "os_compute_api:os-quota-class-sets": "",
     "compute_extension:rescue": "",