From e83a4d72345315a1519543ad37c43aa9003c3a7b Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Fri, 23 Sep 2022 19:42:21 -0400 Subject: [PATCH 1/7] Refactor of LDAP backend to be more efficent --- awx/sso/backends.py | 121 ++++++++++++++++++++++++++++++++------------ 1 file changed, 90 insertions(+), 31 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index 3fbbb23e0a..dc7130f3c1 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -327,13 +327,13 @@ class SAMLAuth(BaseSAMLAuth): return super(SAMLAuth, self).get_user(user_id) -def _update_m2m_from_groups(user, ldap_user, related, opts, remove=True): +def _update_m2m_from_groups(ldap_user_group_dns, opts, remove=True): """ Hepler function to update m2m relationship based on LDAP group membership. """ should_add = False if opts is None: - return + return None elif not opts: pass elif opts is True: @@ -341,17 +341,12 @@ def _update_m2m_from_groups(user, ldap_user, related, opts, remove=True): else: if isinstance(opts, str): opts = [opts] - for group_dn in opts: - if not isinstance(group_dn, str): - continue - if ldap_user._get_groups().is_member_of(group_dn): - should_add = True + # If any of the users groups matches any of the list options + if list(set.intersection(set(ldap_user_group_dns), set(opts))): + should_add = True if should_add: - user.save() - related.add(user) - elif remove and user in related.all(): - user.save() - related.remove(user) + return True + return False @receiver(populate_user, dispatch_uid='populate-ldap-user') @@ -372,7 +367,7 @@ def on_populate_user(sender, **kwargs): # Prefetch user's groups to prevent LDAP queries for each org/team when # checking membership. - ldap_user._get_groups().get_group_dns() + ldap_user_group_dns = ldap_user._get_groups().get_group_dns() # If the LDAP user has a first or last name > $maxlen chars, truncate it for field in ('first_name', 'last_name'): @@ -383,31 +378,70 @@ def on_populate_user(sender, **kwargs): force_user_update = True logger.warning('LDAP user {} has {} > max {} characters'.format(user.username, field, max_len)) - # Update organization membership based on group memberships. org_map = getattr(backend.settings, 'ORGANIZATION_MAP', {}) - for org_name, org_opts in org_map.items(): - org, created = Organization.objects.get_or_create(name=org_name) - remove = bool(org_opts.get('remove', True)) - admins_opts = org_opts.get('admins', None) - remove_admins = bool(org_opts.get('remove_admins', remove)) - _update_m2m_from_groups(user, ldap_user, org.admin_role.members, admins_opts, remove_admins) - auditors_opts = org_opts.get('auditors', None) - remove_auditors = bool(org_opts.get('remove_auditors', remove)) - _update_m2m_from_groups(user, ldap_user, org.auditor_role.members, auditors_opts, remove_auditors) - users_opts = org_opts.get('users', None) - remove_users = bool(org_opts.get('remove_users', remove)) - _update_m2m_from_groups(user, ldap_user, org.member_role.members, users_opts, remove_users) - - # Update team membership based on group memberships. team_map = getattr(backend.settings, 'TEAM_MAP', {}) + + # Move this junk into save of the settings for performance later, there is no need to do that here + # with maybe the exception of someone defining this in settings before the server is started? + # ============================================================================================================== + # Get all of the orgs in the DB by name and create any new org defined in LDAP + + # IS THIS ACTUALLY GETTING JUST NAMES OR DOES IT GET ALL AND THEN GET NAMES FORM THEM ALL? + existing_org_names = list(Organization.objects.all().values_list('name', flat=True)) + for org_name in org_map.keys(): + if org_name not in existing_org_names: + Organization.objects.get_or_create(name=org_name) + # Add the org name to the existing orgs since we created it and we may need it to build the teams below + existing_org_names.append(org_name) + + existing_team_names = list(Team.objects.all().values_list('name', flat=True)) + for team_name, team_opts in team_map.items(): + if 'organization' not in team_opts: + continue + if team_opts['organization'] not in existing_org_names: + Organization.objects.get_or_create(name=team_opts['organization']) + # Append it to the list so that we don't try and create this again (if we needed to) + existing_org_names.append(team_opts['organization']) + if team_name not in existing_team_names: + # Going a little inefficient here, we could prob name/ids above and then set this by ID but I've got limited time r/n + org, created = Organization.objects.get_or_create(name=team_opts['organization']) + Team.objects.get_or_create(name=team_name, organization=org) + # End move some day + # ============================================================================================================== + + # Compute in memory what the state is of the different LDAP orgs + desired_org_states = {} + for org_name, org_opts in org_map.items(): + remove = bool(org_opts.get('remove', True)) + admins_opts = org_opts.get('admins', None) + remove_admins = bool(org_opts.get('remove_admins', remove)) + desired_org_states[org_name] = {} + desired_org_states[org_name]['admin_role'] = _update_m2m_from_groups(ldap_user_group_dns, admins_opts, remove_admins) + auditors_opts = org_opts.get('auditors', None) + remove_auditors = bool(org_opts.get('remove_auditors', remove)) + desired_org_states[org_name]['auditor_role'] = _update_m2m_from_groups(ldap_user_group_dns, auditors_opts, remove_auditors) + users_opts = org_opts.get('users', None) + remove_users = bool(org_opts.get('remove_users', remove)) + desired_org_states[org_name]['member_role'] = _update_m2m_from_groups(ldap_user_group_dns, users_opts, remove_users) + + # If everything returned None (because there was no configuration) we can skip this host + if ( + desired_org_states[org_name]['admin_role'] == None + and desired_org_states[org_name]['auditor_role'] == None + and desired_org_states[org_name]['member_role'] == None + ): + del desired_org_states[org_name] + + # Compute in memory what the state is of the different LDAP teams + desired_team_states = {} for team_name, team_opts in team_map.items(): if 'organization' not in team_opts: continue - org, created = Organization.objects.get_or_create(name=team_opts['organization']) - team, created = Team.objects.get_or_create(name=team_name, organization=org) users_opts = team_opts.get('users', None) remove = bool(team_opts.get('remove', True)) - _update_m2m_from_groups(user, ldap_user, team.member_role.members, users_opts, remove) + state = _update_m2m_from_groups(ldap_user_group_dns, users_opts, remove) + if state is not None: + desired_team_states[team_name] = state # Check if user.profile is available, otherwise force user.save() try: @@ -423,3 +457,28 @@ def on_populate_user(sender, **kwargs): if profile.ldap_dn != ldap_user.dn: profile.ldap_dn = ldap_user.dn profile.save() + + # Get all of the orgs listed in LDAP from the database + roles = ['admin_role', 'auditor_role', 'member_role'] + prefetch_roles = ["{}__members".format(role_name) for role_name in roles] + orgs_tied_to_ldap = Organization.objects.filter(name__in=desired_org_states.keys()).prefetch_related(*prefecth_roles) + # Set the state from memory into the org objects + for org in orgs_tied_to_ldap: + for role in roles: + if desired_org_states[org.name][role] is None: + # If something got a none we just need to continue on + pass + elif desired_org_states[org.name][role]: + getattr(org, role).members.add(user) + elif user in getattr(org, role).members.all(): + getattr(org, role).members.remove(user) + + # Get all of the teams listed in LDAP from the database + teams_tied_to_ldap = Team.objects.filter(name__in=desired_team_states.keys()).prefetch_related('member_role__members') + # Set the state from memory into the team objects + for team in teams_tied_to_ldap: + # We don't need a None check here because it just would have never made it into the dict + if desired_team_states[team.name]: + team.member_role.members.add(user) + elif user in team.member_role.members.all(): + team.member_role.remove(user) From 2dd2931ab2085003232ab08e86479345792b14fa Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Sat, 24 Sep 2022 09:13:12 -0400 Subject: [PATCH 2/7] Fixing bug, updating comments and adding debugging logging --- awx/sso/backends.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index dc7130f3c1..2f2b3ad0cd 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -336,7 +336,7 @@ def _update_m2m_from_groups(ldap_user_group_dns, opts, remove=True): return None elif not opts: pass - elif opts is True: + elif isinstance(opts, bool) and opts is True: should_add = True else: if isinstance(opts, str): @@ -384,9 +384,8 @@ def on_populate_user(sender, **kwargs): # Move this junk into save of the settings for performance later, there is no need to do that here # with maybe the exception of someone defining this in settings before the server is started? # ============================================================================================================== - # Get all of the orgs in the DB by name and create any new org defined in LDAP - # IS THIS ACTUALLY GETTING JUST NAMES OR DOES IT GET ALL AND THEN GET NAMES FORM THEM ALL? + # Get all of the names of orgs in the DB and create any new org defined in LDAP that does not exist in the DB existing_org_names = list(Organization.objects.all().values_list('name', flat=True)) for org_name in org_map.keys(): if org_name not in existing_org_names: @@ -394,16 +393,20 @@ def on_populate_user(sender, **kwargs): # Add the org name to the existing orgs since we created it and we may need it to build the teams below existing_org_names.append(org_name) + # Do the same for teams existing_team_names = list(Team.objects.all().values_list('name', flat=True)) for team_name, team_opts in team_map.items(): if 'organization' not in team_opts: + logger.debug("Team named {} in LDAP team map settings is invalid due to missing organization".format(team_name)) continue + # A team may have an org that we didn't previously evaluate if team_opts['organization'] not in existing_org_names: Organization.objects.get_or_create(name=team_opts['organization']) - # Append it to the list so that we don't try and create this again (if we needed to) + # Append it to the list so that we don't try and create this again if its used by another team existing_org_names.append(team_opts['organization']) if team_name not in existing_team_names: - # Going a little inefficient here, we could prob name/ids above and then set this by ID but I've got limited time r/n + # Going a little inefficient here, we could prob get name/ids above and then set this by ID but I've got limited time r/n + # and this code should only really be getting hit once if an LDAP config change adds a team with an org not built. org, created = Organization.objects.get_or_create(name=team_opts['organization']) Team.objects.get_or_create(name=team_name, organization=org) # End move some day @@ -461,7 +464,7 @@ def on_populate_user(sender, **kwargs): # Get all of the orgs listed in LDAP from the database roles = ['admin_role', 'auditor_role', 'member_role'] prefetch_roles = ["{}__members".format(role_name) for role_name in roles] - orgs_tied_to_ldap = Organization.objects.filter(name__in=desired_org_states.keys()).prefetch_related(*prefecth_roles) + orgs_tied_to_ldap = Organization.objects.filter(name__in=desired_org_states.keys()).prefetch_related(*prefetch_roles) # Set the state from memory into the org objects for org in orgs_tied_to_ldap: for role in roles: @@ -469,8 +472,11 @@ def on_populate_user(sender, **kwargs): # If something got a none we just need to continue on pass elif desired_org_states[org.name][role]: - getattr(org, role).members.add(user) + if user not in getattr(org, role).members.all(): + logger.debug("Adding LDAP user {} to {} as {}".format(user.username, org.name, role)) + getattr(org, role).members.add(user) elif user in getattr(org, role).members.all(): + logger.debug("Removing LDAP user {} from {} in {}".format(user.username, role, org.name)) getattr(org, role).members.remove(user) # Get all of the teams listed in LDAP from the database @@ -479,6 +485,9 @@ def on_populate_user(sender, **kwargs): for team in teams_tied_to_ldap: # We don't need a None check here because it just would have never made it into the dict if desired_team_states[team.name]: - team.member_role.members.add(user) + if user not in team.member_role.members.all(): + logger.debug("Adding LDAP user {} to team {}".format(user.username, team.name)) + team.member_role.members.add(user) elif user in team.member_role.members.all(): - team.member_role.remove(user) + logger.debug("Removing LDAP user {} from team {}".format(user.username, team.name)) + team.member_role.members.remove(user) From 80a0842df1b8ec556f747098301887d5cfc7c4ad Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 26 Sep 2022 10:36:27 -0400 Subject: [PATCH 3/7] Updating comments and remove unneeded variable --- awx/sso/backends.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index 2f2b3ad0cd..c66e3addf6 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -329,23 +329,26 @@ class SAMLAuth(BaseSAMLAuth): def _update_m2m_from_groups(ldap_user_group_dns, opts, remove=True): """ - Hepler function to update m2m relationship based on LDAP group membership. + Hepler function to evaluate the LDAP team/org options to determine if LDAP user should + be a member of the team/org based on their ldap group dns. + + Returns: + True - User should be added + False - User should be removed + None - Users membership should not be changed """ - should_add = False if opts is None: return None elif not opts: pass elif isinstance(opts, bool) and opts is True: - should_add = True + return True else: if isinstance(opts, str): opts = [opts] # If any of the users groups matches any of the list options if list(set.intersection(set(ldap_user_group_dns), set(opts))): - should_add = True - if should_add: - return True + return True return False @@ -427,7 +430,7 @@ def on_populate_user(sender, **kwargs): remove_users = bool(org_opts.get('remove_users', remove)) desired_org_states[org_name]['member_role'] = _update_m2m_from_groups(ldap_user_group_dns, users_opts, remove_users) - # If everything returned None (because there was no configuration) we can skip this host + # If everything returned None (because there was no configuration) we can remove this org from our map if ( desired_org_states[org_name]['admin_role'] == None and desired_org_states[org_name]['auditor_role'] == None From e7c75f35106cffbda82ccb4a41389ed9f631a187 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Wed, 28 Sep 2022 12:21:30 -0400 Subject: [PATCH 4/7] Reverting checking of LDAP groups The initial check performed case insensitive searches and the new method was case sensitive The optimization of the new method is likely not going to contribute noticable slowness --- awx/sso/backends.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index c66e3addf6..5db8e8535b 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -327,7 +327,7 @@ class SAMLAuth(BaseSAMLAuth): return super(SAMLAuth, self).get_user(user_id) -def _update_m2m_from_groups(ldap_user_group_dns, opts, remove=True): +def _update_m2m_from_groups(ldap_user, opts, remove=True): """ Hepler function to evaluate the LDAP team/org options to determine if LDAP user should be a member of the team/org based on their ldap group dns. @@ -347,8 +347,11 @@ def _update_m2m_from_groups(ldap_user_group_dns, opts, remove=True): if isinstance(opts, str): opts = [opts] # If any of the users groups matches any of the list options - if list(set.intersection(set(ldap_user_group_dns), set(opts))): - return True + for group_dn in opts: + if not isinstance(group_dn, str): + continue + if ldap_user._get_groups().is_member_of(group_dn): + return True return False @@ -370,7 +373,7 @@ def on_populate_user(sender, **kwargs): # Prefetch user's groups to prevent LDAP queries for each org/team when # checking membership. - ldap_user_group_dns = ldap_user._get_groups().get_group_dns() + ldap_user._get_groups().get_group_dns() # If the LDAP user has a first or last name > $maxlen chars, truncate it for field in ('first_name', 'last_name'): @@ -422,13 +425,13 @@ def on_populate_user(sender, **kwargs): admins_opts = org_opts.get('admins', None) remove_admins = bool(org_opts.get('remove_admins', remove)) desired_org_states[org_name] = {} - desired_org_states[org_name]['admin_role'] = _update_m2m_from_groups(ldap_user_group_dns, admins_opts, remove_admins) + desired_org_states[org_name]['admin_role'] = _update_m2m_from_groups(ldap_user, admins_opts, remove_admins) auditors_opts = org_opts.get('auditors', None) remove_auditors = bool(org_opts.get('remove_auditors', remove)) - desired_org_states[org_name]['auditor_role'] = _update_m2m_from_groups(ldap_user_group_dns, auditors_opts, remove_auditors) + desired_org_states[org_name]['auditor_role'] = _update_m2m_from_groups(ldap_user, auditors_opts, remove_auditors) users_opts = org_opts.get('users', None) remove_users = bool(org_opts.get('remove_users', remove)) - desired_org_states[org_name]['member_role'] = _update_m2m_from_groups(ldap_user_group_dns, users_opts, remove_users) + desired_org_states[org_name]['member_role'] = _update_m2m_from_groups(ldap_user, users_opts, remove_users) # If everything returned None (because there was no configuration) we can remove this org from our map if ( @@ -445,7 +448,7 @@ def on_populate_user(sender, **kwargs): continue users_opts = team_opts.get('users', None) remove = bool(team_opts.get('remove', True)) - state = _update_m2m_from_groups(ldap_user_group_dns, users_opts, remove) + state = _update_m2m_from_groups(ldap_user, users_opts, remove) if state is not None: desired_team_states[team_name] = state From b5db710c8bf9578001cadeade1c6c20068e02268 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 3 Oct 2022 13:00:33 -0400 Subject: [PATCH 5/7] Multiple enhancements Extrapolating reconciliation of desired and actual states to a function Converting heave prefect related methods to user focus for query optimization Converting from get_or_create to simply create Added memory calculations for query optimization --- awx/sso/backends.py | 140 +++++++++++++++++++++++++------------------- 1 file changed, 80 insertions(+), 60 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index 5db8e8535b..ad3be6ecf3 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -11,9 +11,11 @@ import ldap # Django from django.dispatch import receiver from django.contrib.auth.models import User +from django.contrib.contenttypes.models import ContentType from django.conf import settings as django_settings from django.core.signals import setting_changed from django.utils.encoding import force_str +from django.db.utils import IntegrityError # django-auth-ldap from django_auth_ldap.backend import LDAPSettings as BaseLDAPSettings @@ -391,54 +393,54 @@ def on_populate_user(sender, **kwargs): # with maybe the exception of someone defining this in settings before the server is started? # ============================================================================================================== - # Get all of the names of orgs in the DB and create any new org defined in LDAP that does not exist in the DB - existing_org_names = list(Organization.objects.all().values_list('name', flat=True)) - for org_name in org_map.keys(): - if org_name not in existing_org_names: - Organization.objects.get_or_create(name=org_name) + # Get all of the IDs and names of orgs in the DB and create any new org defined in LDAP that does not exist in the DB + existing_orgs = {} + for (org_id, org_name) in Organization.objects.all().values_list('id', 'name'): + existing_orgs[org_name] = org_id + + # Create any orgs (if needed) for all entries in the org and team maps + for org_name in set(list(org_map.keys()) + [item.get('organization', None) for item in team_map.values()]): + if org_name and org_name not in existing_orgs: + logger.info("LDAP adapter is creating org {}".format(org_name)) + try: + new_org = Organization.objects.create(name=org_name) + except IntegrityError: + # Another thread must have created this org before we did so now we need to get it + new_org = Organization.objects.get(name=org_name) # Add the org name to the existing orgs since we created it and we may need it to build the teams below - existing_org_names.append(org_name) + existing_orgs[org_name] = new_org.id # Do the same for teams existing_team_names = list(Team.objects.all().values_list('name', flat=True)) for team_name, team_opts in team_map.items(): if 'organization' not in team_opts: - logger.debug("Team named {} in LDAP team map settings is invalid due to missing organization".format(team_name)) + # You can't save the LDAP config in the UI w/o an org so if we somehow got this condition its an error + logger.error("Team named {} in LDAP team map settings is invalid due to missing organization".format(team_name)) continue - # A team may have an org that we didn't previously evaluate - if team_opts['organization'] not in existing_org_names: - Organization.objects.get_or_create(name=team_opts['organization']) - # Append it to the list so that we don't try and create this again if its used by another team - existing_org_names.append(team_opts['organization']) if team_name not in existing_team_names: - # Going a little inefficient here, we could prob get name/ids above and then set this by ID but I've got limited time r/n - # and this code should only really be getting hit once if an LDAP config change adds a team with an org not built. - org, created = Organization.objects.get_or_create(name=team_opts['organization']) - Team.objects.get_or_create(name=team_name, organization=org) + try: + Team.objects.create(name=team_name, organization_id=existing_orgs[team_opts['organization']]) + except IntegrityError: + # If another process got here before us that is ok because we don't need the ID from this team or anything + pass # End move some day # ============================================================================================================== # Compute in memory what the state is of the different LDAP orgs + org_roles_and_ldap_attributes = {'admin_role': 'admins', 'auditor_role': 'auditors', 'member_role': 'users'} desired_org_states = {} for org_name, org_opts in org_map.items(): remove = bool(org_opts.get('remove', True)) - admins_opts = org_opts.get('admins', None) - remove_admins = bool(org_opts.get('remove_admins', remove)) desired_org_states[org_name] = {} - desired_org_states[org_name]['admin_role'] = _update_m2m_from_groups(ldap_user, admins_opts, remove_admins) - auditors_opts = org_opts.get('auditors', None) - remove_auditors = bool(org_opts.get('remove_auditors', remove)) - desired_org_states[org_name]['auditor_role'] = _update_m2m_from_groups(ldap_user, auditors_opts, remove_auditors) - users_opts = org_opts.get('users', None) - remove_users = bool(org_opts.get('remove_users', remove)) - desired_org_states[org_name]['member_role'] = _update_m2m_from_groups(ldap_user, users_opts, remove_users) + for org_role_name in org_roles_and_ldap_attributes.keys(): + ldap_name = org_roles_and_ldap_attributes[org_role_name] + opts = org_opts.get(ldap_name, None) + remove = bool(org_opts.get('remove_{}'.format(ldap_name), remove)) + desired_org_states[org_name][org_role_name] = _update_m2m_from_groups(ldap_user, opts, remove) # If everything returned None (because there was no configuration) we can remove this org from our map - if ( - desired_org_states[org_name]['admin_role'] == None - and desired_org_states[org_name]['auditor_role'] == None - and desired_org_states[org_name]['member_role'] == None - ): + # This will prevent us from loading the org in the next query + if all(desired_org_states[org_name][org_role_name] is None for org_role_name in org_roles_and_ldap_attributes.keys()): del desired_org_states[org_name] # Compute in memory what the state is of the different LDAP teams @@ -450,7 +452,7 @@ def on_populate_user(sender, **kwargs): remove = bool(team_opts.get('remove', True)) state = _update_m2m_from_groups(ldap_user, users_opts, remove) if state is not None: - desired_team_states[team_name] = state + desired_team_states[team_name] = {'member_role': state} # Check if user.profile is available, otherwise force user.save() try: @@ -467,33 +469,51 @@ def on_populate_user(sender, **kwargs): profile.ldap_dn = ldap_user.dn profile.save() - # Get all of the orgs listed in LDAP from the database - roles = ['admin_role', 'auditor_role', 'member_role'] - prefetch_roles = ["{}__members".format(role_name) for role_name in roles] - orgs_tied_to_ldap = Organization.objects.filter(name__in=desired_org_states.keys()).prefetch_related(*prefetch_roles) - # Set the state from memory into the org objects - for org in orgs_tied_to_ldap: - for role in roles: - if desired_org_states[org.name][role] is None: - # If something got a none we just need to continue on - pass - elif desired_org_states[org.name][role]: - if user not in getattr(org, role).members.all(): - logger.debug("Adding LDAP user {} to {} as {}".format(user.username, org.name, role)) - getattr(org, role).members.add(user) - elif user in getattr(org, role).members.all(): - logger.debug("Removing LDAP user {} from {} in {}".format(user.username, role, org.name)) - getattr(org, role).members.remove(user) + reconcile_users_org_team_mappings(user, desired_org_states, desired_team_states, 'LDAP') - # Get all of the teams listed in LDAP from the database - teams_tied_to_ldap = Team.objects.filter(name__in=desired_team_states.keys()).prefetch_related('member_role__members') - # Set the state from memory into the team objects - for team in teams_tied_to_ldap: - # We don't need a None check here because it just would have never made it into the dict - if desired_team_states[team.name]: - if user not in team.member_role.members.all(): - logger.debug("Adding LDAP user {} to team {}".format(user.username, team.name)) - team.member_role.members.add(user) - elif user in team.member_role.members.all(): - logger.debug("Removing LDAP user {} from team {}".format(user.username, team.name)) - team.member_role.members.remove(user) + +def reconcile_users_org_team_mappings(user, desired_org_states, desired_team_states, source): + from awx.main.models import Organization, Team + + org_content_type = ContentType.objects.get_for_model(Organization) + team_content_type = ContentType.objects.get_for_model(Team) + + # users_roles is a flat list of IDs + users_roles = list(user.roles.filter(content_type__in=[org_content_type, team_content_type]).values_list('pk', flat=True)) + + for object_type, desired_states, model in [('organization', desired_org_states, Organization), ('team', desired_team_states, Team)]: + # Get all of the roles in the desired states for efficient DB extraction + roles = [] + for sub_dict in desired_states.values(): + for role_name in sub_dict: + if sub_dict[role_name] == None: + continue + if role_name not in roles: + roles.append(role_name) + + # Get a set of named tuples for the org name plus all of the roles we got above + model_roles = getattr(model, 'objects').filter(name__in=desired_states.keys()).values_list(*(['name'] + roles), named=True) + for row in model_roles: + for role_name in roles: + desired_state = desired_states.get(row.name, {}) + if desired_state[role_name] == None: + # The mapping was not defined for this [org/team]/role so we can just pass + pass + + # If somehow the auth adapter knows about an items role but that role is not defined in the DB we are going to print a pretty error + # This is your classic safety net that we should never hit; but here you are reading this comment... good luck and Godspeed. + role_id = getattr(row, role_name, None) + if role_id == None: + logger.error("{} adapter wanted to manage role {} of {} {} but that role is not defined".format(source, role_name, object_type, row.name)) + continue + + if desired_state[role_name]: + # The desired state was the user mapped into the object_type, if the user was not mapped in map them in + if role_id not in users_roles: + logger.debug("{} adapter adding user {} to {} {} as {}".format(source, user.username, object_type, row.name, role_name)) + user.roles.add(role_id) + else: + # The desired state was the user was not mapped into the org, if the user has the permission remove it + if role_id in users_roles: + logger.debug("{} adapter removing user {} permission of {} from {} {}".format(source, user.username, role_name, object_type, row.name)) + user.roles.remove(role_id) From 3a09522d3e670fb46082adf57c41931812d59fda Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 3 Oct 2022 14:01:38 -0400 Subject: [PATCH 6/7] Fixing '== None' and better handeling of {} settings --- awx/sso/backends.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index ad3be6ecf3..26e47de245 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -475,35 +475,45 @@ def on_populate_user(sender, **kwargs): def reconcile_users_org_team_mappings(user, desired_org_states, desired_team_states, source): from awx.main.models import Organization, Team - org_content_type = ContentType.objects.get_for_model(Organization) - team_content_type = ContentType.objects.get_for_model(Team) + content_types = [] + reconcile_items = [] + if desired_org_states: + content_types.append(ContentType.objects.get_for_model(Organization)) + reconcile_items.append(('organization', desired_org_states, Organization)) + if desired_team_states: + content_types.append(ContentType.objects.get_for_model(Team)) + reconcile_items.append(('team', desired_team_states, Team)) - # users_roles is a flat list of IDs - users_roles = list(user.roles.filter(content_type__in=[org_content_type, team_content_type]).values_list('pk', flat=True)) + if not content_types: + # If both desired states were empty we can simply return because there is nothing to reconcile + return - for object_type, desired_states, model in [('organization', desired_org_states, Organization), ('team', desired_team_states, Team)]: + # users_roles is a flat set of IDs + users_roles = set(user.roles.filter(content_type__in=content_types).values_list('pk', flat=True)) + + for object_type, desired_states, model in reconcile_items: # Get all of the roles in the desired states for efficient DB extraction roles = [] for sub_dict in desired_states.values(): for role_name in sub_dict: - if sub_dict[role_name] == None: + if sub_dict[role_name] is None: continue if role_name not in roles: roles.append(role_name) - # Get a set of named tuples for the org name plus all of the roles we got above - model_roles = getattr(model, 'objects').filter(name__in=desired_states.keys()).values_list(*(['name'] + roles), named=True) + # Get a set of named tuples for the org/team name plus all of the roles we got above + model_roles = model.objects.filter(name__in=desired_states.keys()).values_list('name', *roles, named=True) for row in model_roles: for role_name in roles: desired_state = desired_states.get(row.name, {}) - if desired_state[role_name] == None: + if desired_state[role_name] is None: # The mapping was not defined for this [org/team]/role so we can just pass pass # If somehow the auth adapter knows about an items role but that role is not defined in the DB we are going to print a pretty error # This is your classic safety net that we should never hit; but here you are reading this comment... good luck and Godspeed. role_id = getattr(row, role_name, None) - if role_id == None: + if role_id is None: logger.error("{} adapter wanted to manage role {} of {} {} but that role is not defined".format(source, role_name, object_type, row.name)) continue From a4fba37222946a635ef6eb7db2cbfac3ebd6c6fc Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 3 Oct 2022 14:40:15 -0400 Subject: [PATCH 7/7] Changing to handle not only missing but null and empty organization in team map --- awx/sso/backends.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/sso/backends.py b/awx/sso/backends.py index 26e47de245..5daa621165 100644 --- a/awx/sso/backends.py +++ b/awx/sso/backends.py @@ -413,8 +413,8 @@ def on_populate_user(sender, **kwargs): # Do the same for teams existing_team_names = list(Team.objects.all().values_list('name', flat=True)) for team_name, team_opts in team_map.items(): - if 'organization' not in team_opts: - # You can't save the LDAP config in the UI w/o an org so if we somehow got this condition its an error + if not team_opts.get('organization', None): + # You can't save the LDAP config in the UI w/o an org (or '' or null as the org) so if we somehow got this condition its an error logger.error("Team named {} in LDAP team map settings is invalid due to missing organization".format(team_name)) continue if team_name not in existing_team_names: