Fix credential update to ec2 type

It was possible to create a credential without providing a project_id
and later updating it to the ec2 type.

This patch fixes the issue by adding a manual checking in the
manager layer since it needs to check the old credential contents
prior failing the request.

Change-Id: I1eb28a46c89e17d9c990cc798867d1a59714fe5f
Closes-Bug: #1613466

keystone/credential/core.py

@@ -21,6 +21,7 @@ from keystone.common import driver_hints
 from keystone.common import manager
 import keystone.conf
 from keystone.credential.backends import base
+from keystone import exception
 
 
 CONF = keystone.conf.CONF
@@ -52,8 +53,21 @@ class Manager(manager.Manager):
         """Create a credential."""
         return self.driver.create_credential(credential_id, credential)
 
+    def _validate_credential_update(self, credential_id, credential):
+        # ec2 credentials require a "project_id" to be functional. Before we
+        # update, check the case where a non-ec2 credential changes its type
+        # to be "ec2", but has no associated "project_id", either in the
+        # request or already set in the database
+        if (credential.get('type', '').lower() == 'ec2' and
+                not credential.get('project_id')):
+            existing_cred = self.get_credential(credential_id)
+            if not existing_cred['project_id']:
+                raise exception.ValidationError(attribute='project_id',
+                                                target='credential')
+
     def update_credential(self, credential_id, credential):
         """Update an existing credential."""
+        self._validate_credential_update(credential_id, credential)
         return self.driver.update_credential(credential_id, credential)
 
 

keystone/tests/unit/test_v3_credential.py

@@ -172,6 +172,61 @@ class CredentialTestCase(CredentialBaseTestCase):
             body={'credential': ref})
         self.assertValidCredentialResponse(r, ref)
 
+    def test_update_credential_to_ec2_type(self):
+        """Call ``PATCH /credentials/{credential_id}``."""
+        # Create a credential without providing a project_id
+        ref = unit.new_credential_ref(user_id=self.user['id'])
+        r = self.post(
+            '/credentials',
+            body={'credential': ref})
+        self.assertValidCredentialResponse(r, ref)
+        credential_id = r.result.get('credential')['id']
+
+        # Updating the credential to ec2 requires a project_id
+        update_ref = {'type': 'ec2', 'project_id': self.project_id}
+        self.patch(
+            '/credentials/%(credential_id)s' % {
+                'credential_id': credential_id},
+            body={'credential': update_ref})
+
+    def test_update_credential_to_ec2_missing_project_id(self):
+        """Call ``PATCH /credentials/{credential_id}``."""
+        # Create a credential without providing a project_id
+        ref = unit.new_credential_ref(user_id=self.user['id'])
+        r = self.post(
+            '/credentials',
+            body={'credential': ref})
+        self.assertValidCredentialResponse(r, ref)
+        credential_id = r.result.get('credential')['id']
+
+        # Updating such credential to ec2 type without providing a project_id
+        # will fail
+        update_ref = {'type': 'ec2'}
+        self.patch(
+            '/credentials/%(credential_id)s' % {
+                'credential_id': credential_id},
+            body={'credential': update_ref},
+            expected_status=http_client.BAD_REQUEST)
+
+    def test_update_credential_to_ec2_with_previously_set_project_id(self):
+        """Call ``PATCH /credentials/{credential_id}``."""
+        # Create a credential providing a project_id
+        ref = unit.new_credential_ref(user_id=self.user['id'],
+                                      project_id=self.project_id)
+        r = self.post(
+            '/credentials',
+            body={'credential': ref})
+        self.assertValidCredentialResponse(r, ref)
+        credential_id = r.result.get('credential')['id']
+
+        # Since the created credential above already has a project_id, the
+        # update request will not fail
+        update_ref = {'type': 'ec2'}
+        self.patch(
+            '/credentials/%(credential_id)s' % {
+                'credential_id': credential_id},
+            body={'credential': update_ref})
+
     def test_delete_credential(self):
         """Call ``DELETE /credentials/{credential_id}``."""
         self.delete(

releasenotes/notes/bug-1613466-credential-update-ec2-type-8fb51ff3ad3a449c.yaml

@@ -0,0 +1,8 @@
+fixes:
+  - >
+    [`bug 1613466 <https://bugs.launchpad.net/keystone/+bug/1613466>`_]
+    Credentials update to ec2 type originally accepted credentials with no
+    project ID set, this would lead to an error when trying to use such credential.
+    This behavior has been blocked, so creating a non-ec2 credential with no
+    project ID and updating it to ec2 without providing a project ID will fail with
+    a `400 Bad Request` error.