From 0dc5c4edabd5cb0455ffe1c4f8cf8369f64b2197 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Wed, 26 Sep 2018 21:34:13 +0000 Subject: [PATCH] Pass context objects to policy enforcement The oslo.policy library actually accepts context objects as a first class citizen, instead of a hand-built `creds` dictionary. This is a perferred approach because it's easier for services to use oslo.context to generate a context object that they can automatically pass to oslo.policy for enforcement instead of inspecting the context object and building a dictionary manually to pass to oslo.policy. This commit makes allows keystone to partake in this by pulling the keystone request object, which is a subclass of oslo.context's RequestContext object, and uses it in enforcement. Additionally, we're overriding the to_policy_values() method of oslo.context in order to make sure we port keystone-specific values to the policy dict representation of a context object. This ensures we have values present that we rely on with our default policies. This commit also bumps the lower requirement for oslo.policy to make sure we're always using a version that understands context objects. Change-Id: I63e713f4aebf3e8cf5189a6060569d2828bc364d --- keystone/common/context.py | 27 +++++++++++++++++ keystone/common/rbac_enforcer/enforcer.py | 30 +++---------------- .../middleware/auth_context.py | 3 ++ lower-constraints.txt | 2 +- requirements.txt | 2 +- 5 files changed, 36 insertions(+), 28 deletions(-) diff --git a/keystone/common/context.py b/keystone/common/context.py index 6c44ce50ea..441e6a6f76 100644 --- a/keystone/common/context.py +++ b/keystone/common/context.py @@ -38,3 +38,30 @@ class RequestContext(oslo_context.RequestContext): self.authenticated = kwargs.pop('authenticated', False) super(RequestContext, self).__init__(**kwargs) + + def to_policy_values(self): + """Add keystone-specific policy values to policy representation. + + This method converts generic policy values to a dictionary form using + the base implementation from oslo_context.context.RequestContext. + Afterwards, it is going to pull keystone-specific values off the + context and represent them as items in the policy values dictionary. + This is because keystone uses default policies that rely on these + values, so we need to guarantee they are present during policy + enforcement if they are present on the context object. + + This method is automatically called in + oslo_policy.policy.Enforcer.enforce() if oslo.policy knows it's dealing + with a context object. + + """ + # TODO(morgan): Rework this to not need an explicit token render as + # this is a generally poorly designed behavior. The enforcer should not + # rely on a contract of the token's rendered JSON form. This likely + # needs reworking of how we handle the context in oslo.policy. Until + # this is reworked, it is not possible to merge the token render + # function into keystone.api + values = super(RequestContext, self).to_policy_values() + values['token'] = self.token_reference['token'] + values['domain_id'] = self.domain_id if self.domain_id else None + return values diff --git a/keystone/common/rbac_enforcer/enforcer.py b/keystone/common/rbac_enforcer/enforcer.py index 71fbba02ae..f787544ee6 100644 --- a/keystone/common/rbac_enforcer/enforcer.py +++ b/keystone/common/rbac_enforcer/enforcer.py @@ -10,7 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import copy import functools import flask @@ -22,7 +21,6 @@ from keystone.common import authorization from keystone.common import context from keystone.common import policies from keystone.common import provider_api -from keystone.common import render_token from keystone.common import utils import keystone.conf from keystone import exception @@ -79,26 +77,6 @@ class RBACEnforcer(object): extra.update(exc=exception.ForbiddenAction, action=action, do_raise=do_raise) - # NOTE(lbragstad): If there is a token in the credentials dictionary, - # it's going to be an instance of a TokenModel. We'll need to convert - # it to the a token response or dictionary before passing it to - # oslo.policy for enforcement. This is because oslo.policy shouldn't - # know how to deal with an internal object only used within keystone. - # - # TODO(morgan): Rework this to not need an explicit token render as - # this is a generally poorly designed behavior. The enforcer should not - # rely on a contract of the token's rendered JSON form. This likely - # needs reworking of how we handle the context in oslo.policy. Until - # this is reworked, it is not possible to merge the token render - # function into keystone.api - if 'token' in credentials: - token_ref = render_token.render_token_response_from_model( - credentials['token'] - ) - credentials_copy = copy.deepcopy(credentials) - credentials_copy['token'] = token_ref - credentials = credentials_copy - try: return self._enforcer.enforce( rule=action, target=target, creds=credentials, **extra) @@ -398,7 +376,8 @@ class RBACEnforcer(object): policy_dict.update(cls._extract_filter_values(filters)) # Extract the cred data - creds = cls._extract_policy_check_credentials() + ctxt = cls._get_oslo_req_context() + creds = ctxt.to_policy_values() flattened = utils.flatten_dict(policy_dict) if LOG.logger.getEffectiveLevel() <= log.DEBUG: # LOG the Args @@ -410,8 +389,7 @@ class RBACEnforcer(object): {'action': action, 'args': args_str}) # LOG the Cred Data - cred_str = ', '.join( - ['%s=%s' % (k, v) for k, v in creds.items()]) + cred_str = ', '.join(['%s=%s' % (k, v) for k, v in creds.items()]) cred_str = strutils.mask_password(cred_str) LOG.debug('RBAC: Policy Enforcement Cred Data ' '`%(action)s creds(%(cred_str)s)`', @@ -428,7 +406,7 @@ class RBACEnforcer(object): # Instantiate the enforcer object if needed. enforcer_obj = enforcer or cls() enforcer_obj._enforce( - credentials=creds, action=action, target=flattened) + credentials=ctxt, action=action, target=flattened) LOG.debug('RBAC: Authorization granted') @classmethod diff --git a/keystone/server/flask/request_processing/middleware/auth_context.py b/keystone/server/flask/request_processing/middleware/auth_context.py index b883085ba6..92d86ee127 100644 --- a/keystone/server/flask/request_processing/middleware/auth_context.py +++ b/keystone/server/flask/request_processing/middleware/auth_context.py @@ -360,6 +360,9 @@ class AuthContextMiddleware(provider_api.ProviderAPIMixin, self.fill_context(request) def _keystone_specific_values(self, token, request_context): + request_context.token_reference = ( + render_token.render_token_response_from_model(token) + ) if token.domain_scoped: # Domain scoped tokens should never have is_admin_project set # Even if KSA defaults it otherwise. The two mechanisms are diff --git a/lower-constraints.txt b/lower-constraints.txt index b672754937..2a3fce73ba 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -34,7 +34,7 @@ oslo.i18n==3.15.3 oslo.log==3.38.0 oslo.messaging==5.29.0 oslo.middleware==3.31.0 -oslo.policy==1.33.0 +oslo.policy==1.38.0 oslo.serialization==2.18.0 oslo.upgradecheck==0.1.0 oslo.utils==3.33.0 diff --git a/requirements.txt b/requirements.txt index f651e3b007..ba5da875c0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,7 @@ oslo.db>=4.27.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.log>=3.38.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 -oslo.policy>=1.33.0 # Apache-2.0 +oslo.policy>=1.38.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.upgradecheck>=0.1.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0