openstack/barbican
Code Issues Proposed changes

Fix policy for Orders

Browse Source 
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
This commit is contained in:
Douglas Mendizábal 2021-11-12 11:54:26 -06:00 committed by Douglas Mendizábal
parent 31aa926175
commit 5d81a3c453
3 changed files with 27 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

@ -66,6 +66,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

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

@ -12,7 +12,9 @@
from oslo_policy import policy
_MEMBER = "role:member"
_PROJECT_MEMBER = f"{_MEMBER} and project_id:%(target.order.project_id)s"
rules = [
policy.DocumentedRuleDefault(
@ -53,7 +55,8 @@ rules = [
),
policy.DocumentedRuleDefault(
name='order:get',
check_str=f'rule:all_users or {_MEMBER}',
check_str='rule:all_users and project_id:%(target.order.project_id)s '
f'or {_PROJECT_MEMBER}',
scope_types=['project'],
description='Retrieves an orders metadata.',
operations=[
@ -65,7 +68,8 @@ rules = [
),
policy.DocumentedRuleDefault(
name='order:delete',
check_str=f'rule:admin or {_MEMBER}',
check_str='rule:admin and project_id:%(target.order.project_id)s or '
f'{_PROJECT_MEMBER}',
scope_types=['project'],
description='Deletes an order.',
operations=[

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

@ -983,7 +983,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'],
@ -1004,8 +1005,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())
@ -1014,21 +1023,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'],