openstack
/
adjutant
Code Issues Proposed changes

Make reset password emails case insensitive 

* password reset action is no longer case sensitive for emails.
* action also no longer checks email when USERNAME_IS_EMAIL=True
* add test for action to confirm case insensitivity
* make fake keystone manager lower() name checks to match real
  keystone

bug: #1720756
Change-Id: Id1df793ec2f73f4f4e90960eaa55b0fd15b4f8b0
This commit is contained in:
Adrian Turjak 2017-10-03 11:35:40 +13:00
parent ae5c1943c9
commit 950ff0b60f
3 changed files with 119 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
adjutant/actions/v1/tests/test_user_actions.py
View File

@ -438,6 +438,52 @@ class UserActionTests(AdjutantTestCase):
fake_clients.identity_temp_cache['users'][user.id].password,
'123456')
def test_reset_user_password_case_insensitive(self):
"""
Existing user, ensure action is case insensitive.
USERNAME_IS_EMAIL=True
"""
user = mock.Mock()
user.id = 'user_id'
user.name = "test@example.com"
user.email = "test@example.com"
user.domain = 'default'
user.password = "gibberish"
setup_temp_cache({}, {user.id: user})
task = Task.objects.create(
ip_address="0.0.0.0",
keystone_user={
'roles': ['admin', 'project_mod'],
'project_id': 'test_project_id',
'project_domain_id': 'default',
})
data = {
'domain_name': 'Default',
'email': 'TEST@example.com',
'project_name': 'test_project',
}
action = ResetUserPasswordAction(data, task=task, order=1)
action.pre_approve()
self.assertEquals(action.valid, True)
action.post_approve()
self.assertEquals(action.valid, True)
token_data = {'password': '123456'}
action.submit(token_data)
self.assertEquals(action.valid, True)
self.assertEquals(
fake_clients.identity_temp_cache['users'][user.id].password,
'123456')
def test_reset_user_password_no_user(self):
"""
Reset password for a non-existant user.
@ -944,6 +990,54 @@ class UserActionTests(AdjutantTestCase):
fake_clients.identity_temp_cache['users'][user.id].email,
'test@example.com')
@override_settings(USERNAME_IS_EMAIL=False)
def test_reset_user_password_case_insensitive_not_username(self):
"""
Existing user, ensure action is case insensitive.
USERNAME_IS_EMAIL=False
"""
user = mock.Mock()
user.id = 'user_id'
user.name = "test_user"
user.email = "test@example.com"
user.domain = 'default'
user.password = "gibberish"
setup_temp_cache({}, {user.id: user})
task = Task.objects.create(
ip_address="0.0.0.0",
keystone_user={
'roles': ['admin', 'project_mod'],
'project_id': 'test_project_id',
'project_domain_id': 'default',
})
data = {
'domain_name': 'Default',
'username': 'test_USER',
'email': 'TEST@example.com',
'project_name': 'test_project',
}
action = ResetUserPasswordAction(data, task=task, order=1)
action.pre_approve()
self.assertEquals(action.valid, True)
action.post_approve()
self.assertEquals(action.valid, True)
token_data = {'password': '123456'}
action.submit(token_data)
self.assertEquals(action.valid, True)
self.assertEquals(
fake_clients.identity_temp_cache['users'][user.id].password,
'123456')
@override_settings(USERNAME_IS_EMAIL=True)
def test_update_email(self):
"""

36
adjutant/actions/v1/users.py
View File

@ -166,12 +166,11 @@ class ResetUserPasswordAction(UserNameAction, UserMixin):
def __init__(self, *args, **kwargs):
super(ResetUserPasswordAction, self).__init__(*args, **kwargs)
self.blacklist = self.settings.get("blacklisted_roles", {})
self.blacklist = self.settings.get("blacklisted_roles", [])
def _validate_user_roles(self):
id_manager = user_store.IdentityManager()
self.user = id_manager.find_user(self.username, self.domain.id)
roles = id_manager.get_all_roles(self.user)
user_roles = []
@ -182,23 +181,30 @@ class ResetUserPasswordAction(UserNameAction, UserMixin):
self.add_note('Cannot reset users with blacklisted roles.')
return False
if self.user.email == self.email:
self.action.need_token = True
self.set_token_fields(["password"])
self.add_note('Existing user with matching email.')
return True
else:
self.add_note('Existing user with non-matching email.')
return False
return True
def _validate_user_email(self):
# NOTE(adriant): We only need to check the USERNAME_IS_EMAIL=False
# case since '_validate_username_exists' will ensure the True case
if not settings.USERNAME_IS_EMAIL:
if self.user.email.lower() != self.email.lower():
self.add_note('Existing user with non-matching email.')
return False
self.action.need_token = True
self.set_token_fields(["password"])
self.add_note('Existing user with matching email.')
return True
def _validate(self):
# Here, the order of validation matters
# as each one adds new class variables
self.action.valid = (
self._validate_domain_name() and
self._validate_username_exists() and
self._validate_user_roles()
)
self.action.valid = all([
self._validate_domain_name(),
self._validate_username_exists(),
self._validate_user_roles(),
self._validate_user_email(),
])
self.action.save()
def _pre_approve(self):

7
adjutant/common/tests/fake_clients.py
View File

@ -99,7 +99,7 @@ class FakeManager(object):
domain = self._domain_from_id(domain)
global identity_temp_cache
for user in identity_temp_cache['users'].values():
if user.name == name and user.domain == domain.id:
if user.name.lower() == name.lower() and user.domain == domain.id:
return user
return None
@ -218,7 +218,8 @@ class FakeManager(object):
domain = self._domain_from_id(domain)
global identity_temp_cache
for project in identity_temp_cache['projects'].values():
if project.name == project_name and project.domain == domain.id:
if (project.name.lower() == project_name.lower() and
project.domain == domain.id):
return project
return None
@ -259,7 +260,7 @@ class FakeManager(object):
def find_domain(self, domain_name):
global identity_temp_cache
for domain in identity_temp_cache['domains'].values():
if domain.name == domain_name:
if domain.name.lower() == domain_name.lower():
return domain
return None