Merge "Verify project_id when quotas are checked"

api-ref/source/os-quota-sets.inc

@@ -15,11 +15,12 @@ Show A Quota
 
 Show the quota for a project or a project and a user.
 
-To show a quota for a project and a user, specify the ``user_id`` query parameter.
-
 Normal response codes: 200
 
-Error response codes: unauthorized(401), forbidden(403)
+Error response codes: badRequest(400), unauthorized(401), forbidden(403)
+
+- 400 - BadRequest - the tenant_id is not valid in your cloud, perhaps
+  because it was typoed.
 
 Request
 -------
@@ -63,22 +64,18 @@ Update Quotas
 
 Update the quotas for a project or a project and a user.
 
-Users can force the update even if the quota has already been used and the
+Users can force the update even if the quota has already been used and
-
+the reserved quota exceeds the new quota. To force the update, specify
-reserved quota exceeds the new quota.
+the ``"force": True`` attribute in the request body, the default value
-
+is ``false``.
-To force the update, specify the ``"force": True`` attribute in the request
-
-body, the default value is ``false``.
-
-To update a quota for a project and a user, specify the ``user_id`` query
-
-parameter.
 
 Normal response codes: 200
 
 Error response codes: badRequest(400), unauthorized(401), forbidden(403)
 
+- 400 - BadRequest - the tenant_id is not valid in your cloud, perhaps
+  because it was typoed.
+
 Request
 -------
 
@@ -158,7 +155,6 @@ Request
 .. rest_parameters:: parameters.yaml
 
   - tenant_id: tenant_id
-
   - user_id: user_id_query_quota_delete
 
 Response
@@ -175,7 +171,7 @@ Lists the default quotas for a project.
 
 Normal response codes: 200
 
-Error response codes: unauthorized(401), forbidden(403)
+Error response codes: badrequest(400), unauthorized(401), forbidden(403)
 
 Request
 -------
@@ -222,7 +218,10 @@ To show a quota for a project and a user, specify the ``user_id`` query paramete
 
 Normal response codes: 200
 
-Error response codes: unauthorized(401), forbidden(403)
+Error response codes: badrequest(400), unauthorized(401), forbidden(403)
+
+- 400 - BadRequest - the {tenant_id} is not valid in your cloud, perhaps
+  because it was typoed.
 
 Request
 -------

nova/api/openstack/compute/quota_sets.py

@@ -24,6 +24,7 @@ from nova.api.openstack.api_version_request \
     import MIN_WITHOUT_PROXY_API_SUPPORT_VERSION
 from nova.api.openstack.compute.schemas import quota_sets
 from nova.api.openstack import extensions
+from nova.api.openstack import identity
 from nova.api.openstack import wsgi
 from nova.api import validation
 import nova.conf
@@ -86,17 +87,20 @@ class QuotaSetsController(wsgi.Controller):
             return {k: v['limit'] for k, v in values.items()}
 
     @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
-    @extensions.expected_errors(())
+    @extensions.expected_errors(400)
     def show(self, req, id):
         return self._show(req, id, [])
 
     @wsgi.Controller.api_version(MIN_WITHOUT_PROXY_API_SUPPORT_VERSION)  # noqa
+    @extensions.expected_errors(400)
     def show(self, req, id):
         return self._show(req, id, FILTERED_QUOTAS)
 
     def _show(self, req, id, filtered_quotas):
         context = req.environ['nova.context']
         context.can(qs_policies.POLICY_ROOT % 'show', {'project_id': id})
+        identity.verify_project_id(context, id)
+
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
         return self._format_quota_set(id,
@@ -104,18 +108,20 @@ class QuotaSetsController(wsgi.Controller):
             filtered_quotas=filtered_quotas)
 
     @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
-    @extensions.expected_errors(())
+    @extensions.expected_errors(400)
     def detail(self, req, id):
         return self._detail(req, id, [])
 
     @wsgi.Controller.api_version(MIN_WITHOUT_PROXY_API_SUPPORT_VERSION)  # noqa
-    @extensions.expected_errors(())
+    @extensions.expected_errors(400)
     def detail(self, req, id):
         return self._detail(req, id, FILTERED_QUOTAS)
 
     def _detail(self, req, id, filtered_quotas):
         context = req.environ['nova.context']
         context.can(qs_policies.POLICY_ROOT % 'detail', {'project_id': id})
+        identity.verify_project_id(context, id)
+
         user_id = req.GET.get('user_id', None)
         return self._format_quota_set(
             id,
@@ -137,6 +143,8 @@ class QuotaSetsController(wsgi.Controller):
     def _update(self, req, id, body, filtered_quotas):
         context = req.environ['nova.context']
         context.can(qs_policies.POLICY_ROOT % 'update', {'project_id': id})
+        identity.verify_project_id(context, id)
+
         project_id = id
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
@@ -191,18 +199,20 @@ class QuotaSetsController(wsgi.Controller):
             filtered_quotas=filtered_quotas)
 
     @wsgi.Controller.api_version("2.0", MAX_PROXY_API_SUPPORT_VERSION)
-    @extensions.expected_errors(())
+    @extensions.expected_errors(400)
     def defaults(self, req, id):
         return self._defaults(req, id, [])
 
     @wsgi.Controller.api_version(MIN_WITHOUT_PROXY_API_SUPPORT_VERSION)  # noqa
-    @extensions.expected_errors(())
+    @extensions.expected_errors(400)
     def defaults(self, req, id):
         return self._defaults(req, id, FILTERED_QUOTAS)
 
     def _defaults(self, req, id, filtered_quotas):
         context = req.environ['nova.context']
         context.can(qs_policies.POLICY_ROOT % 'defaults', {'project_id': id})
+        identity.verify_project_id(context, id)
+
         values = QUOTAS.get_defaults(context)
         return self._format_quota_set(id, values,
             filtered_quotas=filtered_quotas)

nova/api/openstack/identity.py

@@ -0,0 +1,67 @@
+# Copyright 2017 IBM
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from keystoneauth1 import exceptions as kse
+from keystoneauth1 import session
+from oslo_log import log as logging
+import webob
+
+from nova.i18n import _, _LE, _LI, _LW
+
+LOG = logging.getLogger(__name__)
+
+
+def verify_project_id(context, project_id):
+    """verify that a project_id exists.
+
+    This attempts to verify that a project id exists. If it does not,
+    an HTTPBadRequest is emitted.
+
+    """
+    sess = session.Session(auth=context.get_auth_plugin())
+    try:
+        resp = sess.get('/v3/projects/%s' % project_id,
+                        endpoint_filter={'service_type': 'identity'},
+                        raise_exc=False)
+    except kse.ClientException:
+        # something is wrong, like there isn't a keystone v3 endpoint,
+        # we'll take the pass and default to everything being ok.
+        LOG.exception(_LE("Unable to contact keystone to verify project_id"))
+        return True
+
+    if resp:
+        # All is good with this 20x status
+        return True
+    elif resp.status_code == 404:
+        # we got access, and we know this project is not there
+        raise webob.exc.HTTPBadRequest(
+            explanation=_("Project ID %s is not a valid project.") %
+            project_id)
+    elif resp.status_code == 403:
+        # we don't have enough permission to verify this, so default
+        # to "it's ok".
+        LOG.info(
+            _LI("Insufficient permissions for user %(user)s to verify "
+                "existence of project_id %(pid)s"),
+            {"user": context.user_id, "pid": project_id})
+        return True
+    else:
+        LOG.warning(
+            _LW("Unexpected response from keystone trying to "
+                "verify project_id %(pid)s - resp: %(code)s %(content)s"),
+            {"pid": project_id,
+             "code": resp.status_code,
+             "content": resp.content})
+        # realize we did something wrong, but move on with a warning
+        return True

nova/tests/unit/test_identity.py

@@ -0,0 +1,88 @@
+# Copyright 2017 IBM Corp.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from keystoneauth1 import exceptions as kse
+import webob
+
+from nova.api.openstack import identity
+from nova import test
+
+
+class FakeResponse(object):
+    """A basic response constainer that simulates requests.Response.
+
+    One of the critical things is that a success error code makes the
+    object return true.
+
+    """
+    def __init__(self, status_code, content=""):
+        self.status_code = status_code
+        self.content = content
+
+    def __bool__(self):
+        # python 3
+        return self.__nonzero__()
+
+    def __nonzero__(self):
+        # python 2
+        return self.status_code == 200
+
+
+class IdentityValidationTest(test.NoDBTestCase):
+
+    @mock.patch('keystoneauth1.session.Session.get')
+    def test_good_id(self, get):
+        get.return_value = FakeResponse(200)
+        self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
+        get.assert_called_once_with(
+            '/v3/projects/foo',
+            endpoint_filter={'service_type': 'identity'},
+            raise_exc=False)
+
+    @mock.patch('keystoneauth1.session.Session.get')
+    def test_no_project(self, get):
+        get.return_value = FakeResponse(404)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          identity.verify_project_id,
+                          mock.MagicMock(), "foo")
+        get.assert_called_once_with(
+            '/v3/projects/foo',
+            endpoint_filter={'service_type': 'identity'},
+            raise_exc=False)
+
+    @mock.patch('keystoneauth1.session.Session.get')
+    def test_unknown_id(self, get):
+        get.return_value = FakeResponse(403)
+        self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
+        get.assert_called_once_with(
+            '/v3/projects/foo',
+            endpoint_filter={'service_type': 'identity'},
+            raise_exc=False)
+
+    @mock.patch('keystoneauth1.session.Session.get')
+    def test_unknown_error(self, get):
+        get.return_value = FakeResponse(500, "Oh noes!")
+        self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
+        get.assert_called_once_with(
+            '/v3/projects/foo',
+            endpoint_filter={'service_type': 'identity'},
+            raise_exc=False)
+
+    @mock.patch('keystoneauth1.session.Session.get')
+    def test_early_fail(self, get):
+        get.side_effect = kse.EndpointNotFound()
+        self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))

releasenotes/notes/project_id_validation-568d31c13c3ef735.yaml

@@ -0,0 +1,10 @@
+---
+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
+    valid project_id in the system.