Move list_roles_for_trust enforcement to policies

Without this change, policy enforcement for the GET
/OS-TRUST/trusts/{trust_id}/roles API is hardcoded in the flask
dispatcher code. This is a problem because this enforcement can't be
controlled by the operator, as is the norm. Moreover, it makes the
transition to system-scope and default-roles-aware policies more
difficult because there's no sensible migration from "" to a logical
role-based check string.

This converts the hardcoded enforcement to enforcement via default
policies for GET /OS-TRUST/trusts/{trust_id}/roles. The API specifically
blocks the is_admin user from using it, and since policies aren't loaded
for the is_admin user we need to continue explicitly blocking it.

This change does not use the formal oslo.policy deprecation system
because "" OR'd with the new default is entirely useless as a policy.

Change-Id: Ib339852c9d619b8cbf7a00d45da461377991ba6f
Partial-bug: #1818850
Partial-bug: #1818846

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'],

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:

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',

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
+            )

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()