Browse Source

Merge "Move list_trusts enforcement to default policies"

changes/22/678322/10
Zuul 1 week ago
parent
commit
cdf27685b7

+ 2
- 0
doc/source/getting-started/policy_mapping.rst View File

@@ -138,6 +138,8 @@ identity:revocation_list                                   GET /v3/auth/tokens/O
138 138
 identity:revoke_token                                      DELETE /v3/auth/tokens
139 139
 identity:create_trust                                      POST /v3/OS-TRUST/trusts
140 140
 identity:list_trusts                                       GET /v3/OS-TRUST/trusts
141
+identity:list_trusts_for_trustor                           GET /v3/OS-TRUST/trusts?trustor_user_id={trustor_user_id}
142
+identity:list_trusts_for_trustee                           GET /v3/OS-TRUST/trusts?trustee_user_id={trustee_user_id}
141 143
 identity:list_roles_for_trust                              GET /v3/OS-TRUST/trusts/{trust_id}/roles
142 144
 identity:get_role_for_trust                                GET /v3/OS-TRUST/trusts/{trust_id}/roles/{role_id}
143 145
 identity:delete_trust                                      DELETE /v3/OS-TRUST/trusts/{trust_id}

+ 42
- 21
keystone/api/trusts.py View File

@@ -17,6 +17,8 @@
17 17
 
18 18
 import flask
19 19
 import flask_restful
20
+from oslo_log import log
21
+from oslo_policy import _checks as op_checks
20 22
 from six.moves import http_client
21 23
 
22 24
 from keystone.api._shared import json_home_relations
@@ -24,6 +26,7 @@ from keystone.common import context
24 26
 from keystone.common import json_home
25 27
 from keystone.common import provider_api
26 28
 from keystone.common import rbac_enforcer
29
+from keystone.common.rbac_enforcer import policy
27 30
 from keystone.common import utils
28 31
 from keystone.common import validation
29 32
 from keystone import exception
@@ -32,6 +35,7 @@ from keystone.server import flask as ks_flask
32 35
 from keystone.trust import schema
33 36
 
34 37
 
38
+LOG = log.getLogger(__name__)
35 39
 ENFORCER = rbac_enforcer.RBACEnforcer
36 40
 PROVIDERS = provider_api.ProviderAPIs
37 41
 
@@ -156,32 +160,49 @@ class TrustResource(ks_flask.ResourceBase):
156 160
         return self.wrap_member(trust)
157 161
 
158 162
     def _list_trusts(self):
159
-        ENFORCER.enforce_call(action='identity:list_trusts')
160
-        trusts = []
161 163
         trustor_user_id = flask.request.args.get('trustor_user_id')
162 164
         trustee_user_id = flask.request.args.get('trustee_user_id')
163
-
164
-        if not flask.request.args:
165
-            # NOTE(morgan): Admin can list all trusts.
166
-            ENFORCER.enforce_call(action='admin_required')
167
-            trusts += PROVIDERS.trust_api.list_trusts()
168
-
169
-        # TODO(morgan): Convert the trustor/trustee checks into policy
170
-        # checkstr we can enforce on. This is duplication of code
171
-        # behavior/design as the OS-TRUST controller for ease of review/
172
-        # comparison of previous code. Minor optimizations [checks before db
173
-        # hits] have been done.
174
-        action = _('Cannot list trusts for another user')
175 165
         if trustor_user_id:
176
-            if trustor_user_id != self.oslo_context.user_id:
177
-                raise exception.ForbiddenAction(action=action)
166
+            target = {'trust': {'trustor_user_id': trustor_user_id}}
167
+            ENFORCER.enforce_call(action='identity:list_trusts_for_trustor',
168
+                                  target_attr=target)
169
+        elif trustee_user_id:
170
+            target = {'trust': {'trustee_user_id': trustee_user_id}}
171
+            ENFORCER.enforce_call(action='identity:list_trusts_for_trustee',
172
+                                  target_attr=target)
173
+        else:
174
+            ENFORCER.enforce_call(action='identity:list_trusts')
178 175
 
179
-        if trustee_user_id:
180
-            if trustee_user_id != self.oslo_context.user_id:
181
-                raise exception.ForbiddenAction(action=action)
176
+        trusts = []
182 177
 
183
-        trusts += PROVIDERS.trust_api.list_trusts_for_trustor(trustor_user_id)
184
-        trusts += PROVIDERS.trust_api.list_trusts_for_trustee(trustee_user_id)
178
+        # NOTE(cmurphy) As of Train, the default policies enforce the
179
+        # identity:list_trusts rule and there are new policies in-code to
180
+        # enforce identity:list_trusts_for_trustor and
181
+        # identity:list_trusts_for_trustee. However, in case the
182
+        # identity:list_trusts rule has been locally overridden by the default
183
+        # that would have been produced by the sample config, we need to
184
+        # enforce it again and warn that the behavior is changing.
185
+        rules = policy._ENFORCER._enforcer.rules.get('identity:list_trusts')
186
+        # rule check_str is ""
187
+        if isinstance(rules, op_checks.TrueCheck):
188
+            LOG.warning(
189
+                "The policy check string for rule \"identity:list_trusts\" has been overridden"
190
+                "to \"always true\". In the next release, this will cause the"
191
+                "\"identity:list_trusts\" action to be fully permissive as hardcoded"
192
+                "enforcement will be removed. To correct this issue, either stop overriding the"
193
+                "\"identity:list_trusts\" rule in config to accept the defaults, or explicitly"
194
+                "set a rule that is not empty."
195
+            )
196
+            if not flask.request.args:
197
+                # NOTE(morgan): Admin can list all trusts.
198
+                ENFORCER.enforce_call(action='admin_required')
199
+
200
+        if not flask.request.args:
201
+            trusts += PROVIDERS.trust_api.list_trusts()
202
+        elif trustor_user_id:
203
+            trusts += PROVIDERS.trust_api.list_trusts_for_trustor(trustor_user_id)
204
+        elif trustee_user_id:
205
+            trusts += PROVIDERS.trust_api.list_trusts_for_trustee(trustee_user_id)
185 206
 
186 207
         for trust in trusts:
187 208
             # get_trust returns roles, list_trusts does not

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

@@ -10,11 +10,15 @@
10 10
 # License for the specific language governing permissions and limitations
11 11
 # under the License.
12 12
 
13
+from oslo_policy import _checks
14
+from oslo_policy import policy
13 15
 from oslo_upgradecheck import upgradecheck
14 16
 
17
+from keystone.common import rbac_enforcer
15 18
 import keystone.conf
16 19
 
17 20
 CONF = keystone.conf.CONF
21
+ENFORCER = rbac_enforcer.RBACEnforcer
18 22
 
19 23
 
20 24
 class Checks(upgradecheck.UpgradeCommands):
@@ -26,8 +30,31 @@ class Checks(upgradecheck.UpgradeCommands):
26 30
     policies actually exist within the roles backend.
27 31
     """
28 32
 
29
-    pass
33
+    def check_trust_policies_are_not_empty(self):
34
+        enforcer = policy.Enforcer(CONF)
35
+        ENFORCER.register_rules(enforcer)
36
+        enforcer.load_rules()
37
+        rule = enforcer.rules.get('identity:list_trusts')
38
+        if isinstance(rule, _checks.TrueCheck):
39
+            return upgradecheck.Result(
40
+                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."
48
+            )
49
+        return upgradecheck.Result(
50
+            upgradecheck.Code.SUCCESS, "\"identity:list_trusts\" policy is safe")
51
+
52
+    _upgrade_checks = (
53
+        ("Check trust policies are not empty",
54
+         check_trust_policies_are_not_empty),
55
+    )
30 56
 
31 57
 
32 58
 def main():
59
+    keystone.conf.configure()
33 60
     return upgradecheck.main(CONF, 'keystone', Checks())

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

@@ -14,6 +14,9 @@ from oslo_policy import policy
14 14
 
15 15
 from keystone.common.policies import base
16 16
 
17
+RULE_TRUSTOR = 'user_id:%(target.trust.trustor_user_id)s'
18
+RULE_TRUSTEE = 'user_id:%(target.trust.trustee_user_id)s'
19
+
17 20
 trust_policies = [
18 21
     policy.DocumentedRuleDefault(
19 22
         name=base.IDENTITY % 'create_trust',
@@ -27,13 +30,31 @@ trust_policies = [
27 30
                      'method': 'POST'}]),
28 31
     policy.DocumentedRuleDefault(
29 32
         name=base.IDENTITY % 'list_trusts',
30
-        check_str='',
33
+        check_str=base.RULE_ADMIN_REQUIRED,
31 34
         scope_types=['project'],
32 35
         description='List trusts.',
33 36
         operations=[{'path': '/v3/OS-TRUST/trusts',
34 37
                      'method': 'GET'},
35 38
                     {'path': '/v3/OS-TRUST/trusts',
36 39
                      'method': 'HEAD'}]),
40
+    policy.DocumentedRuleDefault(
41
+        name=base.IDENTITY % 'list_trusts_for_trustor',
42
+        check_str=RULE_TRUSTOR,
43
+        scope_types=['project'],
44
+        description='List trusts for trustor.',
45
+        operations=[{'path': '/v3/OS-TRUST/trusts?trustor_user_id={trustor_user_id}',
46
+                     'method': 'GET'},
47
+                    {'path': '/v3/OS-TRUST/trusts?trustor_user_id={trustor_user_id}',
48
+                     'method': 'HEAD'}]),
49
+    policy.DocumentedRuleDefault(
50
+        name=base.IDENTITY % 'list_trusts_for_trustee',
51
+        check_str=RULE_TRUSTEE,
52
+        scope_types=['project'],
53
+        description='List trusts for trustee.',
54
+        operations=[{'path': '/v3/OS-TRUST/trusts?trustee_user_id={trustee_user_id}',
55
+                     'method': 'GET'},
56
+                    {'path': '/v3/OS-TRUST/trusts?trustee_user_id={trustee_user_id}',
57
+                     'method': 'HEAD'}]),
37 58
     policy.DocumentedRuleDefault(
38 59
         name=base.IDENTITY % 'list_roles_for_trust',
39 60
         check_str='',

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

@@ -12,6 +12,7 @@
12 12
 
13 13
 import uuid
14 14
 
15
+from oslo_serialization import jsonutils
15 16
 from six.moves import http_client
16 17
 
17 18
 from keystone.common import provider_api
@@ -20,6 +21,7 @@ from keystone.tests.common import auth as common_auth
20 21
 from keystone.tests import unit
21 22
 from keystone.tests.unit import base_classes
22 23
 from keystone.tests.unit import ksfixtures
24
+from keystone.tests.unit.ksfixtures import temporaryfile
23 25
 
24 26
 CONF = keystone.conf.CONF
25 27
 PROVIDERS = provider_api.ProviderAPIs
@@ -35,7 +37,13 @@ class TrustTests(base_classes.TestCaseWithBootstrap,
35 37
     def setUp(self):
36 38
         super(TrustTests, self).setUp()
37 39
         self.loadapp()
38
-        self.useFixture(ksfixtures.Policy(self.config_fixture))
40
+        self.policy_file = self.useFixture(temporaryfile.SecureTempFile())
41
+        self.policy_file_name = self.policy_file.file_name
42
+        self.useFixture(
43
+            ksfixtures.Policy(
44
+                self.config_fixture, policy_file=self.policy_file_name
45
+            )
46
+        )
39 47
 
40 48
         domain = PROVIDERS.resource_api.create_domain(
41 49
             uuid.uuid4().hex, unit.new_domain_ref()
@@ -92,6 +100,18 @@ class TrustTests(base_classes.TestCaseWithBootstrap,
92 100
             self.token_id = r.headers['X-Subject-Token']
93 101
             self.trustee_headers = {'X-Auth-Token': self.token_id}
94 102
 
103
+    def _override_policy_old_defaults(self):
104
+        # TODO(cmurphy): This is to simulate what would happen if the operator
105
+        # had generated a sample policy config, or had never removed their old
106
+        # policy files since we adopted policy in code, and had explicitly
107
+        # retained the old "" policy check strings. Remove this once the
108
+        # hardcoded enforcement is removed from the trusts API.
109
+        with open(self.policy_file_name, 'w') as f:
110
+            overridden_policies = {
111
+                'identity:list_trusts': '',
112
+            }
113
+            f.write(jsonutils.dumps(overridden_policies))
114
+
95 115
 
96 116
 class _AdminTestsMixin(object):
97 117
     """Tests for all admin users.
@@ -226,6 +246,18 @@ class SystemAdminTests(TrustTests, _AdminTestsMixin):
226 246
                 expected_status_code=http_client.FORBIDDEN
227 247
             )
228 248
 
249
+    def test_admin_list_all_trusts_overridden_defaults(self):
250
+        self._override_policy_old_defaults()
251
+        PROVIDERS.trust_api.create_trust(
252
+            self.trust_id, **self.trust_data)
253
+
254
+        with self.test_client() as c:
255
+            r = c.get(
256
+                '/v3/OS-TRUST/trusts',
257
+                headers=self.headers
258
+            )
259
+        self.assertEqual(1, len(r.json['trusts']))
260
+
229 261
 
230 262
 class ProjectUserTests(TrustTests):
231 263
     """Tests for all project users."""
@@ -501,3 +533,67 @@ class ProjectUserTests(TrustTests):
501 533
                 headers=self.other_headers,
502 534
                 expected_status_code=http_client.FORBIDDEN
503 535
             )
536
+
537
+    def test_trustor_cannot_list_trusts_for_trustee_overridden_default(self):
538
+        self._override_policy_old_defaults()
539
+        PROVIDERS.trust_api.create_trust(
540
+            self.trust_id, **self.trust_data)
541
+
542
+        with self.test_client() as c:
543
+            c.get(
544
+                ('/v3/OS-TRUST/trusts?trustee_user_id=%s' %
545
+                 self.trustee_user_id),
546
+                headers=self.trustor_headers,
547
+                expected_status_code=http_client.FORBIDDEN
548
+            )
549
+
550
+    def test_trustee_cannot_list_trusts_for_trustor_overridden_default(self):
551
+        self._override_policy_old_defaults()
552
+        PROVIDERS.trust_api.create_trust(
553
+            self.trust_id, **self.trust_data)
554
+
555
+        with self.test_client() as c:
556
+            c.get(
557
+                ('/v3/OS-TRUST/trusts?trustor_user_id=%s' %
558
+                 self.trustor_user_id),
559
+                headers=self.trustee_headers,
560
+                expected_status_code=http_client.FORBIDDEN
561
+            )
562
+
563
+    def test_user_cannot_list_trusts_for_other_trustor_overridden_default(self):
564
+        self._override_policy_old_defaults()
565
+        PROVIDERS.trust_api.create_trust(
566
+            self.trust_id, **self.trust_data)
567
+
568
+        with self.test_client() as c:
569
+            c.get(
570
+                ('/v3/OS-TRUST/trusts?trustor_user_id=%s' %
571
+                 self.trustor_user_id),
572
+                headers=self.other_headers,
573
+                expected_status_code=http_client.FORBIDDEN
574
+            )
575
+
576
+    def test_user_cannot_list_trusts_for_other_trustee_overridden_default(self):
577
+        self._override_policy_old_defaults()
578
+        PROVIDERS.trust_api.create_trust(
579
+            self.trust_id, **self.trust_data)
580
+
581
+        with self.test_client() as c:
582
+            c.get(
583
+                ('/v3/OS-TRUST/trusts?trustee_user_id=%s' %
584
+                 self.trustee_user_id),
585
+                headers=self.other_headers,
586
+                expected_status_code=http_client.FORBIDDEN
587
+            )
588
+
589
+    def test_user_cannot_list_all_trusts_overridden_default(self):
590
+        self._override_policy_old_defaults()
591
+        PROVIDERS.trust_api.create_trust(
592
+            self.trust_id, **self.trust_data)
593
+
594
+        with self.test_client() as c:
595
+            c.get(
596
+                '/v3/OS-TRUST/trusts',
597
+                headers=self.trustee_headers,
598
+                expected_status_code=http_client.FORBIDDEN
599
+            )

+ 39
- 0
keystone/tests/unit/test_cli.py View File

@@ -26,6 +26,7 @@ import oslo_config.fixture
26 26
 from oslo_db.sqlalchemy import migration
27 27
 from oslo_log import log
28 28
 from oslo_serialization import jsonutils
29
+from oslo_upgradecheck import upgradecheck
29 30
 from six.moves import configparser
30 31
 from six.moves import http_client
31 32
 from six.moves import range
@@ -41,6 +42,7 @@ from keystone.cmd.doctor import ldap
41 42
 from keystone.cmd.doctor import security_compliance
42 43
 from keystone.cmd.doctor import tokens
43 44
 from keystone.cmd.doctor import tokens_fernet
45
+from keystone.cmd import status
44 46
 from keystone.common import provider_api
45 47
 from keystone.common.sql import upgrades
46 48
 import keystone.conf
@@ -51,6 +53,7 @@ from keystone.tests import unit
51 53
 from keystone.tests.unit import default_fixtures
52 54
 from keystone.tests.unit.ksfixtures import database
53 55
 from keystone.tests.unit.ksfixtures import ldapdb
56
+from keystone.tests.unit.ksfixtures import policy
54 57
 from keystone.tests.unit.ksfixtures import temporaryfile
55 58
 from keystone.tests.unit import mapping_fixtures
56 59
 
@@ -1843,3 +1846,39 @@ class TestMappingEngineTester(unit.BaseTestCase):
1843 1846
         mapping_engine = cli.MappingEngineTester()
1844 1847
         self.assertRaises(exception.ValidationError,
1845 1848
                           mapping_engine.main)
1849
+
1850
+
1851
+class CliStatusTestCase(unit.SQLDriverOverrides, unit.TestCase):
1852
+
1853
+    def setUp(self):
1854
+        super(CliStatusTestCase, self).setUp()
1855
+        self.load_backends()
1856
+        self.policy_file = self.useFixture(temporaryfile.SecureTempFile())
1857
+        self.policy_file_name = self.policy_file.file_name
1858
+        self.useFixture(
1859
+            policy.Policy(
1860
+                self.config_fixture, policy_file=self.policy_file_name
1861
+            )
1862
+        )
1863
+        self.checks = status.Checks()
1864
+
1865
+    def test_check_safe_trust_policies(self):
1866
+        with open(self.policy_file_name, 'w') as f:
1867
+            overridden_policies = {
1868
+                'identity:list_trusts': ''
1869
+            }
1870
+            f.write(jsonutils.dumps(overridden_policies))
1871
+        result = self.checks.check_trust_policies_are_not_empty()
1872
+        self.assertEqual(upgradecheck.Code.FAILURE, result.code)
1873
+        with open(self.policy_file_name, 'w') as f:
1874
+            overridden_policies = {
1875
+                'identity:list_trusts': 'rule:admin_required'
1876
+            }
1877
+            f.write(jsonutils.dumps(overridden_policies))
1878
+        result = self.checks.check_trust_policies_are_not_empty()
1879
+        self.assertEqual(upgradecheck.Code.SUCCESS, result.code)
1880
+        with open(self.policy_file_name, 'w') as f:
1881
+            overridden_policies = {}
1882
+            f.write(jsonutils.dumps(overridden_policies))
1883
+        result = self.checks.check_trust_policies_are_not_empty()
1884
+        self.assertEqual(upgradecheck.Code.SUCCESS, result.code)

+ 2
- 0
keystone/tests/unit/test_policy.py View File

@@ -194,6 +194,8 @@ class PolicyJsonTestCase(unit.TestCase):
194 194
             'identity:revocation_list',
195 195
             'identity:create_trust',
196 196
             'identity:list_trusts',
197
+            'identity:list_trusts_for_trustor',
198
+            'identity:list_trusts_for_trustee',
197 199
             'identity:list_roles_for_trust',
198 200
             'identity:get_role_for_trust',
199 201
             'identity:delete_trust',

Loading…
Cancel
Save