wsgi: fix base_url finding

The current wsgi.Application.base_url() function does not work correctly
if Keystone runs on something like "http://1.2.3.4/identity" which is now
a default in devstack.

This patch fixes that by using wsgiref.util to parse environment
variable set in WSGI mode to find the real base url and returns the
correct URL. The following environment variables will be used to
produce the effective base url:

  HTTP_HOST
  SERVER_NAME
  SERVER_PORT
  SCRIPT_NAME

Closes-Bug: #1381961
Change-Id: I111c206a8a751ed117c6869f55f8236b29ab88a2

keystone/common/utils.py

@@ -577,3 +577,20 @@ def lower_case_hostname(url):
     # Note: _replace method for named tuples is public and defined in docs
     replaced = parsed._replace(netloc=parsed.netloc.lower())
     return moves.urllib.parse.urlunparse(replaced)
+
+
+def remove_standard_port(url):
+    # remove the default ports specified in RFC2616 and 2818
+    o = moves.urllib.parse.urlparse(url)
+    separator = ':'
+    (host, separator, port) = o.netloc.partition(':')
+    if o.scheme.lower() == 'http' and port == '80':
+        # NOTE(gyee): _replace() is not a private method. It has an
+        # an underscore prefix to prevent conflict with field names.
+        # See https://docs.python.org/2/library/collections.html#
+        # collections.namedtuple
+        o = o._replace(netloc=host)
+    if o.scheme.lower() == 'https' and port == '443':
+        o = o._replace(netloc=host)
+
+    return moves.urllib.parse.urlunparse(o)

keystone/common/wsgi.py

@@ -20,6 +20,7 @@
 
 import copy
 import itertools
+import re
 import wsgiref.util
 
 from oslo_config import cfg
@@ -376,13 +377,19 @@ class Application(BaseApplication):
                 itertools.chain(CONF.items(), CONF.eventlet_server.items()))
 
             url = url % substitutions
+        elif 'environment' in context:
+            url = wsgiref.util.application_uri(context['environment'])
+            # remove version from the URL as it may be part of SCRIPT_NAME but
+            # it should not be part of base URL
+            url = re.sub(r'/v(3|(2\.0))/*$', '', url)
+
+            # now remove the standard port
+            url = utils.remove_standard_port(url)
         else:
-            # NOTE(jamielennox): If url is not set via the config file we
+            # if we don't have enough information to come up with a base URL,
-            # should set it relative to the url that the user used to get here
+            # then fall back to localhost. This should never happen in
-            # so as not to mess with version discovery. This is not perfect.
+            # production environment.
-            # host_url omits the path prefix, but there isn't another good
+            url = 'http://localhost:%d' % CONF.eventlet_server.public_port
-            # solution that will work for all urls.
-            url = context['host_url']
 
         return url.rstrip('/')
 
@@ -812,18 +819,15 @@ def render_exception(error, context=None, request=None, user_locale=None):
     if isinstance(error, exception.AuthPluginException):
         body['error']['identity'] = error.authentication
     elif isinstance(error, exception.Unauthorized):
-        url = CONF.public_endpoint
+        # NOTE(gyee): we only care about the request environment in the
-        if not url:
+        # context. Also, its OK to pass the environemt as it is read-only in
-            if request:
+        # Application.base_url()
-                context = {'host_url': request.host_url}
+        local_context = {}
-            if context:
+        if request:
-                url = Application.base_url(context, 'public')
+            local_context = {'environment': request.environ}
-            else:
+        elif context and 'environment' in context:
-                url = 'http://localhost:%d' % CONF.eventlet_server.public_port
+            local_context = {'environment': context['environment']}
-        else:
+        url = Application.base_url(local_context, 'public')
-            substitutions = dict(
-                itertools.chain(CONF.items(), CONF.eventlet_server.items()))
-            url = url % substitutions
 
         headers.append(('WWW-Authenticate', 'Keystone uri="%s"' % url))
     return render_response(status=(error.code, error.title),

keystone/tests/unit/test_auth.py

@@ -14,6 +14,8 @@
 
 import copy
 import datetime
+import random
+import string
 import uuid
 
 import mock
@@ -41,7 +43,9 @@ CONF = cfg.CONF
 TIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%fZ'
 DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id
 
-HOST_URL = 'http://keystone:5001'
+HOST = ''.join(random.choice(string.ascii_lowercase) for x in range(
+    random.randint(5, 15)))
+HOST_URL = 'http://%s' % (HOST)
 
 
 def _build_user_auth(token=None, user_id=None, username=None,
@@ -871,7 +875,16 @@ class AuthWithTrust(AuthTest):
             token_id=token_id,
             token_data=self.token_provider_api.validate_token(token_id))
         auth_context = authorization.token_to_auth_context(token_ref)
-        return {'environment': {authorization.AUTH_CONTEXT_ENV: auth_context},
+        # NOTE(gyee): if public_endpoint and admin_endpoint are not set, which
+        # is the default, the base url will be constructed from the environment
+        # variables wsgi.url_scheme, SERVER_NAME, SERVER_PORT, and SCRIPT_NAME.
+        # We have to set them in the context so the base url can be constructed
+        # accordingly.
+        return {'environment': {authorization.AUTH_CONTEXT_ENV: auth_context,
+                                'wsgi.url_scheme': 'http',
+                                'SCRIPT_NAME': '/v3',
+                                'SERVER_PORT': '80',
+                                'SERVER_NAME': HOST},
                 'token_id': token_id,
                 'host_url': HOST_URL}
 

keystone/tests/unit/test_wsgi.py

@@ -213,7 +213,9 @@ class ApplicationTest(BaseWSGITest):
 
     def test_render_exception_host(self):
         e = exception.Unauthorized(message=u'\u7f51\u7edc')
-        context = {'host_url': 'http://%s:5000' % uuid.uuid4().hex}
+        req = self._make_request(url='/')
+        context = {'host_url': 'http://%s:5000' % uuid.uuid4().hex,
+                   'environment': req.environ}
         resp = wsgi.render_exception(e, context=context)
 
         self.assertEqual(http_client.UNAUTHORIZED, resp.status_int)
@@ -238,6 +240,77 @@ class ApplicationTest(BaseWSGITest):
         self.assertEqual({'name': u'nonexit\xe8nt'},
                          jsonutils.loads(resp.body))
 
+    def test_base_url(self):
+        class FakeApp(wsgi.Application):
+            def index(self, context):
+                return self.base_url(context, 'public')
+        req = self._make_request(url='/')
+        # NOTE(gyee): according to wsgiref, if HTTP_HOST is present in the
+        # request environment, it will be used to construct the base url.
+        # SERVER_NAME and SERVER_PORT will be ignored. These are standard
+        # WSGI environment variables populated by the webserver.
+        req.environ.update({
+            'SCRIPT_NAME': '/identity',
+            'SERVER_NAME': '1.2.3.4',
+            'wsgi.url_scheme': 'http',
+            'SERVER_PORT': '80',
+            'HTTP_HOST': '1.2.3.4',
+        })
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://1.2.3.4/identity", resp.body)
+
+        # if HTTP_HOST is absent, SERVER_NAME and SERVER_PORT will be used
+        req = self._make_request(url='/')
+        del req.environ['HTTP_HOST']
+        req.environ.update({
+            'SCRIPT_NAME': '/identity',
+            'SERVER_NAME': '1.1.1.1',
+            'wsgi.url_scheme': 'http',
+            'SERVER_PORT': '1234',
+        })
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://1.1.1.1:1234/identity", resp.body)
+
+        # make sure keystone normalize the standard HTTP port 80 by stripping
+        # it
+        req = self._make_request(url='/')
+        req.environ.update({'HTTP_HOST': 'foo:80',
+                            'SCRIPT_NAME': '/identity'})
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://foo/identity", resp.body)
+
+        # make sure keystone normalize the standard HTTPS port 443 by stripping
+        # it
+        req = self._make_request(url='/')
+        req.environ.update({'HTTP_HOST': 'foo:443',
+                            'SCRIPT_NAME': '/identity',
+                            'wsgi.url_scheme': 'https'})
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"https://foo/identity", resp.body)
+
+        # make sure non-standard port is preserved
+        req = self._make_request(url='/')
+        req.environ.update({'HTTP_HOST': 'foo:1234',
+                            'SCRIPT_NAME': '/identity'})
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://foo:1234/identity", resp.body)
+
+        # make sure version portion of the SCRIPT_NAME, '/v2.0',  is stripped
+        # from base url
+        req = self._make_request(url='/')
+        req.environ.update({'HTTP_HOST': 'foo:80',
+                            'SCRIPT_NAME': '/bar/identity/v2.0'})
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://foo/bar/identity", resp.body)
+
+        # make sure version portion of the SCRIPT_NAME, '/v3' is stripped from
+        # base url
+        req = self._make_request(url='/')
+        req.environ.update({'HTTP_HOST': 'foo:80',
+                            'SCRIPT_NAME': '/identity/v3'})
+        resp = req.get_response(FakeApp())
+        self.assertEqual(b"http://foo/identity", resp.body)
+
 
 class ExtensionRouterTest(BaseWSGITest):
     def test_extensionrouter_local_config(self):