From: John Griffith <john.griffith8@gmail.com>
Date: Wed, 19 Aug 2015 17:42:40 +0000 (+0000)
Subject: Filter out extra-specs from type get for non-admin
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0cfa9b62410ab74ce840c3b580318eae6ddfc804;p=openstack-build%2Fcinder-build.git

Filter out extra-specs from type get for non-admin

Currently when a get_volume type call is made to the
db api, we don't check context and auto-fill the
extra-specs info for that type.

Extra specs are intended to be admin only info, but anybody
that calls the API directly to list or get volume-types is
also given this info which is not intended.

This patch just adds a context check to the db api's
private extra-specs builder method.  In the case of
non-admin, we just skip adding the extra-specs.

Change-Id: Ia46695a6c890c06d94e6903ed9c54794107fa132
Closes-Bug: #1351971
Closes-Bug: #1475285
---

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 868909892..7f21362ae 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -459,21 +459,31 @@ def _metadata_refs(metadata_dict, meta_class):
     return metadata_refs
 
 
-def _dict_with_extra_specs(inst_type_query):
+def _dict_with_extra_specs_if_authorized(context, inst_type_query):
     """Convert type query result to dict with extra_spec and rate_limit.
 
     Takes a volume type query returned by sqlalchemy and returns it
     as a dictionary, converting the extra_specs entry from a list
-    of dicts:
+    of dicts.  NOTE the contents of extra-specs are admin readable
+    only.  If the context passed in for this request is not admin
+    then we will return an empty extra-specs dict rather than
+    providing the admin only details.
+
+    Example response with admin context:
 
     'extra_specs' : [{'key': 'k1', 'value': 'v1', ...}, ...]
     to a single dict:
     'extra_specs' : {'k1': 'v1'}
+
     """
+
     inst_type_dict = dict(inst_type_query)
-    extra_specs = {x['key']: x['value']
-                   for x in inst_type_query['extra_specs']}
-    inst_type_dict['extra_specs'] = extra_specs
+    if not is_admin_context(context):
+        del(inst_type_dict['extra_specs'])
+    else:
+        extra_specs = {x['key']: x['value']
+                       for x in inst_type_query['extra_specs']}
+        inst_type_dict['extra_specs'] = extra_specs
     return inst_type_dict
 
 
@@ -2388,7 +2398,8 @@ def volume_type_get_all(context, inactive=False, filters=None):
 
     result = {}
     for row in rows:
-        result[row['name']] = _dict_with_extra_specs(row)
+        result[row['name']] = _dict_with_extra_specs_if_authorized(context,
+                                                                   row)
 
     return result
 
@@ -2421,7 +2432,7 @@ def _volume_type_get(context, id, session=None, inactive=False,
     if not result:
         raise exception.VolumeTypeNotFound(volume_type_id=id)
 
-    vtype = _dict_with_extra_specs(result)
+    vtype = _dict_with_extra_specs_if_authorized(context, result)
 
     if 'projects' in expected_fields:
         vtype['projects'] = [p['project_id'] for p in result['projects']]
@@ -2466,7 +2477,7 @@ def _volume_type_get_by_name(context, name, session=None):
     if not result:
         raise exception.VolumeTypeNotFoundByName(volume_type_name=name)
 
-    return _dict_with_extra_specs(result)
+    return _dict_with_extra_specs_if_authorized(context, result)
 
 
 @require_context
diff --git a/cinder/tests/unit/test_volume_types.py b/cinder/tests/unit/test_volume_types.py
index 6bbc86527..747f4fd3e 100644
--- a/cinder/tests/unit/test_volume_types.py
+++ b/cinder/tests/unit/test_volume_types.py
@@ -404,3 +404,19 @@ class VolumeTypeTestCase(test.TestCase):
         volume_type_id = volume_type.get('id')
         self.assertFalse(volume_types.is_public_volume_type(self.ctxt,
                                                             volume_type_id))
+
+    def test_ensure_no_extra_specs_for_non_admin(self):
+        # non-admin users shouldn't get extra-specs back in type-get/list etc
+        ctxt = context.RequestContext('average-joe',
+                                      'd802f078-0af1-4e6b-8c02-7fac8d4339aa',
+                                      auth_token='token',
+                                      is_admin=False)
+        volume_types.create(self.ctxt, "type-test", is_public=False)
+        vtype = volume_types.get_volume_type_by_name(ctxt, 'type-test')
+        self.assertIsNone(vtype.get('extra_specs', None))
+
+    def test_ensure_extra_specs_for_admin(self):
+        # admin users should get extra-specs back in type-get/list etc
+        volume_types.create(self.ctxt, "type-test", is_public=False)
+        vtype = volume_types.get_volume_type_by_name(self.ctxt, 'type-test')
+        self.assertIsNotNone(vtype.get('extra_specs', None))