From 960159332864740cfbdaeb339ddbaae48238a6d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Douglas=20Mendiz=C3=A1bal?= <dmendiza@redhat.com>
Date: Thu, 4 Nov 2021 16:57:09 -0500
Subject: [PATCH] Fix container consumers rbac policy
This patch modifies the Consumer controller to enable the use of
ownership information in policy checks. e.g. policies that use a target
container:
project_id:%(target.container.project_id)
Story: 2009664
Task: 43872
Depends-On: I8698fc7a9ac849b8c24adfe824ca44dd3e42b999
Change-Id: I1724152839f0f5850f8d32d40b36d1670c0ad996
---
barbican/api/controllers/__init__.py | 25 ++--
barbican/api/controllers/acls.py | 2 +
barbican/api/controllers/consumers.py | 73 +++++------
barbican/api/controllers/containers.py | 17 +--
barbican/api/controllers/orders.py | 3 +-
barbican/api/controllers/quotas.py | 3 +
barbican/api/controllers/secretmeta.py | 6 +-
barbican/api/controllers/secrets.py | 6 +-
barbican/api/controllers/secretstores.py | 3 +
barbican/api/controllers/transportkeys.py | 2 +
barbican/common/policies/consumers.py | 113 +++++++++++++-----
barbican/tests/api/test_resources_policy.py | 67 +++++++++--
.../api/v1/functional/test_acls.py | 34 ++++--
.../fix-story-2009664-042ef282c0dd6b6a.yaml | 13 ++
14 files changed, 245 insertions(+), 122 deletions(-)
create mode 100644 releasenotes/notes/fix-story-2009664-042ef282c0dd6b6a.yaml
diff --git a/barbican/api/controllers/__init__.py b/barbican/api/controllers/__init__.py
index 9cc2d2220..8c9d0a5d2 100644
--- a/barbican/api/controllers/__init__.py
+++ b/barbican/api/controllers/__init__.py
@@ -173,8 +173,22 @@ def flatten(d, parent_key=''):
class ACLMixin(object):
+ def __init__(self):
+ self.secret = None
+ self.container = None
+
def get_acl_tuple(self, req, **kwargs):
- return None, None
+ if self.secret is not None:
+ entity = 'secret'
+ elif self.container is not None:
+ entity = 'container'
+ else:
+ return None, None
+ entity_acls = getattr(getattr(self, entity), '{}_acls'.format(entity))
+ acl = self.get_acl_dict_for_user(req, entity_acls)
+ acl['project_id'] = getattr(self, entity).project.external_id
+ acl['creator_id'] = getattr(self, entity).creator_id
+ return entity, acl
def get_acl_dict_for_user(self, req, acl_list):
"""Get acl operation found for token user in acl list.
@@ -237,12 +251,3 @@ class ACLMixin(object):
acl_dict.update(co_dict)
return acl_dict
-
-
-class SecretACLMixin(ACLMixin):
-
- def get_acl_tuple(self, req, **kwargs):
- acl = self.get_acl_dict_for_user(req, self.secret.secret_acls)
- acl['project_id'] = self.secret.project.external_id
- acl['creator_id'] = self.secret.creator_id
- return 'secret', acl
diff --git a/barbican/api/controllers/acls.py b/barbican/api/controllers/acls.py
index d65f87af2..cf9b0ceb6 100644
--- a/barbican/api/controllers/acls.py
+++ b/barbican/api/controllers/acls.py
@@ -47,6 +47,7 @@ class SecretACLsController(controllers.ACLMixin):
"""Handles SecretACL requests by a given secret id."""
def __init__(self, secret):
+ super().__init__()
self.secret = secret
self.secret_project_id = self.secret.project.external_id
self.acl_repo = repo.get_secret_acl_repository()
@@ -210,6 +211,7 @@ class ContainerACLsController(controllers.ACLMixin):
"""Handles ContainerACL requests by a given container id."""
def __init__(self, container):
+ super().__init__()
self.container = container
self.container_id = container.id
self.acl_repo = repo.get_container_acl_repository()
diff --git a/barbican/api/controllers/consumers.py b/barbican/api/controllers/consumers.py
index ed0585c8a..037d67d6d 100644
--- a/barbican/api/controllers/consumers.py
+++ b/barbican/api/controllers/consumers.py
@@ -46,7 +46,9 @@ def _invalid_consumer_id():
class ContainerConsumerController(controllers.ACLMixin):
"""Handles Container Consumer entity retrieval and deletion requests"""
- def __init__(self, consumer_id):
+ def __init__(self, container, consumer_id):
+ super().__init__()
+ self.container = container
self.consumer_id = consumer_id
self.consumer_repo = repo.get_container_consumer_repository()
self.validator = validators.ContainerConsumerValidator()
@@ -78,8 +80,10 @@ class ContainerConsumerController(controllers.ACLMixin):
class ContainerConsumersController(controllers.ACLMixin):
"""Handles Container Consumer creation requests"""
- def __init__(self, container_id):
- self.container_id = container_id
+ def __init__(self, container):
+ super().__init__()
+ self.container = container
+ self.container_id = self.container.id
self.consumer_repo = repo.get_container_consumer_repository()
self.container_repo = repo.get_container_repository()
self.project_repo = repo.get_project_repository()
@@ -91,7 +95,8 @@ class ContainerConsumersController(controllers.ACLMixin):
def _lookup(self, consumer_id, *remainder):
if not utils.validate_id_is_uuid(consumer_id):
_invalid_consumer_id()()
- return ContainerConsumerController(consumer_id), remainder
+ return ContainerConsumerController(self.container, consumer_id), \
+ remainder
@pecan.expose(generic=True)
def index(self, **kwargs):
@@ -99,7 +104,7 @@ class ContainerConsumersController(controllers.ACLMixin):
@index.when(method='GET', template='json')
@controllers.handle_exceptions(u._('ContainerConsumers(s) retrieval'))
- @controllers.enforce_rbac('consumers:get')
+ @controllers.enforce_rbac('container_consumers:get')
def on_get(self, external_project_id, **kw):
LOG.debug('Start consumers on_get '
'for container-ID %s:', self.container_id)
@@ -137,7 +142,7 @@ class ContainerConsumersController(controllers.ACLMixin):
@index.when(method='POST', template='json')
@controllers.handle_exceptions(u._('ContainerConsumer creation'))
- @controllers.enforce_rbac('consumers:post')
+ @controllers.enforce_rbac('container_consumers:post')
@controllers.enforce_content_types(['application/json'])
def on_post(self, external_project_id, **kwargs):
@@ -145,14 +150,12 @@ class ContainerConsumersController(controllers.ACLMixin):
data = api.load_body(pecan.request, validator=self.validator)
LOG.debug('Start on_post...%s', data)
- container = self._get_container(self.container_id)
-
self.quota_enforcer.enforce(project)
new_consumer = models.ContainerConsumerMetadatum(self.container_id,
project.id,
data)
- self.consumer_repo.create_or_update_from(new_consumer, container)
+ self.consumer_repo.create_or_update_from(new_consumer, self.container)
url = hrefs.convert_consumer_to_href(new_consumer.container_id)
pecan.response.headers['Location'] = url
@@ -160,19 +163,16 @@ class ContainerConsumersController(controllers.ACLMixin):
LOG.info('Created a container consumer for project: %s',
external_project_id)
- return self._return_container_data(self.container_id)
+ return self._return_container_data()
@index.when(method='DELETE', template='json')
@controllers.handle_exceptions(u._('ContainerConsumer deletion'))
- @controllers.enforce_rbac('consumers:delete')
+ @controllers.enforce_rbac('container_consumers:delete')
@controllers.enforce_content_types(['application/json'])
def on_delete(self, external_project_id, **kwargs):
data = api.load_body(pecan.request, validator=self.validator)
LOG.debug('Start on_delete...%s', data)
- project = self.project_repo.find_by_external_project_id(
- external_project_id, suppress_exception=True)
- if not project:
- _consumer_not_found()
+ project = res.get_or_create_project(external_project_id)
consumer = self.consumer_repo.get_by_values(
self.container_id,
@@ -184,9 +184,8 @@ class ContainerConsumersController(controllers.ACLMixin):
_consumer_not_found()
LOG.debug("Found container consumer: %s", consumer)
- container = self._get_container(self.container_id)
owner_of_consumer = consumer.project_id == project.id
- owner_of_container = container.project.external_id \
+ owner_of_container = self.container.project.external_id \
== external_project_id
if not owner_of_consumer and not owner_of_container:
_consumer_ownership_mismatch()
@@ -198,22 +197,13 @@ class ContainerConsumersController(controllers.ACLMixin):
LOG.exception('Problem deleting container consumer')
_consumer_not_found()
- ret_data = self._return_container_data(self.container_id)
+ ret_data = self._return_container_data()
LOG.info('Deleted a container consumer for project: %s',
external_project_id)
return ret_data
- def _get_container(self, container_id):
- container = self.container_repo.get_container_by_id(
- container_id, suppress_exception=True)
- if not container:
- controllers.containers.container_not_found()
- return container
-
- def _return_container_data(self, container_id):
- container = self._get_container(container_id)
-
- dict_fields = container.to_dict_fields()
+ def _return_container_data(self):
+ dict_fields = self.container.to_dict_fields()
for secret_ref in dict_fields['secret_refs']:
hrefs.convert_to_hrefs(secret_ref)
@@ -227,7 +217,9 @@ class ContainerConsumersController(controllers.ACLMixin):
class SecretConsumerController(controllers.ACLMixin):
"""Handles Secret Consumer entity retrieval and deletion requests"""
- def __init__(self, consumer_id):
+ def __init__(self, secret, consumer_id):
+ super().__init__()
+ self.secret = secret
self.consumer_id = consumer_id
self.consumer_repo = repo.get_secret_consumer_repository()
self.validator = validators.SecretConsumerValidator()
@@ -259,8 +251,10 @@ class SecretConsumerController(controllers.ACLMixin):
class SecretConsumersController(controllers.ACLMixin):
"""Handles Secret Consumer creation requests"""
- def __init__(self, secret_id):
- self.secret_id = secret_id
+ def __init__(self, secret):
+ super().__init__()
+ self.secret = secret
+ self.secret_id = secret.id
self.consumer_repo = repo.get_secret_consumer_repository()
self.secret_repo = repo.get_secret_repository()
self.project_repo = repo.get_project_repository()
@@ -272,7 +266,7 @@ class SecretConsumersController(controllers.ACLMixin):
def _lookup(self, consumer_id, *remainder):
if not utils.validate_id_is_uuid(consumer_id):
_invalid_consumer_id()()
- return SecretConsumerController(consumer_id), remainder
+ return SecretConsumerController(self.secret, consumer_id), remainder
@pecan.expose(generic=True)
def index(self, **kwargs):
@@ -280,7 +274,7 @@ class SecretConsumersController(controllers.ACLMixin):
@index.when(method='GET', template='json')
@controllers.handle_exceptions(u._('SecretConsumers(s) retrieval'))
- @controllers.enforce_rbac('consumers:get')
+ @controllers.enforce_rbac('secret_consumers:get')
def on_get(self, external_project_id, **kw):
LOG.debug('Start consumers on_get '
'for secret-ID %s:', self.secret_id)
@@ -318,7 +312,7 @@ class SecretConsumersController(controllers.ACLMixin):
@index.when(method='POST', template='json')
@controllers.handle_exceptions(u._('SecretConsumer creation'))
- @controllers.enforce_rbac('consumers:post')
+ @controllers.enforce_rbac('secret_consumers:post')
@controllers.enforce_content_types(['application/json'])
def on_post(self, external_project_id, **kwargs):
@@ -326,8 +320,6 @@ class SecretConsumersController(controllers.ACLMixin):
data = api.load_body(pecan.request, validator=self.validator)
LOG.debug('Start on_post...%s', data)
- secret = self._get_secret(self.secret_id)
-
self.quota_enforcer.enforce(project)
new_consumer = models.SecretConsumerMetadatum(
@@ -337,7 +329,7 @@ class SecretConsumersController(controllers.ACLMixin):
data["resource_type"],
data["resource_id"],
)
- self.consumer_repo.create_or_update_from(new_consumer, secret)
+ self.consumer_repo.create_or_update_from(new_consumer, self.secret)
url = hrefs.convert_consumer_to_href(new_consumer.secret_id)
pecan.response.headers['Location'] = url
@@ -349,7 +341,7 @@ class SecretConsumersController(controllers.ACLMixin):
@index.when(method='DELETE', template='json')
@controllers.handle_exceptions(u._('SecretConsumer deletion'))
- @controllers.enforce_rbac('consumers:delete')
+ @controllers.enforce_rbac('secret_consumers:delete')
@controllers.enforce_content_types(['application/json'])
def on_delete(self, external_project_id, **kwargs):
data = api.load_body(pecan.request, validator=self.validator)
@@ -368,9 +360,8 @@ class SecretConsumersController(controllers.ACLMixin):
_consumer_not_found()
LOG.debug("Found consumer: %s", consumer)
- secret = self._get_secret(self.secret_id)
owner_of_consumer = consumer.project_id == project.id
- owner_of_secret = secret.project.external_id \
+ owner_of_secret = self.secret.project.external_id \
== external_project_id
if not owner_of_consumer and not owner_of_secret:
_consumer_ownership_mismatch()
diff --git a/barbican/api/controllers/containers.py b/barbican/api/controllers/containers.py
index bac8934c4..1550a968e 100644
--- a/barbican/api/controllers/containers.py
+++ b/barbican/api/controllers/containers.py
@@ -45,21 +45,16 @@ class ContainerController(controllers.ACLMixin):
"""Handles Container entity retrieval and deletion requests."""
def __init__(self, container):
+ super().__init__()
self.container = container
self.container_id = container.id
self.consumer_repo = repo.get_container_consumer_repository()
self.container_repo = repo.get_container_repository()
self.validator = validators.ContainerValidator()
self.consumers = consumers.ContainerConsumersController(
- self.container_id)
+ self.container)
self.acl = acls.ContainerACLsController(self.container)
- def get_acl_tuple(self, req, **kwargs):
- d = self.get_acl_dict_for_user(req, self.container.container_acls)
- d['project_id'] = self.container.project.external_id
- d['creator_id'] = self.container.creator_id
- return 'container', d
-
@pecan.expose(generic=True, template='json')
def index(self, **kwargs):
pecan.abort(405) # HTTP 405 Method Not Allowed as default
@@ -112,6 +107,7 @@ class ContainersController(controllers.ACLMixin):
"""Handles Container creation requests."""
def __init__(self):
+ super().__init__()
self.consumer_repo = repo.get_container_consumer_repository()
self.container_repo = repo.get_container_repository()
self.secret_repo = repo.get_secret_repository()
@@ -230,17 +226,12 @@ class ContainersSecretsController(controllers.ACLMixin):
def __init__(self, container):
LOG.debug('=== Creating ContainerSecretsController ===')
+ super().__init__()
self.container = container
self.container_secret_repo = repo.get_container_secret_repository()
self.secret_repo = repo.get_secret_repository()
self.validator = validators.ContainerSecretValidator()
- def get_acl_tuple(self, req, **kwargs):
- acl = self.get_acl_dict_for_user(req, self.container.container_acls)
- acl['project_id'] = self.container.project.external_id
- acl['creator_id'] = self.container.creator_id
- return ('container', acl)
-
@pecan.expose(generic=True)
def index(self, **kwargs):
pecan.abort(405) # HTTP 405 Method Not Allowed as default
diff --git a/barbican/api/controllers/orders.py b/barbican/api/controllers/orders.py
index 635407ae5..977ae6455 100644
--- a/barbican/api/controllers/orders.py
+++ b/barbican/api/controllers/orders.py
@@ -61,6 +61,7 @@ class OrderController(controllers.ACLMixin):
"""Handles Order retrieval and deletion requests."""
def __init__(self, order, queue_resource=None):
+ super().__init__()
self.order = order
self.order_repo = repo.get_order_repository()
self.queue = queue_resource or async_client.TaskClient()
@@ -96,8 +97,8 @@ class OrdersController(controllers.ACLMixin):
"""Handles Order requests for Secret creation."""
def __init__(self, queue_resource=None):
-
LOG.debug('Creating OrdersController')
+ super().__init__()
self.order_repo = repo.get_order_repository()
self.queue = queue_resource or async_client.TaskClient()
self.type_order_validator = validators.TypeOrderValidator()
diff --git a/barbican/api/controllers/quotas.py b/barbican/api/controllers/quotas.py
index e387da4e3..cdf70a4fe 100644
--- a/barbican/api/controllers/quotas.py
+++ b/barbican/api/controllers/quotas.py
@@ -37,6 +37,7 @@ class QuotasController(controllers.ACLMixin):
def __init__(self):
LOG.debug('=== Creating QuotasController ===')
+ super().__init__()
self.quota_driver = quota.QuotaDriver()
@pecan.expose(generic=True)
@@ -59,6 +60,7 @@ class ProjectQuotasController(controllers.ACLMixin):
def __init__(self, project_id):
LOG.debug('=== Creating ProjectQuotasController ===')
+ super().__init__()
self.passed_project_id = project_id
self.validator = validators.ProjectQuotaValidator()
self.quota_driver = quota.QuotaDriver()
@@ -114,6 +116,7 @@ class ProjectsQuotasController(controllers.ACLMixin):
def __init__(self):
LOG.debug('=== Creating ProjectsQuotaController ===')
+ super().__init__()
self.quota_driver = quota.QuotaDriver()
@pecan.expose()
diff --git a/barbican/api/controllers/secretmeta.py b/barbican/api/controllers/secretmeta.py
index 8215beac7..8f31a8224 100644
--- a/barbican/api/controllers/secretmeta.py
+++ b/barbican/api/controllers/secretmeta.py
@@ -28,11 +28,12 @@ def _secret_metadata_not_found():
pecan.abort(404, u._('Secret metadata not found.'))
-class SecretMetadataController(controllers.SecretACLMixin):
+class SecretMetadataController(controllers.ACLMixin):
"""Handles SecretMetadata requests by a given secret id."""
def __init__(self, secret):
LOG.debug('=== Creating SecretMetadataController ===')
+ super().__init__()
self.secret = secret
self.secret_project_id = self.secret.project.external_id
self.secret_repo = repo.get_secret_repository()
@@ -106,10 +107,11 @@ class SecretMetadataController(controllers.SecretACLMixin):
return {'key': key, 'value': value}
-class SecretMetadatumController(controllers.SecretACLMixin):
+class SecretMetadatumController(controllers.ACLMixin):
def __init__(self, secret):
LOG.debug('=== Creating SecretMetadatumController ===')
+ super().__init__()
self.user_meta_repo = repo.get_secret_user_meta_repository()
self.secret = secret
self.metadatum_validator = validators.NewSecretMetadatumValidator()
diff --git a/barbican/api/controllers/secrets.py b/barbican/api/controllers/secrets.py
index 12d6caf37..d9289b00c 100644
--- a/barbican/api/controllers/secrets.py
+++ b/barbican/api/controllers/secrets.py
@@ -71,13 +71,14 @@ def _request_has_twsk_but_no_transport_key_id():
'transport key id has not been provided.'))
-class SecretController(controllers.SecretACLMixin):
+class SecretController(controllers.ACLMixin):
"""Handles Secret retrieval and deletion requests."""
def __init__(self, secret):
LOG.debug('=== Creating SecretController ===')
+ super().__init__()
self.secret = secret
- self.consumers = consumers.SecretConsumersController(secret.id)
+ self.consumers = consumers.SecretConsumersController(secret)
self.consumer_repo = repo.get_secret_consumer_repository()
self.transport_key_repo = repo.get_transport_key_repository()
@@ -276,6 +277,7 @@ class SecretsController(controllers.ACLMixin):
def __init__(self):
LOG.debug('Creating SecretsController')
+ super().__init__()
self.validator = validators.NewSecretValidator()
self.secret_repo = repo.get_secret_repository()
self.quota_enforcer = quota.QuotaEnforcer('secrets', self.secret_repo)
diff --git a/barbican/api/controllers/secretstores.py b/barbican/api/controllers/secretstores.py
index b328568ed..fb79651be 100644
--- a/barbican/api/controllers/secretstores.py
+++ b/barbican/api/controllers/secretstores.py
@@ -57,6 +57,7 @@ class PreferredSecretStoreController(controllers.ACLMixin):
def __init__(self, secret_store):
LOG.debug('=== Creating PreferredSecretStoreController ===')
+ super().__init__()
self.secret_store = secret_store
self.proj_store_repo = repo.get_project_secret_store_repository()
@@ -103,6 +104,7 @@ class SecretStoreController(controllers.ACLMixin):
def __init__(self, secret_store):
LOG.debug('=== Creating SecretStoreController ===')
+ super().__init__()
self.secret_store = secret_store
@pecan.expose()
@@ -129,6 +131,7 @@ class SecretStoresController(controllers.ACLMixin):
def __init__(self):
LOG.debug('Creating SecretStoresController')
+ super().__init__()
self.secret_stores_repo = repo.get_secret_stores_repository()
self.proj_store_repo = repo.get_project_secret_store_repository()
diff --git a/barbican/api/controllers/transportkeys.py b/barbican/api/controllers/transportkeys.py
index 892b5e0c4..08a834b5b 100644
--- a/barbican/api/controllers/transportkeys.py
+++ b/barbican/api/controllers/transportkeys.py
@@ -43,6 +43,7 @@ class TransportKeyController(controllers.ACLMixin):
def __init__(self, transport_key_id, transport_key_repo=None):
LOG.debug('=== Creating TransportKeyController ===')
+ super().__init__()
self.transport_key_id = transport_key_id
self.repo = transport_key_repo or repo.TransportKeyRepo()
@@ -83,6 +84,7 @@ class TransportKeysController(controllers.ACLMixin):
def __init__(self, transport_key_repo=None):
LOG.debug('Creating TransportKeyController')
+ super().__init__()
self.repo = transport_key_repo or repo.TransportKeyRepo()
self.validator = validators.NewTransportKeyValidator()
diff --git a/barbican/common/policies/consumers.py b/barbican/common/policies/consumers.py
index b99c7b843..3f8ce1f34 100644
--- a/barbican/common/policies/consumers.py
+++ b/barbican/common/policies/consumers.py
@@ -21,10 +21,20 @@ _READER = "role:reader"
_MEMBER = "role:member"
_ADMIN = "role:admin"
_SYSTEM_ADMIN = "role:admin and system_scope:all"
-_PROJECT_MEMBER = f"{_MEMBER} and project_id:%(target.container.project_id)s"
-_PROJECT_ADMIN = f"{_ADMIN} and project_id:%(target.container.project_id)s"
+
+_SECRET_CREATOR = "user_id:%(target.secret.creator_id)s"
+_SECRET_PROJECT = "project_id:%(target.secret.project_id)s"
+_SECRET_MEMBER = f"{_MEMBER} and {_SECRET_PROJECT}"
+_SECRET_ADMIN = f"{_ADMIN} and {_SECRET_PROJECT}"
+_SECRET_ACCESS = (f"{_SECRET_CREATOR} or ({_SECRET_MEMBER} and "
+ f"True:%(target.secret.read_project_access)s)")
+
_CONTAINER_CREATOR = "user_id:%(target.container.creator_id)s"
-_CONTAINER_IS_NOT_PRIVATE = "True:%(target.container.read_project_access)s"
+_CONTAINER_PROJECT = "project_id:%(target.container.project_id)s"
+_CONTAINER_MEMBER = f"{_MEMBER} and {_CONTAINER_PROJECT}"
+_CONTAINER_ADMIN = f"{_ADMIN} and {_CONTAINER_PROJECT}"
+_CONTAINER_ACCESS = (f"{_CONTAINER_CREATOR} or ({_CONTAINER_MEMBER} and "
+ f"True:%(target.container.read_project_access)s)")
rules = [
policy.DocumentedRuleDefault(
@@ -33,27 +43,23 @@ rules = [
'rule:audit or rule:container_non_private_read or ' +
'rule:container_project_creator or ' +
'rule:container_project_admin or rule:container_acl_read' +
- f" or ({_PROJECT_MEMBER} and ({_CONTAINER_CREATOR} or " +
- f"{_CONTAINER_IS_NOT_PRIVATE})) or {_PROJECT_ADMIN} or " +
+ f" or {_CONTAINER_ACCESS} or {_CONTAINER_ADMIN} or " +
f"{_SYSTEM_ADMIN}",
scope_types=['project', 'system'],
- description='List a specific consumer for a given container.',
- operations=[
- {
- 'path': '/v1/containers/{container-id}/consumers/' +
- '{consumer-id}',
- 'method': 'GET'
- }
- ]
+ # This API is unusable. There is no way for a user to get
+ # the consumer-id they would need to send a request.
+ description='DEPRECATED: show information for a specific consumer',
+ operations=[{
+ 'path': '/v1/containers/{container-id}/consumers/{consumer-id}',
+ 'method': 'GET'
+ }]
),
policy.DocumentedRuleDefault(
- name='consumers:get',
- check_str='rule:admin or rule:observer or rule:creator or ' +
- 'rule:audit or rule:container_non_private_read or ' +
+ name='container_consumers:get',
+ check_str='rule:container_non_private_read or ' +
'rule:container_project_creator or ' +
- 'rule:container_project_admin or rule:container_acl_read' +
- f" or ({_PROJECT_MEMBER} and ({_CONTAINER_CREATOR} or " +
- f"{_CONTAINER_IS_NOT_PRIVATE})) or {_PROJECT_ADMIN} or " +
+ 'rule:container_project_admin or rule:container_acl_read ' +
+ f" or {_CONTAINER_ACCESS} or {_CONTAINER_ADMIN} or " +
f"{_SYSTEM_ADMIN}",
scope_types=['project', 'system'],
description='List a containers consumers.',
@@ -65,12 +71,11 @@ rules = [
]
),
policy.DocumentedRuleDefault(
- name='consumers:post',
- check_str='rule:admin or rule:container_non_private_read or ' +
+ name='container_consumers:post',
+ check_str='rule:container_non_private_read or ' +
'rule:container_project_creator or ' +
- 'rule:container_project_admin or rule:container_acl_read' +
- f" or ({_PROJECT_MEMBER} and ({_CONTAINER_CREATOR} or " +
- f"{_CONTAINER_IS_NOT_PRIVATE})) or {_PROJECT_ADMIN} or " +
+ 'rule:container_project_admin or rule:container_acl_read ' +
+ f" or {_CONTAINER_ACCESS} or {_CONTAINER_ADMIN} or " +
f"{_SYSTEM_ADMIN}",
scope_types=['project', 'system'],
description='Creates a consumer.',
@@ -82,19 +87,65 @@ rules = [
]
),
policy.DocumentedRuleDefault(
- name='consumers:delete',
- check_str='rule:admin or rule:container_non_private_read or ' +
+ name='container_consumers:delete',
+ check_str='rule:container_non_private_read or ' +
'rule:container_project_creator or ' +
- 'rule:container_project_admin or rule:container_acl_read' +
- f" or ({_PROJECT_MEMBER} and ({_CONTAINER_CREATOR} or " +
- f"{_CONTAINER_IS_NOT_PRIVATE})) or {_PROJECT_ADMIN} or " +
+ 'rule:container_project_admin or rule:container_acl_read ' +
+ f" or {_CONTAINER_ACCESS} or {_CONTAINER_ADMIN} or " +
f"{_SYSTEM_ADMIN}",
scope_types=['project', 'system'],
description='Deletes a consumer.',
operations=[
{
- 'path': '/v1/containers/{container-id}/consumers/' +
- '{consumer-id}',
+ 'path': '/v1/containers/{container-id}/consumers',
+ 'method': 'DELETE'
+ }
+ ]
+ ),
+ policy.DocumentedRuleDefault(
+ name='secret_consumers:get',
+ check_str='rule:secret_non_private_read or ' +
+ 'rule:secret_project_creator or ' +
+ 'rule:secret_project_admin or rule:secret_acl_read ' +
+ f" or {_SECRET_ACCESS} or {_SECRET_ADMIN} or " +
+ f"{_SYSTEM_ADMIN}",
+ scope_types=['project', 'system'],
+ description='List consumers for a secret.',
+ operations=[
+ {
+ 'path': '/v1/secrets/{secret-id}/consumers',
+ 'method': 'GET'
+ }
+ ]
+ ),
+ policy.DocumentedRuleDefault(
+ name='secret_consumers:post',
+ check_str='rule:secret_non_private_read or ' +
+ 'rule:secret_project_creator or ' +
+ 'rule:secret_project_admin or rule:secret_acl_read ' +
+ f" or {_SECRET_ACCESS} or {_SECRET_ADMIN} or " +
+ f"{_SYSTEM_ADMIN}",
+ scope_types=['project', 'system'],
+ description='Creates a consumer.',
+ operations=[
+ {
+ 'path': '/v1/secrets/{secrets-id}/consumers',
+ 'method': 'POST'
+ }
+ ]
+ ),
+ policy.DocumentedRuleDefault(
+ name='secret_consumers:delete',
+ check_str='rule:secret_non_private_read or ' +
+ 'rule:secret_project_creator or ' +
+ 'rule:secret_project_admin or rule:secret_acl_read ' +
+ f" or {_SECRET_ACCESS} or {_SECRET_ADMIN} or " +
+ f"{_SYSTEM_ADMIN}",
+ scope_types=['project', 'system'],
+ description='Deletes a consumer.',
+ operations=[
+ {
+ 'path': '/v1/secrets/{secrets-id}/consumers',
'method': 'DELETE'
}
]
diff --git a/barbican/tests/api/test_resources_policy.py b/barbican/tests/api/test_resources_policy.py
index bf6ab224a..af984e949 100644
--- a/barbican/tests/api/test_resources_policy.py
+++ b/barbican/tests/api/test_resources_policy.py
@@ -1059,10 +1059,12 @@ class WhenTestingConsumersResource(BaseTestCase):
self.external_project_id = '12345project'
self.container_id = '12345container'
+ self.creator_user_id = '123456CreatorUser'
# Force an error on GET calls that pass RBAC, as we are not testing
# such flows in this test module.
self.consumer_repo = mock.MagicMock()
+ self.container_repo = mock.MagicMock()
get_by_container_id = mock.MagicMock(return_value=None,
side_effect=self
._generate_get_error())
@@ -1072,23 +1074,34 @@ class WhenTestingConsumersResource(BaseTestCase):
self.setup_container_consumer_repository_mock(self.consumer_repo)
self.setup_container_repository_mock()
- self.resource = ConsumersResource(container_id=self.container_id)
+ container = mock.MagicMock()
+ container.id = self.container_id
+ container.project.external_id = self.external_project_id
+ container.creator_id = self.creator_user_id
+ self.container_repo.get_container_by_id.return_value = container
+ self.setup_container_repository_mock(self.container_repo)
+
+ self.resource = ConsumersResource(container)
def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules)
def test_should_pass_create_consumer(self):
self._assert_pass_rbac(['admin'], self._invoke_on_post,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_create_consumer(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],
self._invoke_on_post,
- content_type='application/json')
+ content_type='application/json',
+ user_id='some_other_user',
+ project_id='some_other_project')
def test_should_pass_delete_consumer(self):
self._assert_pass_rbac(['admin'], self._invoke_on_delete,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_delete_consumer(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],
@@ -1097,7 +1110,8 @@ class WhenTestingConsumersResource(BaseTestCase):
def test_should_pass_get_consumers(self):
self._assert_pass_rbac(['admin', 'observer', 'creator', 'audit'],
self._invoke_on_get,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_get_consumers(self):
self._assert_fail_rbac([None, 'bogus'],
@@ -1121,17 +1135,28 @@ class WhenTestingConsumerResource(BaseTestCase):
self.external_project_id = '12345project'
self.consumer_id = '12345consumer'
+ self.container_id = '12345container'
+ self.creator_user_id = '123456CreatorUser'
# Force an error on GET calls that pass RBAC, as we are not testing
# such flows in this test module.
self.consumer_repo = mock.MagicMock()
+ self.container_repo = mock.MagicMock()
fail_method = mock.MagicMock(return_value=None,
side_effect=self._generate_get_error())
self.consumer_repo.get = fail_method
self.setup_project_repository_mock()
self.setup_container_consumer_repository_mock(self.consumer_repo)
- self.resource = ConsumerResource(consumer_id=self.consumer_id)
+
+ container = mock.MagicMock()
+ container.id = self.container_id
+ container.project.external_id = self.external_project_id
+ container.creator_id = self.creator_user_id
+ self.container_repo.get_container_by_id.return_value = container
+ self.setup_container_repository_mock(self.container_repo)
+
+ self.resource = ConsumerResource(container, self.consumer_id)
def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules)
@@ -1292,6 +1317,7 @@ class WhenTestingSecretConsumersResource(BaseTestCase):
self.external_project_id = '12345project'
self.secret_id = '12345secret'
+ self.creator_user_id = '123456CreatorUser'
# Force an error on GET calls that pass RBAC, as we are not testing
# such flows in this test module.
@@ -1301,27 +1327,35 @@ class WhenTestingSecretConsumersResource(BaseTestCase):
._generate_get_error())
self.consumer_repo.get_by_secret_id = get_by_secret_id
+ secret = mock.MagicMock()
+ secret.id = self.secret_id
+ secret.project.external_id = self.external_project_id
+ secret.creator_id = self.creator_user_id
+
self.setup_project_repository_mock()
self.setup_secret_consumer_repository_mock(self.consumer_repo)
self.setup_secret_repository_mock()
- self.resource = SecretConsumersResource(secret_id=self.secret_id)
+ self.resource = SecretConsumersResource(secret)
def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules)
def test_should_pass_create_consumer(self):
self._assert_pass_rbac(['admin'], self._invoke_on_post,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_create_consumer(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],
self._invoke_on_post,
- content_type='application/json')
+ content_type='application/json',
+ project_id='some_other_id')
def test_should_pass_delete_consumer(self):
self._assert_pass_rbac(['admin'], self._invoke_on_delete,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_delete_consumer(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],
@@ -1330,12 +1364,14 @@ class WhenTestingSecretConsumersResource(BaseTestCase):
def test_should_pass_get_consumers(self):
self._assert_pass_rbac(['admin', 'observer', 'creator', 'audit'],
self._invoke_on_get,
- content_type='application/json')
+ content_type='application/json',
+ project_id=self.external_project_id)
def test_should_raise_get_consumers(self):
self._assert_fail_rbac([None, 'bogus'],
self._invoke_on_get,
- content_type='application/json')
+ content_type='application/json',
+ project_id='some_other_id')
def _invoke_on_post(self):
self.resource.on_post(self.req, self.resp)
@@ -1354,6 +1390,7 @@ class WhenTestingSecretConsumerResource(BaseTestCase):
self.external_project_id = '12345project'
self.consumer_id = '12345consumer'
+ self.creator_user_id = '123456CreatorUser'
# Force an error on GET calls that pass RBAC, as we are not testing
# such flows in this test module.
@@ -1362,9 +1399,13 @@ class WhenTestingSecretConsumerResource(BaseTestCase):
side_effect=self._generate_get_error())
self.consumer_repo.get = fail_method
+ secret = mock.MagicMock()
+ secret.project.external_id = self.external_project_id
+ secret.creator_id = self.creator_user_id
+
self.setup_project_repository_mock()
self.setup_secret_consumer_repository_mock(self.consumer_repo)
- self.resource = SecretConsumerResource(consumer_id=self.consumer_id)
+ self.resource = SecretConsumerResource(secret, self.consumer_id)
def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules)
diff --git a/functionaltests/api/v1/functional/test_acls.py b/functionaltests/api/v1/functional/test_acls.py
index db19ca268..e701a9a41 100644
--- a/functionaltests/api/v1/functional/test_acls.py
+++ b/functionaltests/api/v1/functional/test_acls.py
@@ -136,20 +136,22 @@ test_data_read_container_consumer_acl_only = {
test_data_delete_container_consumer_acl_only = {
'with_admin_a': {'user': admin_a, 'expected_return': 200},
- 'with_creator_a': {'user': creator_a, 'expected_return': 403},
- 'with_observer_a': {'user': observer_a, 'expected_return': 403},
- 'with_auditor_a': {'user': auditor_a, 'expected_return': 403},
+ 'with_creator_a': {'user': creator_a, 'expected_return': 200},
+ 'with_observer_a': {'user': observer_a, 'expected_return': 200},
+ 'with_auditor_a': {'user': auditor_a, 'expected_return': 200},
+ # the consumer being deleted is owned by project a, so attempts
+ # to remove it with users from project b below should fail
'with_admin_b': {'user': admin_b, 'expected_return': 403},
'with_observer_b': {'user': observer_b, 'expected_return': 403},
}
test_data_create_container_consumer_acl_only = {
'with_admin_a': {'user': admin_a, 'expected_return': 200},
- 'with_creator_a': {'user': creator_a, 'expected_return': 403},
- 'with_observer_a': {'user': observer_a, 'expected_return': 403},
- 'with_auditor_a': {'user': auditor_a, 'expected_return': 403},
+ 'with_creator_a': {'user': creator_a, 'expected_return': 200},
+ 'with_observer_a': {'user': observer_a, 'expected_return': 200},
+ 'with_auditor_a': {'user': auditor_a, 'expected_return': 200},
'with_admin_b': {'user': admin_b, 'expected_return': 200},
- 'with_observer_b': {'user': observer_b, 'expected_return': 403},
+ 'with_observer_b': {'user': observer_b, 'expected_return': 200},
}
@@ -272,7 +274,14 @@ class AclTestCase(base.TestCase):
@utils.parameterized_dataset(test_data_delete_container_consumer_acl_only)
def test_container_acl_remove_consumer(self, user, expected_return):
- """Acl access will not allow you to delete a consumer"""
+ """Test DELETE /v1/containers/{container-id}/consumers
+
+ Test default policy for deleting a consumer set by admin_a
+ from a private container owned by creator_a.
+
+ Each user in the data set is added to the ACL and then used
+ to delete the consumer set by admin_a.
+ """
container_ref = self.store_container(user_name=creator_a,
admin=admin_a)
consumer_model = get_consumer_model()
@@ -298,7 +307,14 @@ class AclTestCase(base.TestCase):
@utils.parameterized_dataset(test_data_create_container_consumer_acl_only)
def test_container_acl_create_consumer(self, user, expected_return):
- """Acl access will not allow you to add a consumer"""
+ """Test POST /v1/containers/{container_id}/consumers
+
+ Test default policy for adding consumers to a container owned by
+ creator_a and set to private.
+
+ Each user in the data set is added to the ACL and then used
+ to POST a new consumer.
+ """
container_ref = self.store_container(user_name=creator_a,
admin=admin_a)
diff --git a/releasenotes/notes/fix-story-2009664-042ef282c0dd6b6a.yaml b/releasenotes/notes/fix-story-2009664-042ef282c0dd6b6a.yaml
new file mode 100644
index 000000000..090816d3f
--- /dev/null
+++ b/releasenotes/notes/fix-story-2009664-042ef282c0dd6b6a.yaml
@@ -0,0 +1,13 @@
+---
+fixes:
+ - |
+ Fixed Story 2009664 - Fixed the Consumer controller to be able to use the
+ associated Container's ownership information in policy checks.
+security:
+ - |
+ Part of the fix for Story 2009664 required renaming the policy for
+ Container Consumers from "consumers:get" to "container_consumers:get",
+ "consumers:post" to "container_consumers:post", and "consumers:delete"
+ to "container_consumers:delete". If you are using custom policies to
+ override the default policies you will need to update them to use the
+ new names.