Merge "Stop adding entry in local_user while updating ephemerals" into stable/train

4 files changed, 80 insertions, 2 deletions

diff --git a/keystone/identity/backends/sql_model.py b/keystone/identity/backends/sql_model.py
index 8798d326c..22ab4ef83 100644
--- a/keystone/identity/backends/sql_model.py
+++ b/keystone/identity/backends/sql_model.py
@@ -82,9 +82,13 @@ class User(sql.ModelBase, sql.ModelDictMixinWithExtras):
 
     @name.setter
     def name(self, value):
-        if not self.local_user:
+        if self.federated_users:
+            self.federated_users[0].display_name = value
+        elif self.local_user:
+            self.local_user.name = value
+        else:
             self.local_user = LocalUser()
-        self.local_user.name = value
+            self.local_user.name = value
 
     @name.expression
     def name(cls):
diff --git a/keystone/tests/unit/identity/shadow_users/test_core.py b/keystone/tests/unit/identity/shadow_users/test_core.py
index ca037733d..dd6fdfb6f 100644
--- a/keystone/tests/unit/identity/shadow_users/test_core.py
+++ b/keystone/tests/unit/identity/shadow_users/test_core.py
@@ -12,6 +12,7 @@
 
 import uuid
 
+from keystone.common import driver_hints
 from keystone.common import provider_api
 
 PROVIDERS = provider_api.ProviderAPIs
@@ -62,3 +63,33 @@ class ShadowUsersCoreTests(object):
 
         # The shadowed users still share the same unique ID.
         self.assertEqual(shadow_user1['id'], shadow_user2['id'])
+
+    def test_shadow_federated_user_not_creating_a_local_user(self):
+        PROVIDERS.identity_api.shadow_federated_user(
+            self.federated_user['idp_id'],
+            self.federated_user['protocol_id'],
+            self.federated_user['unique_id'],
+            self.federated_user['display_name'],
+            "some_id@mail.provider")
+
+        hints = driver_hints.Hints()
+        hints.add_filter('name', self.federated_user['display_name'])
+        users = PROVIDERS.identity_api.list_users(hints=hints)
+
+        self.assertEqual(1, len(users))
+
+        # Avoid caching
+        self.federated_user['display_name'] = uuid.uuid4().hex
+
+        PROVIDERS.identity_api.shadow_federated_user(
+            self.federated_user['idp_id'],
+            self.federated_user['protocol_id'],
+            self.federated_user['unique_id'],
+            self.federated_user['display_name'],
+            "some_id@mail.provider")
+
+        hints.add_filter('name', self.federated_user['display_name'])
+        users = PROVIDERS.identity_api.list_users(hints=hints)
+
+        # The number os users must remain 1
+        self.assertEqual(1, len(users))
diff --git a/keystone/tests/unit/test_v3_identity.py b/keystone/tests/unit/test_v3_identity.py
index c377a96b8..fcab4ff21 100644
--- a/keystone/tests/unit/test_v3_identity.py
+++ b/keystone/tests/unit/test_v3_identity.py
@@ -396,6 +396,26 @@ class IdentityTestCase(test_v3.RestfulTestCase):
         self.delete('/groups/%(group_id)s/users/%(user_id)s' % {
             'group_id': self.group_id, 'user_id': self.user['id']})
 
+    def test_update_ephemeral_user(self):
+        federated_user_a = model.FederatedUser()
+        federated_user_b = model.FederatedUser()
+        federated_user_a.idp_id = 'a_idp'
+        federated_user_b.idp_id = 'b_idp'
+        federated_user_a.display_name = 'federated_a'
+        federated_user_b.display_name = 'federated_b'
+        federated_users = [federated_user_a, federated_user_b]
+
+        user_a = model.User()
+        user_a.federated_users = federated_users
+
+        self.assertEqual(federated_user_a.display_name, user_a.name)
+        self.assertIsNone(user_a.password)
+
+        user_a.name = 'new_federated_a'
+
+        self.assertEqual('new_federated_a', user_a.name)
+        self.assertIsNone(user_a.local_user)
+
     def test_update_user(self):
         """Call ``PATCH /users/{user_id}``."""
         user = unit.new_user_ref(domain_id=self.domain_id)
diff --git a/releasenotes/notes/bug-1848342-317c9e4afa65a3ff.yaml b/releasenotes/notes/bug-1848342-317c9e4afa65a3ff.yaml
new file mode 100644
index 000000000..25026551b
--- /dev/null
+++ b/releasenotes/notes/bug-1848342-317c9e4afa65a3ff.yaml
@@ -0,0 +1,23 @@
+---
+fixes:
+  - |
+    [`bug 1848342 <https://bugs.launchpad.net/keystone/+bug/1848342>`_]
+    There was an inconsistency in the ephemeral user update flow. Every time a
+    federated user logged in, keystone created an entry in the local_user
+    table instead of just updating the entries in the user and federated_user
+    tables, which caused duplicate entries when listing users. Now, the
+    keystone will not create the entry in the local_user table while updating
+    an ephemeral user.
+
+    If you are affected by this bug, a fix in the keystone database will be
+    needed so we recommend to dump the users' tables before doing this process:
+
+    mysql db example:
+        - mysqldump -h <mysql host> -p -P <mysql port> -u keystone keystone federated_user local_user user > user_tables.sql
+        - mysql -h <mysql host> -D keystone -p -P <mysql port> -u keystone -e 'delete from local_user where user_id in (select user_id from federated_user);'
+
+    SQL:
+     - delete from local_user where user_id in (select user_id from federated_user);
+
+
+