From 1f120b5649ba03aa5b2490a82c08b77c580f12d7 Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Fri, 17 Feb 2017 07:55:43 -0500 Subject: [PATCH] Verify project id for flavor access calls This includes project id verification for flavor access calls. Closes-Bug: #1544989 Implements bp:validate-project-with-keystone Change-Id: I2620c3ebc2a6dc131946602f8aa36ec0b6e782e0 --- api-ref/source/os-flavor-access.inc | 7 ++++- nova/api/openstack/compute/flavor_access.py | 3 ++ .../openstack/compute/test_flavor_access.py | 28 +++++++++++++++++++ ...roject_id_validation-568d31c13c3ef735.yaml | 13 +++++---- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/api-ref/source/os-flavor-access.inc b/api-ref/source/os-flavor-access.inc index 48b69accbeb..5d4b21420c1 100644 --- a/api-ref/source/os-flavor-access.inc +++ b/api-ref/source/os-flavor-access.inc @@ -57,6 +57,9 @@ Normal response codes: 200 Error response codes: badRequest(400), unauthorized(401), forbidden(403), itemNotFound(404), conflict(409) +- 400 - BadRequest - if the `tenant` is not found in your OpenStack + deployment, a 400 is returned to prevent typos on the API call. + Request ------- @@ -100,6 +103,9 @@ Normal response codes: 200 Error response codes: badRequest(400), unauthorized(401), forbidden(403), itemNotFound(404), conflict(409) +- 400 - BadRequest - if the `tenant` is not found in your OpenStack + deployment, a 400 is returned to prevent typos on the API call. + Request ------- @@ -128,4 +134,3 @@ Response .. literalinclude:: ../../doc/api_samples/flavor-access/flavor-access-remove-tenant-resp.json :language: javascript - diff --git a/nova/api/openstack/compute/flavor_access.py b/nova/api/openstack/compute/flavor_access.py index 163a327f3e2..b591212d691 100644 --- a/nova/api/openstack/compute/flavor_access.py +++ b/nova/api/openstack/compute/flavor_access.py @@ -21,6 +21,7 @@ from nova.api.openstack import common from nova.api.openstack.compute.schemas import flavor_access from nova.api.openstack import extensions +from nova.api.openstack import identity from nova.api.openstack import wsgi from nova.api import validation from nova import exception @@ -95,6 +96,7 @@ def _add_tenant_access(self, req, id, body): vals = body['addTenantAccess'] tenant = vals['tenant'] + identity.verify_project_id(context, tenant) flavor = common.get_flavor(context, id) @@ -120,6 +122,7 @@ def _remove_tenant_access(self, req, id, body): vals = body['removeTenantAccess'] tenant = vals['tenant'] + identity.verify_project_id(context, tenant) # NOTE(gibi): We have to load a flavor from the db here as # flavor.remove_access() will try to emit a notification and that needs diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_access.py b/nova/tests/unit/api/openstack/compute/test_flavor_access.py index d600582137f..2495eb1fda5 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_access.py @@ -383,6 +383,34 @@ def test_delete_tenant_access_with_no_tenant(self, mock_api_get): self.assertRaises(self.validation_ex, remove_access, req, '2', body=body) + @mock.patch('nova.api.openstack.identity.verify_project_id', + side_effect=exc.HTTPBadRequest( + explanation="Project ID proj2 is not a valid project.")) + def test_add_tenant_access_with_invalid_tenant(self, mock_verify): + """Tests the case that the tenant does not exist in Keystone.""" + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + body = {'addTenantAccess': {'tenant': 'proj2'}} + self.assertRaises(exc.HTTPBadRequest, + self.flavor_action_controller._add_tenant_access, + req, '2', body=body) + mock_verify.assert_called_once_with( + req.environ['nova.context'], 'proj2') + + @mock.patch('nova.api.openstack.identity.verify_project_id', + side_effect=exc.HTTPBadRequest( + explanation="Project ID proj2 is not a valid project.")) + def test_remove_tenant_access_with_invalid_tenant(self, mock_verify): + """Tests the case that the tenant does not exist in Keystone.""" + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + body = {'removeTenantAccess': {'tenant': 'proj2'}} + self.assertRaises(exc.HTTPBadRequest, + self.flavor_action_controller._remove_tenant_access, + req, '2', body=body) + mock_verify.assert_called_once_with( + req.environ['nova.context'], 'proj2') + class FlavorAccessPolicyEnforcementV21(test.NoDBTestCase): diff --git a/releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml b/releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml index a7b51ceb64a..8315d4bb98b 100644 --- a/releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml +++ b/releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml @@ -1,10 +1,11 @@ --- fixes: - | - API calls to /os-quota-sets/* will now attempt to validate the - project_id being opperated on with keystone. If the user has - enough permissions in user, and the keystone project does not - exist, a 400 will be returned to prevent invalidate quota data - from being put in the Nova database. This fixes an effective - silent error where this would be stored even if this was not a + API calls to ``/os-quota-sets`` and flavor access will now attempt + to validate the project_id being operated on with Keystone. If + the user token has enough permissions to perform + ``GET /v3/projects/{project_id}``, and the Keystone project + does not exist, a 400 BadRequest will be returned to prevent invalid + project data from being put in the Nova database. This fixes an effective + silent error where the project_id would be stored even if it was not a valid project_id in the system.