From 1d4e40252884f054d692e82e170d3f69228ef7ee Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Thu, 29 Nov 2018 21:01:52 +0000 Subject: [PATCH] Update limit policies for system admin This change makes the policy definitions for admin limit operations consistent with the other limit policies. Subsequent patches will incorporate: - domain user test coverage - project user test coverage Change-Id: Id3f6159af505fbe81ff83cfaa346f2178f2d8e77 Closes-Bug: 1805372 Related-Bug: 1805880 --- keystone/common/policies/limit.py | 6 +- .../tests/unit/protection/v3/test_limits.py | 95 +++++++++++++++++++ keystone/tests/unit/test_limits.py | 64 +++++++++++++ keystone/tests/unit/test_v3_resource.py | 1 + .../notes/bug-1805372-af4ebf4b19500b72.yaml | 21 ++-- 5 files changed, 176 insertions(+), 11 deletions(-) diff --git a/keystone/common/policies/limit.py b/keystone/common/policies/limit.py index a50e4fd889..f6f8178ffa 100644 --- a/keystone/common/policies/limit.py +++ b/keystone/common/policies/limit.py @@ -50,21 +50,21 @@ limit_policies = [ 'method': 'HEAD'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'create_limits', - check_str=base.RULE_ADMIN_REQUIRED, + check_str=base.SYSTEM_ADMIN, scope_types=['system'], description='Create limits.', operations=[{'path': '/v3/limits', 'method': 'POST'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'update_limit', - check_str=base.RULE_ADMIN_REQUIRED, + check_str=base.SYSTEM_ADMIN, scope_types=['system'], description='Update limit.', operations=[{'path': '/v3/limits/{limit_id}', 'method': 'PATCH'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'delete_limit', - check_str=base.RULE_ADMIN_REQUIRED, + check_str=base.SYSTEM_ADMIN, scope_types=['system'], description='Delete limit.', operations=[{'path': '/v3/limits/{limit_id}', diff --git a/keystone/tests/unit/protection/v3/test_limits.py b/keystone/tests/unit/protection/v3/test_limits.py index d364344a9f..210c0cfa25 100644 --- a/keystone/tests/unit/protection/v3/test_limits.py +++ b/keystone/tests/unit/protection/v3/test_limits.py @@ -203,3 +203,98 @@ class SystemMemberTests(base_classes.TestCaseWithBootstrap, r = c.post('/v3/auth/tokens', json=auth) self.token_id = r.headers['X-Subject-Token'] self.headers = {'X-Auth-Token': self.token_id} + + +class SystemAdminTests(base_classes.TestCaseWithBootstrap, + common_auth.AuthTestMixin): + + def setUp(self): + super(SystemAdminTests, self).setUp() + self.loadapp() + self.useFixture(ksfixtures.Policy(self.config_fixture)) + self.config_fixture.config(group='oslo_policy', enforce_scope=True) + + # Reuse the system administrator account created during + # ``keystone-manage bootstrap`` + self.user_id = self.bootstrapper.admin_user_id + auth = self.build_authentication_request( + user_id=self.user_id, + password=self.bootstrapper.admin_password, + system=True + ) + + # Grab a token using the persona we're testing and prepare headers + # for requests we'll be making in the tests. + with self.test_client() as c: + r = c.post('/v3/auth/tokens', json=auth) + self.token_id = r.headers['X-Subject-Token'] + self.headers = {'X-Auth-Token': self.token_id} + + def test_user_can_get_a_limit(self): + limits = _create_limit_and_dependencies() + limit = limits[0] + + with self.test_client() as c: + r = c.get('/v3/limits/%s' % limit['id'], headers=self.headers) + self.assertEqual(limit['id'], r.json['limit']['id']) + + def test_user_can_list_limits(self): + limits = _create_limit_and_dependencies() + limit = limits[0] + + with self.test_client() as c: + r = c.get('/v3/limits', headers=self.headers) + self.assertTrue(len(r.json['limits']) == 1) + self.assertEqual(limit['id'], r.json['limits'][0]['id']) + + def test_user_can_create_limits(self): + service = PROVIDERS.catalog_api.create_service( + uuid.uuid4().hex, unit.new_service_ref() + ) + + registered_limit = unit.new_registered_limit_ref( + service_id=service['id'], id=uuid.uuid4().hex + ) + registered_limits = ( + PROVIDERS.unified_limit_api.create_registered_limits( + [registered_limit] + ) + ) + registered_limit = registered_limits[0] + + project = PROVIDERS.resource_api.create_project( + uuid.uuid4().hex, + unit.new_project_ref(domain_id=CONF.identity.default_domain_id) + ) + + create = { + 'limits': [ + unit.new_limit_ref( + project_id=project['id'], service_id=service['id'], + resource_name=registered_limit['resource_name'], + resource_limit=5 + ) + ] + } + + with self.test_client() as c: + c.post('/v3/limits', json=create, headers=self.headers) + + def test_user_can_update_limits(self): + limits = _create_limit_and_dependencies() + limit = limits[0] + + update = {'limits': {'description': uuid.uuid4().hex}} + + with self.test_client() as c: + c.patch( + '/v3/limits/%s' % limit['id'], json=update, + headers=self.headers + ) + + def test_user_can_delete_limits(self): + limits = _create_limit_and_dependencies() + limit = limits[0] + + with self.test_client() as c: + c.delete('/v3/limits/%s' % limit['id'], headers=self.headers) diff --git a/keystone/tests/unit/test_limits.py b/keystone/tests/unit/test_limits.py index 462f9414da..814bd5b2fb 100644 --- a/keystone/tests/unit/test_limits.py +++ b/keystone/tests/unit/test_limits.py @@ -413,6 +413,7 @@ class RegisteredLimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_ref = { @@ -546,6 +547,7 @@ class RegisteredLimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) id = r.result['registered_limits'][0]['id'] @@ -603,6 +605,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] @@ -619,6 +622,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] @@ -637,6 +641,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] @@ -655,6 +660,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.BAD_REQUEST) def test_create_multi_limit(self): @@ -668,6 +674,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref1, ref2]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] for key in ['service_id', 'resource_name', 'resource_limit']: @@ -684,6 +691,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref1]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] self.assertEqual(1, len(limits)) @@ -698,6 +706,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref2, ref3]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limits = r.result['limits'] self.assertEqual(2, len(limits)) @@ -713,6 +722,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [input_limit]}, + token=self.system_admin_token, expected_status=http_client.BAD_REQUEST) def test_create_limit_duplicate(self): @@ -723,10 +733,12 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CONFLICT) def test_create_limit_without_reference_registered_limit(self): @@ -737,6 +749,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_update_limit(self): @@ -748,6 +761,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_ref = { 'resource_limit': 5, @@ -756,6 +770,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_ref}, + token=self.system_admin_token, expected_status=http_client.OK) new_limits = r.result['limit'] @@ -769,6 +784,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.patch( '/limits/%s' % uuid.uuid4().hex, body={'limit': update_ref}, + token=self.system_admin_token, expected_status=http_client.NOT_FOUND) def test_update_limit_with_invalid_input(self): @@ -780,6 +796,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) limit_id = r.result['limits'][0]['id'] @@ -794,11 +811,13 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.patch( '/limits/%s' % limit_id, body={'limit': input_limit}, + token=self.system_admin_token, expected_status=http_client.BAD_REQUEST) def test_list_limit(self): r = self.get( '/limits', + token=self.system_admin_token, expected_status=http_client.OK) self.assertEqual([], r.result.get('limits')) @@ -812,6 +831,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref1, ref2]}, + token=self.system_admin_token, expected_status=http_client.CREATED) id1 = r.result['limits'][0]['id'] r = self.get( @@ -889,6 +909,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): self.post( '/limits', body={'limits': [ref1, ref2]}, + token=self.system_admin_token, expected_status=http_client.CREATED) # non system scoped request will get the limits in its project. @@ -937,6 +958,7 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref1, ref2]}, + token=self.system_admin_token, expected_status=http_client.CREATED) id1 = r.result['limits'][0]['id'] self.get('/limits/fake_id', @@ -959,14 +981,18 @@ class LimitsTestCase(test_v3.RestfulTestCase): r = self.post( '/limits', body={'limits': [ref1, ref2]}, + token=self.system_admin_token, expected_status=http_client.CREATED) id1 = r.result['limits'][0]['id'] self.delete('/limits/%s' % id1, + token=self.system_admin_token, expected_status=http_client.NO_CONTENT) self.delete('/limits/fake_id', + token=self.system_admin_token, expected_status=http_client.NOT_FOUND) r = self.get( '/limits', + token=self.system_admin_token, expected_status=http_client.OK) limits = r.result['limits'] self.assertEqual(len(limits), 1) @@ -976,6 +1002,13 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): def setUp(self): super(StrictTwoLevelLimitsTestCase, self).setUp() + # Most of these tests require system-scoped tokens. Let's have one on + # hand so that we can use it in tests when we need it. + PROVIDERS.assignment_api.create_system_grant_for_user( + self.user_id, self.role_id + ) + self.system_admin_token = self.get_system_scoped_token() + # create two hierarchical projects trees for test. # A D # / \ / \ @@ -1022,6 +1055,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_B['id'], @@ -1032,6 +1066,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_C['id'], @@ -1042,6 +1077,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) def test_create_child_limit_break_hierarchical_tree(self): @@ -1061,6 +1097,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_B['id'], @@ -1071,6 +1108,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_C['id'], @@ -1081,6 +1119,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_create_child_with_default_parent(self): @@ -1101,6 +1140,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_C['id'], @@ -1111,6 +1151,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_create_parent_limit(self): @@ -1126,6 +1167,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_A['id'], @@ -1136,6 +1178,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) def test_create_parent_limit_break_hierarchical_tree(self): @@ -1151,6 +1194,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref = unit.new_limit_ref(project_id=self.project_A['id'], @@ -1161,6 +1205,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_create_multi_limits(self): @@ -1201,6 +1246,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_A, ref_B, ref_C, ref_D, ref_E, ref_F]}, + token=self.system_admin_token, expected_status=http_client.CREATED) def test_create_multi_limits_invalid_input(self): @@ -1242,6 +1288,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_A, ref_B, ref_C, ref_D, ref_E, ref_F]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_create_multi_limits_break_hierarchical_tree(self): @@ -1272,6 +1319,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_A, ref_B, ref_E]}, + token=self.system_admin_token, expected_status=http_client.CREATED) ref_C = unit.new_limit_ref(project_id=self.project_C['id'], @@ -1287,6 +1335,7 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_C, ref_D]}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_update_child_limit(self): @@ -1312,16 +1361,19 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_A, ref_B]}, + token=self.system_admin_token, expected_status=http_client.CREATED) r = self.post( '/limits', body={'limits': [ref_C]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_dict = {'resource_limit': 9} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.OK) def test_update_child_limit_break_hierarchical_tree(self): @@ -1347,16 +1399,19 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): self.post( '/limits', body={'limits': [ref_A, ref_B]}, + token=self.system_admin_token, expected_status=http_client.CREATED) r = self.post( '/limits', body={'limits': [ref_C]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_dict = {'resource_limit': 11} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_update_child_limit_with_default_parent(self): @@ -1377,18 +1432,21 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): r = self.post( '/limits', body={'limits': [ref_C]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_dict = {'resource_limit': 9} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.OK) update_dict = {'resource_limit': 11} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) def test_update_parent_limit(self): @@ -1414,16 +1472,19 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): r = self.post( '/limits', body={'limits': [ref_A]}, + token=self.system_admin_token, expected_status=http_client.CREATED) self.post( '/limits', body={'limits': [ref_B, ref_C]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_dict = {'resource_limit': 8} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.OK) def test_update_parent_limit_break_hierarchical_tree(self): @@ -1449,14 +1510,17 @@ class StrictTwoLevelLimitsTestCase(LimitsTestCase): r = self.post( '/limits', body={'limits': [ref_A]}, + token=self.system_admin_token, expected_status=http_client.CREATED) self.post( '/limits', body={'limits': [ref_B, ref_C]}, + token=self.system_admin_token, expected_status=http_client.CREATED) update_dict = {'resource_limit': 6} self.patch( '/limits/%s' % r.result['limits'][0]['id'], body={'limit': update_dict}, + token=self.system_admin_token, expected_status=http_client.FORBIDDEN) diff --git a/keystone/tests/unit/test_v3_resource.py b/keystone/tests/unit/test_v3_resource.py index 7c39129a3e..4149b91ad2 100644 --- a/keystone/tests/unit/test_v3_resource.py +++ b/keystone/tests/unit/test_v3_resource.py @@ -1126,6 +1126,7 @@ class ResourceTestCase(test_v3.RestfulTestCase, self.post( '/limits', body={'limits': [limit1, limit2, limit3]}, + token=system_admin_token, expected_status=http_client.CREATED) # "include_limits" should work together with "parents_as_list" or # "subtree_as_list". Only using "include_limits" really does nothing. diff --git a/releasenotes/notes/bug-1805372-af4ebf4b19500b72.yaml b/releasenotes/notes/bug-1805372-af4ebf4b19500b72.yaml index 7cab01bb31..ad1a8fca40 100644 --- a/releasenotes/notes/bug-1805372-af4ebf4b19500b72.yaml +++ b/releasenotes/notes/bug-1805372-af4ebf4b19500b72.yaml @@ -2,24 +2,29 @@ features: - | [`bug 1805372 `_] - The registered limit API now supports the ``admin``, ``member``, and - ``reader`` default roles. + The registered limit and limit API now support the ``admin``, + ``member``, and ``reader`` default roles. upgrade: - | [`bug 1805372 `_] - The following registered limit policy check strings have changed - in favor of more clear and concise defaults: + Several of the registered limit and limit policies have been + deprecated. The following policies now use ``role:admin and + system_scope:all`` instead of ``rule:admin_required``: * ``identity:create_registered_limits`` * ``identity:update_registered_limit`` * ``identity:delete_registered_limit`` + * ``identity:create_limits`` + * ``identity:update_limit`` + * ``identity:delete_limit`` These policies are not being formally deprecated because the - unified limits API is still considered experiemental. Please + unified limits API is still considered experimental. These + new default automatically account for system-scope. Please consider these new defaults if your deployment overrides the - registered limit policies. + registered limit or limit policies. security: - | [`bug 1805372 `_] - The registered limit API now uses system-scope and default - roles to provide better accessibility to users in a secure way. + The registered limit and limit APIs now uses system-scope and default roles + to provide better accessibility to users in a secure way.