Cascade delete federated_user fk

The bug was caused by a foreign key in the federated_user table. This
key prevents a protocol from being deleted after a successful
authentication has happened (so the creation of a federated user
via shadowing). We take advantage of the same foreign key by adding the
cascade delete behavior to it.

Closes-Bug: 1642692

Change-Id: I3b3e265d20f0cfe0ee10c6a274d9bdf4e840b742

keystone/common/sql/contract_repo/versions/013_protocol_cascade_delete_for_federated_user.py

@@ -0,0 +1,31 @@
+# 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
+
+    federated_table = sql.Table('federated_user', meta, autoload=True)
+    protocol_table = sql.Table('federation_protocol', meta, autoload=True)
+
+    migrate.ForeignKeyConstraint(
+        columns=[federated_table.c.protocol_id, federated_table.c.idp_id],
+        refcolumns=[protocol_table.c.id, protocol_table.c.idp_id]).drop()
+
+    migrate.ForeignKeyConstraint(
+        columns=[federated_table.c.protocol_id, federated_table.c.idp_id],
+        refcolumns=[protocol_table.c.id, protocol_table.c.idp_id],
+        ondelete='CASCADE').create()

keystone/common/sql/data_migration_repo/versions/013_protocol_cascade_delete_for_federated_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/013_protocol_cascade_delete_for_federated_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/tests/unit/test_sql_banned_operations.py

@@ -281,7 +281,14 @@ class TestKeystoneDataMigrations(
         # timestamp to datetime and updates the initial value in the contract
         # phase. Adding an exception here to pass data migrations banned tests,
         # otherwise fails.
-        4
+        4,
+        # Migration 013 updates a foreign key constraint at the federated_user
+        # table. It is a composite key pointing to the procotol.id and
+        # protocol.idp_id columns. Since we can't create a new foreign key
+        # before dropping the old one and the operations happens in the same
+        # upgrade phase, adding an exception here to pass the data migration
+        # banned tests.
+        13
     ]
 
     def setUp(self):
@@ -334,7 +341,14 @@ class TestKeystoneContractSchemaMigrations(
         # could be a performance issue for existing large password tables, as
         # the migration is not batched. However, it's a compromise and not
         # likely going to be a problem for operators.
-        4
+        4,
+        # Migration 013 updates a foreign key constraint at the federated_user
+        # table. It is a composite key pointing to the procotol.id and
+        # protocol.idp_id columns. Since we can't create a new foreign key
+        # before dropping the old one and the operations happens in the same
+        # upgrade phase, adding an exception here to pass the contract
+        # banned tests.
+        13
     ]
 
     def setUp(self):

keystone/tests/unit/test_sql_upgrade.py

@@ -1904,6 +1904,88 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
         idp_table = sqlalchemy.Table(idp_name, self.metadata, autoload=True)
         self.assertFalse(idp_table.c.domain_id.nullable)
 
+    def test_migration_013_protocol_cascade_delete_for_federated_user(self):
+        if self.engine.name == 'sqlite':
+            self.skipTest('sqlite backend does not support foreign keys')
+
+        self.expand(12)
+        self.migrate(12)
+        self.contract(12)
+
+        # This test requires a bit of setup to properly work, first we create
+        # an identity provider, mapping and a protocol. Then, we create a
+        # federated user and delete the protocol. We expect the federated user
+        # to be deleted as well.
+
+        session = self.sessionmaker()
+
+        def _create_protocol():
+            domain = {
+                'id': uuid.uuid4().hex,
+                'name': uuid.uuid4().hex,
+                'domain_id': resource_base.NULL_DOMAIN_ID,
+                'is_domain': True,
+                'parent_id': None
+            }
+            self.insert_dict(session, 'project', domain)
+
+            idp = {'id': uuid.uuid4().hex, 'enabled': True,
+                   'domain_id': domain['id']}
+            self.insert_dict(session, 'identity_provider', idp)
+
+            mapping = {'id': uuid.uuid4().hex, 'rules': json.dumps([])}
+            self.insert_dict(session, 'mapping', mapping)
+
+            protocol = {'id': uuid.uuid4().hex, 'idp_id': idp['id'],
+                        'mapping_id': mapping['id']}
+            protocol_table = sqlalchemy.Table(
+                'federation_protocol', self.metadata, autoload=True)
+            self.insert_dict(session, 'federation_protocol', protocol,
+                             table=protocol_table)
+
+            return protocol, protocol_table
+
+        def _create_federated_user(idp_id, protocol_id):
+            user = {'id': uuid.uuid4().hex}
+            self.insert_dict(session, 'user', user)
+
+            # NOTE(rodrigods): do not set the ID, the engine will do that
+            # for us and we won't need it later.
+            federated_user = {
+                'user_id': user['id'], 'idp_id': idp_id,
+                'protocol_id': protocol_id, 'unique_id': uuid.uuid4().hex}
+            federated_table = sqlalchemy.Table(
+                'federated_user', self.metadata, autoload=True)
+            self.insert_dict(session, 'federated_user', federated_user,
+                             table=federated_table)
+
+            return federated_user, federated_table
+
+        protocol, protocol_table = _create_protocol()
+        federated_user, federated_table = _create_federated_user(
+            protocol['idp_id'], protocol['id'])
+
+        # before updating the foreign key, we won't be able to delete the
+        # protocol
+        self.assertRaises(db_exception.DBError,
+                          session.execute,
+                          protocol_table.delete().where(
+                              protocol_table.c.id == protocol['id']))
+
+        self.expand(13)
+        self.migrate(13)
+        self.contract(13)
+
+        # now we are able to delete the protocol
+        session.execute(
+            protocol_table.delete().where(
+                protocol_table.c.id == protocol['id']))
+
+        # assert the cascade deletion worked
+        federated_users = session.query(federated_table).filter_by(
+            protocol_id=federated_user['protocol_id']).all()
+        self.assertThat(federated_users, matchers.HasLength(0))
+
 
 class MySQLOpportunisticFullMigration(FullMigration):
     FIXTURE = test_base.MySQLOpportunisticFixture

keystone/tests/unit/test_v3_federation.py

@@ -3007,6 +3007,21 @@ class FederatedUserTests(test_v3.RestfulTestCase, FederatedSetupMixin):
             self.assertNotIn(project['id'], user_project_ids)
             user_project_ids.append(project['id'])
 
+    def test_delete_protocol_after_federated_authentication(self):
+        # Create a protocol
+        protocol = self.proto_ref(mapping_id=self.mapping['id'])
+        self.federation_api.create_protocol(
+            self.IDP, protocol['id'], protocol)
+
+        # Authenticate to create a new federated_user entry with a foreign
+        # key pointing to the protocol
+        r = self._issue_unscoped_token()
+        user_id = r.json_body['token']['user']['id']
+        self.assertNotEmpty(self.identity_api.get_user(user_id))
+
+        # Now we should be able to delete the protocol
+        self.federation_api.delete_protocol(self.IDP, protocol['id'])
+
     def _authenticate_via_saml(self):
         r = self._issue_unscoped_token()
         unscoped_token = r.headers['X-Subject-Token']

releasenotes/notes/bug-1642692-d669c8fcf9e171d9.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    [`bug 1642692 <https://bugs.launchpad.net/keystone/+bug/1642692>`_]
+    All federated_user entries will be deleted if the ``protocol`` it used
+    to first authenticate is deleted.