openstack/adjutant
Code Issues Proposed changes

Fix issues around LDAP backed Keystone

Browse Source 
Invite user workflow now defaults to domain_id from
the project.

Create project workflow now default to getting domain
and parent id from config.

Identity manager now has setting to flag the inability to
edit/create users, which some actions now support.

Fix an issue with email comparison when username_is_email was true.

Change-Id: I8548914e3d2283b17f3015595ea72c4c8084d7f5
This commit is contained in:
Adrian Turjak 2019-01-17 14:36:25 +13:00
parent 6a849eec3e
commit fef06515c9
8 changed files with 168 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
adjutant/actions/v1/projects.py
View File

@ -14,6 +14,7 @@
from uuid import uuid4
from django.conf import settings
from django.utils import timezone
from adjutant.common import user_store
@ -43,7 +44,7 @@ class NewProjectAction(BaseAction, ProjectMixin, UserMixin):
def _validate(self):
self.action.valid = validate_steps([
self._validate_domain_id,
self._validate_parent_project,
self._validate_keystone_user_parent_project,
self._validate_project_absent,
])
self.action.save()
@ -57,7 +58,7 @@ class NewProjectAction(BaseAction, ProjectMixin, UserMixin):
return super(NewProjectAction, self)._validate_domain_id()
def _validate_parent_project(self):
def _validate_keystone_user_parent_project(self):
if self.parent_id:
keystone_user = self.action.task.keystone_user
@ -65,7 +66,7 @@ class NewProjectAction(BaseAction, ProjectMixin, UserMixin):
self.add_note(
'Parent id does not match keystone user project.')
return False
return super(NewProjectAction, self)._validate_parent_project()
return self._validate_parent_project()
return True
def _pre_approve(self):
@ -151,29 +152,41 @@ class NewProjectWithUserAction(UserNameAction, ProjectMixin, UserMixin):
user = id_manager.find_user(self.username, self.domain_id)
if not user:
self.add_note(
"No user present with username '%s'. "
"Need to create new user." % self.username)
if not id_manager.can_edit_users:
self.add_note(
'Identity backend does not support user editing, '
'cannot create new user.')
return False
# add to cache to use in template
self.action.task.cache['user_state'] = "default"
self.action.need_token = True
self.set_token_fields(["password"])
self.add_note("No user present with username '%s'." %
self.username)
return True
if user.email != self.email:
if (not settings.USERNAME_IS_EMAIL
and getattr(user, 'email', None) != self.email):
self.add_note("Existing user '%s' with non-matching email." %
self.username)
return False
if not user.enabled:
self.add_note(
"Existing disabled user '%s' with matching email." %
self.email)
if not id_manager.can_edit_users:
self.add_note(
'Identity backend does not support user editing, '
'cannot renable user.')
return False
self.action.state = "disabled"
# add to cache to use in template
self.action.task.cache['user_state'] = "disabled"
self.action.need_token = True
# as they are disabled we'll reset their password
self.set_token_fields(["password"])
self.add_note(
"Existing disabled user '%s' with matching email." %
self.email)
return True
else:
self.action.state = "existing"

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

@ -45,28 +45,41 @@ class NewUserAction(UserNameAction, ProjectMixin, UserMixin):
# this may mean we need a token.
user = self._get_target_user()
if not user:
self.add_note(
"No user present with username '%s'. "
"Need to create new user." % self.username)
if not id_manager.can_edit_users:
self.add_note(
'Identity backend does not support user editing, '
'cannot create new user.')
return False
self.action.need_token = True
# add to cache to use in template
self.action.task.cache['user_state'] = "default"
self.set_token_fields(["password"])
self.add_note(
'No user present with username. Need to create new user.')
return True
if user.email != self.email:
if (not settings.USERNAME_IS_EMAIL
and getattr(user, 'email', None) != self.email):
self.add_note(
'Found matching username, but email did not match. '
'Reporting as invalid.')
return False
if not user.enabled:
self.add_note(
"Existing disabled user '%s' with matching email." %
self.email)
if not id_manager.can_edit_users:
self.add_note(
'Identity backend does not support user editing, '
'cannot renable user.')
return False
self.action.need_token = True
self.action.state = "disabled"
# add to cache to use in template
self.action.task.cache['user_state'] = "disabled"
# as they are disabled we'll reset their password
self.set_token_fields(["password"])
self.add_note(
'Existing disabled user with matching email.')
return True
# role_validation
@ -192,7 +205,9 @@ class ResetUserPasswordAction(UserNameAction, UserMixin):
# 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 and self.user.email.lower() != self.email.lower():
if (self.user and (
getattr(self.user, 'email', None).lower()
!= self.email.lower())):
self.add_note('Existing user with non-matching email.')
return False

4
adjutant/api/v1/openstack.py
View File

@ -119,6 +119,10 @@ class UserList(tasks.InviteUser):
if notification.error:
status = "Failed"
for action in task.actions:
if not action.valid:
status = "Invalid"
task_data = {}
for action in task.actions:
task_data.update(action.action_data)

10
adjutant/api/v1/tasks.py
View File

@ -321,6 +321,10 @@ class CreateProject(TaskView):
# we need to set the region the resources will be created in:
request.data['region'] = class_conf.get('default_region')
# domain
request.data['domain_id'] = class_conf.get(
'default_domain_id', 'default')
# parent_id for new project, if null defaults to domain:
request.data['parent_id'] = class_conf.get('default_parent_id')
@ -372,6 +376,12 @@ class InviteUser(TaskView):
or request.data['project_id'] is None):
request.data['project_id'] = request.keystone_user['project_id']
# Default domain_id to the keystone user's project
if ('domain_id' not in request.data
or request.data['domain_id'] is None):
request.data['domain_id'] = \
request.keystone_user['project_domain_id']
processed, status = self.process_actions(request)
errors = processed.get('errors', None)

99
adjutant/api/v1/tests/test_api_taskview.py
View File

@ -1433,3 +1433,102 @@ class TaskViewTests(AdjutantAPITestCase):
"Error: KeyError('Forced key error.') while setting up "
"task. See task itself for details."]})
self.assertEqual(new_notification.task, new_task)
@override_settings(KEYSTONE={'can_edit_users': False})
def test_user_invite_cant_edit_users(self):
"""
When can_edit_users is false, and a new user is invited,
the task should be marked as invalid if the user doesn't
already exist.
"""
project = fake_clients.FakeProject(name="test_project")
setup_identity_cache(projects=[project])
url = "/v1/actions/InviteUser"
headers = {
'project_name': "test_project",
'project_id': project.id,
'roles': "project_admin,_member_,project_mod",
'username': "user",
'user_id': "test_user_id",
'authenticated': True
}
data = {'username': 'new_user', 'email': "new@example.com",
'roles': ["_member_"], 'project_id': project.id}
response = self.client.post(url, data, format='json', headers=headers)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {'errors': ['actions invalid']})
@override_settings(KEYSTONE={'can_edit_users': False})
def test_user_invite_cant_edit_users_existing_user(self):
"""
When can_edit_users is false, and a new user is invited,
the task should be marked as valid if the user exists.
"""
project = fake_clients.FakeProject(name="test_project")
user = fake_clients.FakeUser(name="test@example.com")
setup_identity_cache(projects=[project], users=[user])
url = "/v1/actions/InviteUser"
headers = {
'project_name': "test_project",
'project_id': project.id,
'roles': "project_admin,_member_,project_mod",
'username': "user",
'user_id': "test_user_id",
'authenticated': True
}
data = {'username': 'new_user', 'email': "test@example.com",
'roles': ["_member_"], 'project_id': project.id}
response = self.client.post(url, data, format='json', headers=headers)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json(), {'notes': ['created token']})
@override_settings(KEYSTONE={'can_edit_users': False})
def test_project_create_cant_edit_users(self):
"""
When can_edit_users is false, and a new signup comes in,
the task should be marked as invalid if it needs to
create a new user.
Will return OK (as task doesn't auto_approve), but task will
actually be invalid.
"""
setup_identity_cache()
url = "/v1/actions/CreateProject"
data = {'project_name': "test_project", 'email': "test@example.com"}
response = self.client.post(url, data, format='json')
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json(), {'notes': ['task created']})
task = Task.objects.all()[0]
action_models = task.actions
actions = [act.get_action() for act in action_models]
self.assertFalse(all([act.valid for act in actions]))
@override_settings(KEYSTONE={'can_edit_users': False})
def test_project_create_cant_edit_users_existing_user(self):
"""
When can_edit_users is false, and a new signup comes in,
the task should be marked as valid if the user already
exists.
Will return OK (as task doesn't auto_approve), but task will
actually be valid.
"""
user = fake_clients.FakeUser(name="test@example.com")
setup_identity_cache(users=[user])
url = "/v1/actions/CreateProject"
data = {'project_name': "test_project", 'email': "test@example.com"}
response = self.client.post(url, data, format='json')
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json(), {'notes': ['task created']})
task = Task.objects.all()[0]
action_models = task.actions
actions = [act.get_action() for act in action_models]
self.assertTrue(all([act.valid for act in actions]))

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

@ -46,7 +46,7 @@ class FakeProject(object):
class FakeUser(object):
def __init__(self, name, password, domain_id='default',
def __init__(self, name, password="123", domain_id='default',
enabled=True, default_project_id=None,
**kwargs):
self.id = uuid4().hex
@ -158,6 +158,11 @@ def setup_identity_cache(projects=None, users=None, role_assignments=None,
class FakeManager(object):
def __init__(self):
# TODO(adriant): decide if we want to have some function calls
# throw errors if this is false.
self.can_edit_users = settings.KEYSTONE.get('can_edit_users', True)
def _project_from_id(self, project):
if isinstance(project, FakeProject):
return project

4
adjutant/common/user_store.py
View File

@ -59,6 +59,10 @@ class IdentityManager(object): # pragma: no cover
def __init__(self):
self.ks_client = get_keystoneclient()
# TODO(adriant): decide if we want to have some function calls
# throw errors if this is false.
self.can_edit_users = settings.KEYSTONE.get('can_edit_users', True)
def find_user(self, name, domain):
try:
users = self.ks_client.users.list(name=name, domain=domain)

3
conf/conf.yaml
View File

@ -47,7 +47,7 @@ EMAIL_SETTINGS:
# to have different values.
USERNAME_IS_EMAIL: True
# Keystone admin credentials:
# Keystone config
KEYSTONE:
username: admin
password: openstack
@ -55,6 +55,7 @@ KEYSTONE:
# MUST BE V3 API:
auth_url: http://localhost/identity/v3
domain_id: default
can_edit_users: True
HORIZON_URL: http://localhost:8080/