From 1d7c96d6a3e839dc64ae590da08cbee4b2108273 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Fri, 8 Jul 2016 15:55:39 +1000 Subject: [PATCH] Require auth_context middleware in the pipeline Keystone has shipped with the auth_context middleware in the pipeline now for a number of releases. In newer parts of the code we rely on it for auth information and in others we don't. A prime example of this is the @protected handlers, if the auth_context middleware isn't present it rebuilds this information. This is yet another place where keystone validates a token. Set a flag on the context to indicate that the request was validated by auth_context middleware and raise Unauthenticated if not. This essentially requires deployments to have the middleware enabled and reduces validation to that point. This will help unify the code that is behind a protected endpoint as we can rely on the context. Change-Id: Iceca967e5bf698df13da28d0875c441f2a980e90 --- keystone/common/context.py | 1 + keystone/common/controller.py | 31 +++++------------------------ keystone/common/request.py | 19 +++++++++++++++++- keystone/middleware/auth.py | 4 ++++ keystone/tests/unit/core.py | 3 ++- keystone/tests/unit/test_v3_auth.py | 17 ---------------- 6 files changed, 30 insertions(+), 45 deletions(-) diff --git a/keystone/common/context.py b/keystone/common/context.py index d6c7f9e2dc..240efc21eb 100644 --- a/keystone/common/context.py +++ b/keystone/common/context.py @@ -29,6 +29,7 @@ class RequestContext(oslo_context.RequestContext): self.username = kwargs.pop('username', None) self.user_domain_name = kwargs.pop('user_domain_name', None) self.project_domain_name = kwargs.pop('project_domain_name', None) + self.authenticated = kwargs.pop('authenticated', False) super(RequestContext, self).__init__(**kwargs) @classmethod diff --git a/keystone/common/controller.py b/keystone/common/controller.py index af1d8abed9..352b785ddb 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -79,32 +79,7 @@ def _build_policy_check_credentials(self, action, context, kwargs): 'action': action, 'kwargs': kwargs_str}) - # see if auth context has already been created. If so use it. - if ('environment' in context and - authorization.AUTH_CONTEXT_ENV in context['environment']): - LOG.debug('RBAC: using auth context from the request environment') - return context['environment'].get(authorization.AUTH_CONTEXT_ENV) - - # There is no current auth context, build it from the incoming token. - # TODO(morganfainberg): Collapse this logic with AuthContextMiddleware - # in a sane manner as this just mirrors the logic in AuthContextMiddleware - try: - LOG.debug('RBAC: building auth context from the incoming auth token') - token_ref = token_model.KeystoneToken( - token_id=context['token_id'], - token_data=self.token_provider_api.validate_token( - context['token_id'])) - # NOTE(jamielennox): whilst this maybe shouldn't be within this - # function it would otherwise need to reload the token_ref from - # backing store. - wsgi.validate_token_bind(context, token_ref) - except exception.TokenNotFound: - LOG.warning(_LW('RBAC: Invalid token')) - raise exception.Unauthorized() - - auth_context = authorization.token_to_auth_context(token_ref) - - return auth_context + return context['environment'].get(authorization.AUTH_CONTEXT_ENV, {}) def protected(callback=None): @@ -123,6 +98,8 @@ def protected(callback=None): def wrapper(f): @functools.wraps(f) def inner(self, request, *args, **kwargs): + request.assert_authenticated() + if request.context.is_admin: LOG.warning(_LW('RBAC: Bypassing authorization')) elif callback is not None: @@ -205,6 +182,8 @@ def filterprotected(*filters, **callback): def _filterprotected(f): @functools.wraps(f) def wrapper(self, request, **kwargs): + request.assert_authenticated() + if not request.context.is_admin: # The target dict for the policy check will include: # diff --git a/keystone/common/request.py b/keystone/common/request.py index acff09a30f..37f6dd14a1 100644 --- a/keystone/common/request.py +++ b/keystone/common/request.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + import webob from webob.descriptors import environ_getter @@ -17,13 +19,14 @@ from keystone.common import authorization from keystone.common import context import keystone.conf from keystone import exception -from keystone.i18n import _ +from keystone.i18n import _, _LW # Environment variable used to pass the request context CONTEXT_ENV = 'openstack.context' CONF = keystone.conf.CONF +LOG = logging.getLogger(__name__) class Request(webob.Request): @@ -73,6 +76,20 @@ class Request(webob.Request): def auth_context(self): return self.environ.get(authorization.AUTH_CONTEXT_ENV, {}) + def assert_authenticated(self): + """Ensure that the current request has been authenticated.""" + if not self.context: + LOG.warning(_LW('An authenticated call was made and there is ' + 'no request.context. This means the ' + 'auth_context middleware is not in place. You ' + 'must have this middleware in your pipeline ' + 'to perform authenticated calls')) + raise exception.Unauthorized() + + if not self.context.authenticated: + # auth_context didn't decode anything we can use + raise exception.Unauthorized() + auth_type = environ_getter('AUTH_TYPE', None) remote_domain = environ_getter('REMOTE_DOMAIN', None) context = environ_getter(context.REQUEST_CONTEXT_ENV, None) diff --git a/keystone/middleware/auth.py b/keystone/middleware/auth.py index c32a05be7f..e65671c32f 100644 --- a/keystone/middleware/auth.py +++ b/keystone/middleware/auth.py @@ -154,6 +154,7 @@ class AuthContextMiddleware(auth_token.BaseAuthProtocol): # The request context stores itself in thread-local memory for logging. request_context = context.RequestContext( request_id=request.environ.get('openstack.request_id'), + authenticated=False, overwrite=True) request.environ[context.REQUEST_CONTEXT_ENV] = request_context @@ -203,6 +204,9 @@ class AuthContextMiddleware(auth_token.BaseAuthProtocol): 'context will be set.') return + # set authenticated to flag to keystone that a token has been validated + request_context.authenticated = True + # The attributes of request_context are put into the logs. This is a # common pattern for all the OpenStack services. In all the other # projects these are IDs, so set the attributes to IDs here rather than diff --git a/keystone/tests/unit/core.py b/keystone/tests/unit/core.py index 09dfb27002..b2085fc899 100644 --- a/keystone/tests/unit/core.py +++ b/keystone/tests/unit/core.py @@ -594,7 +594,8 @@ class TestCase(BaseTestCase): if not environ.get(context.REQUEST_CONTEXT_ENV): environ[context.REQUEST_CONTEXT_ENV] = context.RequestContext( - is_admin=is_admin) + is_admin=is_admin, + authenticated=kwargs.pop('authenticated', True)) req = request.Request.blank(path=path, **kwargs) req.context_dict['is_admin'] = is_admin diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py index 2fbaa01276..33a6fb4e67 100644 --- a/keystone/tests/unit/test_v3_auth.py +++ b/keystone/tests/unit/test_v3_auth.py @@ -4699,23 +4699,6 @@ class TestTrustChain(test_v3.RestfulTestCase): expected_status=http_client.FORBIDDEN) -class TestAPIProtectionWithoutAuthContextMiddleware(test_v3.RestfulTestCase): - def test_api_protection_with_no_auth_context_in_env(self): - auth_data = self.build_authentication_request( - user_id=self.default_domain_user['id'], - password=self.default_domain_user['password'], - project_id=self.project['id']) - token = self.get_requested_token(auth_data) - auth_controller = auth.controllers.Auth() - # all we care is that auth context is not in the environment and - # 'token_id' is used to build the auth context instead - request = self.make_request() - request.context_dict['subject_token_id'] = token - request.context_dict['token_id'] = token - r = auth_controller.validate_token(request) - self.assertEqual(http_client.OK, r.status_code) - - class TestAuthContext(unit.TestCase): def setUp(self): super(TestAuthContext, self).setUp()