From c29ff68a6f91d020ee248f70ba6f2a7a2801013b Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Mon, 6 Jul 2015 19:51:48 -0500 Subject: [PATCH] AuthContextMiddleware admin token handling The AuthContextMiddleware shouldn't be re-implementing the AdminTokenAuthMiddleware but using the request environment context setting that the admin token middleware sets up. This makes it so that admin token handling is in one place rather than duplicating it and allows for an alternative implementation of the admin token middleware. The old behavior is left in place as deprecated to be removed in a future release. ReleaseNotesImpact - The paste.ini file is changed to put the admin token middleware first so that the auth context middleware can use the results. It's deprecated to have the admin token middleware after the auth context middleware. Closes-Bug: 1473553 Change-Id: I658213699ac4af0abd08f893d9cf18ef0af5827d --- keystone/middleware/auth.py | 81 +++++++++++++------ keystone/tests/unit/test_v3.py | 23 +++++- .../notes/admin_token-a5678d712783c145.yaml | 14 ++++ 3 files changed, 90 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/admin_token-a5678d712783c145.yaml diff --git a/keystone/middleware/auth.py b/keystone/middleware/auth.py index d3fc50f82a..909675243f 100644 --- a/keystone/middleware/auth.py +++ b/keystone/middleware/auth.py @@ -13,6 +13,7 @@ from oslo_config import cfg from oslo_context import context as oslo_context from oslo_log import log +from oslo_log import versionutils from keystone.common import authorization from keystone.common import tokenless_auth @@ -34,14 +35,55 @@ __all__ = ('AuthContextMiddleware',) class AuthContextMiddleware(wsgi.Middleware): """Build the authentication context from the request auth token.""" - def _build_auth_context(self, request, token_id): + def _build_auth_context(self, request): + + # NOTE(gyee): token takes precedence over SSL client certificates. + # This will preserve backward compatibility with the existing + # behavior. Tokenless authorization with X.509 SSL client + # certificate is effectively disabled if no trusted issuers are + # provided. + + token_id = None + if core.AUTH_TOKEN_HEADER in request.headers: + token_id = request.headers[core.AUTH_TOKEN_HEADER].strip() + + is_admin = request.environ.get(core.CONTEXT_ENV, {}).get('is_admin', + False) + if is_admin: + # NOTE(gyee): no need to proceed any further as we already know + # this is an admin request. + auth_context = {} + return auth_context, token_id, is_admin + + if token_id: + # In this case the client sent in a token. + auth_context, is_admin = self._build_token_auth_context( + request, token_id) + return auth_context, token_id, is_admin + + # No token, maybe the client presented an X.509 certificate. + + if self._validate_trusted_issuer(request.environ): + auth_context = self._build_tokenless_auth_context( + request.environ) + return auth_context, None, False + + LOG.debug('There is either no auth token in the request or ' + 'the certificate issuer is not trusted. No auth ' + 'context will be set.') + + return None, None, False + + def _build_token_auth_context(self, request, token_id): if token_id == CONF.admin_token: - # NOTE(gyee): no need to proceed any further as the special admin - # token is being handled by AdminTokenAuthMiddleware. This code - # will not be impacted even if AdminTokenAuthMiddleware is removed - # from the pipeline as "is_admin" is default to "False". This code - # is independent of AdminTokenAuthMiddleware. - return {} + versionutils.report_deprecated_feature( + LOG, + _LW('Auth context checking for the admin token is deprecated ' + 'as of the Mitaka release and will be removed in the O ' + 'release. Update keystone-paste.ini so that ' + 'admin_token_auth is before build_auth_context in the ' + 'paste pipelines.')) + return {}, True context = {'token_id': token_id} context['environment'] = request.environ @@ -53,7 +95,7 @@ class AuthContextMiddleware(wsgi.Middleware): # TODO(gyee): validate_token_bind should really be its own # middleware wsgi.validate_token_bind(context, token_ref) - return authorization.token_to_auth_context(token_ref) + return authorization.token_to_auth_context(token_ref), False except exception.TokenNotFound: LOG.warning(_LW('RBAC: Invalid token')) raise exception.Unauthorized() @@ -154,23 +196,13 @@ class AuthContextMiddleware(wsgi.Middleware): LOG.warning(msg) return - # NOTE(gyee): token takes precedence over SSL client certificates. - # This will preserve backward compatibility with the existing - # behavior. Tokenless authorization with X.509 SSL client - # certificate is effectively disabled if no trusted issuers are - # provided. - if core.AUTH_TOKEN_HEADER in request.headers: - token_id = request.headers[core.AUTH_TOKEN_HEADER].strip() - request_context.auth_token = token_id + auth_context, token_id, is_admin = self._build_auth_context(request) - auth_context = self._build_auth_context(request, token_id) - elif self._validate_trusted_issuer(request.environ): - auth_context = self._build_tokenless_auth_context( - request.environ) - else: - LOG.debug('There is either no auth token in the request or ' - 'the certificate issuer is not trusted. No auth ' - 'context will be set.') + request_context.auth_token = token_id + request_context.is_admin = is_admin + + if auth_context is None: + # The client didn't send any auth info, so don't set auth context. return # The attributes of request_context are put into the logs. This is a @@ -182,7 +214,6 @@ class AuthContextMiddleware(wsgi.Middleware): request_context.domain = auth_context.get('domain_id') request_context.user_domain = auth_context.get('user_domain_id') request_context.project_domain = auth_context.get('project_domain_id') - request_context.is_admin = request.environ.get('is_admin', False) LOG.debug('RBAC: auth_context: %s', auth_context) request.environ[authorization.AUTH_CONTEXT_ENV] = auth_context diff --git a/keystone/tests/unit/test_v3.py b/keystone/tests/unit/test_v3.py index e464d88d3a..efd2ad94a8 100644 --- a/keystone/tests/unit/test_v3.py +++ b/keystone/tests/unit/test_v3.py @@ -14,6 +14,7 @@ import uuid +import mock from oslo_config import cfg import oslo_context.context from oslo_serialization import jsonutils @@ -28,6 +29,7 @@ from keystone.common import cache from keystone.common.validation import validators from keystone import exception from keystone import middleware +from keystone.middleware import auth as middleware_auth from keystone.tests.common import auth as common_auth from keystone.tests import unit from keystone.tests.unit import rest @@ -1345,12 +1347,27 @@ class AuthContextMiddlewareAdminTokenTestCase(RestfulTestCase): self.assertEqual('body', resp.text) # just to make sure it worked return resp.request - def test_admin_token_auth_context(self): - # test to make sure AuthContextMiddleware does not attempt to build - # auth context if the incoming auth token is the special admin token + def test_admin_auth_context(self): + # test to make sure AuthContextMiddleware does not attempt to build the + # auth context if the admin_token middleware indicates it's admin + # already. + token_id = uuid.uuid4().hex # token doesn't matter. + # the admin_token middleware sets is_admin in the context. + extra_environ = {middleware.CONTEXT_ENV: {'is_admin': True}} + req = self._middleware_request(token_id, extra_environ) + auth_context = req.environ.get(authorization.AUTH_CONTEXT_ENV) + self.assertDictEqual({}, auth_context) + + @mock.patch.object(middleware_auth.versionutils, + 'report_deprecated_feature') + def test_admin_token_auth_context_deprecated(self, mock_report_deprecated): + # For backwards compatibility AuthContextMiddleware will check that the + # admin token (as configured in the CONF file) is present and not + # attempt to build the auth context. This is deprecated. req = self._middleware_request(CONF.admin_token) auth_context = req.environ.get(authorization.AUTH_CONTEXT_ENV) self.assertDictEqual({}, auth_context) + self.assertEqual(1, mock_report_deprecated.call_count) # NOTE(gyee): test AuthContextMiddleware here instead of test_middleware.py diff --git a/releasenotes/notes/admin_token-a5678d712783c145.yaml b/releasenotes/notes/admin_token-a5678d712783c145.yaml new file mode 100644 index 0000000000..5fb2ef7f35 --- /dev/null +++ b/releasenotes/notes/admin_token-a5678d712783c145.yaml @@ -0,0 +1,14 @@ +--- +upgrade: + - > + [`bug 1473553 `_] + The ``keystone-paste.ini`` must be updated to put the ``admin_token_auth`` + middleware before ``build_auth_context``. See the sample + ``keystone-paste.ini`` for the correct ``pipeline`` value. Having + ``admin_token_auth`` after ``build_auth_context`` is deprecated and will + not be supported in a future release. +deprecations: + - > + [`blueprint deprecated-as-of-mitaka `_] + The ``admin_token_auth`` filter must now be placed before the + ``build_auth_context`` filter in ``keystone-paste.ini``.