From 82fd155eb16fb806f4a44a2197dca03e999a05c7 Mon Sep 17 00:00:00 2001 From: wangzh21 Date: Fri, 23 Aug 2019 15:12:22 +0800 Subject: [PATCH] Enable api v2 policy check Change-Id: Ie1de7e23e938a174b0963bbf9e301b7ad5567fe6 --- cyborg/api/controllers/v2/arqs.py | 11 ++-- cyborg/api/controllers/v2/device_profiles.py | 11 ++-- cyborg/common/policy.py | 59 +++++++++---------- cyborg/tests/unit/api/base.py | 12 +++- .../unit/api/controllers/v2/test_arqs.py | 16 +++++ .../controllers/v2/test_device_profiles.py | 30 +++++++++- 6 files changed, 95 insertions(+), 44 deletions(-) diff --git a/cyborg/api/controllers/v2/arqs.py b/cyborg/api/controllers/v2/arqs.py index c2061f6f..f25cc755 100644 --- a/cyborg/api/controllers/v2/arqs.py +++ b/cyborg/api/controllers/v2/arqs.py @@ -25,6 +25,7 @@ from cyborg.api.controllers import link from cyborg.api.controllers import types from cyborg.api import expose from cyborg.common import exception +from cyborg.common import policy from cyborg import objects LOG = log.getLogger(__name__) @@ -107,7 +108,7 @@ class ARQsController(base.CyborgController): except Exception: return None - # @policy.authorize_wsgi("cyborg:arq", "create", False) + @policy.authorize_wsgi("cyborg:arq", "create", False) @expose.expose(ARQCollection, body=types.jsontype, status_code=http_client.CREATED) def post(self, req): @@ -159,7 +160,7 @@ class ARQsController(base.CyborgController): LOG.info('[arqs] post returned: %s', ret) return ret - # @policy.authorize_wsgi("cyborg:arq", "get_one") + @policy.authorize_wsgi("cyborg:arq", "get_one") @expose.expose(ARQ, wtypes.text) def get_one(self, uuid): """Get a single ARQ by UUID.""" @@ -167,7 +168,7 @@ class ARQsController(base.CyborgController): extarq = objects.ExtARQ.get(context, uuid) return ARQ.convert_with_links(extarq.arq) - # @policy.authorize_wsgi("cyborg:arq", "get_all") + @policy.authorize_wsgi("cyborg:arq", "get_all", False) @expose.expose(ARQCollection, wtypes.text, types.uuid) def get_all(self, bind_state=None, instance=None): """Retrieve a list of arqs.""" @@ -203,7 +204,7 @@ class ARQsController(base.CyborgController): LOG.info('[arqs:get_all] Returned: %s', ret) return ret - # @policy.authorize_wsgi("cyborg:arq", "delete") + @policy.authorize_wsgi("cyborg:arq", "delete") @expose.expose(None, wtypes.text, wtypes.text, status_code=http_client.NO_CONTENT) def delete(self, arqs=None, instance=None): @@ -260,7 +261,7 @@ class ARQsController(base.CyborgController): raise exception.PatchError(reason=reason) return valid_fields - # @policy.authorize_wsgi("cyborg:arq", "update") + @policy.authorize_wsgi("cyborg:arq", "update", False) @expose.expose(None, body=types.jsontype, status_code=http_client.ACCEPTED) def patch(self, patch_list): diff --git a/cyborg/api/controllers/v2/device_profiles.py b/cyborg/api/controllers/v2/device_profiles.py index 1fb9d6a6..2bab95a5 100644 --- a/cyborg/api/controllers/v2/device_profiles.py +++ b/cyborg/api/controllers/v2/device_profiles.py @@ -27,6 +27,7 @@ from cyborg.api.controllers import link from cyborg.api.controllers import types from cyborg.api import expose from cyborg.common import exception +from cyborg.common import policy from cyborg import objects LOG = log.getLogger(__name__) @@ -84,7 +85,7 @@ class DeviceProfileCollection(object): class DeviceProfilesController(base.CyborgController): """REST controller for Device Profiles.""" - # @policy.authorize_wsgi("cyborg:device_profile", "create", False) + @policy.authorize_wsgi("cyborg:device_profile", "create", False) @expose.expose('json', body=types.jsontype, status_code=http_client.CREATED) def post(self, req_devprof_list): @@ -164,7 +165,7 @@ class DeviceProfilesController(base.CyborgController): return api_obj_devprofs - # @policy.authorize_wsgi("cyborg:device_profile", "get_all") + @policy.authorize_wsgi("cyborg:device_profile", "get_all", False) @expose.expose('json', wtypes.text) def get_all(self, name=None): """Retrieve a list of device profiles.""" @@ -181,7 +182,7 @@ class DeviceProfilesController(base.CyborgController): return wsme.api.Response(ret, status_code=http_client.OK, return_type=wsme.types.DictType) - # @policy.authorize_wsgi("cyborg:device_profile", "get_one") + @policy.authorize_wsgi("cyborg:device_profile", "get_one") @expose.expose('json', wtypes.text) def get_one(self, uuid): """Retrieve a single device profile by uuid.""" @@ -202,14 +203,14 @@ class DeviceProfilesController(base.CyborgController): return wsme.api.Response(ret, status_code=http_client.OK, return_type=wsme.types.DictType) - # @policy.authorize_wsgi("cyborg:device_profile", "delete") + @policy.authorize_wsgi("cyborg:device_profile", "delete") @expose.expose(None, wtypes.text, status_code=http_client.NO_CONTENT) def delete(self, value): """Delete one or more device_profiles. URL: /device_profiles/{uuid} OR /device_profiles?value=foo,bar - :param value: Tis should be one of these two: + :param value: This should be one of these two: - UUID of a device_profile. - Comma-delimited list of device profile names. """ diff --git a/cyborg/common/policy.py b/cyborg/common/policy.py index c8fb084b..46e05894 100644 --- a/cyborg/common/policy.py +++ b/cyborg/common/policy.py @@ -73,40 +73,37 @@ default_policies = [ # All of these may be overridden by configuration, but we can # depend on their existence throughout the code. -accelerator_policies = [ - policy.RuleDefault('cyborg:accelerator:get', +accelerator_request_policies = [ + policy.RuleDefault('cyborg:arq:get_all', 'rule:default', - description='Retrieve accelerator records'), - policy.RuleDefault('cyborg:accelerator:create', + description='Retrieve accelerator request records.'), + policy.RuleDefault('cyborg:arq:get_one', + 'rule:default', + description='Get an accelerator request record.'), + policy.RuleDefault('cyborg:arq:create', 'rule:allow', - description='Create accelerator records'), - policy.RuleDefault('cyborg:accelerator:delete', + description='Create accelerator request records.'), + policy.RuleDefault('cyborg:arq:delete', 'rule:default', - description='Delete accelerator records'), - policy.RuleDefault('cyborg:accelerator:update', + description='Delete accelerator request records.'), + policy.RuleDefault('cyborg:arq:update', 'rule:default', - description='Update accelerator records'), + description='Update accelerator request records.'), ] -deployable_policies = [ - policy.RuleDefault('cyborg:deployable:get_one', - 'rule:allow', - description='Show deployable detail'), - policy.RuleDefault('cyborg:deployable:get_all', - 'rule:allow', - description='Retrieve all deployable records'), - policy.RuleDefault('cyborg:deployable:create', - 'rule:admin_api', - description='Create deployable records'), - policy.RuleDefault('cyborg:deployable:delete', - 'rule:admin_api', - description='Delete deployable records'), - policy.RuleDefault('cyborg:deployable:update', - 'rule:admin_api', - description='Update deployable records'), - policy.RuleDefault('cyborg:deployable:program', - 'rule:allow', - description='Program deployable(FPGA) records'), +device_profile_policies = [ + policy.RuleDefault('cyborg:device_profile:get_all', + 'rule:default', + description='Retrieve device_profile records.'), + policy.RuleDefault('cyborg:device_profile:get_one', + 'rule:default', + description='Get a device_profile record.'), + policy.RuleDefault('cyborg:device_profile:create', + 'rule:is_admin', + description='Create device_profile records.'), + policy.RuleDefault('cyborg:device_profile:delete', + 'rule:default', + description='Delete device_profile records.'), ] fpga_policies = [ @@ -124,9 +121,9 @@ fpga_policies = [ def list_policies(): return default_policies \ - + accelerator_policies \ - + deployable_policies \ - + fpga_policies + + fpga_policies \ + + accelerator_request_policies \ + + device_profile_policies @lockutils.synchronized('policy_enforcer', 'cyborg-') diff --git a/cyborg/tests/unit/api/base.py b/cyborg/tests/unit/api/base.py index a93d01b8..c62480ce 100644 --- a/cyborg/tests/unit/api/base.py +++ b/cyborg/tests/unit/api/base.py @@ -16,6 +16,7 @@ """Base classes for API tests.""" from oslo_config import cfg +from oslo_context import context import pecan import pecan.testing @@ -107,6 +108,10 @@ class BaseApiTest(base.DbTestCase): headers=headers, extra_environ=extra_environ, status=status, method="post") + def gen_context(self, value, **kwargs): + ct = context.RequestContext.from_dict(value, **kwargs) + return ct + def gen_headers(self, context, **kw): """Generate a header for a simulated HTTP request to Pecan test app. @@ -119,6 +124,10 @@ class BaseApiTest(base.DbTestCase): """ ct = context.to_dict() ct.update(kw) + if ct.get("is_admin"): + role = "admin" + else: + role = "user" headers = { 'X-User-Name': ct.get("user_name") or "user", 'X-User-Id': @@ -131,9 +140,8 @@ class BaseApiTest(base.DbTestCase): 'X-User-Domain-Name': ct.get("domain_name") or "no_domain", 'X-Auth-Token': ct.get("auth_token") or "b9764005b8c145bf972634fb16a826e8", - 'X-Roles': ct.get("roles") or "cyborg" + 'X-Roles': ct.get("roles") or role } - return headers def get_json(self, path, expect_errors=False, headers=None, diff --git a/cyborg/tests/unit/api/controllers/v2/test_arqs.py b/cyborg/tests/unit/api/controllers/v2/test_arqs.py index 0e9a481a..8919e92f 100644 --- a/cyborg/tests/unit/api/controllers/v2/test_arqs.py +++ b/cyborg/tests/unit/api/controllers/v2/test_arqs.py @@ -111,3 +111,19 @@ class TestARQsController(v2_test.APITestV2): args = '?' + "instance=" + instance response = self.delete(url + args, headers=self.headers) self.assertEqual(http_client.NO_CONTENT, response.status_int) + + def test_delete_with_non_default(self): + value = {"is_admin": False, "roles": "user", "is_admin_project": False} + ct = self.gen_context(value) + headers = self.gen_headers(ct) + url = self.ARQ_URL + arq = self.fake_extarqs[0].arq + args = '?' + "arqs=" + str(arq['uuid']) + exc = None + try: + self.delete(url + args, headers=headers) + except Exception as e: + exc = e + # Cyborg does not raise different exception when policy check failed + # now, improve this case with assertRaises later. + self.assertIn("Bad response: 403 Forbidden", exc.args[0]) diff --git a/cyborg/tests/unit/api/controllers/v2/test_device_profiles.py b/cyborg/tests/unit/api/controllers/v2/test_device_profiles.py index 2ceafb43..72613a4b 100644 --- a/cyborg/tests/unit/api/controllers/v2/test_device_profiles.py +++ b/cyborg/tests/unit/api/controllers/v2/test_device_profiles.py @@ -23,7 +23,6 @@ from cyborg.tests.unit import fake_device_profile class TestDeviceProfileController(v2_test.APITestV2): - DP_URL = '/device_profiles' def setUp(self): @@ -72,6 +71,20 @@ class TestDeviceProfileController(v2_test.APITestV2): for in_dp, out_dp in zip(self.fake_dp_objs, out_dps): self._validate_dp(in_dp, out_dp) + def test_create_with_non_admin(self): + value = {"is_admin": False, "roles": "user", "is_admin_project": False} + ct = self.gen_context(value) + headers = self.gen_headers(ct) + dp = [self.fake_dps[0]] + exc = None + try: + self.post_json(self.DP_URL, dp, headers=headers) + except Exception as e: + exc = e + # Cyborg does not raise different exception when policy check failed + # now, improve this case with assertRaises later. + self.assertIn("Bad response: 403 Forbidden", exc.args[0]) + @mock.patch('cyborg.objects.DeviceProfile.create') def test_create(self, mock_obj_dp): dp = [self.fake_dps[0]] @@ -92,3 +105,18 @@ class TestDeviceProfileController(v2_test.APITestV2): url = self.DP_URL + "?value=mydp" response = self.delete(url, headers=self.headers) self.assertEqual(http_client.NO_CONTENT, response.status_int) + + def test_delete_with_non_default(self): + value = {"is_admin": False, "roles": "user", "is_admin_project": False} + ct = self.gen_context(value) + headers = self.gen_headers(ct) + dp = self.fake_dp_objs[0] + url = self.DP_URL + '/%s' + exc = None + try: + self.delete(url % dp['uuid'], headers=headers) + except Exception as e: + exc = e + # Cyborg does not raise different exception when policy check failed + # now, improve this case with assertRaises later. + self.assertIn("Bad response: 403 Forbidden", exc.args[0])