Skip to content

Commit

Permalink
Verify project id for flavor access calls
Browse files Browse the repository at this point in the history
This includes project id verification for flavor access calls.

Closes-Bug: #1544989

Implements bp:validate-project-with-keystone

Change-Id: I2620c3ebc2a6dc131946602f8aa36ec0b6e782e0
  • Loading branch information
sdague authored and mriedem committed May 4, 2017
1 parent 7f050fe commit 1f120b5
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 7 deletions.
7 changes: 6 additions & 1 deletion api-ref/source/os-flavor-access.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------

Expand Down Expand Up @@ -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
-------

Expand Down Expand Up @@ -128,4 +134,3 @@ Response

.. literalinclude:: ../../doc/api_samples/flavor-access/flavor-access-remove-tenant-resp.json
:language: javascript

3 changes: 3 additions & 0 deletions nova/api/openstack/compute/flavor_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down
28 changes: 28 additions & 0 deletions nova/tests/unit/api/openstack/compute/test_flavor_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down
13 changes: 7 additions & 6 deletions releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 1f120b5

Please sign in to comment.