From 409b482253dec248ed828e92e52b09d4c02e51dd Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Fri, 29 Sep 2017 16:16:29 +0200 Subject: [PATCH] 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 --- doc/source/middlewarearchitecture.rst | 3 +- keystonemiddleware/auth_token/__init__.py | 15 ++++++---- keystonemiddleware/auth_token/_identity.py | 10 ++++--- keystonemiddleware/auth_token/_opts.py | 20 ++++++++++++- keystonemiddleware/s3_token.py | 13 ++++++-- .../auth_token/test_auth_token_middleware.py | 27 +++++++++-------- .../tests/unit/auth_token/test_cache.py | 2 +- .../tests/unit/auth_token/test_config.py | 12 ++++---- keystonemiddleware/tests/unit/test_opts.py | 2 ++ .../tests/unit/test_s3_token_middleware.py | 30 +++++++++++++++++-- .../rename-auth-uri-d223d883f5898aee.yaml | 9 ++++++ 11 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/rename-auth-uri-d223d883f5898aee.yaml diff --git a/doc/source/middlewarearchitecture.rst b/doc/source/middlewarearchitecture.rst index 2df21eb5..28ff0c67 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 6cb1f93c..8e4d0424 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 88d7f620..d02dcfc3 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 488a4824..7e687950 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 be444739..d8d8e78b 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 2a3a66d3..a130b199 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 df677bf7..6fa1ef2d 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 8cfa35d4..6b824afe 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 18c40463..5768011d 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 bb8fcc39..f9857a9f 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 00000000..1c3e3fa4 --- /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.