Merge "Drop the compatibility password column"
This commit is contained in:
commit
74b2be3a41
|
@ -0,0 +1,21 @@
|
||||||
|
# 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
|
||||||
|
|
||||||
|
password_table = sql.Table('password', meta, autoload=True)
|
||||||
|
password_table.c.password.drop()
|
|
@ -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
|
|
@ -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
|
|
@ -237,8 +237,7 @@ class Identity(base.IdentityDriverBase):
|
||||||
if unique_cnt > 0:
|
if unique_cnt > 0:
|
||||||
for password_ref in user_ref.local_user.passwords[-unique_cnt:]:
|
for password_ref in user_ref.local_user.passwords[-unique_cnt:]:
|
||||||
if password_hashing.check_password(
|
if password_hashing.check_password(
|
||||||
password,
|
password, password_ref.password_hash):
|
||||||
password_ref.password_hash or password_ref.password):
|
|
||||||
raise exception.PasswordHistoryValidationError(
|
raise exception.PasswordHistoryValidationError(
|
||||||
unique_count=unique_cnt)
|
unique_count=unique_cnt)
|
||||||
|
|
||||||
|
|
|
@ -105,10 +105,7 @@ class User(sql.ModelBase, sql.ModelDictMixinWithExtras):
|
||||||
def password(self):
|
def password(self):
|
||||||
"""Return the current password."""
|
"""Return the current password."""
|
||||||
if self.password_ref:
|
if self.password_ref:
|
||||||
if self.password_ref.password_hash is not None:
|
return self.password_ref.password_hash
|
||||||
return self.password_ref.password_hash
|
|
||||||
else:
|
|
||||||
return self.password_ref.password
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@property
|
@property
|
||||||
|
@ -149,19 +146,17 @@ class User(sql.ModelBase, sql.ModelDictMixinWithExtras):
|
||||||
new_password_ref = Password()
|
new_password_ref = Password()
|
||||||
|
|
||||||
hashed_passwd = None
|
hashed_passwd = None
|
||||||
hashed_compat = None
|
|
||||||
if value is not None:
|
if value is not None:
|
||||||
# NOTE(notmorgan): hash the passwords, never directly bind the
|
# NOTE(notmorgan): hash the passwords, never directly bind the
|
||||||
# "value" in the unhashed form to hashed_passwd or hashed_compat
|
# "value" in the unhashed form to hashed_passwd to ensure the
|
||||||
# to ensure the unhashed password cannot end up in the db. If an
|
# unhashed password cannot end up in the db. If an unhashed
|
||||||
# unhashed password ends up in the DB, it cannot be used for auth,
|
# password ends up in the DB, it cannot be used for auth, it is
|
||||||
# it is however incorrect and could leak user credentials (due to
|
# however incorrect and could leak user credentials (due to users
|
||||||
# users doing insecure things such as sharing passwords across
|
# doing insecure things such as sharing passwords across
|
||||||
# different systems) to unauthorized parties.
|
# different systems) to unauthorized parties.
|
||||||
hashed_passwd = password_hashing.hash_password(value)
|
hashed_passwd = password_hashing.hash_password(value)
|
||||||
|
|
||||||
new_password_ref.password_hash = hashed_passwd
|
new_password_ref.password_hash = hashed_passwd
|
||||||
new_password_ref.password = hashed_compat
|
|
||||||
new_password_ref.created_at = now
|
new_password_ref.created_at = now
|
||||||
new_password_ref.expires_at = self._get_password_expires_at(now)
|
new_password_ref.expires_at = self._get_password_expires_at(now)
|
||||||
self.local_user.passwords.append(new_password_ref)
|
self.local_user.passwords.append(new_password_ref)
|
||||||
|
@ -185,7 +180,7 @@ class User(sql.ModelBase, sql.ModelDictMixinWithExtras):
|
||||||
|
|
||||||
@password.expression
|
@password.expression
|
||||||
def password(cls):
|
def password(cls):
|
||||||
return Password.password
|
return Password.password_hash
|
||||||
|
|
||||||
# NOTE(stevemar): we use a hybrid property here because we leverage the
|
# NOTE(stevemar): we use a hybrid property here because we leverage the
|
||||||
# expression method, see `@enabled.expression` and `User._enabled` below.
|
# expression method, see `@enabled.expression` and `User._enabled` below.
|
||||||
|
@ -289,15 +284,11 @@ class LocalUser(sql.ModelBase, sql.ModelDictMixin):
|
||||||
|
|
||||||
class Password(sql.ModelBase, sql.ModelDictMixin):
|
class Password(sql.ModelBase, sql.ModelDictMixin):
|
||||||
__tablename__ = 'password'
|
__tablename__ = 'password'
|
||||||
attributes = ['id', 'local_user_id', 'password', 'password_hash',
|
attributes = ['id', 'local_user_id', 'password_hash', 'created_at',
|
||||||
'created_at', 'expires_at']
|
'expires_at']
|
||||||
id = sql.Column(sql.Integer, primary_key=True)
|
id = sql.Column(sql.Integer, primary_key=True)
|
||||||
local_user_id = sql.Column(sql.Integer, sql.ForeignKey('local_user.id',
|
local_user_id = sql.Column(sql.Integer, sql.ForeignKey('local_user.id',
|
||||||
ondelete='CASCADE'))
|
ondelete='CASCADE'))
|
||||||
# TODO(notmorgan): in the Q release the "password" field can be dropped as
|
|
||||||
# long as data migration exists to move the hashes over to the
|
|
||||||
# password_hash column if no value is in the password_hash column.
|
|
||||||
password = sql.Column(sql.String(128), nullable=True)
|
|
||||||
password_hash = sql.Column(sql.String(255), nullable=True)
|
password_hash = sql.Column(sql.String(255), nullable=True)
|
||||||
|
|
||||||
# TODO(lbragstad): Once Rocky opens for development, the _created_at and
|
# TODO(lbragstad): Once Rocky opens for development, the _created_at and
|
||||||
|
|
|
@ -61,18 +61,6 @@ class UserPasswordHashingTestsNoCompat(test_backend_sql.SqlTests):
|
||||||
self.config_fixture.config(group='identity',
|
self.config_fixture.config(group='identity',
|
||||||
password_hash_algorithm='scrypt')
|
password_hash_algorithm='scrypt')
|
||||||
|
|
||||||
def test_password_hashing_compat_not_set_used(self):
|
|
||||||
with sql.session_for_read() as session:
|
|
||||||
user_ref = PROVIDERS.identity_api._get_user(
|
|
||||||
session, self.user_foo['id']
|
|
||||||
)
|
|
||||||
self.assertIsNone(user_ref.password_ref.password)
|
|
||||||
self.assertIsNotNone(user_ref.password_ref.password_hash)
|
|
||||||
self.assertEqual(user_ref.password,
|
|
||||||
user_ref.password_ref.password_hash)
|
|
||||||
self.assertTrue(password_hashing.check_password(
|
|
||||||
self.user_foo['password'], user_ref.password))
|
|
||||||
|
|
||||||
def test_configured_algorithm_used(self):
|
def test_configured_algorithm_used(self):
|
||||||
with sql.session_for_read() as session:
|
with sql.session_for_read() as session:
|
||||||
user_ref = PROVIDERS.identity_api._get_user(
|
user_ref = PROVIDERS.identity_api._get_user(
|
||||||
|
|
|
@ -152,7 +152,6 @@ class SqlModels(SqlTests):
|
||||||
def test_password_model(self):
|
def test_password_model(self):
|
||||||
cols = (('id', sql.Integer, None),
|
cols = (('id', sql.Integer, None),
|
||||||
('local_user_id', sql.Integer, None),
|
('local_user_id', sql.Integer, None),
|
||||||
('password', sql.String, 128),
|
|
||||||
('password_hash', sql.String, 255),
|
('password_hash', sql.String, 255),
|
||||||
('created_at', sql.DateTime, None),
|
('created_at', sql.DateTime, None),
|
||||||
('expires_at', sql.DateTime, None),
|
('expires_at', sql.DateTime, None),
|
||||||
|
|
|
@ -3179,6 +3179,29 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
|
||||||
}
|
}
|
||||||
role_table.insert().values(role_without_description).execute()
|
role_table.insert().values(role_without_description).execute()
|
||||||
|
|
||||||
|
def test_migration_054_drop_old_password_column(self):
|
||||||
|
self.expand(53)
|
||||||
|
self.migrate(53)
|
||||||
|
self.contract(53)
|
||||||
|
|
||||||
|
password_table = 'password'
|
||||||
|
self.assertTableColumns(
|
||||||
|
password_table,
|
||||||
|
['id', 'local_user_id', 'password', 'password_hash',
|
||||||
|
'self_service', 'created_at_int', 'created_at', 'expires_at_int',
|
||||||
|
'expires_at']
|
||||||
|
)
|
||||||
|
|
||||||
|
self.expand(54)
|
||||||
|
self.migrate(54)
|
||||||
|
self.contract(54)
|
||||||
|
|
||||||
|
self.assertTableColumns(
|
||||||
|
password_table,
|
||||||
|
['id', 'local_user_id', 'password_hash', 'self_service',
|
||||||
|
'created_at_int', 'created_at', 'expires_at_int', 'expires_at']
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class MySQLOpportunisticFullMigration(FullMigration):
|
class MySQLOpportunisticFullMigration(FullMigration):
|
||||||
FIXTURE = db_fixtures.MySQLOpportunisticFixture
|
FIXTURE = db_fixtures.MySQLOpportunisticFixture
|
||||||
|
|
Loading…
Reference in New Issue