Skip to content

Commit

Permalink
Increase length of username in DB
Browse files Browse the repository at this point in the history
Length of username in database may be too short for X.509 DNs and 255
seems a sane value for it.

Fixes bug #1081932

Change-Id: Ie8f696845ea15d37cf13f3fe7978b22deac798b0
  • Loading branch information
alvarolopez authored and Morgan Fainberg committed Aug 15, 2013
1 parent 049c5c7 commit 8d6055e
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 7 deletions.
7 changes: 4 additions & 3 deletions keystone/clean.py
Expand Up @@ -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


Expand All @@ -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):
Expand Down
31 changes: 31 additions & 0 deletions 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))
2 changes: 1 addition & 1 deletion keystone/identity/backends/sql.py
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions keystone/tests/test_backend.py
Expand Up @@ -1628,7 +1628,7 @@ def test_update_project_invalid_name_fails(self):
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,
Expand Down Expand Up @@ -1701,7 +1701,7 @@ def test_update_user_long_name_fails(self):
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',
Expand Down
2 changes: 1 addition & 1 deletion keystone/tests/test_backend_sql.py
Expand Up @@ -81,7 +81,7 @@ def assertExpectedSchema(self, table, cols):

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),
Expand Down
36 changes: 36 additions & 0 deletions keystone/tests/test_sql_upgrade.py
Expand Up @@ -556,6 +556,42 @@ def insert_dict(self, session, table_name, d):
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)

Expand Down

0 comments on commit 8d6055e

Please sign in to comment.