Make user to nonlocal_user a 1:1 relationship

The table relationship between 'user' and 'nonlocal_user' should be
1 to 1, which is consistent with 'user' to 'local_user'. However, it's
mistakenly 1 to many. In fact, the backend code treats 'user' to
'nonlocal_user' as 1:1 and wouldn't allow duplicates, so this will have
zero impact on existing deployments. This patch fixes this by making the
user_id column unique.

Closes-Bug: #1649412
Partially-Implements: bp support-federated-attr
Change-Id: Ib371df18f3fb2c67e5421cf0bf4551183902cf00

keystone/common/sql/contract_repo/versions/011_contract_user_id_unique_for_nonlocal_user.py

@@ -0,0 +1,23 @@
+# 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 migrate
+import sqlalchemy as sql
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    nonlocal_user = sql.Table('nonlocal_user', meta, autoload=True)
+    migrate.UniqueConstraint(nonlocal_user.c.user_id,
+                             name='ixu_nonlocal_user_user_id').create()

keystone/common/sql/data_migration_repo/versions/011_expand_user_id_unique_for_nonlocal_user.py

@@ -0,0 +1,15 @@
+#    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.
+
+
+def upgrade(migrate_engine):
+    pass

keystone/common/sql/expand_repo/versions/011_expand_user_id_unique_for_nonlocal_user.py

@@ -0,0 +1,15 @@
+# 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.
+
+
+def upgrade(migrate_engine):
+    pass

keystone/identity/backends/sql_model.py

@@ -42,11 +42,12 @@ class User(sql.ModelBase, sql.DictBase):
                                        lazy='subquery',
                                        cascade='all,delete-orphan',
                                        backref='user')
-    nonlocal_users = orm.relationship('NonLocalUser',
-                                      single_parent=True,
-                                      lazy='subquery',
-                                      cascade='all,delete-orphan',
-                                      backref='user')
+    nonlocal_user = orm.relationship('NonLocalUser',
+                                     uselist=False,
+                                     single_parent=True,
+                                     lazy='subquery',
+                                     cascade='all,delete-orphan',
+                                     backref='user')
     created_at = sql.Column(sql.DateTime, nullable=True)
     last_active_at = sql.Column(sql.Date, nullable=True)
 
@@ -55,8 +56,8 @@ class User(sql.ModelBase, sql.DictBase):
     def name(self):
         if self.local_user:
             return self.local_user.name
-        elif self.nonlocal_users:
-            return self.nonlocal_users[0].name
+        elif self.nonlocal_user:
+            return self.nonlocal_user.name
         elif self.federated_users:
             return self.federated_users[0].display_name
         else:
@@ -140,8 +141,8 @@ class User(sql.ModelBase, sql.DictBase):
     def domain_id(self):
         if self.local_user:
             return self.local_user.domain_id
-        elif self.nonlocal_users:
-            return self.nonlocal_users[0].domain_id
+        elif self.nonlocal_user:
+            return self.nonlocal_user.domain_id
         else:
             return None
 
@@ -273,7 +274,7 @@ class NonLocalUser(sql.ModelBase, sql.ModelDictMixin):
     domain_id = sql.Column(sql.String(64), primary_key=True)
     name = sql.Column(sql.String(255), primary_key=True)
     user_id = sql.Column(sql.String(64), sql.ForeignKey('user.id',
-                                                        ondelete='CASCADE'))
+                         ondelete='CASCADE'), unique=True)
 
 
 class Group(sql.ModelBase, sql.DictBase):

keystone/identity/shadow_backends/sql.py

@@ -105,7 +105,7 @@ class ShadowUsers(base.ShadowUsersDriverBase):
                 new_nonlocal_user_dict)
             new_user_ref = model.User.from_dict(new_user_dict)
             new_user_ref.created_at = datetime.datetime.utcnow()
-            new_user_ref.nonlocal_users.append(new_nonlocal_user_ref)
+            new_user_ref.nonlocal_user = new_nonlocal_user_ref
             session.add(new_user_ref)
             return identity_base.filter_user(new_user_ref.to_dict())
 

keystone/tests/unit/identity/test_backends.py

@@ -1351,6 +1351,23 @@ class ShadowUsersTests(object):
         user_ref = self._get_user_ref(new_nonlocal_user['id'])
         self.assertIsNone(user_ref.local_user)
 
+    def test_nonlocal_user_unique_user_id_constraint(self):
+        user_ref = unit.new_user_ref(domain_id=CONF.identity.default_domain_id)
+        user = self.shadow_users_api.create_nonlocal_user(user_ref)
+        # attempt to create a nonlocal_user with the same user_id
+        nonlocal_user = {
+            'domain_id': CONF.identity.default_domain_id,
+            'name': uuid.uuid4().hex,
+            'user_id': user['id']
+        }
+        self.assertRaises(sql.DBDuplicateEntry, self._add_nonlocal_user,
+                          nonlocal_user)
+
+    def _add_nonlocal_user(self, nonlocal_user):
+        with sql.session_for_write() as session:
+            nonlocal_user_ref = model.NonLocalUser.from_dict(nonlocal_user)
+            session.add(nonlocal_user_ref)
+
     def test_get_user(self):
         user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id)
         user.pop('email')

keystone/tests/unit/test_sql_upgrade.py

@@ -295,6 +295,15 @@ class SqlMigrateBase(test_base.DbTestCase):
         table = sqlalchemy.Table(table_name, self.metadata, autoload=True)
         return index_name in [idx.name for idx in table.indexes]
 
+    def does_unique_constraint_exist(self, table_name, column_name):
+        inspector = reflection.Inspector.from_engine(self.engine)
+        constraints = inspector.get_unique_constraints(table_name)
+        for c in constraints:
+            if (len(c['column_names']) == 1 and
+                    column_name in c['column_names']):
+                return True
+        return False
+
 
 class SqlLegacyRepoUpgradeTests(SqlMigrateBase):
     def test_blank_db_to_start(self):
@@ -1816,6 +1825,18 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
             'revocation_event',
             'ix_revocation_event_audit_id_issued_before'))
 
+    def test_migration_011_user_id_unique_for_nonlocal_user(self):
+        table_name = 'nonlocal_user'
+        column = 'user_id'
+        self.expand(10)
+        self.migrate(10)
+        self.contract(10)
+        self.assertFalse(self.does_unique_constraint_exist(table_name, column))
+        self.expand(11)
+        self.migrate(11)
+        self.contract(11)
+        self.assertTrue(self.does_unique_constraint_exist(table_name, column))
+
 
 class MySQLOpportunisticFullMigration(FullMigration):
     FIXTURE = test_base.MySQLOpportunisticFixture