Browse Source

Merge "Move delete_trust enforcement to default policies"

changes/22/678322/10
Zuul 1 week ago
parent
commit
30a3f9f4e6

+ 36
- 10
keystone/api/trusts.py View File

@@ -46,6 +46,18 @@ TRUST_ID_PARAMETER_RELATION = _build_parameter_relation(
46 46
     parameter_name='trust_id')
47 47
 
48 48
 
49
+def _build_trust_target_enforcement():
50
+    target = {}
51
+    # NOTE(cmurphy) unlike other APIs, in the event the trust doesn't exist or
52
+    # has 0 remaining uses, we actually do expect it to return a 404 and not a
53
+    # 403, so don't catch NotFound here (lp#1840288)
54
+    target['trust'] = PROVIDERS.trust_api.get_trust(
55
+        flask.request.view_args.get('trust_id')
56
+    )
57
+
58
+    return target
59
+
60
+
49 61
 def _trustor_trustee_only(trust):
50 62
     user_id = flask.request.environ.get(context.REQUEST_CONTEXT_ENV).user_id
51 63
     if user_id not in [trust.get('trustee_user_id'),
@@ -263,18 +275,32 @@ class TrustResource(ks_flask.ResourceBase):
263 275
         return self.wrap_member(return_trust), http_client.CREATED
264 276
 
265 277
     def delete(self, trust_id):
266
-        ENFORCER.enforce_call(action='identity:delete_trust')
278
+        ENFORCER.enforce_call(action='identity:delete_trust',
279
+                              build_target=_build_trust_target_enforcement)
267 280
         self._check_unrestricted()
268 281
 
269
-        trust = PROVIDERS.trust_api.get_trust(trust_id)
270
-
271
-        # TODO(morgan): convert this check to an oslo_policy checkstr that
272
-        # can be referenced/enforced on.
273
-        if (self.oslo_context.user_id != trust.get('trustor_user_id') and
274
-                not self.oslo_context.is_admin):
275
-            action = _('Only admin or trustor can delete a trust')
276
-            raise exception.ForbiddenAction(action=action)
277
-
282
+        # NOTE(cmurphy) As of Train, the default policies enforce the
283
+        # identity:delete_trust rule. However, in case the
284
+        # identity:delete_trust rule has been locally overridden by the
285
+        # default that would have been produced by the sample config, we need
286
+        # to enforce it again and warn that the behavior is changing.
287
+        rules = policy._ENFORCER._enforcer.rules.get('identity:delete_trust')
288
+        # rule check_str is ""
289
+        if isinstance(rules, op_checks.TrueCheck):
290
+            LOG.warning(
291
+                "The policy check string for rule \"identity:delete_trust\" "
292
+                "has been overridden to \"always true\". In the next release, "
293
+                "this will cause the" "\"identity:delete_trust\" action to "
294
+                "be fully permissive as hardcoded enforcement will be "
295
+                "removed. To correct this issue, either stop overriding the "
296
+                "\"identity:delete_trust\" rule in config to accept the "
297
+                "defaults, or explicitly set a rule that is not empty."
298
+            )
299
+            trust = PROVIDERS.trust_api.get_trust(trust_id)
300
+            if (self.oslo_context.user_id != trust.get('trustor_user_id') and
301
+                    not self.oslo_context.is_admin):
302
+                action = _('Only admin or trustor can delete a trust')
303
+                raise exception.ForbiddenAction(action=action)
278 304
         PROVIDERS.trust_api.delete_trust(trust_id,
279 305
                                          initiator=self.audit_initiator)
280 306
         return '', http_client.NO_CONTENT

+ 15
- 10
keystone/cmd/status.py View File

@@ -34,20 +34,25 @@ class Checks(upgradecheck.UpgradeCommands):
34 34
         enforcer = policy.Enforcer(CONF)
35 35
         ENFORCER.register_rules(enforcer)
36 36
         enforcer.load_rules()
37
-        rule = enforcer.rules.get('identity:list_trusts')
38
-        if isinstance(rule, _checks.TrueCheck):
37
+        rules = ['identity:list_trusts', 'identity:delete_trust']
38
+        failed_rules = []
39
+        for rule in rules:
40
+            current_rule = enforcer.rules.get(rule)
41
+            if isinstance(current_rule, _checks.TrueCheck):
42
+                failed_rules.append(rule)
43
+        if any(failed_rules):
39 44
             return upgradecheck.Result(
40 45
                 upgradecheck.Code.FAILURE,
41
-                "Policy check string for \"identity:list_trusts\" is "
42
-                "overridden to \"\", \"@\", or []. In the next release, "
43
-                "this will cause the \"identity:list_trusts\" action to be "
44
-                "fully permissive as hardcoded enforcement will be removed. "
45
-                "To correct this issue, either stop overriding this rule in "
46
-                "config to accept the defaults, or explicitly set a rule that "
47
-                "is not empty."
46
+                "Policy check string for rules \"%s\" are overridden to "
47
+                "\"\", \"@\", or []. In the next release, this will cause "
48
+                "these rules to be fully permissive as hardcoded enforcement "
49
+                "will be removed. To correct this issue, either stop "
50
+                "overriding these rules in config to accept the defaults, or "
51
+                "explicitly set check strings that are not empty." %
52
+                "\", \"".join(failed_rules)
48 53
             )
49 54
         return upgradecheck.Result(
50
-            upgradecheck.Code.SUCCESS, "\"identity:list_trusts\" policy is safe")
55
+            upgradecheck.Code.SUCCESS, 'Trust policies are safe.')
51 56
 
52 57
     _upgrade_checks = (
53 58
         ("Check trust policies are not empty",

+ 1
- 1
keystone/common/policies/trust.py View File

@@ -75,7 +75,7 @@ trust_policies = [
75 75
                      'method': 'HEAD'}]),
76 76
     policy.DocumentedRuleDefault(
77 77
         name=base.IDENTITY % 'delete_trust',
78
-        check_str='',
78
+        check_str=RULE_TRUSTOR,
79 79
         scope_types=['project'],
80 80
         description='Revoke trust.',
81 81
         operations=[{'path': '/v3/OS-TRUST/trusts/{trust_id}',

+ 50
- 1
keystone/tests/unit/protection/v3/test_trusts.py View File

@@ -109,6 +109,7 @@ class TrustTests(base_classes.TestCaseWithBootstrap,
109 109
         with open(self.policy_file_name, 'w') as f:
110 110
             overridden_policies = {
111 111
                 'identity:list_trusts': '',
112
+                'identity:delete_trust': '',
112 113
             }
113 114
             f.write(jsonutils.dumps(overridden_policies))
114 115
 
@@ -258,6 +259,19 @@ class SystemAdminTests(TrustTests, _AdminTestsMixin):
258 259
             )
259 260
         self.assertEqual(1, len(r.json['trusts']))
260 261
 
262
+    def test_admin_cannot_delete_trust_for_user_overridden_defaults(self):
263
+        # only the is_admin admin can do this
264
+        self._override_policy_old_defaults()
265
+        ref = PROVIDERS.trust_api.create_trust(
266
+            self.trust_id, **self.trust_data)
267
+
268
+        with self.test_client() as c:
269
+            c.delete(
270
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
271
+                headers=self.headers,
272
+                expected_status_code=http_client.FORBIDDEN
273
+            )
274
+
261 275
 
262 276
 class ProjectUserTests(TrustTests):
263 277
     """Tests for all project users."""
@@ -573,7 +587,7 @@ class ProjectUserTests(TrustTests):
573 587
                 expected_status_code=http_client.FORBIDDEN
574 588
             )
575 589
 
576
-    def test_user_cannot_list_trusts_for_other_trustee_overridden_default(self):
590
+    def test_user_cannot_list_trusts_for_trustee_overridden_default(self):
577 591
         self._override_policy_old_defaults()
578 592
         PROVIDERS.trust_api.create_trust(
579 593
             self.trust_id, **self.trust_data)
@@ -597,3 +611,38 @@ class ProjectUserTests(TrustTests):
597 611
                 headers=self.trustee_headers,
598 612
                 expected_status_code=http_client.FORBIDDEN
599 613
             )
614
+
615
+    def test_trustor_can_delete_trust_overridden_default(self):
616
+        self._override_policy_old_defaults()
617
+        ref = PROVIDERS.trust_api.create_trust(
618
+            self.trust_id, **self.trust_data)
619
+
620
+        with self.test_client() as c:
621
+            c.delete(
622
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
623
+                headers=self.trustor_headers
624
+            )
625
+
626
+    def test_trustee_cannot_delete_trust_overridden_default(self):
627
+        self._override_policy_old_defaults()
628
+        ref = PROVIDERS.trust_api.create_trust(
629
+            self.trust_id, **self.trust_data)
630
+
631
+        with self.test_client() as c:
632
+            c.delete(
633
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
634
+                headers=self.trustee_headers,
635
+                expected_status_code=http_client.FORBIDDEN
636
+            )
637
+
638
+    def test_user_cannot_delete_trust_for_other_user_overridden_default(self):
639
+        self._override_policy_old_defaults()
640
+        ref = PROVIDERS.trust_api.create_trust(
641
+            self.trust_id, **self.trust_data)
642
+
643
+        with self.test_client() as c:
644
+            c.delete(
645
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
646
+                headers=self.other_headers,
647
+                expected_status_code=http_client.FORBIDDEN
648
+            )

+ 4
- 2
keystone/tests/unit/test_cli.py View File

@@ -1865,14 +1865,16 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase):
1865 1865
     def test_check_safe_trust_policies(self):
1866 1866
         with open(self.policy_file_name, 'w') as f:
1867 1867
             overridden_policies = {
1868
-                'identity:list_trusts': ''
1868
+                'identity:list_trusts': '',
1869
+                'identity:delete_trust': ''
1869 1870
             }
1870 1871
             f.write(jsonutils.dumps(overridden_policies))
1871 1872
         result = self.checks.check_trust_policies_are_not_empty()
1872 1873
         self.assertEqual(upgradecheck.Code.FAILURE, result.code)
1873 1874
         with open(self.policy_file_name, 'w') as f:
1874 1875
             overridden_policies = {
1875
-                'identity:list_trusts': 'rule:admin_required'
1876
+                'identity:list_trusts': 'rule:admin_required',
1877
+                'identity:delete_trust': 'rule:admin_required'
1876 1878
             }
1877 1879
             f.write(jsonutils.dumps(overridden_policies))
1878 1880
         result = self.checks.check_trust_policies_are_not_empty()

Loading…
Cancel
Save