Support X_IS_ADMIN_PROJECT header on auth response

In order to make the 'admin' role more sensible, keystone added a
header to authentication responses X_IS_ADMIN_PROJECT indicating
whether or not a request was authenticated against the administrative
project (in devstack, this is configured to be the 'admin' project).

This patch adds support for the header. It defaults to True if the
header is not set, in accordance with other projects, for the reason
that older keystone deployments (or those where this feature is not
enabled) would otherwise not be useable.

The header is configured by admin_project_name and
admin_project_domain_name in keystone's keystone.conf. If these are
set, a request with the admin role against a non-admin project should
no longer be able to retrieve resources belonging to other tenants even
if all_projects is set. If the keystone options are *not* set, then
all admin-role requests will be able to retrieve resources belonging to
other tenants.

See also https://bugs.launchpad.net/keystone/+bug/968696

Change-Id: Iaa6e6b6e85d0474a9e1fa1cf6c7d8012a9557188
Closes-Bug: #1626589

doc/source/authentication.rst

@@ -131,6 +131,31 @@ operations are allowed.
 See http://docs.openstack.org/developer/oslo.policy/api.html for details on
 rule formatting.
 
+During the last few cycles concerns were raised about the scope of the
+``admin`` role within openstack. Many services consider any token scoped with
+the ``admin`` role to have access to resources within any project. With the
+introduction of keystone v3 it is possible to create users with the admin role
+on a particular project, but not with the intention of them seeing resources in
+other projects.
+
+Keystone added two configuration options called ``admin_project_name`` and
+``admin_project_domain_name`` to attempt to address this. If a request is
+authenticated against a the project whose name is ``admin_project_name``
+in the ``admin_project_domain_name`` domain, a flag is set on the
+authentication response headers indicating that the user is authenticated
+against the administrative project. This can then be supported by the policy
+rule (in Searchlight's ``policy.json``)::
+
+    "is_admin_context": "role:admin and is_admin_project:True"
+
+Since devstack configures keystone to support those options, this is the
+default in Searchlight. To maintain backwards compatibility, if your keystone
+is *not* configured to set these options, any token with the ``admin`` role
+will be assumed to have administrative powers (this approach has been taken
+by other Openstack services).
+
+For more history see https://bugs.launchpad.net/keystone/+bug/968696.
+
 Access to operations
 --------------------
 

etc/policy.json

@@ -1,6 +1,6 @@
 {
-    "context_is_admin":  "role:admin",
-    "admin_or_owner":  "is_admin:True or project_id:%(project_id)s",
+    "context_is_admin":  "role:admin and is_admin_project:True",
+    "admin_or_owner":  "rule:context_is_admin or project_id:%(project_id)s",
     "default": "",
 
     "search:query": "rule:admin_or_owner",

searchlight/api/middleware/context.py

@@ -124,16 +124,23 @@ class ContextMiddleware(BaseContextMiddleware):
                 raise webob.exc.HTTPInternalServerError(
                     _('Invalid service catalog json.'))
 
+        # TODO(sjmc7) consider changing this to use RequestContext.from_environ
+        # rather than parsing headers ourselves.
+        # Default this to true is the header's missing; older versions
+        # of oslo_context don't include it and some keystone configurations
+        # may not include it either. Look at defaulting to False in ocata
+        is_admin_project = req.headers.get('X-Is-Admin-Project',
+                                           'true').lower() == 'true'
         kwargs = {
             'user': req.headers.get('X-User-Id'),
             'tenant': req.headers.get('X-Tenant-Id'),
             'roles': roles,
-            'is_admin': CONF.admin_role.strip().lower() in roles,
             'auth_token': req.headers.get('X-Auth-Token', deprecated_token),
             'owner_is_tenant': CONF.owner_is_tenant,
             'service_catalog': service_catalog,
             'policy_enforcer': self.policy_enforcer,
             'request_id': req.headers.get('X-Openstack-Request-ID'),
+            'is_admin_project': is_admin_project
         }
 
         return searchlight.context.RequestContext(**kwargs)

searchlight/tests/etc/policy.json

@@ -1,6 +1,6 @@
 {
-    "context_is_admin":  "role:admin",
-    "admin_or_owner":  "is_admin:True or project_id:%(project_id)s",
+    "context_is_admin":  "role:admin and is_admin_project:True",
+    "admin_or_owner":  "rule:context_is_admin or project_id:%(project_id)s",
     "default": "",
 
     "search:query": "rule:admin_or_owner",

searchlight/tests/functional/__init__.py

@@ -514,12 +514,17 @@ class FunctionalTest(test_utils.BaseTestCase):
             content = jsonutils.loads(content)
         return response, content
 
-    def _search_request(self, body, tenant, role="member", decode_json=True):
+    def _search_request(self, body, tenant, role="member", decode_json=True,
+                        is_admin_project=None):
         """Conduct a search against all elasticsearch indices unless specified
         in `body`. Returns the response and json-decoded content.
         """
+        extra_headers = {}
+        if is_admin_project is not None:
+            extra_headers['X-Is-Admin-Project'] = str(is_admin_project)
+
         return self._request("POST", "/search", tenant, body,
-                             role, decode_json)
+                             role, decode_json, extra_headers)
 
     def _facet_request(self, tenant, doc_type=None, role="member",
                        decode_json=True, include_fields=None,

searchlight/tests/functional/test_api.py

@@ -907,6 +907,68 @@ class TestSearchApi(functional.FunctionalTest):
                                                       decode_json=False)
         self.assertEqual(403, response.status)
 
+    def test_is_admin_project(self):
+        """The X-IS-ADMIN-PROJECT header is the current solution to the
+        problems caused by the overloaded 'admin' role. It's only set if
+        configured in keystone, and defaults to True for back compatibility.
+        """
+        one_tenant_doc = {
+            u'addresses': {},
+            u'id': 'abcdef',
+            u'tenant_id': TENANT1,
+            u'user_id': USER1,
+            u'image': {u'id': u'a'},
+            u'flavor': {u'id': u'1'},
+            u'created_at': u'2016-04-07T15:49:35Z',
+            u'updated_at': u'2016-04-07T15:51:35Z'
+        }
+        two_tenant_doc = {
+            u'addresses': {},
+            u'id': '12341234',
+            u'tenant_id': TENANT2,
+            u'user_id': USER1,
+            u'image': {u'id': u'a'},
+            u'flavor': {u'id': u'1'},
+            u'created_at': u'2016-04-07T15:49:35Z',
+            u'updated_at': u'2016-04-07T15:51:35Z'
+        }
+
+        # A request with HTTP_X_IS_ADMIN_PROJECT=false should not allow a
+        # TENANT1-scoped request to retrieve both servers if set to True it
+        # should (assuming they also have the admin role)
+        servers_plugin = self.initialized_plugins['OS::Nova::Server']
+        with mock.patch(nova_version_getter, return_value=fake_version_list):
+            self._index(servers_plugin, [test_utils.DictObj(**one_tenant_doc),
+                                         test_utils.DictObj(**two_tenant_doc)])
+
+        query = {"type": "OS::Nova::Server",
+                 "all_projects": True,
+                 "query": {"match_all": {}},
+                 "sort": "id"}
+        # First try admin request, is-admin-project false
+        response, json_content = self._search_request(query,
+                                                      TENANT1,
+                                                      role="admin",
+                                                      is_admin_project=False)
+        self.assertEqual(['abcdef_USER'],
+                         [h['_id'] for h in json_content['hits']['hits']])
+
+        # Now try with is_admin_project True
+        response, json_content = self._search_request(query,
+                                                      TENANT1,
+                                                      role="admin",
+                                                      is_admin_project=True)
+        self.assertEqual(['12341234_ADMIN', 'abcdef_ADMIN'],
+                         [h['_id'] for h in json_content['hits']['hits']])
+
+        # Now try with is_admin_project True but member role
+        response, json_content = self._search_request(query,
+                                                      TENANT1,
+                                                      role="member",
+                                                      is_admin_project=True)
+        self.assertEqual(['abcdef_USER'],
+                         [h['_id'] for h in json_content['hits']['hits']])
+
 
 class TestServerServicePolicies(functional.FunctionalTest):
     def _write_policy_file(self, filename, rules):

searchlight/tests/unit/test_context_middleware.py

@@ -34,3 +34,17 @@ class TestContextMiddleware(test_utils.BaseTestCase):
         # Validate that request-id do not starts with 'req-req-'
         self.assertFalse(resp_req_id.startswith('req-req-'))
         self.assertTrue(resp_req_id.startswith('req-'))
+
+    def test_is_admin_project(self):
+        middleware = context.ContextMiddleware(None)
+        req = webob.Request.blank('/')
+        req_context = middleware._get_authenticated_context(req)
+        self.assertTrue(req_context.is_admin_project)
+
+        req = webob.Request.blank('/', headers={'X-Is-Admin-Project': 'True'})
+        req_context = middleware._get_authenticated_context(req)
+        self.assertTrue(req_context.is_admin_project)
+
+        req = webob.Request.blank('/', headers={'X-Is-Admin-Project': 'False'})
+        req_context = middleware._get_authenticated_context(req)
+        self.assertFalse(req_context.is_admin_project)