diff --git a/api-guide/source/acls.rst b/api-guide/source/acls.rst index 6f137fda2..24d97eac7 100644 --- a/api-guide/source/acls.rst +++ b/api-guide/source/acls.rst @@ -48,7 +48,8 @@ Following ACL rules are defined and used as `OR` in resource access policy: * ACL based access is allowed when token user is present in secret/container operation specific ACL user list e.g. token user present in `read` users list. * When secret/container resource is marked private, then project-level RBAC users access is not - allowed. + allowed. e.g. When a secret is marked private, only the user who created it + or a user with the "admin" role on the project will be able to remove it. .. note:: diff --git a/barbican/common/policies/secrets.py b/barbican/common/policies/secrets.py index 17e24aacf..5989e836b 100644 --- a/barbican/common/policies/secrets.py +++ b/barbican/common/policies/secrets.py @@ -72,6 +72,8 @@ rules = [ name='secret:delete', check_str='rule:secret_project_admin or ' + 'rule:secret_project_creator or ' + + '(rule:secret_project_creator_role and ' + + 'not rule:secret_private_read) or ' + f"({_PROJECT_MEMBER} and ({_SECRET_CREATOR} or " + f"{_SECRET_IS_NOT_PRIVATE})) or {_PROJECT_ADMIN}", scope_types=['project'], diff --git a/barbican/tests/api/test_resources_policy.py b/barbican/tests/api/test_resources_policy.py index bf6ab224a..6b1cd3a69 100644 --- a/barbican/tests/api/test_resources_policy.py +++ b/barbican/tests/api/test_resources_policy.py @@ -663,15 +663,26 @@ class WhenTestingSecretResource(BaseTestCase): content_type="application/octet-stream") def test_should_pass_delete_secret(self): - self._assert_pass_rbac(['admin'], self._invoke_on_delete, + self._assert_pass_rbac(['admin', 'creator'], self._invoke_on_delete, user_id=self.user_id, project_id=self.external_project_id) def test_should_raise_delete_secret(self): - """A non-admin user cannot delete other user's secret. + """Only admin and creator can delete secrets User id is different from initial user who has created the secret. """ + self._assert_fail_rbac([None, 'audit', 'observer', 'bogus'], + self._invoke_on_delete, + user_id=self.user_id, + project_id=self.external_project_id) + + def test_should_raise_delete_private_secret(self): + self.acl_list.pop() # remove read acl from default setup + acl_read = models.SecretACL(secret_id=self.secret_id, operation='read', + project_access=False, + user_ids=['anyRandomUserX', 'aclUser1']) + self.acl_list.append(acl_read) self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'], self._invoke_on_delete, user_id=self.user_id, diff --git a/doc/source/admin/access_control.rst b/doc/source/admin/access_control.rst index 8f5eae6d8..2d56cf895 100644 --- a/doc/source/admin/access_control.rst +++ b/doc/source/admin/access_control.rst @@ -58,11 +58,10 @@ admin by the project for which the admin role is scoped. creator - Users with this role are allowed to create new resources and can only - delete resources which are originally created (owned) by them. Users with - this role cannot delete other user's resources managed within same project. - They are also allowed full access to existing secrets owned by the project - in scope. + Users with this role are allowed to create new resources and can also + delete resources which are owned by the project for which the creator role + is scoped. They are also allowed full access to existing secrets owned by + the project in scope. observer Users with this role are allowed to access to existing resources but are @@ -76,10 +75,14 @@ Access Control List API ----------------------- There are some limitations that result from scoping ownership of a secret at the -project level. For example, there is no easy way for a user to upload a secret -for which only they have access. There is also no easy way to grant a user -access to only a single secret. +project level. For example, it is not possible to grant a user access to a +single secret, as granting a role on a project would allow access to all +all secrets owned by that project. -To address this limitations the Key Manager service includes an Access Control +Additionally, there is no easy way to upload a private secret (i.e. a secret +that only you have access to) without creating a new project for which only +you have roles assigned on it. + +To address these limitations the Key Manager service includes an Access Control List (ACL) API. For full details see the `ACL API User Guide `__ diff --git a/functionaltests/api/v1/functional/test_secrets_rbac.py b/functionaltests/api/v1/functional/test_secrets_rbac.py index 6fce00d52..e9d977fd4 100644 --- a/functionaltests/api/v1/functional/test_secrets_rbac.py +++ b/functionaltests/api/v1/functional/test_secrets_rbac.py @@ -90,7 +90,7 @@ test_data_rbac_delete_secret = { 'with_creator_a': {'user': creator_a, 'admin': creator_a, 'expected_return': 204}, 'with_creator_a_2': {'user': creator_a_2, 'admin': creator_a, - 'expected_return': 403}, + 'expected_return': 204}, 'with_observer_a': {'user': observer_a, 'admin': admin_a, 'expected_return': 403}, 'with_auditor_a': {'user': auditor_a, 'admin': admin_a, diff --git a/releasenotes/notes/fix-story-2009791-allow-creator-delete-06dd3eb670d0e624.yaml b/releasenotes/notes/fix-story-2009791-allow-creator-delete-06dd3eb670d0e624.yaml new file mode 100644 index 000000000..a2a837336 --- /dev/null +++ b/releasenotes/notes/fix-story-2009791-allow-creator-delete-06dd3eb670d0e624.yaml @@ -0,0 +1,11 @@ +--- +security: + - | + Fixed Story #2009791: Users with the "creator" role on a project can now + delete secrets owned by the project even if the user is different than + the user that originally created the secret. Previous to this fix a user + with the "creator" role was only allowed to delete a secret owned by the + project if they were also the same user that originally created, which + was inconsistent with the way that deletes are handled by other OpenStack + projects that integrate with Barbican. This change does not affect private + secrets (i.e. secrets with the "project-access" flag set to "false").