Update local_id limit to 255 characters

This avoids the "String length exceeded." error, when using LDAP
domain specific backend in case the user uses a user id
attribute, which can exceed the previous constraint of 64 chars.

Change-Id: I923a2a2a5e79c8f265ff436e96258288dddb867b
Closes-Bug: #1929066
Resolves: rhbz#1959345

9 files changed, 120 insertions, 3 deletions

diff --git a/doc/source/admin/domain-specific-config.inc b/doc/source/admin/domain-specific-config.inc
index 3797e3078..2d8f9936a 100644
--- a/doc/source/admin/domain-specific-config.inc
+++ b/doc/source/admin/domain-specific-config.inc
@@ -146,6 +146,12 @@ then the same public ID will be created. This is useful if you are running
 multiple keystones and want to ensure the same ID would be generated whichever
 server you hit.
 
+.. NOTE::
+
+    In case of the LDAP backend, the names of users and groups are not hashed.
+    As a result, these are length limited to 255 characters. Longer names
+    will result in an error.
+
 While keystone will dynamically maintain the identity mapping, including
 removing entries when entities are deleted via the keystone, for those entities
 in backends that are managed outside of keystone (e.g. a read-only LDAP),
diff --git a/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py b/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py
new file mode 100644
index 000000000..2b09cbc99
--- /dev/null
+++ b/keystone/common/sql/contract_repo/versions/079_contract_update_local_id_limit.py
@@ -0,0 +1,18 @@
+#    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.
+
+# This is a placeholder for Ussuri backports. Do not use this number for new
+# Victoria work. New Victoria work starts after all the placeholders.
+
+
+def upgrade(migrate_engine):
+    pass
diff --git a/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py b/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py
new file mode 100644
index 000000000..2b09cbc99
--- /dev/null
+++ b/keystone/common/sql/data_migration_repo/versions/079_migrate_update_local_id_limit.py
@@ -0,0 +1,18 @@
+#    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.
+
+# This is a placeholder for Ussuri backports. Do not use this number for new
+# Victoria work. New Victoria work starts after all the placeholders.
+
+
+def upgrade(migrate_engine):
+    pass
diff --git a/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py b/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py
new file mode 100644
index 000000000..20db83851
--- /dev/null
+++ b/keystone/common/sql/expand_repo/versions/079_expand_update_local_id_limit.py
@@ -0,0 +1,24 @@
+#    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 sqlalchemy as sql
+
+
+def upgrade(migrate_engine):
+
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    id_mapping_table = sql.Table(
+        'id_mapping', meta, autoload=True
+    )
+    id_mapping_table.c.local_id.alter(type=sql.String(255))
diff --git a/keystone/identity/mapping_backends/sql.py b/keystone/identity/mapping_backends/sql.py
index 676d14492..6fadd6a0b 100644
--- a/keystone/identity/mapping_backends/sql.py
+++ b/keystone/identity/mapping_backends/sql.py
@@ -21,7 +21,7 @@ class IDMapping(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'id_mapping'
     public_id = sql.Column(sql.String(64), primary_key=True)
     domain_id = sql.Column(sql.String(64), nullable=False)
-    local_id = sql.Column(sql.String(64), nullable=False)
+    local_id = sql.Column(sql.String(255), nullable=False)
     # NOTE(henry-nash): Postgres requires a name to be defined for an Enum
     entity_type = sql.Column(
         sql.Enum(identity_mapping.EntityType.USER,
diff --git a/keystone/tests/unit/test_backend_id_mapping_sql.py b/keystone/tests/unit/test_backend_id_mapping_sql.py
index baee34e99..7729d53c6 100644
--- a/keystone/tests/unit/test_backend_id_mapping_sql.py
+++ b/keystone/tests/unit/test_backend_id_mapping_sql.py
@@ -33,7 +33,7 @@ class SqlIDMappingTable(test_backend_sql.SqlModels):
     def test_id_mapping(self):
         cols = (('public_id', sql.String, 64),
                 ('domain_id', sql.String, 64),
-                ('local_id', sql.String, 64),
+                ('local_id', sql.String, 255),
                 ('entity_type', sql.Enum, None))
         self.assertExpectedSchema('id_mapping', cols)
 
@@ -169,6 +169,26 @@ class SqlIDMapping(test_backend_sql.SqlTests):
         self.assertEqual(
             public_id, PROVIDERS.id_mapping_api.get_public_id(local_entity))
 
+    def test_id_mapping_handles_ids_greater_than_64_characters(self):
+        initial_mappings = len(mapping_sql.list_id_mappings())
+        local_id = 'Aa' * 100
+        local_entity = {'domain_id': self.domainA['id'],
+                        'local_id': local_id,
+                        'entity_type': mapping.EntityType.GROUP}
+
+        # Check no mappings for the new local entity
+        self.assertIsNone(PROVIDERS.id_mapping_api.get_public_id(local_entity))
+
+        # Create the new mapping and then read it back
+        public_id = PROVIDERS.id_mapping_api.create_id_mapping(local_entity)
+        self.assertThat(mapping_sql.list_id_mappings(),
+                        matchers.HasLength(initial_mappings + 1))
+        self.assertEqual(
+            public_id, PROVIDERS.id_mapping_api.get_public_id(local_entity))
+        self.assertEqual(
+            local_id,
+            PROVIDERS.id_mapping_api.get_id_mapping(public_id)['local_id'])
+
     def test_delete_public_id_is_silent(self):
         # Test that deleting an invalid public key is silent
         PROVIDERS.id_mapping_api.delete_id_mapping(uuid.uuid4().hex)
diff --git a/keystone/tests/unit/test_sql_banned_operations.py b/keystone/tests/unit/test_sql_banned_operations.py
index 9916657da..220714075 100644
--- a/keystone/tests/unit/test_sql_banned_operations.py
+++ b/keystone/tests/unit/test_sql_banned_operations.py
@@ -263,7 +263,12 @@ class TestKeystoneExpandSchemaMigrations(
         # timestamp to datetime and updates the initial value in the contract
         # phase. Adding an exception here to pass expand banned tests,
         # otherwise fails.
-        4
+        4,
+
+        # Migration 79 changes a varchar column length, doesn't
+        # convert the data within that column/table and doesn't rebuild
+        # indexes.
+        79
     ]
 
     def setUp(self):
diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py
index 50c28707a..502032f66 100644
--- a/keystone/tests/unit/test_sql_upgrade.py
+++ b/keystone/tests/unit/test_sql_upgrade.py
@@ -3499,6 +3499,25 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
             ['id', 'domain_id', 'enabled', 'description',
              'authorization_ttl'])
 
+    def test_migration_079_expand_update_local_id_limit(self):
+        self.expand(78)
+        self.migrate(78)
+        self.contract(78)
+
+        id_mapping_table = sqlalchemy.Table('id_mapping',
+                                            self.metadata, autoload=True)
+        # assert local_id column is a string of 64 characters (before)
+        self.assertEqual('VARCHAR(64)', str(id_mapping_table.c.local_id.type))
+
+        self.expand(79)
+        self.migrate(79)
+        self.contract(79)
+
+        id_mapping_table = sqlalchemy.Table('id_mapping',
+                                            self.metadata, autoload=True)
+        # assert local_id column is a string of 255 characters (after)
+        self.assertEqual('VARCHAR(255)', str(id_mapping_table.c.local_id.type))
+
 
 class MySQLOpportunisticFullMigration(FullMigration):
     FIXTURE = db_fixtures.MySQLOpportunisticFixture
diff --git a/releasenotes/notes/bug-1929066-6e741c9182620a37.yaml b/releasenotes/notes/bug-1929066-6e741c9182620a37.yaml
new file mode 100644
index 000000000..0acd1abc9
--- /dev/null
+++ b/releasenotes/notes/bug-1929066-6e741c9182620a37.yaml
@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    [`bug 1929066 <https://bugs.launchpad.net/keystone/+bug/1929066>`_]
+    Increase the length of the `local_id` column in the `id_mapping` table
+    to accommodate LDAP group names that result in names greater than
+    64 characters.