diff --git a/nova/context.py b/nova/context.py index 51ef6ef52990..76c0086f6344 100644 --- a/nova/context.py +++ b/nova/context.py @@ -228,9 +228,8 @@ class RequestContext(context.RequestContext): :param action: string representing the action to be checked. :param target: dictionary representing the object of the action for object creation this should be a dictionary representing the - location of the object e.g. ``{'project_id': context.project_id}``. - If None, then this default target will be considered: - {'project_id': self.project_id, 'user_id': self.user_id} + location of the object + e.g. ``{'project_id': instance.project_id}``. :param fatal: if False, will return False when an exception.Forbidden occurs. @@ -240,9 +239,6 @@ class RequestContext(context.RequestContext): :return: returns a non-False value (not necessarily "True") if authorized and False if not authorized and fatal is False. """ - if target is None: - target = self.default_target() - try: return policy.authorize(self, action, target) except exception.Forbidden: @@ -250,9 +246,6 @@ class RequestContext(context.RequestContext): raise return False - def default_target(self): - return {'project_id': self.project_id, 'user_id': self.user_id} - def to_policy_values(self): policy = super(RequestContext, self).to_policy_values() policy['is_admin'] = self.is_admin diff --git a/nova/policy.py b/nova/policy.py index cd5273547b21..4f211872b3ff 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -122,7 +122,7 @@ def set_rules(rules, overwrite=True, use_conf=False): _ENFORCER.set_rules(rules, overwrite, use_conf) -def authorize(context, action, target, do_raise=True, exc=None): +def authorize(context, action, target=None, do_raise=True, exc=None): """Verifies that the action is valid on the target in this context. :param context: nova context @@ -133,7 +133,9 @@ def authorize(context, action, target, do_raise=True, exc=None): ``volume:attach_volume`` :param target: dictionary representing the object of the action for object creation this should be a dictionary representing the - location of the object e.g. ``{'project_id': context.project_id}`` + location of the object e.g. ``{'project_id': instance.project_id}`` + If None, then this default target will be considered: + {'project_id': self.project_id, 'user_id': self.user_id} :param do_raise: if True (the default), raises PolicyNotAuthorized; if False, returns False :param exc: Class of the exception to raise if the check fails. @@ -154,6 +156,12 @@ def authorize(context, action, target, do_raise=True, exc=None): credentials = context.to_policy_values() if not exc: exc = exception.PolicyNotAuthorized + + # Legacy fallback for emtpy target from context.can() + # should be removed once we improve testing and scope checks + if target is None: + target = default_target(context) + try: result = _ENFORCER.authorize(action, target, credentials, do_raise=do_raise, exc=exc, action=action) @@ -168,6 +176,10 @@ def authorize(context, action, target, do_raise=True, exc=None): return result +def default_target(context): + return {'project_id': context.project_id, 'user_id': context.user_id} + + def check_is_admin(context): """Whether or not roles contains 'admin' role according to policy setting. @@ -176,7 +188,7 @@ def check_is_admin(context): init() # the target is user-self credentials = context.to_policy_values() - target = context.default_target() + target = default_target(context) return _ENFORCER.authorize('context_is_admin', target, credentials) diff --git a/nova/tests/unit/test_context.py b/nova/tests/unit/test_context.py index c640477ab802..3fde2cb5c39b 100644 --- a/nova/tests/unit/test_context.py +++ b/nova/tests/unit/test_context.py @@ -216,8 +216,7 @@ class ContextTestCase(test.NoDBTestCase): self.assertTrue(result) mock_authorize.assert_called_once_with( - ctxt, mock.sentinel.rule, - {'project_id': ctxt.project_id, 'user_id': ctxt.user_id}) + ctxt, mock.sentinel.rule, None) @mock.patch.object(context.policy, 'authorize') def test_can_fatal(self, mock_authorize): diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 2917d701e007..952c47ebdb3a 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -134,6 +134,10 @@ class PolicyTestCase(test.NoDBTestCase): target_not_mine = {'project_id': 'another'} action = "example:my_file" policy.authorize(self.context, action, target_mine) + # check we fallback to context.project_id + # TODO(johngarbutt): longer term we need to remove this and make + # the target a required param. + policy.authorize(self.context, action) self.assertRaises(exception.PolicyNotAuthorized, policy.authorize, self.context, action, target_not_mine)