openstack
/
barbican
Code Issues Proposed changes

Fix policy for Orders 

This patch adds checks to make sure that the project_id of the token
matches the project_id that owns the Order.

Currently, having a role on any project will allow the request to be
processed, which results in a 404 - Not Found instead of 401 -
Forbidden.

Change-Id: Ie0e6f6edae40e47d45afbe92fd509032cb091b1a
(cherry picked from commit 5d81a3c453)
(cherry picked from commit 382b5086a2)
(cherry picked from commit ea7451e32c)
(cherry picked from commit 85b9feecd2)
(cherry picked from commit 93c5636f9c)
This commit is contained in:
Douglas Mendizábal 2021-11-12 11:54:26 -06:00 committed by Douglas Mendizábal
parent 459e321d55
commit 0e52699830
3 changed files with 23 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
barbican/api/controllers/orders.py
View File

@ -67,6 +67,12 @@ class OrderController(controllers.ACLMixin):
self.queue = queue_resource or async_client.TaskClient()
self.type_order_validator = validators.TypeOrderValidator()
def get_acl_tuple(self, req, **kwargs):
acl = dict()
acl['project_id'] = self.order.project.external_id
acl['creator_id'] = self.order.creator_id
return 'order', acl
@pecan.expose(generic=True)
def index(self, **kwargs):
pecan.abort(405) # HTTP 405 Method Not Allowed as default

4
barbican/common/policies/orders.py
View File

@ -52,7 +52,7 @@ rules = [
),
policy.DocumentedRuleDefault(
name='order:get',
check_str='rule:all_users',
check_str='rule:all_users and project_id:%(target.order.project_id)s',
scope_types=[],
description='Retrieves an orders metadata.',
operations=[
@ -64,7 +64,7 @@ rules = [
),
policy.DocumentedRuleDefault(
name='order:delete',
check_str='rule:admin',
check_str='rule:admin and project_id:%(target.order.project_id)s',
scope_types=[],
description='Deletes an order.',
operations=[

19
barbican/tests/api/test_resources_policy.py
View File

@ -975,7 +975,8 @@ class WhenTestingOrdersResource(BaseTestCase):
def test_should_pass_get_orders(self):
self._assert_pass_rbac(['admin', 'observer', 'creator'],
self._invoke_on_get)
self._invoke_on_get,
project_id=self.external_project_id)
def test_should_raise_get_orders(self):
self._assert_fail_rbac([None, 'audit', 'bogus'],
@ -996,8 +997,16 @@ class WhenTestingOrderResource(BaseTestCase):
self.external_project_id = '12345project'
self.order_id = '12345order'
order = mock.MagicMock()
order.id = self.order_id
order.project.external_id = self.external_project_id
order.creator_id = 'CRE-A-TOR-UUID'
# Force an error on GET and DELETE calls that pass RBAC,
# as we are not testing such flows in this test module.
mock_to_dict = mock.MagicMock()
mock_to_dict.side_effect = self._generate_get_error()
order.to_dict_fields = mock_to_dict
self.order_repo = mock.MagicMock()
fail_method = mock.MagicMock(return_value=None,
side_effect=self._generate_get_error())
@ -1006,21 +1015,23 @@ class WhenTestingOrderResource(BaseTestCase):
self.setup_order_repository_mock(self.order_repo)
self.resource = OrderResource(self.order_id)
self.resource = OrderResource(order)
def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules)
def test_should_pass_get_order(self):
self._assert_pass_rbac(['admin', 'observer', 'creator', 'audit'],
self._invoke_on_get)
self._invoke_on_get,
project_id=self.external_project_id)
def test_should_raise_get_order(self):
self._assert_fail_rbac([None, 'bogus'],
self._invoke_on_get)
def test_should_pass_delete_order(self):
self._assert_pass_rbac(['admin'], self._invoke_on_delete)
self._assert_pass_rbac(['admin'], self._invoke_on_delete,
project_id=self.external_project_id)
def test_should_raise_delete_order(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],