From 7d8f2fcfb9a63b23a7c809f1cbb7fd0eddfa2fac Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 14 Oct 2016 12:07:03 -0400 Subject: [PATCH] Refactor Authorization: Merged the decorator logic into a single function that calls check_policy. Moved authorization logic in authorize.py Prep for oslo-context fixes Change-Id: Id1452db131740c68b3756a819792f9a60cff239c --- keystone/assignment/controllers.py | 6 +- keystone/auth/controllers.py | 5 +- keystone/common/authorization.py | 153 +++++++++++++++++++++ keystone/common/controller.py | 197 +++++++--------------------- keystone/common/utils.py | 19 --- keystone/common/wsgi.py | 26 +--- keystone/contrib/ec2/controllers.py | 3 +- keystone/oauth1/controllers.py | 6 +- 8 files changed, 213 insertions(+), 202 deletions(-) diff --git a/keystone/assignment/controllers.py b/keystone/assignment/controllers.py index 64dcfc5f40..c6ac2243a6 100644 --- a/keystone/assignment/controllers.py +++ b/keystone/assignment/controllers.py @@ -22,9 +22,9 @@ from oslo_log import log from six.moves import urllib from keystone.assignment import schema +from keystone.common import authorization from keystone.common import controller from keystone.common import dependency -from keystone.common import utils from keystone.common import validation from keystone.common import wsgi import keystone.conf @@ -50,7 +50,7 @@ class TenantAssignment(controller.V2Controller): Doesn't care about token scopedness. """ - token_ref = utils.get_token_ref(request.context_dict) + token_ref = authorization.get_token_ref(request.context_dict) tenant_refs = ( self.assignment_api.list_projects_for_user(token_ref.user_id)) @@ -964,7 +964,7 @@ class RoleAssignmentV3(controller.V3Controller): """ ref = {} - for filter, value in protection_info['filter_attr'].items(): + for filter, value in protection_info.get('filter_attr', {}).items(): if filter == 'scope.project.id' and value: ref['project'] = self.resource_api.get_project(value) diff --git a/keystone/auth/controllers.py b/keystone/auth/controllers.py index 7f711686c7..1cc47d8cf8 100644 --- a/keystone/auth/controllers.py +++ b/keystone/auth/controllers.py @@ -19,6 +19,7 @@ import six from keystone.auth import core from keystone.auth import schema +from keystone.common import authorization from keystone.common import controller from keystone.common import dependency from keystone.common import utils @@ -298,7 +299,7 @@ class Auth(controller.V3Controller): @controller.protected() def check_token(self, request): token_id = request.context_dict.get('subject_token_id') - window_seconds = self._token_validation_window(request) + window_seconds = authorization.token_validation_window(request) token_data = self.token_provider_api.validate_token( token_id, window_seconds=window_seconds) # NOTE(morganfainberg): The code in @@ -314,7 +315,7 @@ class Auth(controller.V3Controller): @controller.protected() def validate_token(self, request): token_id = request.context_dict.get('subject_token_id') - window_seconds = self._token_validation_window(request) + window_seconds = authorization.token_validation_window(request) include_catalog = 'nocatalog' not in request.params token_data = self.token_provider_api.validate_token( token_id, window_seconds=window_seconds) diff --git a/keystone/common/authorization.py b/keystone/common/authorization.py index 0095c8488e..0ddaf26896 100644 --- a/keystone/common/authorization.py +++ b/keystone/common/authorization.py @@ -15,14 +15,19 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import copy from oslo_log import log +from oslo_utils import strutils +from keystone.common import utils +from keystone import conf from keystone import exception from keystone.i18n import _ from keystone.models import token_model +CONF = conf.CONF AUTH_CONTEXT_ENV = 'KEYSTONE_AUTH_CONTEXT' """Environment variable used to convey the Keystone auth context. @@ -111,3 +116,151 @@ def token_to_auth_context(token): auth_context['is_admin_project'] = token.is_admin_project return auth_context + + +def assert_admin(app, request): + """Ensure the user is an admin. + + :raises keystone.exception.Unauthorized: if a token could not be + found/authorized, a user is invalid, or a tenant is + invalid/not scoped. + :raises keystone.exception.Forbidden: if the user is not an admin and + does not have the admin role + + """ + request.assert_authenticated() + + if not request.context.is_admin: + user_token_ref = get_token_ref(request.context_dict) + + creds = copy.deepcopy(user_token_ref.metadata) + + try: + creds['user_id'] = user_token_ref.user_id + except exception.UnexpectedError: + LOG.debug('Invalid user') + raise exception.Unauthorized(_('Invalid user')) + + if user_token_ref.project_scoped: + creds['tenant_id'] = user_token_ref.project_id + else: + LOG.debug('Invalid tenant') + raise exception.Unauthorized(_('Invalid tenant')) + + creds['roles'] = user_token_ref.role_names + # Accept either is_admin or the admin role + app.policy_api.enforce(creds, 'admin_required', {}) + + +def _build_policy_check_credentials(action, context, kwargs): + kwargs_str = ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs]) + kwargs_str = strutils.mask_password(kwargs_str) + msg = 'RBAC: Authorizing %(action)s(%(kwargs)s)' + LOG.debug(msg, {'action': action, 'kwargs': kwargs_str}) + + return context['environment'].get(AUTH_CONTEXT_ENV, {}) + + +def _handle_member_from_driver(self, policy_dict, **kwargs): + # Check to see if we need to include the target entity in our + # policy checks. We deduce this by seeing if the class has + # specified a get_member() method and that kwargs contains the + # appropriate entity id. + if (hasattr(self, 'get_member_from_driver') and + self.get_member_from_driver is not None): + key = '%s_id' % self.member_name + if key in kwargs: + ref = self.get_member_from_driver(kwargs[key]) + policy_dict['target'] = {self.member_name: ref} + + +def token_validation_window(request): + # NOTE(jamielennox): it's dumb that i have to put this here. We should + # only validate subject token in one place. + + allow_expired = request.params.get('allow_expired') + allow_expired = strutils.bool_from_string(allow_expired, default=False) + return CONF.token.allow_expired_window if allow_expired else 0 + + +def _handle_subject_token_id(self, request, policy_dict): + if request.context_dict.get('subject_token_id') is not None: + window_seconds = token_validation_window(request) + + token_ref = token_model.KeystoneToken( + token_id=request.context_dict['subject_token_id'], + token_data=self.token_provider_api.validate_token( + request.context_dict['subject_token_id'], + window_seconds=window_seconds)) + policy_dict.setdefault('target', {}) + policy_dict['target'].setdefault(self.member_name, {}) + policy_dict['target'][self.member_name]['user_id'] = ( + token_ref.user_id) + try: + user_domain_id = token_ref.user_domain_id + except exception.UnexpectedError: + user_domain_id = None + if user_domain_id: + policy_dict['target'][self.member_name].setdefault( + 'user', {}) + policy_dict['target'][self.member_name][ + 'user'].setdefault('domain', {}) + policy_dict['target'][self.member_name]['user'][ + 'domain']['id'] = ( + user_domain_id) + + +def check_protection(controller, request, prep_info, target_attr=None, + *args, **kwargs): + """Provide call protection for complex target attributes. + + As well as including the standard parameters from the original API + call (which is passed in prep_info), this call will add in any + additional entities or attributes (passed in target_attr), so that + they can be referenced by policy rules. + + """ + if request.context.is_admin: + LOG.warning('RBAC: Bypassing authorization') + return + else: + action = 'identity:%s' % prep_info['f_name'] + # TODO(henry-nash) need to log the target attributes as well + creds = _build_policy_check_credentials( + action, request.context_dict, prep_info['input_attr']) + # Build the dict the policy engine will check against from both the + # parameters passed into the call we are protecting (which was + # stored in the prep_info by protected()), plus the target + # attributes provided. + policy_dict = {} + _handle_member_from_driver(controller, policy_dict, **kwargs) + _handle_subject_token_id(controller, request, policy_dict) + + if target_attr: + policy_dict = {'target': target_attr} + policy_dict.update(prep_info['input_attr']) + if 'filter_attr' in prep_info: + policy_dict.update(prep_info['filter_attr']) + + for key in kwargs: + policy_dict[key] = kwargs[key] + controller.policy_api.enforce(creds, + action, + utils.flatten_dict(policy_dict)) + LOG.debug('RBAC: Authorization granted') + + +def get_token_ref(context): + """Retrieve KeystoneToken object from the auth context and returns it. + + :param dict context: The request context. + :raises keystone.exception.Unauthorized: If auth context cannot be found. + :returns: The KeystoneToken object. + """ + try: + # Retrieve the auth context that was prepared by AuthContextMiddleware. + auth_context = (context['environment'][AUTH_CONTEXT_ENV]) + return auth_context['token'] + except KeyError: + LOG.warning("Couldn't find the auth context.") + raise exception.Unauthorized() diff --git a/keystone/common/controller.py b/keystone/common/controller.py index 71da4646cc..be5449e516 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -17,19 +17,16 @@ import uuid from oslo_log import log from oslo_log import versionutils -from oslo_utils import strutils import six from keystone.common import authorization from keystone.common import dependency from keystone.common import driver_hints -from keystone.common import policy from keystone.common import utils from keystone.common import wsgi import keystone.conf from keystone import exception from keystone.i18n import _ -from keystone.models import token_model LOG = log.getLogger(__name__) @@ -72,15 +69,6 @@ def v2_auth_deprecated(f): return wrapper() -def _build_policy_check_credentials(self, action, context, kwargs): - kwargs_str = ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs]) - kwargs_str = strutils.mask_password(kwargs_str) - msg = 'RBAC: Authorizing %(action)s(%(kwargs)s)' - LOG.debug(msg, {'action': action, 'kwargs': kwargs_str}) - - return context['environment'].get(authorization.AUTH_CONTEXT_ENV, {}) - - def protected(callback=None): """Wrap API calls with role based access controls (RBAC). @@ -97,69 +85,12 @@ def protected(callback=None): def wrapper(f): @functools.wraps(f) def inner(self, request, *args, **kwargs): - request.assert_authenticated() + check_function = authorization.check_protection + if callback is not None: + check_function = callback - if request.context.is_admin: - LOG.warning('RBAC: Bypassing authorization') - elif callback is not None: - prep_info = {'f_name': f.__name__, - 'input_attr': kwargs} - callback(self, - request, - prep_info, - *args, - **kwargs) - else: - action = 'identity:%s' % f.__name__ - creds = _build_policy_check_credentials(self, - action, - request.context_dict, - kwargs) - - policy_dict = {} - - # Check to see if we need to include the target entity in our - # policy checks. We deduce this by seeing if the class has - # specified a get_member() method and that kwargs contains the - # appropriate entity id. - if (hasattr(self, 'get_member_from_driver') and - self.get_member_from_driver is not None): - key = '%s_id' % self.member_name - if key in kwargs: - ref = self.get_member_from_driver(kwargs[key]) - policy_dict['target'] = {self.member_name: ref} - - # TODO(henry-nash): Move this entire code to a member - # method inside v3 Auth - if request.context_dict.get('subject_token_id') is not None: - window_seconds = self._token_validation_window(request) - token_ref = token_model.KeystoneToken( - token_id=request.context_dict['subject_token_id'], - token_data=self.token_provider_api.validate_token( - request.context_dict['subject_token_id'], - window_seconds=window_seconds)) - policy_dict.setdefault('target', {}) - policy_dict['target'].setdefault(self.member_name, {}) - policy_dict['target'][self.member_name]['user_id'] = ( - token_ref.user_id) - try: - user_domain_id = token_ref.user_domain_id - except exception.UnexpectedError: - user_domain_id = None - if user_domain_id: - policy_dict['target'][self.member_name].setdefault( - 'user', {}) - policy_dict['target'][self.member_name][ - 'user'].setdefault('domain', {}) - policy_dict['target'][self.member_name]['user'][ - 'domain']['id'] = ( - user_domain_id) - - # Add in the kwargs, which means that any entity provided as a - # parameter for calls like create and update will be included. - policy_dict.update(kwargs) - policy.enforce(creds, action, utils.flatten_dict(policy_dict)) - LOG.debug('RBAC: Authorization granted') + protected_wrapper( + self, f, check_function, request, None, *args, **kwargs) return f(self, request, *args, **kwargs) return inner return wrapper @@ -178,62 +109,55 @@ def filterprotected(*filters, **callback): entities needed and then call check_protection() in the V3Controller class. """ + def _handle_filters(filters, request): + target = dict() + if filters: + for item in filters: + if item in request.params: + target[item] = request.params[item] + + LOG.debug('RBAC: Adding query filter params (%s)', ( + ', '.join(['%s=%s' % (item, target[item]) + for item in target]))) + return target + def _filterprotected(f): @functools.wraps(f) def wrapper(self, request, **kwargs): - request.assert_authenticated() + filter_attr = _handle_filters(filters, request) + check_function = authorization.check_protection + if 'callback' in callback and callback['callback'] is not None: + # A callback has been specified to load additional target + # data, so pass it the formal url params as well as the + # list of filters, so it can augment these and then call + # the check_protection() method. + check_function = callback['callback'] - if not request.context.is_admin: - # The target dict for the policy check will include: - # - # - Any query filter parameters - # - Data from the main url (which will be in the kwargs - # parameter), which although most of our APIs do not utilize, - # in theory you could have. - # - - # First build the dict of filter parameters - target = dict() - if filters: - for item in filters: - if item in request.params: - target[item] = request.params[item] - - LOG.debug('RBAC: Adding query filter params (%s)', ( - ', '.join(['%s=%s' % (item, target[item]) - for item in target]))) - - if 'callback' in callback and callback['callback'] is not None: - # A callback has been specified to load additional target - # data, so pass it the formal url params as well as the - # list of filters, so it can augment these and then call - # the check_protection() method. - prep_info = {'f_name': f.__name__, - 'input_attr': kwargs, - 'filter_attr': target} - callback['callback'](self, - request, - prep_info, - **kwargs) - else: - # No callback, so we are going to check the protection here - action = 'identity:%s' % f.__name__ - creds = _build_policy_check_credentials( - self, action, request.context_dict, kwargs) - # Add in any formal url parameters - for key in kwargs: - target[key] = kwargs[key] - - policy.enforce(creds, action, utils.flatten_dict(target)) - - LOG.debug('RBAC: Authorization granted') - else: - LOG.warning('RBAC: Bypassing authorization') + protected_wrapper( + self, f, check_function, request, filter_attr, **kwargs) return f(self, request, filters, **kwargs) return wrapper return _filterprotected +# Unified calls for the decorators above. +# TODO(ayoung): Continue the refactoring. Always call check_protection +# explicitly, by removing the calls to check protection from the callbacks. +# Instead, have a call to the callbacks inserted prior to the call to +# `check_protection`. +def protected_wrapper(self, f, check_function, request, filter_attr, + *args, **kwargs): + request.assert_authenticated() + if request.context.is_admin: + LOG.warning('RBAC: Bypassing authorization') + return + prep_info = {'f_name': f.__name__, + 'input_attr': kwargs} + if (filter_attr): + prep_info['filter_attr'] = filter_attr + check_function(self, request, prep_info, *args, **kwargs) + + class V2Controller(wsgi.Application): """Base controller class for Identity API v2.""" @@ -682,7 +606,7 @@ class V3Controller(wsgi.Application): if domain_id: return domain_id - token_ref = utils.get_token_ref(request.context_dict) + token_ref = authorization.get_token_ref(request.context_dict) if token_ref.domain_scoped: return token_ref.domain_id @@ -751,26 +675,7 @@ class V3Controller(wsgi.Application): they can be referenced by policy rules. """ - if request.context.is_admin: - LOG.warning('RBAC: Bypassing authorization') - else: - action = 'identity:%s' % prep_info['f_name'] - # TODO(henry-nash) need to log the target attributes as well - creds = _build_policy_check_credentials(self, action, - request.context_dict, - prep_info['input_attr']) - # Build the dict the policy engine will check against from both the - # parameters passed into the call we are protecting (which was - # stored in the prep_info by protected()), plus the target - # attributes provided. - policy_dict = {} - if target_attr: - policy_dict = {'target': target_attr} - policy_dict.update(prep_info['input_attr']) - if 'filter_attr' in prep_info: - policy_dict.update(prep_info['filter_attr']) - policy.enforce(creds, action, utils.flatten_dict(policy_dict)) - LOG.debug('RBAC: Authorization granted') + authorization.check_protection(self, request, prep_info, target_attr) @classmethod def filter_params(cls, ref): @@ -787,11 +692,3 @@ class V3Controller(wsgi.Application): for blocked_param in blocked_keys: del ref[blocked_param] return ref - - def _token_validation_window(self, request): - # NOTE(jamielennox): it's dumb that i have to put this here. We should - # only validate subject token in one place. - allow_expired = request.params.get('allow_expired') - allow_expired = strutils.bool_from_string(allow_expired, default=False) - - return CONF.token.allow_expired_window if allow_expired else 0 diff --git a/keystone/common/utils.py b/keystone/common/utils.py index 3f6d004251..b113bd094e 100644 --- a/keystone/common/utils.py +++ b/keystone/common/utils.py @@ -34,7 +34,6 @@ import passlib.hash import six from six import moves -from keystone.common import authorization import keystone.conf from keystone import exception from keystone.i18n import _ @@ -526,24 +525,6 @@ def isotime(at=None, subsecond=False): return st -def get_token_ref(context): - """Retrieve KeystoneToken object from the auth context and returns it. - - :param dict context: The request context. - :raises keystone.exception.Unauthorized: If auth context cannot be found. - :returns: The KeystoneToken object. - """ - try: - # Retrieve the auth context that was prepared by AuthContextMiddleware. - auth_context = (context['environment'] - [authorization.AUTH_CONTEXT_ENV]) - return auth_context['token'] - except KeyError: - msg = _("Couldn't find the auth context.") - LOG.warning(msg) - raise exception.Unauthorized(msg) - - URL_RESERVED_CHARS = ":/?#[]@!$&'()*+,;=" diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index 75f34a40b6..bbe7e2a46c 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -18,7 +18,6 @@ """Utility methods for working with WSGI servers.""" -import copy import functools import itertools import re @@ -35,9 +34,9 @@ from six.moves import http_client import webob.dec import webob.exc +from keystone.common import authorization from keystone.common import dependency from keystone.common import json_home -from keystone.common import policy from keystone.common import request as request_mod from keystone.common import utils import keystone.conf @@ -292,28 +291,7 @@ class Application(BaseApplication): does not have the admin role """ - request.assert_authenticated() - - if not request.context.is_admin: - user_token_ref = utils.get_token_ref(request.context_dict) - - creds = copy.deepcopy(user_token_ref.metadata) - - try: - creds['user_id'] = user_token_ref.user_id - except exception.UnexpectedError: - LOG.debug('Invalid user') - raise exception.Unauthorized(_('Invalid user')) - - if user_token_ref.project_scoped: - creds['tenant_id'] = user_token_ref.project_id - else: - LOG.debug('Invalid tenant') - raise exception.Unauthorized(_('Invalid tenant')) - - creds['roles'] = user_token_ref.role_names - # Accept either is_admin or the admin role - policy.enforce(creds, 'admin_required', {}) + authorization.assert_admin(self, request) def _attribute_is_empty(self, ref, attribute): """Determine if the attribute in ref is empty or None.""" diff --git a/keystone/contrib/ec2/controllers.py b/keystone/contrib/ec2/controllers.py index 3e04485de8..7a20adea0f 100644 --- a/keystone/contrib/ec2/controllers.py +++ b/keystone/contrib/ec2/controllers.py @@ -41,6 +41,7 @@ from oslo_serialization import jsonutils import six from six.moves import http_client +from keystone.common import authorization from keystone.common import controller from keystone.common import dependency from keystone.common import utils @@ -327,7 +328,7 @@ class Ec2Controller(Ec2ControllerCommon, controller.V2Controller): :raises keystone.exception.Forbidden: when token is invalid """ - token_ref = utils.get_token_ref(context) + token_ref = authorization.get_token_ref(context) if token_ref.user_id != user_id: raise exception.Forbidden(_('Token belongs to another user')) diff --git a/keystone/oauth1/controllers.py b/keystone/oauth1/controllers.py index 6dffa6a82e..1e24ef73db 100644 --- a/keystone/oauth1/controllers.py +++ b/keystone/oauth1/controllers.py @@ -19,9 +19,9 @@ from oslo_serialization import jsonutils from oslo_utils import timeutils from six.moves import http_client +from keystone.common import authorization from keystone.common import controller from keystone.common import dependency -from keystone.common import utils from keystone.common import validation from keystone.common import wsgi import keystone.conf @@ -92,7 +92,7 @@ class ConsumerCrudV3(controller.V3Controller): @controller.protected() def delete_consumer(self, request, consumer_id): - user_token_ref = utils.get_token_ref(request.context_dict) + user_token_ref = authorization.get_token_ref(request.context_dict) payload = {'user_id': user_token_ref.user_id, 'consumer_id': consumer_id} _emit_user_oauth_consumer_token_invalidate(payload) @@ -396,7 +396,7 @@ class OAuthControllerV3(controller.V3Controller): authed_roles.add(role['id']) # verify the authorizing user has the roles - user_token = utils.get_token_ref(request.context_dict) + user_token = authorization.get_token_ref(request.context_dict) user_id = user_token.user_id project_id = req_token['requested_project_id'] user_roles = self.assignment_api.get_roles_for_user_and_project(