Merge "Increase length of username in DB"

6 files changed, 75 insertions, 7 deletions

diff --git a/keystone/clean.py b/keystone/clean.py
index c1d01ec8..7684210a 100644
--- a/keystone/clean.py
+++ b/keystone/clean.py
@@ -44,10 +44,11 @@ def check_enabled(property_name, enabled):
     return bool(enabled)
 
 
-def check_name(property_name, name):
+def check_name(property_name, name, min_length=1, max_length=64):
     check_type('%s name' % property_name, name, basestring, 'str or unicode')
     name = name.strip()
-    check_length('%s name' % property_name, name)
+    check_length('%s name' % property_name, name,
+                 min_length=min_length, max_length=max_length)
     return name
 
 
@@ -64,7 +65,7 @@ def project_enabled(enabled):
 
 
 def user_name(name):
-    return check_name('User', name)
+    return check_name('User', name, max_length=255)
 
 
 def user_enabled(enabled):
diff --git a/keystone/common/sql/migrate_repo/versions/032_username_length.py b/keystone/common/sql/migrate_repo/versions/032_username_length.py
new file mode 100644
index 00000000..636ebd75
--- /dev/null
+++ b/keystone/common/sql/migrate_repo/versions/032_username_length.py
@@ -0,0 +1,31 @@
+import sqlalchemy as sql
+from sqlalchemy.orm import sessionmaker
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    user_table = sql.Table('user', meta, autoload=True)
+    user_table.c.name.alter(type=sql.String(255))
+
+
+def downgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    user_table = sql.Table('user', meta, autoload=True)
+    if migrate_engine.name != 'mysql':
+        # NOTE(aloga): sqlite does not enforce length on the
+        # VARCHAR types: http://www.sqlite.org/faq.html#q9
+        # postgresql and DB2 do not truncate.
+        maker = sessionmaker(bind=migrate_engine)
+        session = maker()
+        for user in session.query(user_table).all():
+            values = {'name': user.name[:64]}
+            update = (user_table.update().
+                      where(user_table.c.id == user.id).
+                      values(values))
+            migrate_engine.execute(update)
+
+        session.commit()
+        session.close()
+    user_table.c.name.alter(type=sql.String(64))
diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py
index bff41106..65a34a8a 100644
--- a/keystone/identity/backends/sql.py
+++ b/keystone/identity/backends/sql.py
@@ -26,7 +26,7 @@ class User(sql.ModelBase, sql.DictBase):
     __tablename__ = 'user'
     attributes = ['id', 'name', 'domain_id', 'password', 'enabled']
     id = sql.Column(sql.String(64), primary_key=True)
-    name = sql.Column(sql.String(64), nullable=False)
+    name = sql.Column(sql.String(255), nullable=False)
     domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'),
                            nullable=False)
     password = sql.Column(sql.String(128))
diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py
index 8622b10d..52628985 100644
--- a/keystone/tests/test_backend.py
+++ b/keystone/tests/test_backend.py
@@ -1628,7 +1628,7 @@ class IdentityTests(object):
                           tenant)
 
     def test_create_user_long_name_fails(self):
-        user = {'id': 'fake1', 'name': 'a' * 65,
+        user = {'id': 'fake1', 'name': 'a' * 256,
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.assertRaises(exception.ValidationError,
                           self.identity_api.create_user,
@@ -1701,7 +1701,7 @@ class IdentityTests(object):
         user = {'id': 'fake1', 'name': 'fake1',
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.identity_api.create_user('fake1', user)
-        user['name'] = 'a' * 65
+        user['name'] = 'a' * 256
         self.assertRaises(exception.ValidationError,
                           self.identity_api.update_user,
                           'fake1',
diff --git a/keystone/tests/test_backend_sql.py b/keystone/tests/test_backend_sql.py
index 773ae862..24159eb6 100644
--- a/keystone/tests/test_backend_sql.py
+++ b/keystone/tests/test_backend_sql.py
@@ -81,7 +81,7 @@ class SqlModels(SqlTests):
 
     def test_user_model(self):
         cols = (('id', sql.String, 64),
-                ('name', sql.String, 64),
+                ('name', sql.String, 255),
                 ('password', sql.String, 128),
                 ('domain_id', sql.String, 64),
                 ('enabled', sql.Boolean, None),
diff --git a/keystone/tests/test_sql_upgrade.py b/keystone/tests/test_sql_upgrade.py
index e904d6a7..7d60ced4 100644
--- a/keystone/tests/test_sql_upgrade.py
+++ b/keystone/tests/test_sql_upgrade.py
@@ -556,6 +556,42 @@ class SqlUpgradeTests(SqlMigrateBase):
         insert.execute(d)
         session.commit()
 
+    def test_upgrade_31_to_32(self):
+        self.upgrade(32)
+
+        user_table = self.select_table("user")
+        self.assertEquals(user_table.c.name.type.length, 255)
+
+    def test_downgrade_32_to_31(self):
+        self.upgrade(32)
+        session = self.Session()
+        # NOTE(aloga): we need a different metadata object
+        user_table = sqlalchemy.Table('user',
+                                      sqlalchemy.MetaData(),
+                                      autoload=True,
+                                      autoload_with=self.engine)
+        user_id = uuid.uuid4().hex
+        ins = user_table.insert().values(
+            {'id': user_id,
+             'name': 'a' * 255,
+             'password': uuid.uuid4().hex,
+             'enabled': True,
+             'domain_id': DEFAULT_DOMAIN_ID,
+             'extra': '{}'})
+        session.execute(ins)
+        session.commit()
+
+        self.downgrade(31)
+        # Check that username has been truncated
+        q = session.query(user_table.c.name)
+        q = q.filter(user_table.c.id == user_id)
+        r = q.one()
+        user_name = r[0]
+        self.assertEquals(len(user_name), 64)
+
+        user_table = self.select_table("user")
+        self.assertEquals(user_table.c.name.type.length, 64)
+
     def test_downgrade_to_0(self):
         self.upgrade(self.max_version)