Browse Source

Merge "Move get_trust enforcement to default policies"

changes/22/678322/10
Zuul 1 week ago
parent
commit
20bea870a8

+ 32
- 2
keystone/api/trusts.py View File

@@ -164,9 +164,39 @@ class TrustResource(ks_flask.ResourceBase):
164 164
         return roles
165 165
 
166 166
     def _get_trust(self, trust_id):
167
-        ENFORCER.enforce_call(action='identity:get_trust')
167
+        ENFORCER.enforce_call(action='identity:get_trust',
168
+                              build_target=_build_trust_target_enforcement)
169
+
170
+        # NOTE(cmurphy) look up trust before doing is_admin authorization - to
171
+        # maintain the API contract, we expect a missing trust to raise a 404
172
+        # before we get to enforcement (lp#1840288)
168 173
         trust = PROVIDERS.trust_api.get_trust(trust_id)
169
-        _trustor_trustee_only(trust)
174
+
175
+        if self.oslo_context.is_admin:
176
+            # policies are not loaded for the is_admin context, so need to
177
+            # block access here
178
+            raise exception.ForbiddenAction(
179
+                action=_('Requested user has no relation to this trust'))
180
+
181
+        # NOTE(cmurphy) As of Train, the default policies enforce the
182
+        # identity:get_trust rule. However, in case the
183
+        # identity:get_trust rule has been locally overridden by the
184
+        # default that would have been produced by the sample config, we need
185
+        # to enforce it again and warn that the behavior is changing.
186
+        rules = policy._ENFORCER._enforcer.rules.get('identity:get_trust')
187
+        # rule check_str is ""
188
+        if isinstance(rules, op_checks.TrueCheck):
189
+            LOG.warning(
190
+                "The policy check string for rule \"identity:get_trust\" "
191
+                "has been overridden to \"always true\". In the next release, "
192
+                "this will cause the" "\"identity:get_trust\" action to "
193
+                "be fully permissive as hardcoded enforcement will be "
194
+                "removed. To correct this issue, either stop overriding the "
195
+                "\"identity:get_trust\" rule in config to accept the "
196
+                "defaults, or explicitly set a rule that is not empty."
197
+            )
198
+            _trustor_trustee_only(trust)
199
+
170 200
         _normalize_trust_expires_at(trust)
171 201
         _normalize_trust_roles(trust)
172 202
         return self.wrap_member(trust)

+ 5
- 1
keystone/cmd/status.py View File

@@ -34,7 +34,11 @@ class Checks(upgradecheck.UpgradeCommands):
34 34
         enforcer = policy.Enforcer(CONF)
35 35
         ENFORCER.register_rules(enforcer)
36 36
         enforcer.load_rules()
37
-        rules = ['identity:list_trusts', 'identity:delete_trust']
37
+        rules = [
38
+            'identity:list_trusts',
39
+            'identity:delete_trust',
40
+            'identity:get_trust'
41
+        ]
38 42
         failed_rules = []
39 43
         for rule in rules:
40 44
             current_rule = enforcer.rules.get(rule)

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

@@ -82,7 +82,7 @@ trust_policies = [
82 82
                      'method': 'DELETE'}]),
83 83
     policy.DocumentedRuleDefault(
84 84
         name=base.IDENTITY % 'get_trust',
85
-        check_str='',
85
+        check_str=RULE_TRUSTOR + ' or ' + RULE_TRUSTEE,
86 86
         scope_types=['project'],
87 87
         description='Get trust.',
88 88
         operations=[{'path': '/v3/OS-TRUST/trusts/{trust_id}',

+ 36
- 0
keystone/tests/unit/protection/v3/test_trusts.py View File

@@ -110,6 +110,7 @@ class TrustTests(base_classes.TestCaseWithBootstrap,
110 110
             overridden_policies = {
111 111
                 'identity:list_trusts': '',
112 112
                 'identity:delete_trust': '',
113
+                'identity:get_trust': '',
113 114
             }
114 115
             f.write(jsonutils.dumps(overridden_policies))
115 116
 
@@ -272,6 +273,18 @@ class SystemAdminTests(TrustTests, _AdminTestsMixin):
272 273
                 expected_status_code=http_client.FORBIDDEN
273 274
             )
274 275
 
276
+    def test_admin_cannot_get_trust_for_other_user_overridden_defaults(self):
277
+        self._override_policy_old_defaults()
278
+        PROVIDERS.trust_api.create_trust(
279
+            self.trust_id, **self.trust_data)
280
+
281
+        with self.test_client() as c:
282
+            c.get(
283
+                '/v3/OS-TRUST/trusts/%s' % self.trust_id,
284
+                headers=self.headers,
285
+                expected_status_code=http_client.FORBIDDEN
286
+            )
287
+
275 288
 
276 289
 class ProjectUserTests(TrustTests):
277 290
     """Tests for all project users."""
@@ -646,3 +659,26 @@ class ProjectUserTests(TrustTests):
646 659
                 headers=self.other_headers,
647 660
                 expected_status_code=http_client.FORBIDDEN
648 661
             )
662
+
663
+    def test_user_can_get_trust_of_whom_they_are_the_trustor_overridden_default(self):
664
+        self._override_policy_old_defaults()
665
+        ref = PROVIDERS.trust_api.create_trust(
666
+            self.trust_id, **self.trust_data)
667
+
668
+        with self.test_client() as c:
669
+            c.get(
670
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
671
+                headers=self.trustor_headers
672
+            )
673
+
674
+    def test_user_can_get_trust_delegated_to_them_overridden_default(self):
675
+        self._override_policy_old_defaults()
676
+        ref = PROVIDERS.trust_api.create_trust(
677
+            self.trust_id, **self.trust_data)
678
+
679
+        with self.test_client() as c:
680
+            r = c.get(
681
+                '/v3/OS-TRUST/trusts/%s' % ref['id'],
682
+                headers=self.trustee_headers
683
+            )
684
+        self.assertEqual(r.json['trust']['id'], self.trust_id)

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

@@ -1866,7 +1866,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase):
1866 1866
         with open(self.policy_file_name, 'w') as f:
1867 1867
             overridden_policies = {
1868 1868
                 'identity:list_trusts': '',
1869
-                'identity:delete_trust': ''
1869
+                'identity:delete_trust': '',
1870
+                'identity:get_trust': ''
1870 1871
             }
1871 1872
             f.write(jsonutils.dumps(overridden_policies))
1872 1873
         result = self.checks.check_trust_policies_are_not_empty()
@@ -1874,7 +1875,8 @@ class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase):
1874 1875
         with open(self.policy_file_name, 'w') as f:
1875 1876
             overridden_policies = {
1876 1877
                 'identity:list_trusts': 'rule:admin_required',
1877
-                'identity:delete_trust': 'rule:admin_required'
1878
+                'identity:delete_trust': 'rule:admin_required',
1879
+                'identity:get_trust': 'rule:admin_required'
1878 1880
             }
1879 1881
             f.write(jsonutils.dumps(overridden_policies))
1880 1882
         result = self.checks.check_trust_policies_are_not_empty()

Loading…
Cancel
Save