diff --git a/keystone/api/trusts.py b/keystone/api/trusts.py index c70c654147..f45a6f5309 100644 --- a/keystone/api/trusts.py +++ b/keystone/api/trusts.py @@ -164,9 +164,39 @@ class TrustResource(ks_flask.ResourceBase): return roles def _get_trust(self, trust_id): - ENFORCER.enforce_call(action='identity:get_trust') + ENFORCER.enforce_call(action='identity:get_trust', + build_target=_build_trust_target_enforcement) + + # NOTE(cmurphy) look up trust before doing is_admin authorization - to + # maintain the API contract, we expect a missing trust to raise a 404 + # before we get to enforcement (lp#1840288) trust = PROVIDERS.trust_api.get_trust(trust_id) - _trustor_trustee_only(trust) + + 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')) + + # NOTE(cmurphy) As of Train, the default policies enforce the + # identity:get_trust rule. However, in case the + # identity:get_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:get_trust') + # rule check_str is "" + if isinstance(rules, op_checks.TrueCheck): + LOG.warning( + "The policy check string for rule \"identity:get_trust\" " + "has been overridden to \"always true\". In the next release, " + "this will cause the" "\"identity:get_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 self.wrap_member(trust) diff --git a/keystone/cmd/status.py b/keystone/cmd/status.py index edb90db804..9a0adcd567 100644 --- a/keystone/cmd/status.py +++ b/keystone/cmd/status.py @@ -34,7 +34,11 @@ class Checks(upgradecheck.UpgradeCommands): enforcer = policy.Enforcer(CONF) ENFORCER.register_rules(enforcer) enforcer.load_rules() - rules = ['identity:list_trusts', 'identity:delete_trust'] + rules = [ + 'identity:list_trusts', + 'identity:delete_trust', + 'identity:get_trust' + ] failed_rules = [] for rule in rules: current_rule = enforcer.rules.get(rule) diff --git a/keystone/common/policies/trust.py b/keystone/common/policies/trust.py index e945534930..b8da273cd6 100644 --- a/keystone/common/policies/trust.py +++ b/keystone/common/policies/trust.py @@ -82,7 +82,7 @@ trust_policies = [ 'method': 'DELETE'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'get_trust', - check_str='', + check_str=RULE_TRUSTOR + ' or ' + RULE_TRUSTEE, scope_types=['project'], description='Get trust.', operations=[{'path': '/v3/OS-TRUST/trusts/{trust_id}', diff --git a/keystone/tests/unit/protection/v3/test_trusts.py b/keystone/tests/unit/protection/v3/test_trusts.py index cef4c976c1..a5ba49bdcf 100644 --- a/keystone/tests/unit/protection/v3/test_trusts.py +++ b/keystone/tests/unit/protection/v3/test_trusts.py @@ -110,6 +110,7 @@ class TrustTests(base_classes.TestCaseWithBootstrap, overridden_policies = { 'identity:list_trusts': '', 'identity:delete_trust': '', + 'identity:get_trust': '', } f.write(jsonutils.dumps(overridden_policies)) @@ -272,6 +273,18 @@ class SystemAdminTests(TrustTests, _AdminTestsMixin): expected_status_code=http_client.FORBIDDEN ) + def test_admin_cannot_get_trust_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' % self.trust_id, + headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + class ProjectUserTests(TrustTests): """Tests for all project users.""" @@ -646,3 +659,26 @@ class ProjectUserTests(TrustTests): headers=self.other_headers, expected_status_code=http_client.FORBIDDEN ) + + def test_user_can_get_trust_of_whom_they_are_the_trustor_overridden_default(self): + self._override_policy_old_defaults() + ref = PROVIDERS.trust_api.create_trust( + self.trust_id, **self.trust_data) + + with self.test_client() as c: + c.get( + '/v3/OS-TRUST/trusts/%s' % ref['id'], + headers=self.trustor_headers + ) + + def test_user_can_get_trust_delegated_to_them_overridden_default(self): + self._override_policy_old_defaults() + ref = 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' % ref['id'], + headers=self.trustee_headers + ) + self.assertEqual(r.json['trust']['id'], self.trust_id) diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index f5b73f87c7..91e59e6f0a 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -1866,7 +1866,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase): with open(self.policy_file_name, 'w') as f: overridden_policies = { 'identity:list_trusts': '', - 'identity:delete_trust': '' + 'identity:delete_trust': '', + 'identity:get_trust': '' } f.write(jsonutils.dumps(overridden_policies)) result = self.checks.check_trust_policies_are_not_empty() @@ -1874,7 +1875,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase): with open(self.policy_file_name, 'w') as f: overridden_policies = { 'identity:list_trusts': 'rule:admin_required', - 'identity:delete_trust': 'rule:admin_required' + 'identity:delete_trust': 'rule:admin_required', + 'identity:get_trust': 'rule:admin_required' } f.write(jsonutils.dumps(overridden_policies)) result = self.checks.check_trust_policies_are_not_empty()