From 72bedeba7f44963216d1017b6e6bb40a07dc2945 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Tue, 5 Mar 2019 21:25:16 +0000 Subject: [PATCH] Make system members the same as system readers for credentials It was decided some time ago that allowing system-members the ability to do certain things that system-readers can't do, but not as much as system-admins, isn't really all that helpful. Unfortunately, the credential API was one of the first APIs we migrated to formally adopting scope types and default roles. The credential update policy was still allowing system-members to access it, despite us deciding against it. This commit updates the policy to be consistent with the patterns we use for default roles across the rest of keystone's API. Change-Id: If11ded59cb191a4d8bf531689b8827c3bfbb39fa --- keystone/common/policies/credential.py | 6 +----- .../tests/unit/protection/v3/test_credentials.py | 13 ++++++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/keystone/common/policies/credential.py b/keystone/common/policies/credential.py index f04ba5438d..340f308b3c 100644 --- a/keystone/common/policies/credential.py +++ b/keystone/common/policies/credential.py @@ -19,10 +19,6 @@ SYSTEM_READER_OR_CRED_OWNER = ( '(role:reader and system_scope:all) ' 'or user_id:%(target.credential.user_id)s' ) -SYSTEM_MEMBER_OR_CRED_OWNER = ( - '(role:member and system_scope:all) ' - 'or user_id:%(target.credential.user_id)s' -) SYSTEM_ADMIN_OR_CRED_OWNER = ( '(role:admin and system_scope:all) ' 'or user_id:%(target.credential.user_id)s' @@ -93,7 +89,7 @@ credential_policies = [ ), policy.DocumentedRuleDefault( name=base.IDENTITY % 'update_credential', - check_str=SYSTEM_MEMBER_OR_CRED_OWNER, + check_str=SYSTEM_ADMIN_OR_CRED_OWNER, scope_types=['system', 'project'], description='Update credential.', operations=[{'path': '/v3/credentials/{credential_id}', diff --git a/keystone/tests/unit/protection/v3/test_credentials.py b/keystone/tests/unit/protection/v3/test_credentials.py index 21a26519ef..b681e7ee57 100644 --- a/keystone/tests/unit/protection/v3/test_credentials.py +++ b/keystone/tests/unit/protection/v3/test_credentials.py @@ -768,7 +768,7 @@ class SystemMemberTests(base_classes.TestCaseWithBootstrap, expected_status_code=http_client.FORBIDDEN ) - def test_user_can_update_credentials_for_others(self): + def test_user_cannot_update_credentials_for_others(self): user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) user_password = user['password'] user = PROVIDERS.identity_api.create_user(user) @@ -803,16 +803,19 @@ class SystemMemberTests(base_classes.TestCaseWithBootstrap, with self.test_client() as c: update = {'credential': {'blob': uuid.uuid4().hex}} path = '/v3/credentials/%s' % credential_id - c.patch(path, json=update, headers=self.headers) + c.patch( + path, json=update, headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) - def test_user_cannot_update_non_existant_credential_not_found(self): + def test_user_cannot_update_non_existant_credential_forbidden(self): with self.test_client() as c: update = {'credential': {'blob': uuid.uuid4().hex}} c.patch( '/v3/credentials/%s' % uuid.uuid4().hex, json=update, headers=self.headers, - expected_status_code=http_client.NOT_FOUND + expected_status_code=http_client.FORBIDDEN ) def test_user_cannot_delete_credentials_for_others(self): @@ -1131,7 +1134,7 @@ class ProjectAdminTests(base_classes.TestCaseWithBootstrap, 'identity:get_credential': cp.SYSTEM_READER_OR_CRED_OWNER, 'identity:list_credentials': cp.SYSTEM_READER_OR_CRED_OWNER, 'identity:create_credential': cp.SYSTEM_ADMIN_OR_CRED_OWNER, - 'identity:update_credential': cp.SYSTEM_MEMBER_OR_CRED_OWNER, + 'identity:update_credential': cp.SYSTEM_ADMIN_OR_CRED_OWNER, 'identity:delete_credential': cp.SYSTEM_ADMIN_OR_CRED_OWNER } f.write(jsonutils.dumps(overridden_policies))