Move X-Auth-Url logic to auth_url middleware

Refactoring the auth_password middleware to move X-Auth-Url logic
into the auth_url middleware, so that all X-Auth-Url logic is
handled in one place.

This also adds the auth_url middleware in front of the auth_password
middleware, so that there should be no behavior change

Co-Authored-By: Richard Lee <rblee88@gmail.com>
Related-Bug: #1259364
Change-Id: I3819cbf1a4c4955752dc7c804b0add1bab2b962c

etc/heat/api-paste.ini

@@ -12,7 +12,7 @@ pipeline = faultwrap ssl versionnegotiation authurl authtoken context apiv1app
 #   flavor = standalone
 #
 [pipeline:heat-api-standalone]
-pipeline = faultwrap ssl versionnegotiation authpassword context apiv1app
+pipeline = faultwrap ssl versionnegotiation authurl authpassword context apiv1app
 
 # heat-api pipeline for custom cloud backends
 # i.e. in heat.conf:

heat/common/auth_password.py

@@ -17,12 +17,8 @@
 
 from keystoneclient.v2_0 import client as keystone_client
 from keystoneclient import exceptions as keystone_exceptions
-from oslo.config import cfg
-from webob.exc import HTTPBadRequest
 from webob.exc import HTTPUnauthorized
 
-from heat.openstack.common import importutils
-
 
 class KeystonePasswordAuthProtocol(object):
     """
@@ -35,16 +31,6 @@ class KeystonePasswordAuthProtocol(object):
     def __init__(self, app, conf):
         self.app = app
         self.conf = conf
-        auth_url = None
-        if not cfg.CONF.auth_password.multi_cloud:
-            if 'auth_uri' in self.conf:
-                auth_url = self.conf['auth_uri']
-            else:
-                # Import auth_token to have keystone_authtoken settings setup.
-                importutils.import_module(
-                    'keystoneclient.middleware.auth_token')
-                auth_url = cfg.CONF.keystone_authtoken['auth_uri']
-        self.auth_url = auth_url
 
     def __call__(self, env, start_response):
         """Authenticate incoming request."""
@@ -52,12 +38,7 @@ class KeystonePasswordAuthProtocol(object):
         password = env.get('HTTP_X_AUTH_KEY')
         # Determine tenant id from path.
         tenant = env.get('PATH_INFO').split('/')[1]
-        auth_url = self.auth_url
+        auth_url = env.get('HTTP_X_AUTH_URL')
-        if cfg.CONF.auth_password.multi_cloud:
-            auth_url = env.get('HTTP_X_AUTH_URL')
-            error = self._validate_auth_url(env, start_response, auth_url)
-            if error:
-                return error
         if not tenant:
             return self._reject_request(env, start_response, auth_url)
         try:
@@ -70,7 +51,7 @@ class KeystonePasswordAuthProtocol(object):
                 keystone_exceptions.AuthorizationFailure):
             return self._reject_request(env, start_response, auth_url)
         env['keystone.token_info'] = client.auth_ref
-        env.update(self._build_user_headers(client.auth_ref, auth_url))
+        env.update(self._build_user_headers(client.auth_ref))
         return self.app(env, start_response)
 
     def _reject_request(self, env, start_response, auth_url):
@@ -79,7 +60,7 @@ class KeystonePasswordAuthProtocol(object):
         resp = HTTPUnauthorized('Authentication required', headers)
         return resp(env, start_response)
 
-    def _build_user_headers(self, token_info, auth_url):
+    def _build_user_headers(self, token_info):
         """Build headers that represent authenticated user from auth token."""
         tenant_id = token_info['token']['tenant']['id']
         tenant_name = token_info['token']['tenant']['name']
@@ -99,7 +80,6 @@ class KeystonePasswordAuthProtocol(object):
             'HTTP_X_ROLES': roles,
             'HTTP_X_SERVICE_CATALOG': service_catalog,
             'HTTP_X_AUTH_TOKEN': auth_token,
-            'HTTP_X_AUTH_URL': auth_url,
             # DEPRECATED
             'HTTP_X_USER': user_name,
             'HTTP_X_TENANT_ID': tenant_id,
@@ -110,20 +90,6 @@ class KeystonePasswordAuthProtocol(object):
 
         return headers
 
-    def _validate_auth_url(self, env, start_response, auth_url):
-        """Validate auth_url to ensure it can be used."""
-        if not auth_url:
-            resp = HTTPBadRequest(_('Request missing required header '
-                                    'X-Auth-Url'))
-            return resp(env, start_response)
-        allowed = cfg.CONF.auth_password.allowed_auth_uris
-        if auth_url not in allowed:
-            resp = HTTPUnauthorized(_('Header X-Auth-Url "%s" not an allowed '
-                                      'endpoint')
-                                    % auth_url)
-            return resp(env, start_response)
-        return None
-
 
 def filter_factory(global_conf, **local_conf):
     """Returns a WSGI filter app for use with paste.deploy."""

heat/common/auth_url.py

@@ -14,6 +14,8 @@
 # limitations under the License.
 
 from oslo.config import cfg
+from webob.exc import HTTPBadRequest
+from webob.exc import HTTPUnauthorized
 
 from heat.common import wsgi
 from heat.openstack.common import importutils
@@ -35,8 +37,25 @@ class AuthUrlFilter(wsgi.Middleware):
             importutils.import_module(auth_token_module)
             return cfg.CONF.keystone_authtoken.auth_uri
 
+    def _validate_auth_url(self, auth_url):
+        """Validate auth_url to ensure it can be used."""
+        if not auth_url:
+            raise HTTPBadRequest(_('Request missing required header '
+                                   'X-Auth-Url'))
+        allowed = cfg.CONF.auth_password.allowed_auth_uris
+        if auth_url not in allowed:
+            raise HTTPUnauthorized(_('Header X-Auth-Url "%s" not an allowed '
+                                     'endpoint')
+                                   % auth_url)
+        return True
+
     def process_request(self, req):
-        req.headers['X-Auth-Url'] = self.auth_url
+        auth_url = self.auth_url
+        if cfg.CONF.auth_password.multi_cloud:
+            auth_url = req.headers.get('X-Auth-Url')
+            self._validate_auth_url(auth_url)
+
+        req.headers['X-Auth-Url'] = auth_url
         return None
 
 

heat/tests/test_auth_password.py

@@ -17,7 +17,6 @@
 
 from keystoneclient.v2_0 import client as keystone_client
 from keystoneclient.exceptions import Unauthorized
-from oslo.config import cfg
 import webob
 
 from heat.common.auth_password import KeystonePasswordAuthProtocol
@@ -98,6 +97,7 @@ class KeystonePasswordAuthProtocolTest(HeatTestCase):
         req = webob.Request.blank('/tenant_id1/')
         req.headers['X_AUTH_USER'] = 'user_name1'
         req.headers['X_AUTH_KEY'] = 'goodpassword'
+        req.headers['X_AUTH_URL'] = self.config['auth_uri']
         self.middleware(req.environ, self._start_fake_response)
         self.m.VerifyAll()
 
@@ -112,6 +112,7 @@ class KeystonePasswordAuthProtocolTest(HeatTestCase):
         req = webob.Request.blank('/tenant_id1/')
         req.headers['X_AUTH_USER'] = 'user_name1'
         req.headers['X_AUTH_KEY'] = 'badpassword'
+        req.headers['X_AUTH_URL'] = self.config['auth_uri']
         self.middleware(req.environ, self._start_fake_response)
         self.m.VerifyAll()
         self.assertEqual(401, self.response_status)
@@ -120,62 +121,3 @@ class KeystonePasswordAuthProtocolTest(HeatTestCase):
         req = webob.Request.blank('/')
         self.middleware(req.environ, self._start_fake_response)
         self.assertEqual(401, self.response_status)
-
-    def test_multi_cloud(self):
-        allowed_auth_uris = ['http://multicloud.test.com:5000/v2.0']
-        cfg.CONF.set_override('multi_cloud', True, group='auth_password')
-        auth_url = 'http://multicloud.test.com:5000/v2.0'
-        cfg.CONF.set_override('allowed_auth_uris',
-                              allowed_auth_uris,
-                              group='auth_password')
-        self.app = FakeApp(
-            expected_env={'HTTP_X_AUTH_URL': auth_url})
-        self.middleware = KeystonePasswordAuthProtocol(self.app, self.config)
-
-        mock_client = self.m.CreateMock(keystone_client.Client)
-        self.m.StubOutWithMock(keystone_client, 'Client')
-        keystone_client.Client(
-            username='user_name1', password='goodpassword',
-            tenant_id='tenant_id1', auth_url=auth_url).AndReturn(mock_client)
-        mock_client.auth_ref = TOKEN_RESPONSE
-        self.m.ReplayAll()
-        req = webob.Request.blank('/tenant_id1/')
-        req.headers['X_AUTH_USER'] = 'user_name1'
-        req.headers['X_AUTH_KEY'] = 'goodpassword'
-        req.headers['X_AUTH_URL'] = auth_url
-        self.middleware(req.environ, self._start_fake_response)
-        self.m.VerifyAll()
-
-    def test_multi_cloud_empty_allowed_uris(self):
-        cfg.CONF.set_override('multi_cloud', True, group='auth_password')
-        auth_url = 'http://multicloud.test.com:5000/v2.0'
-        cfg.CONF.set_override('allowed_auth_uris',
-                              [],
-                              group='auth_password')
-        req = webob.Request.blank('/tenant_id1/')
-        req.headers['X_AUTH_USER'] = 'user_name1'
-        req.headers['X_AUTH_KEY'] = 'goodpassword'
-        req.headers['X_AUTH_URL'] = auth_url
-        self.middleware(req.environ, self._start_fake_response)
-        self.assertEqual(401, self.response_status)
-
-    def test_multi_cloud_target_not_allowed(self):
-        cfg.CONF.set_override('multi_cloud', True, group='auth_password')
-        auth_url = 'http://multicloud.test.com:5000/v2.0'
-        cfg.CONF.set_override('allowed_auth_uris',
-                              ['http://some.other.url:5000/v2.0'],
-                              group='auth_password')
-        req = webob.Request.blank('/tenant_id1/')
-        req.headers['X_AUTH_USER'] = 'user_name1'
-        req.headers['X_AUTH_KEY'] = 'goodpassword'
-        req.headers['X_AUTH_URL'] = auth_url
-        self.middleware(req.environ, self._start_fake_response)
-        self.assertEqual(401, self.response_status)
-
-    def test_multi_cloud_no_auth_url(self):
-        cfg.CONF.set_override('multi_cloud', True, group='auth_password')
-        req = webob.Request.blank('/tenant_id1/')
-        req.headers['X_AUTH_USER'] = 'user_name1'
-        req.headers['X_AUTH_KEY'] = 'goodpassword'
-        self.middleware(req.environ, self._start_fake_response)
-        self.assertEqual(400, self.response_status)

heat/tests/test_auth_url.py

@@ -36,6 +36,7 @@ class AuthUrlFilterTest(HeatTestCase):
         super(AuthUrlFilterTest, self).setUp()
         self.app = FakeApp()
         self.config = {'auth_uri': 'foobar'}
+        self.middleware = auth_url.AuthUrlFilter(self.app, self.config)
 
     @mock.patch.object(auth_url.cfg, 'CONF')
     def test_adds_default_auth_url_from_keystone_authtoken(self, mock_cfg):
@@ -49,15 +50,56 @@ class AuthUrlFilterTest(HeatTestCase):
         self.assertEqual('foobar', req.headers['X-Auth-Url'])
 
     def test_overwrites_auth_url_from_headers_with_local_config(self):
-        self.middleware = auth_url.AuthUrlFilter(self.app, self.config)
         req = webob.Request.blank('/tenant_id/')
         req.headers['X-Auth-Url'] = 'should_be_overwritten'
         self.middleware(req)
         self.assertEqual('foobar', req.headers['X-Auth-Url'])
 
     def test_reads_auth_url_from_local_config(self):
-        self.middleware = auth_url.AuthUrlFilter(self.app, self.config)
         req = webob.Request.blank('/tenant_id/')
         self.middleware(req)
         self.assertIn('X-Auth-Url', req.headers)
         self.assertEqual('foobar', req.headers['X-Auth-Url'])
+
+    @mock.patch.object(auth_url.AuthUrlFilter, '_validate_auth_url')
+    @mock.patch.object(auth_url.cfg, 'CONF')
+    def test_multicloud_reads_auth_url_from_headers(self, mock_cfg, mock_val):
+        mock_cfg.auth_password.multi_cloud = True
+        mock_val.return_value = True
+        req = webob.Request.blank('/tenant_id/')
+        req.headers['X-Auth-Url'] = 'overwrites config'
+        self.middleware(req)
+        self.assertIn('X-Auth-Url', req.headers)
+        self.assertEqual('overwrites config', req.headers['X-Auth-Url'])
+
+    @mock.patch.object(auth_url.AuthUrlFilter, '_validate_auth_url')
+    @mock.patch.object(auth_url.cfg, 'CONF')
+    def test_multicloud_validates_auth_url(self, mock_cfg, mock_validate):
+        mock_cfg.auth_password.multi_cloud = True
+        req = webob.Request.blank('/tenant_id/')
+
+        self.middleware(req)
+        self.assertTrue(mock_validate.called)
+
+    def test_validate_auth_url_with_missing_url(self):
+        self.assertRaises(auth_url.HTTPBadRequest,
+                          self.middleware._validate_auth_url,
+                          auth_url='')
+
+        self.assertRaises(auth_url.HTTPBadRequest,
+                          self.middleware._validate_auth_url,
+                          auth_url=None)
+
+    @mock.patch.object(auth_url.cfg, 'CONF')
+    def test_validate_auth_url_with_url_not_allowed(self, mock_cfg):
+        mock_cfg.auth_password.allowed_auth_uris = ['foobar']
+
+        self.assertRaises(auth_url.HTTPUnauthorized,
+                          self.middleware._validate_auth_url,
+                          auth_url='not foobar')
+
+    @mock.patch.object(auth_url.cfg, 'CONF')
+    def test_validate_auth_url_with_valid_url(self, mock_cfg):
+        mock_cfg.auth_password.allowed_auth_uris = ['foobar']
+
+        self.assertTrue(self.middleware._validate_auth_url('foobar'))