diff --git a/ironic/api/controllers/v1/allocation.py b/ironic/api/controllers/v1/allocation.py index 73aca3a51e..ab6c6e5418 100644 --- a/ironic/api/controllers/v1/allocation.py +++ b/ironic/api/controllers/v1/allocation.py @@ -323,9 +323,11 @@ class AllocationsController(pecan.rest.RestController): except exception.HTTPForbidden: cdict = api.request.context.to_policy_values() project = cdict.get('project_id') - if project and project != allocation.get('owner'): + if (project and allocation.get('owner') + and project != allocation.get('owner')): raise - if project and not CONF.oslo_policy.enforce_new_defaults: + if (allocation.get('owner') + and not CONF.oslo_policy.enforce_new_defaults): api_utils.check_policy('baremetal:allocation:create_pre_rbac') api_utils.check_policy('baremetal:allocation:create_restricted') self._check_allowed_allocation_fields(allocation) diff --git a/ironic/tests/unit/api/controllers/v1/test_allocation.py b/ironic/tests/unit/api/controllers/v1/test_allocation.py index 39a12784e8..1be54db809 100644 --- a/ironic/tests/unit/api/controllers/v1/test_allocation.py +++ b/ironic/tests/unit/api/controllers/v1/test_allocation.py @@ -1042,20 +1042,6 @@ class TestPost(test_api_base.BaseApiTest): headers=self.headers) self.assertEqual('123456', result['owner']) - def test_create_allocation_is_restricted_until_scope_enforcement(self): - cfg.CONF.set_override('enforce_new_defaults', False, - group='oslo_policy') - cfg.CONF.set_override('auth_strategy', 'keystone') - # We're setting ourselves to be a random ID and member - # which is allowed to create an allocation. - self.headers['X-Project-ID'] = '1135' - self.headers['X-Roles'] = 'admin, member, reader' - self.headers['X-Is-Admin-Project'] = 'False' - adict = apiutils.allocation_post_data() - response = self.post_json('/allocations', adict, expect_errors=True, - headers=self.headers) - self.assertEqual(http_client.FORBIDDEN, response.status_int) - def test_backfill(self): node = obj_utils.create_test_node(self.context) adict = apiutils.allocation_post_data(node=node.uuid) @@ -1171,6 +1157,29 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + @mock.patch.object(policy, 'authorize', autospec=True) + def test_create_restricted_allocation_deprecated_without_owner( + self, mock_authorize): + cfg.CONF.set_override('enforce_new_defaults', False, + group='oslo_policy') + + def mock_authorize_function(rule, target, creds): + if rule == 'baremetal:allocation:create': + raise exception.HTTPForbidden(resource='fake') + return True + mock_authorize.side_effect = mock_authorize_function + owner = '12345' + adict = apiutils.allocation_post_data() + headers = {api_base.Version.string: '1.60', 'X-Project-Id': owner} + response = self.post_json('/allocations', adict, headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + self.assertEqual(owner, response.json['owner']) + result = self.get_json('/allocations/%s' % adict['uuid'], + headers=headers) + self.assertEqual(adict['uuid'], result['uuid']) + self.assertEqual(owner, result['owner']) + @mock.patch.object(policy, 'authorize', autospec=True) def test_create_restricted_allocation_with_owner(self, mock_authorize): def mock_authorize_function(rule, target, creds): diff --git a/releasenotes/notes/restricted-allocation-creation-fix-a70dfcbcb9996602.yaml b/releasenotes/notes/restricted-allocation-creation-fix-a70dfcbcb9996602.yaml new file mode 100644 index 0000000000..147b56f4c0 --- /dev/null +++ b/releasenotes/notes/restricted-allocation-creation-fix-a70dfcbcb9996602.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes restricted allocation creation for old policy defaults. This involves + a check that ensures that the user is not trying to create an allocation with + an owner other than themselves. This check is updated to also see if the + user is actually trying to set an allocation owner.