diff --git a/keystone/api/trusts.py b/keystone/api/trusts.py index f45a6f5309..374b7351c3 100644 --- a/keystone/api/trusts.py +++ b/keystone/api/trusts.py @@ -340,13 +340,48 @@ class TrustResource(ks_flask.ResourceBase): # URL additions and does not have a collection key/member_key, we use # the flask-restful Resource, not the keystone ResourceBase class RolesForTrustListResource(flask_restful.Resource): + + @property + def oslo_context(self): + return flask.request.environ.get(context.REQUEST_CONTEXT_ENV, None) + def get(self, trust_id): - ENFORCER.enforce_call(action='identity:list_roles_for_trust') + ENFORCER.enforce_call(action='identity:list_roles_for_trust', + build_target=_build_trust_target_enforcement) + # NOTE(morgan): This duplicates a little of the .get_trust from the # main resource, as it needs some of the same logic. However, due to # how flask-restful works, this should be fully encapsulated + + if self.oslo_context.is_admin: + # policies are not loaded for the is_admin context, so need to + # block access here + raise exception.ForbiddenAction( + action=_('Requested user has no relation to this trust')) + trust = PROVIDERS.trust_api.get_trust(trust_id) - _trustor_trustee_only(trust) + + # NOTE(cmurphy) As of Train, the default policies enforce the + # identity:list_roles_for_trust rule. However, in case the + # identity:list_roles_for_trust rule has been locally overridden by the + # default that would have been produced by the sample config, we need + # to enforce it again and warn that the behavior is changing. + rules = policy._ENFORCER._enforcer.rules.get( + 'identity:list_roles_for_trust') + # rule check_str is "" + if isinstance(rules, op_checks.TrueCheck): + LOG.warning( + "The policy check string for rule " + "\"identity:list_roles_for_trust\" has been overridden to " + "\"always true\". In the next release, this will cause the " + "\"identity:list_roles_for_trust\" action to be fully " + "permissive as hardcoded enforcement will be removed. To " + "correct this issue, either stop overriding the " + "\"identity:get_trust\" rule in config to accept the " + "defaults, or explicitly set a rule that is not empty." + ) + _trustor_trustee_only(trust) + _normalize_trust_expires_at(trust) _normalize_trust_roles(trust) return {'roles': trust['roles'], diff --git a/keystone/cmd/status.py b/keystone/cmd/status.py index 9a0adcd567..e8e32d44cc 100644 --- a/keystone/cmd/status.py +++ b/keystone/cmd/status.py @@ -37,7 +37,8 @@ class Checks(upgradecheck.UpgradeCommands): rules = [ 'identity:list_trusts', 'identity:delete_trust', - 'identity:get_trust' + 'identity:get_trust', + 'identity:list_roles_for_trust' ] failed_rules = [] for rule in rules: diff --git a/keystone/common/policies/trust.py b/keystone/common/policies/trust.py index b8da273cd6..55d6515fe1 100644 --- a/keystone/common/policies/trust.py +++ b/keystone/common/policies/trust.py @@ -57,7 +57,7 @@ trust_policies = [ 'method': 'HEAD'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'list_roles_for_trust', - check_str='', + check_str=RULE_TRUSTOR + ' or ' + RULE_TRUSTEE, scope_types=['project'], description='List roles delegated by a trust.', operations=[{'path': '/v3/OS-TRUST/trusts/{trust_id}/roles', diff --git a/keystone/tests/unit/protection/v3/test_trusts.py b/keystone/tests/unit/protection/v3/test_trusts.py index a5ba49bdcf..78528975b5 100644 --- a/keystone/tests/unit/protection/v3/test_trusts.py +++ b/keystone/tests/unit/protection/v3/test_trusts.py @@ -111,6 +111,7 @@ class TrustTests(base_classes.TestCaseWithBootstrap, 'identity:list_trusts': '', 'identity:delete_trust': '', 'identity:get_trust': '', + 'identity:list_roles_for_trust': '', } f.write(jsonutils.dumps(overridden_policies)) @@ -285,6 +286,18 @@ class SystemAdminTests(TrustTests, _AdminTestsMixin): expected_status_code=http_client.FORBIDDEN ) + def test_admin_cannot_list_roles_for_other_user_overridden_defaults(self): + self._override_policy_old_defaults() + PROVIDERS.trust_api.create_trust( + self.trust_id, **self.trust_data) + + with self.test_client() as c: + c.get( + '/v3/OS-TRUST/trusts/%s/roles' % self.trust_id, + headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + class ProjectUserTests(TrustTests): """Tests for all project users.""" @@ -682,3 +695,41 @@ class ProjectUserTests(TrustTests): headers=self.trustee_headers ) self.assertEqual(r.json['trust']['id'], self.trust_id) + + def test_trustor_can_list_trust_roles_overridden_default(self): + self._override_policy_old_defaults() + PROVIDERS.trust_api.create_trust( + self.trust_id, **self.trust_data) + + with self.test_client() as c: + r = c.get( + '/v3/OS-TRUST/trusts/%s/roles' % self.trust_id, + headers=self.trustor_headers + ) + self.assertEqual(r.json['roles'][0]['id'], + self.bootstrapper.member_role_id) + + def test_trustee_can_list_trust_roles_overridden_default(self): + self._override_policy_old_defaults() + PROVIDERS.trust_api.create_trust( + self.trust_id, **self.trust_data) + + with self.test_client() as c: + r = c.get( + '/v3/OS-TRUST/trusts/%s/roles' % self.trust_id, + headers=self.trustee_headers + ) + self.assertEqual(r.json['roles'][0]['id'], + self.bootstrapper.member_role_id) + + def test_user_cannot_list_trust_roles_other_user_overridden_default(self): + self._override_policy_old_defaults() + PROVIDERS.trust_api.create_trust( + self.trust_id, **self.trust_data) + + with self.test_client() as c: + c.get( + '/v3/OS-TRUST/trusts/%s/roles' % self.trust_id, + headers=self.other_headers, + expected_status_code=http_client.FORBIDDEN + ) diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index 91e59e6f0a..76012241b1 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -1867,7 +1867,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase): overridden_policies = { 'identity:list_trusts': '', 'identity:delete_trust': '', - 'identity:get_trust': '' + 'identity:get_trust': '', + 'identity:list_roles_for_trust': '' } f.write(jsonutils.dumps(overridden_policies)) result = self.checks.check_trust_policies_are_not_empty() @@ -1876,7 +1877,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase): overridden_policies = { 'identity:list_trusts': 'rule:admin_required', 'identity:delete_trust': 'rule:admin_required', - 'identity:get_trust': 'rule:admin_required' + 'identity:get_trust': 'rule:admin_required', + 'identity:list_roles_for_trust': 'rule:admin_required' } f.write(jsonutils.dumps(overridden_policies)) result = self.checks.check_trust_policies_are_not_empty()