Http 400 when project enabled is not a boolean

Having enabled="true" in json data when updating tenant will produce 500

When updating a project, no type check was performed on the enabled
attribute. Therefore, if enabled value in JSON/XML is not a boolean but
a string, keystone responds with an incorrect Http 500 error code and
the stacktrace.

The change introduces a type validation of the enabled attribute at
identity manager. If the type is not a boolean, keystone now returns an
appropriate Http 400 error code with a message pointing a bad format for
the attribute.

Test cases have been added to file test_backend and test_content_types
for testing the case when enabled attribute is a string or int when
updating project.

Fixes bug #1191384

Change-Id: I86dd7e71d4bac1e3fd6fcabaa1a2136a47722e5f

4 files changed, 77 insertions, 2 deletions

diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index 205c8849..1c2a5508 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -93,6 +93,12 @@ class Manager(manager.Manager):
             tenant['description'] = ''
         return self.driver.create_project(tenant_id, tenant)
 
+    def update_project(self, context, tenant_id, tenant_ref):
+        tenant = tenant_ref.copy()
+        if 'enabled' in tenant:
+            tenant['enabled'] = clean.project_enabled(tenant['enabled'])
+        return self.driver.update_project(tenant_id, tenant)
+
 
 class Driver(object):
     """Interface description for an Identity driver."""
diff --git a/tests/test_backend.py b/tests/test_backend.py
index 9e506928..e8042ab1 100644
--- a/tests/test_backend.py
+++ b/tests/test_backend.py
@@ -1553,6 +1553,25 @@ class IdentityTests(object):
                           'fake1',
                           user)
 
+    def test_update_project_invalid_enabled_type_string(self):
+            project = {'id': uuid.uuid4().hex,
+                       'name': uuid.uuid4().hex,
+                       'enabled': True,
+                       'domain_id': DEFAULT_DOMAIN_ID}
+            self.identity_man.create_project(EMPTY_CONTEXT,
+                                             project['id'],
+                                             project)
+            project_ref = self.identity_api.get_project(project['id'])
+            self.assertEqual(project_ref['enabled'], True)
+
+            # Strings are not valid boolean values
+            project['enabled'] = "false"
+            self.assertRaises(exception.ValidationError,
+                              self.identity_man.update_project,
+                              EMPTY_CONTEXT,
+                              project['id'],
+                              project)
+
     def test_create_project_invalid_enabled_type_string(self):
         project = {'id': uuid.uuid4().hex,
                    'name': uuid.uuid4().hex,
@@ -1773,11 +1792,22 @@ class IdentityTests(object):
         tenant_ref = self.identity_api.get_project('fake1')
         self.assertEqual(tenant_ref['enabled'], tenant['enabled'])
 
+        # If not present, enabled field should not be updated
+        del tenant['enabled']
+        self.identity_api.update_project('fake1', tenant)
+        tenant_ref = self.identity_api.get_project('fake1')
+        self.assertEqual(tenant_ref['enabled'], False)
+
         tenant['enabled'] = True
         self.identity_api.update_project('fake1', tenant)
         tenant_ref = self.identity_api.get_project('fake1')
         self.assertEqual(tenant_ref['enabled'], tenant['enabled'])
 
+        del tenant['enabled']
+        self.identity_api.update_project('fake1', tenant)
+        tenant_ref = self.identity_api.get_project('fake1')
+        self.assertEqual(tenant_ref['enabled'], True)
+
     def test_add_user_to_group(self):
         domain = self._get_domain_fixture()
         new_group = {'id': uuid.uuid4().hex, 'domain_id': domain['id'],
diff --git a/tests/test_backend_ldap.py b/tests/test_backend_ldap.py
index 60b9f4ba..ecdc7465 100644
--- a/tests/test_backend_ldap.py
+++ b/tests/test_backend_ldap.py
@@ -113,7 +113,7 @@ class LDAPIdentity(test.TestCase, test_backend.IdentityTests):
         tenant_ref = self.identity_api.get_project('fake1')
         self.assertEqual(tenant_ref['id'], 'fake1')
 
-        tenant['enabled'] = 'False'
+        tenant['enabled'] = False
         self.identity_api.update_project('fake1', tenant)
 
         self.identity_api.delete_project('fake1')
@@ -133,7 +133,7 @@ class LDAPIdentity(test.TestCase, test_backend.IdentityTests):
                           'fake1',
                           tenant)
 
-        self.tenant_bar['enabled'] = 'False'
+        self.tenant_bar['enabled'] = False
         self.assertRaises(exception.ForbiddenAction,
                           self.identity_api.update_project,
                           self.tenant_bar['id'],
diff --git a/tests/test_content_types.py b/tests/test_content_types.py
index c3b804a7..fd99eae8 100644
--- a/tests/test_content_types.py
+++ b/tests/test_content_types.py
@@ -1059,3 +1059,42 @@ class XmlTestCase(RestfulTestCase, CoreApiTests):
         self.assertIsNotNone(r.result.get('tenant'))
         self.assertValidTenant(r.result['tenant'])
         self.assertEqual(r.result['tenant'].get('description'), "")
+
+    def test_create_project_invalid_enabled_type_string(self):
+        # Forbidden usage of string for 'enabled' field in JSON and XML
+        token = self.get_scoped_token()
+
+        r = self.admin_request(
+            method='POST',
+            path='/v2.0/tenants',
+            body={
+                'tenant': {
+                    'name': uuid.uuid4().hex,
+                    # In XML, only "true|false" are converted to boolean.
+                    'enabled': "False",
+                },
+            },
+            token=token,
+            expected_status=400)
+        self.assertValidErrorResponse(r)
+
+    def test_update_project_invalid_enabled_type_string(self):
+        # Forbidden usage of string for 'enabled' field in JSON and XML
+        token = self.get_scoped_token()
+
+        path = '/v2.0/tenants/%(tenant_id)s' % {
+               'tenant_id': self.tenant_bar['id'],
+        }
+
+        r = self.admin_request(
+            method='PUT',
+            path=path,
+            body={
+                'tenant': {
+                    # In XML, only "true|false" are converted to boolean.
+                    'enabled': "False",
+                },
+            },
+            token=token,
+            expected_status=400)
+        self.assertValidErrorResponse(r)