Rename auth_uri to www_authenticate_uri

The [keystone_authtoken]/auth_uri middleware parameter has been causing
extreme confusion amongst operators and developers ever since the
keystonemiddleware started accepting keystoneauth plugin parameters
including auth_url. The two parameters look identical and yet have
completely different meanings and are both required. This patch
deprecates auth_uri and renames it to www_authenticate_uri, which more
accurately describes the WWW-Authenticate header it is configuring and
is dissimilar to any other keystone_authtoken middleware parameter. This
also renames the internal variable names for consistency with the config
option.

Change-Id: I0cf11da3d395749df28077427689fdafc8a6b981

11 files changed, 106 insertions, 37 deletions

diff --git a/doc/source/middlewarearchitecture.rst b/doc/source/middlewarearchitecture.rst
index 2df21eb..28ff0c6 100644
--- a/doc/source/middlewarearchitecture.rst
+++ b/doc/source/middlewarearchitecture.rst
@@ -261,7 +261,8 @@ swift/cloud files and for legacy Rackspace use. If the token isn't present and
 the middleware is configured to not delegate auth responsibility, it will
 respond to the HTTP request with HTTPUnauthorized, returning the header
 ``WWW-Authenticate`` with the value `Keystone uri='...'` to indicate where to
-request a token. The auth_uri returned is configured  with the middleware.
+request a token. The URI returned is configured with the
+``www_authenticate_uri`` option.
 
 The authentication middleware extends the HTTP request with the header
 ``X-Identity-Status``.  If a request is successfully authenticated, the value
diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py
index 6cb1f93..8e4d042 100644
--- a/keystonemiddleware/auth_token/__init__.py
+++ b/keystonemiddleware/auth_token/__init__.py
@@ -583,17 +583,20 @@ class AuthProtocol(BaseAuthProtocol):
         self._session = self._create_session()
         self._identity_server = self._create_identity_server()
 
-        self._auth_uri = self._conf.get('auth_uri')
-        if not self._auth_uri:
+        self._www_authenticate_uri = self._conf.get('www_authenticate_uri')
+        if not self._www_authenticate_uri:
+            self._www_authenticate_uri = self._conf.get('auth_uri')
+        if not self._www_authenticate_uri:
             self.log.warning(
-                'Configuring auth_uri to point to the public identity '
-                'endpoint is required; clients may not be able to '
+                'Configuring www_authenticate_uri to point to the public '
+                'identity endpoint is required; clients may not be able to '
                 'authenticate against an admin endpoint')
 
             # FIXME(dolph): drop support for this fallback behavior as
             # documented in bug 1207517.
 
-            self._auth_uri = self._identity_server.auth_uri
+            self._www_authenticate_uri = \
+                self._identity_server.www_authenticate_uri
 
         self._signing_directory = _signing_dir.SigningDirectory(
             directory_name=self._conf.get('signing_dir'), log=self.log)
@@ -687,7 +690,7 @@ class AuthProtocol(BaseAuthProtocol):
 
     @property
     def _reject_auth_headers(self):
-        header_val = 'Keystone uri=\'%s\'' % self._auth_uri
+        header_val = 'Keystone uri=\'%s\'' % self._www_authenticate_uri
         return [('WWW-Authenticate', header_val)]
 
     def _token_hashes(self, token):
diff --git a/keystonemiddleware/auth_token/_identity.py b/keystonemiddleware/auth_token/_identity.py
index 88d7f62..d02dcfc 100644
--- a/keystonemiddleware/auth_token/_identity.py
+++ b/keystonemiddleware/auth_token/_identity.py
@@ -147,16 +147,18 @@ class IdentityServer(object):
         self._request_strategy_obj = None
 
     @property
-    def auth_uri(self):
-        auth_uri = self._adapter.get_endpoint(interface=plugin.AUTH_INTERFACE)
+    def www_authenticate_uri(self):
+        www_authenticate_uri = self._adapter.get_endpoint(
+            interface=plugin.AUTH_INTERFACE)
 
         # NOTE(jamielennox): This weird stripping of the prefix hack is
         # only relevant to the legacy case. We urljoin '/' to get just the
         # base URI as this is the original behaviour.
         if isinstance(self._adapter.auth, _auth.AuthTokenPlugin):
-            auth_uri = urllib.parse.urljoin(auth_uri, '/').rstrip('/')
+            www_authenticate_uri = urllib.parse.urljoin(
+                www_authenticate_uri, '/').rstrip('/')
 
-        return auth_uri
+        return www_authenticate_uri
 
     @property
     def auth_version(self):
diff --git a/keystonemiddleware/auth_token/_opts.py b/keystonemiddleware/auth_token/_opts.py
index 488a482..7e68795 100644
--- a/keystonemiddleware/auth_token/_opts.py
+++ b/keystonemiddleware/auth_token/_opts.py
@@ -28,7 +28,7 @@ from keystonemiddleware.auth_token import _base
 # options via CONF.
 
 _OPTS = [
-    cfg.StrOpt('auth_uri',
+    cfg.StrOpt('www_authenticate_uri',
                # FIXME(dolph): should be default='http://127.0.0.1:5000/v2.0/',
                # or (depending on client support) an unversioned, publicly
                # accessible identity endpoint (see bug 1207517). Further, we
@@ -38,6 +38,7 @@ _OPTS = [
                # This wasn't an option originally when many auth_token
                # deployments were configured with the "ADMIN" token and
                # endpoint combination.
+               deprecated_name='auth_uri',
                help='Complete "public" Identity API endpoint. This endpoint'
                ' should not be an "admin" endpoint, as it should be accessible'
                ' by all end users. Unauthenticated clients are redirected to'
@@ -47,6 +48,23 @@ _OPTS = [
                ' should *not* be the same endpoint the service user utilizes'
                ' for validating tokens, because normal end users may not be'
                ' able to reach that endpoint.'),
+    cfg.StrOpt('auth_uri',
+               deprecated_for_removal=True,
+               deprecated_reason='The auth_uri option is deprecated in favor'
+               ' of www_authenticate_uri and will be removed in the S '
+               ' release.',
+               deprecated_since='Queens',
+               help='Complete "public" Identity API endpoint. This endpoint'
+               ' should not be an "admin" endpoint, as it should be accessible'
+               ' by all end users. Unauthenticated clients are redirected to'
+               ' this endpoint to authenticate. Although this endpoint should'
+               ' ideally be unversioned, client support in the wild varies.'
+               ' If you\'re using a versioned v2 endpoint here, then this'
+               ' should *not* be the same endpoint the service user utilizes'
+               ' for validating tokens, because normal end users may not be'
+               ' able to reach that endpoint. This option is deprecated in'
+               ' favor of www_authenticate_uri and will be removed in the S'
+               ' release.'),
     cfg.StrOpt('auth_version',
                help='API version of the admin Identity API endpoint.'),
     cfg.BoolOpt('delay_auth_decision',
diff --git a/keystonemiddleware/s3_token.py b/keystonemiddleware/s3_token.py
index be44473..d8d8e78 100644
--- a/keystonemiddleware/s3_token.py
+++ b/keystonemiddleware/s3_token.py
@@ -57,13 +57,20 @@ class S3Token(object):
         self._reseller_prefix = conf.get('reseller_prefix', 'AUTH_')
         # where to find the auth service (we use this to validate tokens)
 
-        self._request_uri = conf.get('auth_uri')
+        self._request_uri = conf.get('www_authenticate_uri')
+        auth_uri = conf.get('auth_uri')
+        if not self._request_uri and auth_uri:
+            self._logger.warning(
+                "Use of the auth_uri option was deprecated "
+                "in the Queens release in favor of www_authenticate_uri. This "
+                "option will be removed in the S release.")
+            self._request_uri = auth_uri
         if not self._request_uri:
             self._logger.warning(
                 "Use of the auth_host, auth_port, and auth_protocol "
                 "configuration options was deprecated in the Newton release "
-                "in favor of auth_uri. These options may be removed in a "
-                "future release.")
+                "in favor of www_authenticate_uri. These options will be "
+                "removed in the S release.")
             auth_host = conf.get('auth_host')
             auth_port = int(conf.get('auth_port', 35357))
             auth_protocol = conf.get('auth_protocol', 'https')
diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
index 2a3a66d..a130b19 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
@@ -280,7 +280,7 @@ class BaseAuthTokenMiddlewareTest(base.BaseAuthTokenTestCase):
             'identity_uri': 'https://keystone.example.com:1234/testadmin/',
             'signing_dir': signing_dir,
             'auth_version': auth_version,
-            'auth_uri': 'https://keystone.example.com:1234',
+            'www_authenticate_uri': 'https://keystone.example.com:1234',
             'admin_user': uuid.uuid4().hex,
         }
 
@@ -460,7 +460,7 @@ class GeneralAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
     def test_config_revocation_cache_timeout(self):
         conf = {
             'revocation_cache_time': '24',
-            'auth_uri': 'https://keystone.example.com:1234',
+            'www_authenticate_uri': 'https://keystone.example.com:1234',
             'admin_user': uuid.uuid4().hex
         }
         middleware = auth_token.AuthProtocol(self.fake_app, conf)
@@ -591,12 +591,12 @@ class CommonAuthTokenMiddlewareTest(object):
             'auth_host': '2001:2013:1:f101::1',
             'auth_port': '1234',
             'auth_protocol': 'http',
-            'auth_uri': None,
+            'www_authenticate_uri': None,
             'auth_version': 'v3.0',
         }
         middleware = self.create_simple_middleware(conf=conf)
         self.assertEqual('http://[2001:2013:1:f101::1]:1234',
-                         middleware._auth_uri)
+                         middleware._www_authenticate_uri)
 
     def assert_valid_request_200(self, token, with_catalog=True):
         resp = self.call_middleware(headers={'X-Auth-Token': token})
@@ -1982,10 +1982,10 @@ class DelayedAuthTests(BaseAuthTokenMiddlewareTest):
 
     def test_header_in_401(self):
         body = uuid.uuid4().hex
-        auth_uri = 'http://local.test'
+        www_authenticate_uri = 'http://local.test'
         conf = {'delay_auth_decision': 'True',
                 'auth_version': 'v3.0',
-                'auth_uri': auth_uri}
+                'www_authenticate_uri': www_authenticate_uri}
 
         middleware = self.create_simple_middleware(status='401 Unauthorized',
                                                    body=body,
@@ -1993,11 +1993,11 @@ class DelayedAuthTests(BaseAuthTokenMiddlewareTest):
         resp = self.call(middleware, expected_status=401)
         self.assertEqual(six.b(body), resp.body)
 
-        self.assertEqual("Keystone uri='%s'" % auth_uri,
+        self.assertEqual("Keystone uri='%s'" % www_authenticate_uri,
                          resp.headers['WWW-Authenticate'])
 
     def test_delayed_auth_values(self):
-        conf = {'auth_uri': 'http://local.test'}
+        conf = {'www_authenticate_uri': 'http://local.test'}
         status = '401 Unauthorized'
 
         middleware = self.create_simple_middleware(status=status, conf=conf)
@@ -2005,7 +2005,7 @@ class DelayedAuthTests(BaseAuthTokenMiddlewareTest):
 
         for v in ('True', '1', 'on', 'yes'):
             conf = {'delay_auth_decision': v,
-                    'auth_uri': 'http://local.test'}
+                    'www_authenticate_uri': 'http://local.test'}
 
             middleware = self.create_simple_middleware(status=status,
                                                        conf=conf)
@@ -2013,7 +2013,7 @@ class DelayedAuthTests(BaseAuthTokenMiddlewareTest):
 
         for v in ('False', '0', 'no'):
             conf = {'delay_auth_decision': v,
-                    'auth_uri': 'http://local.test'}
+                    'www_authenticate_uri': 'http://local.test'}
 
             middleware = self.create_simple_middleware(status=status,
                                                        conf=conf)
@@ -2021,8 +2021,11 @@ class DelayedAuthTests(BaseAuthTokenMiddlewareTest):
 
     def test_auth_plugin_with_no_tokens(self):
         body = uuid.uuid4().hex
-        auth_uri = 'http://local.test'
-        conf = {'delay_auth_decision': True, 'auth_uri': auth_uri}
+        www_authenticate_uri = 'http://local.test'
+        conf = {
+            'delay_auth_decision': True,
+            'www_authenticate_uri': www_authenticate_uri
+        }
 
         middleware = self.create_simple_middleware(body=body, conf=conf)
         resp = self.call(middleware)
diff --git a/keystonemiddleware/tests/unit/auth_token/test_cache.py b/keystonemiddleware/tests/unit/auth_token/test_cache.py
index df677bf..6fa1ef2 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_cache.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_cache.py
@@ -80,7 +80,7 @@ class NoMemcacheAuthToken(base.BaseAuthTokenTestCase):
             'auth_host': 'keystone.example.com',
             'auth_port': '1234',
             'memcached_servers': ','.join(MEMCACHED_SERVERS),
-            'auth_uri': 'https://keystone.example.com:1234',
+            'www_authenticate_uri': 'https://keystone.example.com:1234',
         }
 
         self.create_simple_middleware(conf=conf)
diff --git a/keystonemiddleware/tests/unit/auth_token/test_config.py b/keystonemiddleware/tests/unit/auth_token/test_config.py
index 8cfa35d..6b824af 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_config.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_config.py
@@ -36,7 +36,7 @@ class TestAuthPluginLocalOsloConfig(base.BaseAuthTokenTestCase):
         # in the 'keystone_authtoken' group. Additional options, from
         # plugins, are registered dynamically so must not be used here.
         self.oslo_options = {
-            'auth_uri': uuid.uuid4().hex,
+            'www_authenticate_uri': uuid.uuid4().hex,
             'identity_uri': uuid.uuid4().hex,
         }
 
@@ -56,14 +56,14 @@ class TestAuthPluginLocalOsloConfig(base.BaseAuthTokenTestCase):
 
         self.file_options = {
             'auth_type': 'password',
-            'auth_uri': uuid.uuid4().hex,
+            'www_authenticate_uri': uuid.uuid4().hex,
             'password': uuid.uuid4().hex,
         }
 
         content = ("[keystone_authtoken]\n"
                    "auth_type=%(auth_type)s\n"
-                   "auth_uri=%(auth_uri)s\n"
-                   "auth_url=%(auth_uri)s\n"
+                   "www_authenticate_uri=%(www_authenticate_uri)s\n"
+                   "auth_url=%(www_authenticate_uri)s\n"
                    "password=%(password)s\n" % self.file_options)
 
         self.conf_file_fixture = self.useFixture(
@@ -108,5 +108,5 @@ class TestAuthPluginLocalOsloConfig(base.BaseAuthTokenTestCase):
         for option in self.oslo_options:
             self.assertEqual(self.oslo_options[option],
                              conf_get(app, option))
-        self.assertNotEqual(self.file_options['auth_uri'],
-                            conf_get(app, 'auth_uri'))
+        self.assertNotEqual(self.file_options['www_authenticate_uri'],
+                            conf_get(app, 'www_authenticate_uri'))
diff --git a/keystonemiddleware/tests/unit/test_opts.py b/keystonemiddleware/tests/unit/test_opts.py
index 18c4046..5768011 100644
--- a/keystonemiddleware/tests/unit/test_opts.py
+++ b/keystonemiddleware/tests/unit/test_opts.py
@@ -35,6 +35,7 @@ class OptsTestCase(utils.TestCase):
             'auth_host',
             'auth_port',
             'auth_protocol',
+            'www_authenticate_uri',
             'auth_uri',
             'identity_uri',
             'auth_version',
@@ -86,6 +87,7 @@ class OptsTestCase(utils.TestCase):
 
         # This is the sample config generator list WITHOUT deprecations
         expected_opt_names = [
+            'www_authenticate_uri',
             'auth_uri',
             'auth_version',
             'delay_auth_decision',
diff --git a/keystonemiddleware/tests/unit/test_s3_token_middleware.py b/keystonemiddleware/tests/unit/test_s3_token_middleware.py
index bb8fcc3..f9857a9 100644
--- a/keystonemiddleware/tests/unit/test_s3_token_middleware.py
+++ b/keystonemiddleware/tests/unit/test_s3_token_middleware.py
@@ -12,12 +12,14 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import fixtures
 import mock
 from oslo_serialization import jsonutils
 import requests
 from requests_mock.contrib import fixture as rm_fixture
 import six
 from six.moves import urllib
+from testtools import matchers
 import webob
 
 from keystonemiddleware import s3_token
@@ -39,14 +41,14 @@ class FakeApp(object):
 
 class S3TokenMiddlewareTestBase(utils.TestCase):
 
-    TEST_AUTH_URI = 'https://fakehost/identity'
-    TEST_URL = '%s/v2.0/s3tokens' % (TEST_AUTH_URI, )
+    TEST_WWW_AUTHENTICATE_URI = 'https://fakehost/identity'
+    TEST_URL = '%s/v2.0/s3tokens' % (TEST_WWW_AUTHENTICATE_URI, )
 
     def setUp(self):
         super(S3TokenMiddlewareTestBase, self).setUp()
 
         self.conf = {
-            'auth_uri': self.TEST_AUTH_URI,
+            'www_authenticate_uri': self.TEST_WWW_AUTHENTICATE_URI,
         }
 
         self.requests_mock = self.useFixture(rm_fixture.Fixture())
@@ -225,3 +227,25 @@ class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase):
         s3_invalid_req = self.middleware._deny_request('InvalidURI')
         self.assertEqual(resp.body, s3_invalid_req.body)
         self.assertEqual(resp.status_int, s3_invalid_req.status_int)
+
+
+class S3TokenMiddlewareTestDeprecatedOptions(S3TokenMiddlewareTestBase):
+    def setUp(self):
+        super(S3TokenMiddlewareTestDeprecatedOptions, self).setUp()
+        self.conf = {
+            'auth_uri': self.TEST_WWW_AUTHENTICATE_URI,
+        }
+        self.logger = self.useFixture(fixtures.FakeLogger())
+        self.middleware = s3_token.S3Token(FakeApp(), self.conf)
+
+        self.requests_mock.post(self.TEST_URL,
+                                status_code=201,
+                                json=GOOD_RESPONSE)
+
+    def test_logs_warning(self):
+        req = webob.Request.blank('/')
+        self.middleware(req.environ, self.start_fake_response)
+        self.assertEqual(self.response_status, 200)
+        log = "Use of the auth_uri option was deprecated in the Queens " \
+            "release in favor of www_authenticate_uri."
+        self.assertThat(self.logger.output, matchers.Contains(log))
diff --git a/releasenotes/notes/rename-auth-uri-d223d883f5898aee.yaml b/releasenotes/notes/rename-auth-uri-d223d883f5898aee.yaml
new file mode 100644
index 0000000..1c3e3fa
--- /dev/null
+++ b/releasenotes/notes/rename-auth-uri-d223d883f5898aee.yaml
@@ -0,0 +1,9 @@
+---
+deprecations:
+  - |
+    The auth_uri parameter of keystone_authtoken is deprecated in favor of
+    www_authenticate_uri. The auth_uri option was often confused with the
+    auth_url parameter of the keystoneauth plugin, which was also effectively
+    always required. The parameter refers to the WWW-Authenticate header that is
+    returned when the user needs to be redirected to the Identity service for
+    authentication.