Fix UsedLimitsController's authorizer to soft

The rule "limits_extension:used_limits" is used by
"UsedLimitsController" which is a controller extension and it extends
"limits" api. Most of controller extensions authorize user's context
by applying the function "extensions.soft_extension_authorizer()",
but "UsedLimitsController" applies "extensions.extension_authorizer"
and this may cause 403 Forbidden error.

In this patch, I changed UsedLimitsController's authorizer to
"soft_extension_authorizer".

APIImpact

Co-Authored-By: ZhuChunzhan <zhucz@cn.fujitsu.com>
Change-Id: I8a4163ca89236b35c2c6ba10bcd98f8c42ef9089
Closes-Bug: #1497868

diff --git a/cinder/api/contrib/used_limits.py b/cinder/api/contrib/used_limits.py
index 52887ba802adb49b019f125e29e569a070f40124..d32967a3e7bd97ff5924f18314b5e403fb80178b 100644 (file)
--- a/cinder/api/contrib/used_limits.py
+++ b/cinder/api/contrib/used_limits.py
@@ -18,7 +18,7 @@ from cinder import quota
 
 QUOTAS = quota.QUOTAS
 
-authorize = extensions.extension_authorizer('limits', 'used_limits')
+authorize = extensions.soft_extension_authorizer('limits', 'used_limits')
 
 
 class UsedLimitsController(wsgi.Controller):
@@ -26,25 +26,24 @@ class UsedLimitsController(wsgi.Controller):
     @wsgi.extends
     def index(self, req, resp_obj):
         context = req.environ['cinder.context']
-        authorize(context)
-
-        quotas = QUOTAS.get_project_quotas(context, context.project_id,
-                                           usages=True)
-
-        quota_map = {
-            'totalVolumesUsed': 'volumes',
-            'totalGigabytesUsed': 'gigabytes',
-            'totalSnapshotsUsed': 'snapshots',
-            'totalBackupsUsed': 'backups',
-            'totalBackupGigabytesUsed': 'backup_gigabytes'
-        }
-
-        used_limits = {}
-        for display_name, single_quota in quota_map.items():
-            if single_quota in quotas:
-                used_limits[display_name] = quotas[single_quota]['in_use']
-
-        resp_obj.obj['limits']['absolute'].update(used_limits)
+        if authorize(context):
+            quotas = QUOTAS.get_project_quotas(context, context.project_id,
+                                               usages=True)
+
+            quota_map = {
+                'totalVolumesUsed': 'volumes',
+                'totalGigabytesUsed': 'gigabytes',
+                'totalSnapshotsUsed': 'snapshots',
+                'totalBackupsUsed': 'backups',
+                'totalBackupGigabytesUsed': 'backup_gigabytes'
+            }
+
+            used_limits = {}
+            for display_name, single_quota in quota_map.items():
+                if single_quota in quotas:
+                    used_limits[display_name] = quotas[single_quota]['in_use']
+
+            resp_obj.obj['limits']['absolute'].update(used_limits)
 
 
 class Used_limits(extensions.ExtensionDescriptor):

diff --git a/cinder/tests/unit/api/contrib/test_used_limits.py b/cinder/tests/unit/api/contrib/test_used_limits.py
index 7a292060586274f5003a993951a71b4e61f9396c..437c2b55e5757641a184452eb496b624307feaa6 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_used_limits.py
+++ b/cinder/tests/unit/api/contrib/test_used_limits.py
@@ -13,9 +13,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+
 from cinder.api.contrib import used_limits
 from cinder.api.openstack import wsgi
-from cinder import quota
+from cinder import exception
 from cinder import test
 from cinder.tests.unit.api import fakes
 
@@ -31,7 +33,9 @@ class UsedLimitsTestCase(test.TestCase):
         super(UsedLimitsTestCase, self).setUp()
         self.controller = used_limits.UsedLimitsController()
 
-    def test_used_limits(self):
+    @mock.patch('cinder.quota.QUOTAS.get_project_quotas')
+    @mock.patch('cinder.policy.enforce')
+    def test_used_limits(self, _mock_policy_enforce, _mock_get_project_quotas):
         fake_req = FakeRequest(fakes.FakeRequestContext('fake', 'fake'))
         obj = {
             "limits": {
@@ -50,17 +54,30 @@ class UsedLimitsTestCase(test.TestCase):
         for display_name, q in quota_map.items():
             limits[q] = {'limit': 2,
                          'in_use': 1}
+        _mock_get_project_quotas.return_value = limits
 
-        def stub_get_project_quotas(context, project_id, usages=True):
-            return limits
-
-        self.stubs.Set(quota.QUOTAS, "get_project_quotas",
-                       stub_get_project_quotas)
-
-        self.mox.ReplayAll()
+        # allow user to access used limits
+        _mock_policy_enforce.return_value = None
 
         self.controller.index(fake_req, res)
         abs_limits = res.obj['limits']['absolute']
         for used_limit, value in abs_limits.items():
             self.assertEqual(value,
                              limits[quota_map[used_limit]]['in_use'])
+
+        obj = {
+            "limits": {
+                "rate": [],
+                "absolute": {},
+            },
+        }
+        res = wsgi.ResponseObject(obj)
+
+        # unallow user to access used limits
+        _mock_policy_enforce.side_effect = exception.NotAuthorized
+
+        self.controller.index(fake_req, res)
+        abs_limits = res.obj['limits']['absolute']
+        self.assertNotIn('totalVolumesUsed', abs_limits)
+        self.assertNotIn('totalGigabytesUsed', abs_limits)
+        self.assertNotIn('totalSnapshotsUsed', abs_limits)