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)
This commit is contained in:
Douglas Mendizábal 2021-11-12 11:54:26 -06:00 committed by Douglas Mendizábal
parent 4d619b4c95
commit 93c5636f9c
3 changed files with 23 additions and 6 deletions

View File

@ -66,6 +66,12 @@ class OrderController(controllers.ACLMixin):
self.queue = queue_resource or async_client.TaskClient() self.queue = queue_resource or async_client.TaskClient()
self.type_order_validator = validators.TypeOrderValidator() 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) @pecan.expose(generic=True)
def index(self, **kwargs): def index(self, **kwargs):
pecan.abort(405) # HTTP 405 Method Not Allowed as default pecan.abort(405) # HTTP 405 Method Not Allowed as default

View File

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

View File

@ -983,7 +983,8 @@ class WhenTestingOrdersResource(BaseTestCase):
def test_should_pass_get_orders(self): def test_should_pass_get_orders(self):
self._assert_pass_rbac(['admin', 'observer', 'creator'], 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): def test_should_raise_get_orders(self):
self._assert_fail_rbac([None, 'audit', 'bogus'], self._assert_fail_rbac([None, 'audit', 'bogus'],
@ -1004,8 +1005,16 @@ class WhenTestingOrderResource(BaseTestCase):
self.external_project_id = '12345project' self.external_project_id = '12345project'
self.order_id = '12345order' 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, # Force an error on GET and DELETE calls that pass RBAC,
# as we are not testing such flows in this test module. # 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() self.order_repo = mock.MagicMock()
fail_method = mock.MagicMock(return_value=None, fail_method = mock.MagicMock(return_value=None,
side_effect=self._generate_get_error()) side_effect=self._generate_get_error())
@ -1014,21 +1023,23 @@ class WhenTestingOrderResource(BaseTestCase):
self.setup_order_repository_mock(self.order_repo) 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): def test_rules_should_be_loaded(self):
self.assertIsNotNone(self.policy_enforcer.rules) self.assertIsNotNone(self.policy_enforcer.rules)
def test_should_pass_get_order(self): def test_should_pass_get_order(self):
self._assert_pass_rbac(['admin', 'observer', 'creator', 'audit'], 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): def test_should_raise_get_order(self):
self._assert_fail_rbac([None, 'bogus'], self._assert_fail_rbac([None, 'bogus'],
self._invoke_on_get) self._invoke_on_get)
def test_should_pass_delete_order(self): 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): def test_should_raise_delete_order(self):
self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'], self._assert_fail_rbac([None, 'audit', 'observer', 'creator', 'bogus'],