From 252c23b1b80bfbc0e9b54ac31a5b97c117cf3d77 Mon Sep 17 00:00:00 2001
From: Vishakha Agarwal <agarwalvishakha18@gmail.com>
Date: Fri, 15 May 2020 14:13:40 +0530
Subject: [PATCH] Disable EC2 credentials access_id update

Without this patch user can alter EC2 credential access_id and user
cannot use it anymore as an ec2 auth token since EC2 credential
access ID is used to calculate an ID of the "credential" [1] and it
doesn't update the EC2 credential ID with new access ID. This leads
to unwanted EC2 credentials stored in database.

As per the discussion of keystone team [2] we decided to block patching
of "access_id" attribute.

[1] https://github.com/openstack/keystone/blob/7bb6314e40d6947294260324e84a58de191f8609/keystone/api/users.py#L363
[2]http://eavesdrop.openstack.org/irclogs/%23openstack-meeting-alt/%23openstack-meeting-alt.2020-05-12.log.html#t2020-05-12T17:45:20

Closes-Bug: #1872753
Change-Id: I1f6ce3927c2881d9a2d7dcda3ccd29e0a82e45a9
---
 keystone/api/credentials.py                   |  3 +-
 keystone/tests/unit/test_v3_credential.py     | 28 +++++++++++++++++++
 .../notes/bug-1872753-e2a934eac919ccde.yaml   |  8 ++++++
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/bug-1872753-e2a934eac919ccde.yaml

diff --git a/keystone/api/credentials.py b/keystone/api/credentials.py
index 2b81000e09..90b53dd686 100644
--- a/keystone/api/credentials.py
+++ b/keystone/api/credentials.py
@@ -176,7 +176,8 @@ class CredentialResource(ks_flask.ResourceBase):
             if isinstance(old_blob, str):
                 old_blob = jsonutils.loads(old_blob)
             # if there was a scope set, prevent changing it or unsetting it
-            for key in ['trust_id', 'app_cred_id', 'access_token_id']:
+            for key in ['trust_id', 'app_cred_id', 'access_token_id',
+                        'access_id']:
                 if old_blob.get(key) != new_blob.get(key):
                     message = _('%s can not be updated for credential') % key
                     raise exception.ValidationError(message=message)
diff --git a/keystone/tests/unit/test_v3_credential.py b/keystone/tests/unit/test_v3_credential.py
index 4cce2d1bf8..6573f4402a 100644
--- a/keystone/tests/unit/test_v3_credential.py
+++ b/keystone/tests/unit/test_v3_credential.py
@@ -407,6 +407,34 @@ class CredentialTestCase(CredentialBaseTestCase):
             body={'credential': update_ref},
             expected_status=http.client.BAD_REQUEST)
 
+    def test_update_ec2_credential_change_access_id(self):
+        """Call ``PATCH /credentials/{credential_id}``."""
+        blob, ref = unit.new_ec2_credential(user_id=self.user['id'],
+                                            project_id=self.project_id)
+        blob['access_id'] = uuid.uuid4().hex
+        ref['blob'] = json.dumps(blob)
+        r = self.post(
+            '/credentials',
+            body={'credential': ref})
+        self.assertValidCredentialResponse(r, ref)
+        credential_id = r.result.get('credential')['id']
+        # Try changing to a different access_id
+        blob['access_id'] = uuid.uuid4().hex
+        update_ref = {'blob': json.dumps(blob)}
+        self.patch(
+            '/credentials/%(credential_id)s' % {
+                'credential_id': credential_id},
+            body={'credential': update_ref},
+            expected_status=http.client.BAD_REQUEST)
+        # Try removing the access_id
+        del blob['access_id']
+        update_ref = {'blob': json.dumps(blob)}
+        self.patch(
+            '/credentials/%(credential_id)s' % {
+                'credential_id': credential_id},
+            body={'credential': update_ref},
+            expected_status=http.client.BAD_REQUEST)
+
     def test_delete_credential(self):
         """Call ``DELETE /credentials/{credential_id}``."""
         self.delete(
diff --git a/releasenotes/notes/bug-1872753-e2a934eac919ccde.yaml b/releasenotes/notes/bug-1872753-e2a934eac919ccde.yaml
new file mode 100644
index 0000000000..5f8790537e
--- /dev/null
+++ b/releasenotes/notes/bug-1872753-e2a934eac919ccde.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - >
+    [`bug 1872753 <https://bugs.launchpad.net/keystone/+bug/1872753>`_]
+    Added validation to the EC2 credential API to prevent altering the ``access_id``
+    field in the blob attribute. This prevents accidentally orphaning an EC2 credential
+    resource when an altered ``access_id`` no longer resolves to the credential's
+    resource ID.