From 3e46dcfc7e356e31e08c3eacc884f2a84f236360 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Tue, 7 Oct 2025 12:24:11 -0700 Subject: [PATCH 01/20] add warning to project_remove_users template re: primary user removal --- .../templates/project/project_remove_users.html | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/coldfront/core/project/templates/project/project_remove_users.html b/coldfront/core/project/templates/project/project_remove_users.html index 3d8b7034d6..116e12b16d 100644 --- a/coldfront/core/project/templates/project/project_remove_users.html +++ b/coldfront/core/project/templates/project/project_remove_users.html @@ -17,10 +17,17 @@

Remove users from project: {{project.title}}

remove that user's data from the allocations.

- To be removed from a lab, the user must not have the lab as their primary - group. If you would like to remove a user that has your lab as their primary - group, please - contact FASRC support. + {% if request.user.is_superuser %} + As a superuser, you can remove any user from the project. Please be aware that + removing a user that has your lab as their primary group may cause issues with + that user's access to other resources. If you are unsure, please + contact FASRC support or the ColdFront system administrator. + {% else %} + To be removed from a lab, the user must not have the lab as their primary + group. If you would like to remove a user that has your lab as their primary + group, please + contact FASRC support. + {% endif %}

{% csrf_token %} From 19a99fdf99204ab7d02a94d0d7c7a69abf364af8 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Tue, 7 Oct 2025 13:25:42 -0700 Subject: [PATCH 02/20] make primary group users selectable by admins --- .../project/project_remove_users.html | 38 +++++++++++++------ coldfront/core/project/views.py | 12 ++++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/coldfront/core/project/templates/project/project_remove_users.html b/coldfront/core/project/templates/project/project_remove_users.html index 116e12b16d..daebebffda 100644 --- a/coldfront/core/project/templates/project/project_remove_users.html +++ b/coldfront/core/project/templates/project/project_remove_users.html @@ -8,7 +8,7 @@

Remove users from project: {{project.title}}


-{% if formset or users_no_removal %} +{% if formset or primary_group_users %}

@@ -39,6 +39,7 @@

Remove users from project: {{project.title}}

# + Is Primary Username First Name Last Name @@ -47,21 +48,36 @@

Remove users from project: {{project.title}}

- {% for user in users_no_removal %} - - - - {{ user.username }} - {{ user.first_name }} - {{ user.last_name }} - {{ user.email }} - {{ user.role }} - + {% for user in primary_group_users %} + {% if request.user.is_superuser %} + + {{ user.selected }} + {{ forloop.counter }} + + {{ user.username.value }} + {{ user.first_name.value }} + {{ user.last_name.value }} + {{ user.email.value }} + {{ user.role.value }} + + {% else %} + + + + + {{ user.username }} + {{ user.first_name }} + {{ user.last_name }} + {{ user.email }} + {{ user.role }} + + {% endif %} {% endfor %} {% for form in formset %} {{ form.selected }} {{ forloop.counter }} + {{ form.username.value }} {{ form.first_name.value }} {{ form.last_name.value }} diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 148345fb30..1e0c3586c8 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -840,7 +840,7 @@ def get(self, request, *args, **kwargs): else: users_to_remove = users_list - users_no_removal = [u for u in users_list if u not in users_to_remove] + primary_group_users = [u for u in users_list if u not in users_to_remove] context = {} @@ -850,9 +850,15 @@ def get(self, request, *args, **kwargs): ) formset = formset(initial=users_to_remove, prefix='userform') context['formset'] = formset - + if self.request.user.is_superuser: + primaryuser_formset = formset_factory( + ProjectRemoveUserForm, max_num=len(primary_group_users) + ) + primaryuser_formset = primaryuser_formset(initial=primary_group_users, prefix='userform') + context['primary_group_users'] = primaryuser_formset + else: + context['primary_group_users'] = primary_group_users context['project'] = get_object_or_404(Project, pk=pk) - context['users_no_removal'] = users_no_removal context['EMAIL_TICKET_SYSTEM_ADDRESS'] = EMAIL_TICKET_SYSTEM_ADDRESS return render(request, self.template_name, context) From 031fcc2849cc9c71c6c269463807b3943a1018f1 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 10 Dec 2025 15:12:48 -0800 Subject: [PATCH 03/20] add Deactivated ProjectUserStatusChoice to add_default_project_choices.py --- .../management/commands/add_default_project_choices.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/coldfront/core/project/management/commands/add_default_project_choices.py b/coldfront/core/project/management/commands/add_default_project_choices.py index d66e03b46a..ddcaca7271 100644 --- a/coldfront/core/project/management/commands/add_default_project_choices.py +++ b/coldfront/core/project/management/commands/add_default_project_choices.py @@ -21,7 +21,9 @@ def handle(self, *args, **options): for choice in ['User', 'General Manager', 'Storage Manager', 'Access Manager', 'PI']: ProjectUserRoleChoice.objects.get_or_create(name=choice) - for choice in ['Active', 'Pending - Add', 'Pending - Remove', 'Denied', 'Removed', ]: + for choice in [ + 'Active', 'Pending - Add', 'Pending - Remove', 'Denied', 'Removed', 'Deactivated' + ]: ProjectUserStatusChoice.objects.get_or_create(name=choice) for attribute_type in ('Date', 'Float', 'Int', 'Text', 'Yes/No'): From abd074dff9b7493b1a288cb5a61bb458c78a08d8 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Thu, 11 Dec 2025 12:33:40 -0800 Subject: [PATCH 04/20] change ldap membership update to only assign "Removed" status to users no longer part of the group --- coldfront/plugins/ldap/utils.py | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index 7912820009..a21d4e1295 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -345,9 +345,32 @@ def pi_is_active(self): """Return true if PI's account is both unexpired and not disabled.""" return user_valid(self.pi) - def compare_active_members_projectusers(self): + def compare_members_projectusers(self): """Compare ADGroup members to ProjectUsers. + Returns + ------- + members_only : list + users present in the members list and not as Coldfront ProjectUsers. + projuser_only : list + Coldfront ProjectUsers missing from the members list. + """ + ### check presence of ADGroup members in Coldfront ### + logger.debug('membership data collected for %s\nraw ADUser data for %s users', + self.name, len(self.members)) + ad_users = [u['sAMAccountName'][0] for u in self.members] + proj_usernames = [ + pu.user.username for pu in self.project.projectuser_set.filter( + (~Q(status__name='Removed'))) + ] + logger.debug('projectusernames: %s', proj_usernames) + + members_only, _, projuser_only = uniques_and_intersection(ad_users, proj_usernames) + return (members_only, projuser_only) + + def compare_active_members_projectusers(self): + """Compare ADGroup active members to ProjectUsers. + Returns ------- members_only : list @@ -521,7 +544,7 @@ def collect_update_project_status_membership(): active_pi_group_projs_statuschange.update(status=project_active_status) for group in active_pi_groups: - ad_users_not_added, remove_projuser_names = group.compare_active_members_projectusers() + ad_users_not_added, remove_projuser_names = group.compare_members_projectusers() # handle any AD users not in Coldfront if ad_users_not_added: @@ -575,7 +598,7 @@ def collect_update_project_status_membership(): ### identify inactive ProjectUsers, slate for status change ### remove_projusers = group.project.projectuser_set.filter( - user__username__in=remove_projuser_names) + user__username__in=remove_projuser_names).exclude(status__name='Removed') logger.debug("remove_projusers - projectusers slated for removal:\n %s", remove_projusers) projectusers_to_remove.extend(list(remove_projusers)) From 6cb493bee9dec511a8363673db061cf8fd23a2e1 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Thu, 11 Dec 2025 17:08:27 -0800 Subject: [PATCH 05/20] move ldap signals to separate file, add methods for user deactivation and reactivation --- coldfront/core/project/views.py | 7 +- coldfront/plugins/ldap/apps.py | 4 + coldfront/plugins/ldap/signals.py | 127 ++++++++++++++++++++++++ coldfront/plugins/ldap/utils.py | 157 ++++++++---------------------- 4 files changed, 173 insertions(+), 122 deletions(-) create mode 100644 coldfront/plugins/ldap/signals.py diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 1e0c3586c8..6275ef7c52 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -854,7 +854,8 @@ def get(self, request, *args, **kwargs): primaryuser_formset = formset_factory( ProjectRemoveUserForm, max_num=len(primary_group_users) ) - primaryuser_formset = primaryuser_formset(initial=primary_group_users, prefix='userform') + primaryuser_formset = primaryuser_formset( + initial=primary_group_users, prefix='userform') context['primary_group_users'] = primaryuser_formset else: context['primary_group_users'] = primary_group_users @@ -867,14 +868,12 @@ def post(self, request, *args, **kwargs): project_obj = get_object_or_404(Project, pk=pk) users_to_remove = self.get_users_to_remove(project_obj) - # if ldap is activated, prevent selection of users with project corresponding to primary group + # if ldap is activated, identify users with project corresponding to primary group signal_response = project_filter_users_to_remove.send( sender=self.__class__, users_to_remove=users_to_remove, project=project_obj ) if signal_response: users_to_remove = signal_response[0][1] - else: - users_to_remove = users_to_remove formset = formset_factory(ProjectRemoveUserForm, max_num=len(users_to_remove)) formset = formset(request.POST, initial=users_to_remove, prefix='userform') diff --git a/coldfront/plugins/ldap/apps.py b/coldfront/plugins/ldap/apps.py index b1bb53ece5..d0ca465405 100644 --- a/coldfront/plugins/ldap/apps.py +++ b/coldfront/plugins/ldap/apps.py @@ -3,3 +3,7 @@ class LdapConfig(AppConfig): name = 'coldfront.plugins.ldap' + + + def ready(self): + import coldfront.plugins.ldap.signals diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py new file mode 100644 index 0000000000..6c0504c7c6 --- /dev/null +++ b/coldfront/plugins/ldap/signals.py @@ -0,0 +1,127 @@ +import logging +from django.dispatch import receiver +from django.contrib.auth import get_user_model +from coldfront.core.field_of_science.models import FieldOfScience +from coldfront.core.project.signals import ( + project_filter_users_to_remove, + project_preremove_projectuser, + project_make_projectuser, + project_create, + project_post_create, +) +from coldfront.core.project.models import ( + ProjectUserRoleChoice, + ProjectUserStatusChoice, + ProjectUser, +) +from coldfront.plugins.ldap.utils import LDAPConn + +if 'coldfront.plugins.sftocf' in import_from_settings('INSTALLED_APPS', []): + from coldfront.plugins.sftocf.signals import ( + starfish_add_aduser, + starfish_remove_aduser, + starfish_add_adgroup, + ) + + +logger = logging.getLogger(__name__) + +@receiver(project_create) +def identify_ad_group(sender, **kwargs): + """Confirm that a project's name corresponds to an existing AD group""" + project_title = kwargs['project_title'] + try: + ad_conn = LDAPConn() + members, manager = ad_conn.return_group_members_manager(project_title) + except Exception as e: + logger.exception( + "error encountered retrieving members and manager for Project %s: %s", + project_title, e + ) + raise ValueError(f"ldap error: {e}") from e + + try: + ifx_pi = get_user_model().objects.get(username=manager['sAMAccountName'][0]) + except Exception as e: + raise ValueError(f"issue retrieving pi's ifxuser entry: {e}") from e + return ifx_pi + +@receiver(project_post_create) +def update_new_project(sender, **kwargs): + """Update the new project using the AD group information""" + project = kwargs['project_obj'] + try: + ad_conn = LDAPConn() + members, manager = ad_conn.return_group_members_manager(project.title) + except Exception as e: + raise ValueError(f"ldap connection error: {e}") + # locate field_of_science + if 'department' in manager.keys() and manager['department']: + field_of_science_name=manager['department'][0] + logger.debug('field_of_science_name %s', field_of_science_name) + field_of_science_obj, created = FieldOfScience.objects.get_or_create( + description=field_of_science_name, defaults={'is_selectable':'True'} + ) + if created: + logger.info('added new field_of_science: %s', field_of_science_name) + else: + raise ValueError(f'no department for AD group {project.title}, will not add unless fixed') + + project.field_of_science = field_of_science_obj + project.pi = get_user_model().objects.get(username=manager['sAMAccountName'][0]) + project.save() + for member in members: + role_name = "User" if member['sAMAccountName'][0] != manager['sAMAccountName'][0] else "PI" + try: + user_obj = get_user_model().objects.get(username=member['sAMAccountName'][0]) + except get_user_model().DoesNotExist: + logger.warning('User %s not found when trying to add to Project %s', + member['sAMAccountName'][0], project.title) + continue + ProjectUser.objects.create( + project=project, + user=user_obj, + role=ProjectUserRoleChoice.objects.get(name=role_name), + status=ProjectUserStatusChoice.objects.get(name='Active'), + ) + logger.info('added User %s to Project %s as %s', + user_obj.username, project.title, role_name, + extra={ 'category': 'database_change:ProjectUser', 'status': 'success' } + ) + +@receiver(project_filter_users_to_remove) +def filter_project_users_to_remove(sender, **kwargs): + users_to_remove = kwargs['users_to_remove'] + usernames = [u['username'] for u in users_to_remove] + ldap_conn = LDAPConn() + users_main_group = ldap_conn.users_in_primary_group(usernames, kwargs['project'].title) + users_to_remove = [ + u for u in users_to_remove if u['username'] not in users_main_group + ] + return users_to_remove + +@receiver(project_make_projectuser) +def add_user_to_group(sender, **kwargs): + ldap_conn = LDAPConn() + ldap_conn.add_user_to_group(kwargs['user_name'], kwargs['group_name']) + +@receiver(project_preremove_projectuser) +def remove_member_from_group(sender, **kwargs): + ldap_conn = LDAPConn() + ldap_conn.remove_member_from_group(kwargs['user_name'], kwargs['group_name']) + +if 'coldfront.plugins.sftocf' in import_from_settings('INSTALLED_APPS', []): + @receiver(starfish_add_aduser) + def starfish_add_user(sender, **kwargs): + ldap_conn = LDAPConn() + ldap_conn.add_user_to_group(kwargs['username'], 'starfish_users') + + @receiver(starfish_remove_aduser) + def starfish_remove_user(sender, **kwargs): + ldap_conn = LDAPConn() + ldap_conn.remove_member_from_group(kwargs['username'], 'starfish_users') + + @receiver(starfish_add_adgroup) + def starfish_add_group(sender, **kwargs): + ldap_conn = LDAPConn() + ldap_conn.add_group_to_group(kwargs['groupname'], 'starfish_users') diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index a21d4e1295..bbcf6e8878 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -4,22 +4,15 @@ import logging import operator from functools import reduce +from datetime import datetime from django.db.models import Q -from django.dispatch import receiver from django.utils import timezone from django.contrib.auth import get_user_model -from ldap3 import Connection, Server, ALL_ATTRIBUTES +from ldap3 import Connection, Server, ALL_ATTRIBUTES, MODIFY_REPLACE from ldap3.extend.microsoft.addMembersToGroups import ad_add_members_to_groups from ldap3.extend.microsoft.removeMembersFromGroups import ad_remove_members_from_groups -from coldfront.core.project.signals import ( - project_filter_users_to_remove, - project_preremove_projectuser, - project_make_projectuser, - project_create, - project_post_create, -) from coldfront.core.utils.common import ( import_from_settings, uniques_and_intersection ) @@ -37,17 +30,15 @@ ProjectUser, ) -if 'coldfront.plugins.sftocf' in import_from_settings('INSTALLED_APPS', []): - from coldfront.plugins.sftocf.signals import ( - starfish_add_aduser, - starfish_remove_aduser, - starfish_add_adgroup, - ) logger = logging.getLogger(__name__) username_ignore_list = import_from_settings('username_ignore_list', []) +def now_filetime(): + FILETIME_EPOCH = datetime(1601, 1, 1) + return int((datetime.utcnow() - FILETIME_EPOCH).total_seconds() * 10**7) + class LDAPException(Exception): """The base exception class for all LDAPExceptions""" @@ -228,15 +219,20 @@ def remove_member_from_group(self, user_name, group_name): group = self.return_group_by_name(group_name) # get user user = self.return_user_by_name(user_name) - if user['gidNumber'] == group['gidNumber']: - raise ValueError( - "Group is member's primary group. Please contact FASRC support to remove this member from your group.") member_dn = user['distinguishedName'] group_dn = group['distinguishedName'] member_name = user['sAMAccountName'] group_name = group['sAMAccountName'] member_sid = user['objectSid'] group_sid = group['objectSid'] + if user['gidNumber'] == group['gidNumber']: + logger.warning("Deactivating member %s (sid %s) from primary group %s (sid %s)", + member_name, member_sid, group_name, group_sid, + extra={ 'category': 'integration:AD' } + ) + result = self.deactivate_user(user['distinguishedName']) + return result + try: result = ad_remove_members_from_groups( self.conn, [member_dn], group_dn, fix=True, raise_error=True @@ -253,6 +249,31 @@ def remove_member_from_group(self, user_name, group_name): ) return result + def deactivate_user(self, username): + user = self.return_user_by_name(username) + user_dn = user['distinguishedName'] + filetime_now = now_filetime() + result = self.conn.modify( + user_dn, + changes={ + 'accountExpires': [(MODIFY_REPLACE, [str(filetime_now)])], + 'userAccountControl': [(MODIFY_REPLACE, ['514'])] + } + ) + return result + + def reactivate_user(self, username): + user = self.return_user_by_name(username) + user_dn = user['distinguishedName'] + result = self.conn.modify( + user_dn, + changes={ + 'accountExpires': [(MODIFY_REPLACE, ['9223372036854775807'])], + 'userAccountControl': [(MODIFY_REPLACE, ['512'])] + } + ) + return result + def users_in_primary_group(self, usernames, groupname): """ Return list of usernames representing users that are members of the @@ -773,103 +794,3 @@ def add_new_projects(groupusercollections, errortracker): for errortype in errortracker: logger.warning('AD groups with %s: %s', errortype, errortracker[errortype]) return added_projects, errortracker - -@receiver(project_create) -def identify_ad_group(sender, **kwargs): - """Confirm that a project's name corresponds to an existing AD group""" - project_title = kwargs['project_title'] - try: - ad_conn = LDAPConn() - members, manager = ad_conn.return_group_members_manager(project_title) - except Exception as e: - logger.exception( - "error encountered retrieving members and manager for Project %s: %s", - project_title, e - ) - raise ValueError(f"ldap error: {e}") from e - - try: - ifx_pi = get_user_model().objects.get(username=manager['sAMAccountName'][0]) - except Exception as e: - raise ValueError(f"issue retrieving pi's ifxuser entry: {e}") from e - return ifx_pi - -@receiver(project_post_create) -def update_new_project(sender, **kwargs): - """Update the new project using the AD group information""" - project = kwargs['project_obj'] - try: - ad_conn = LDAPConn() - members, manager = ad_conn.return_group_members_manager(project.title) - except Exception as e: - raise ValueError(f"ldap connection error: {e}") - # locate field_of_science - if 'department' in manager.keys() and manager['department']: - field_of_science_name=manager['department'][0] - logger.debug('field_of_science_name %s', field_of_science_name) - field_of_science_obj, created = FieldOfScience.objects.get_or_create( - description=field_of_science_name, defaults={'is_selectable':'True'} - ) - if created: - logger.info('added new field_of_science: %s', field_of_science_name) - else: - raise ValueError(f'no department for AD group {project.title}, will not add unless fixed') - - project.field_of_science = field_of_science_obj - project.pi = get_user_model().objects.get(username=manager['sAMAccountName'][0]) - project.save() - for member in members: - role_name = "User" if member['sAMAccountName'][0] != manager['sAMAccountName'][0] else "PI" - try: - user_obj = get_user_model().objects.get(username=member['sAMAccountName'][0]) - except get_user_model().DoesNotExist: - logger.warning('User %s not found when trying to add to Project %s', - member['sAMAccountName'][0], project.title) - continue - ProjectUser.objects.create( - project=project, - user=user_obj, - role=ProjectUserRoleChoice.objects.get(name=role_name), - status=ProjectUserStatusChoice.objects.get(name='Active'), - ) - logger.info('added User %s to Project %s as %s', - user_obj.username, project.title, role_name, - extra={ 'category': 'database_change:ProjectUser', 'status': 'success' } - ) - -@receiver(project_filter_users_to_remove) -def filter_project_users_to_remove(sender, **kwargs): - users_to_remove = kwargs['users_to_remove'] - usernames = [u['username'] for u in users_to_remove] - ldap_conn = LDAPConn() - users_main_group = ldap_conn.users_in_primary_group(usernames, kwargs['project'].title) - users_to_remove = [ - u for u in users_to_remove if u['username'] not in users_main_group - ] - return users_to_remove - -@receiver(project_make_projectuser) -def add_user_to_group(sender, **kwargs): - ldap_conn = LDAPConn() - ldap_conn.add_user_to_group(kwargs['user_name'], kwargs['group_name']) - -@receiver(project_preremove_projectuser) -def remove_member_from_group(sender, **kwargs): - ldap_conn = LDAPConn() - ldap_conn.remove_member_from_group(kwargs['user_name'], kwargs['group_name']) - -if 'coldfront.plugins.sftocf' in import_from_settings('INSTALLED_APPS', []): - @receiver(starfish_add_aduser) - def starfish_add_user(sender, **kwargs): - ldap_conn = LDAPConn() - ldap_conn.add_user_to_group(kwargs['username'], 'starfish_users') - - @receiver(starfish_remove_aduser) - def starfish_remove_user(sender, **kwargs): - ldap_conn = LDAPConn() - ldap_conn.remove_member_from_group(kwargs['username'], 'starfish_users') - - @receiver(starfish_add_adgroup) - def starfish_add_group(sender, **kwargs): - ldap_conn = LDAPConn() - ldap_conn.add_group_to_group(kwargs['groupname'], 'starfish_users') From 2a0f9ac273d4254ff74cec3dda0e88f37b5c3815 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 17 Dec 2025 15:50:19 -0800 Subject: [PATCH 06/20] add import_from_settings to ldap signals.py --- .../core/project/templates/project/project_remove_users.html | 4 ++-- coldfront/core/project/views.py | 3 ++- coldfront/plugins/ldap/management/commands/add_projects.py | 2 +- coldfront/plugins/ldap/signals.py | 1 + coldfront/plugins/ldap/utils.py | 1 - 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/coldfront/core/project/templates/project/project_remove_users.html b/coldfront/core/project/templates/project/project_remove_users.html index daebebffda..8b86a851fe 100644 --- a/coldfront/core/project/templates/project/project_remove_users.html +++ b/coldfront/core/project/templates/project/project_remove_users.html @@ -19,8 +19,8 @@

Remove users from project: {{project.title}}

{% if request.user.is_superuser %} As a superuser, you can remove any user from the project. Please be aware that - removing a user that has your lab as their primary group may cause issues with - that user's access to other resources. If you are unsure, please + removing a user from their primary group will cause their account to be deactivated and + prevent access to computational resources. If you are unsure, please contact FASRC support or the ColdFront system administrator. {% else %} To be removed from a lab, the user must not have the lab as their primary diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 6275ef7c52..f32ca095c0 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -244,7 +244,8 @@ def get_context_data(self, **kwargs): allocations = allocations.filter( status__name__in=['Active', 'Paid', 'Ready for Review','Payment Requested'] ).distinct().order_by('-end_date') - storage_allocations = allocations.filter(resources__resource_type__name='Storage').order_by('resources__name') + storage_allocations = allocations.filter( + resources__resource_type__name='Storage').order_by('resources__name') compute_allocations = allocations.filter(resources__resource_type__name='Cluster') allocation_total = {'allocation_user_count': 0, 'size': 0, 'cost': 0, 'usage':0} for allocation in storage_allocations: diff --git a/coldfront/plugins/ldap/management/commands/add_projects.py b/coldfront/plugins/ldap/management/commands/add_projects.py index c546f02c4e..5ef9b11be9 100644 --- a/coldfront/plugins/ldap/management/commands/add_projects.py +++ b/coldfront/plugins/ldap/management/commands/add_projects.py @@ -2,7 +2,7 @@ from datetime import datetime import pandas as pd -from django.core.management.base import BaseCommand, CommandError +from django.core.management.base import BaseCommand from coldfront.plugins.ldap.utils import import_projects_projectusers diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 6c0504c7c6..28a6e5b816 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -1,6 +1,7 @@ import logging from django.dispatch import receiver from django.contrib.auth import get_user_model +from coldfront.core.utils.common import import_from_settings from coldfront.core.field_of_science.models import FieldOfScience from coldfront.core.project.signals import ( project_filter_users_to_remove, diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index bbcf6e8878..566fb2cced 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -232,7 +232,6 @@ def remove_member_from_group(self, user_name, group_name): ) result = self.deactivate_user(user['distinguishedName']) return result - try: result = ad_remove_members_from_groups( self.conn, [member_dn], group_dn, fix=True, raise_error=True From 2d922ffae28fb7ff0634aef958aabd3beab8f06c Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 17 Dec 2025 16:51:29 -0800 Subject: [PATCH 07/20] reconfigure user removal form creation and templating --- coldfront/core/project/forms.py | 11 ++- .../project/project_remove_users.html | 69 ++++++++++--------- coldfront/core/project/views.py | 13 +--- coldfront/plugins/ldap/signals.py | 6 +- 4 files changed, 48 insertions(+), 51 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index 3c2c88edd1..fdf0c5168f 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -56,8 +56,14 @@ def __init__(self, request_user, project_pk, *args, **kwargs): allocation_query_set = project_obj.allocation_set.filter( resources__is_allocatable=True, is_locked=False, status__name__in=PENDING_ACTIVE_ALLOCATION_STATUSES).distinct() - allocation_choices = [(allocation.id, "%s (%s) %s" % (allocation.get_parent_resource.name, allocation.get_parent_resource.resource_type.name, - allocation.description if allocation.description else '')) for allocation in allocation_query_set] + allocation_choices = [( + allocation.id, "%s (%s) %s" % ( + allocation.get_parent_resource.name, + allocation.get_parent_resource.resource_type.name, + allocation.description if allocation.description else '' + ) + ) for allocation in allocation_query_set] + allocation_choices_sorted = [] allocation_choices_sorted = sorted(allocation_choices, key=lambda x: x[1][0].lower()) allocation_choices.insert(0, ('__select_all__', 'Select All')) @@ -74,6 +80,7 @@ class ProjectRemoveUserForm(forms.Form): last_name = forms.CharField(max_length=150, required=False, disabled=True) email = forms.EmailField(max_length=100, required=False, disabled=True) role = forms.CharField(max_length=30, disabled=True) + primary_group = forms.BooleanField(required=False, disabled=True) selected = forms.BooleanField(initial=False, required=False) diff --git a/coldfront/core/project/templates/project/project_remove_users.html b/coldfront/core/project/templates/project/project_remove_users.html index 8b86a851fe..3917d87710 100644 --- a/coldfront/core/project/templates/project/project_remove_users.html +++ b/coldfront/core/project/templates/project/project_remove_users.html @@ -48,43 +48,44 @@

Remove users from project: {{project.title}}

- {% for user in primary_group_users %} - {% if request.user.is_superuser %} - - {{ user.selected }} - {{ forloop.counter }} - - {{ user.username.value }} - {{ user.first_name.value }} - {{ user.last_name.value }} - {{ user.email.value }} - {{ user.role.value }} - + {% for form in formset %} + {% if form.primary_group.value %} + {% if request.user.is_superuser %} + + {{ form.selected }} + {{ forloop.counter }} + + {{ form.username.value }} + {{ form.first_name.value }} + {{ form.last_name.value }} + {{ form.email.value }} + {{ form.role.value }} + + {% else %} + + + {{ forloop.counter }} + + {{ form.username }} + {{ form.first_name }} + {{ form.last_name }} + {{ form.email }} + {{ form.role }} + + {% endif %} {% else %} - - - - - {{ user.username }} - {{ user.first_name }} - {{ user.last_name }} - {{ user.email }} - {{ user.role }} - + + {{ form.selected }} + {{ forloop.counter }} + + {{ form.username.value }} + {{ form.first_name.value }} + {{ form.last_name.value }} + {{ form.email.value }} + {{ form.role.value }} + {% endif %} {% endfor %} - {% for form in formset %} - - {{ form.selected }} - {{ forloop.counter }} - - {{ form.username.value }} - {{ form.first_name.value }} - {{ form.last_name.value }} - {{ form.email.value }} - {{ form.role.value }} - - {% endfor %}
diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index f32ca095c0..5eb214c66f 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -832,7 +832,7 @@ def get(self, request, *args, **kwargs): project_obj = get_object_or_404(Project, pk=pk) users_list = self.get_users_to_remove(project_obj) - # if ldap is activated, prevent selection of users with project corresponding to primary group + # if ldap activated, prevent selection of users with project corresponding to primary group signal_response = project_filter_users_to_remove.send( sender=self.__class__, users_to_remove=users_list, project=project_obj ) @@ -841,8 +841,6 @@ def get(self, request, *args, **kwargs): else: users_to_remove = users_list - primary_group_users = [u for u in users_list if u not in users_to_remove] - context = {} if users_to_remove: @@ -851,15 +849,6 @@ def get(self, request, *args, **kwargs): ) formset = formset(initial=users_to_remove, prefix='userform') context['formset'] = formset - if self.request.user.is_superuser: - primaryuser_formset = formset_factory( - ProjectRemoveUserForm, max_num=len(primary_group_users) - ) - primaryuser_formset = primaryuser_formset( - initial=primary_group_users, prefix='userform') - context['primary_group_users'] = primaryuser_formset - else: - context['primary_group_users'] = primary_group_users context['project'] = get_object_or_404(Project, pk=pk) context['EMAIL_TICKET_SYSTEM_ADDRESS'] = EMAIL_TICKET_SYSTEM_ADDRESS return render(request, self.template_name, context) diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 28a6e5b816..a52e334fef 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -96,9 +96,9 @@ def filter_project_users_to_remove(sender, **kwargs): usernames = [u['username'] for u in users_to_remove] ldap_conn = LDAPConn() users_main_group = ldap_conn.users_in_primary_group(usernames, kwargs['project'].title) - users_to_remove = [ - u for u in users_to_remove if u['username'] not in users_main_group - ] + for user in users_to_remove: + user['primary_group'] = user['username'] in users_main_group + users_to_remove = sorted(users_to_remove, key=lambda x: x['primary_group'], reverse=True) return users_to_remove @receiver(project_make_projectuser) From 608c8d91b5d81e3886160fab6bad5db910be3dab Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 17 Dec 2025 17:37:17 -0800 Subject: [PATCH 08/20] add AD user deactivation on signal post --- coldfront/core/project/views.py | 39 ++++++++++++++++++++++++++----- coldfront/plugins/ldap/signals.py | 5 +++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 5eb214c66f..f13aa1712e 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -876,12 +876,33 @@ def post(self, request, *args, **kwargs): messages.error(request, error) return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': pk})) projectuser_status_removed = ProjectUserStatusChoice.objects.get(name='Removed') + projectuser_status_deactivated = ProjectUserStatusChoice.objects.get(name='Deactivated') allocationuser_status_removed = AllocationUserStatusChoice.objects.get(name='Removed') for form in formset: user_form_data = form.cleaned_data + if user_form_data['selected']: user_obj = get_user_model().objects.get(username=user_form_data.get('username')) + + if user_form_data['primary_group'] and not request.user.is_superuser: + failed_user_removals += [ + f"{project_obj.title} is user {user_obj.username}'s primary group" + ] + logger.warning( + "non-admin attempted removal of primary group user. request_user=%s,member=%s,project=%s", + request.user, user_form_data['username'], project_obj.title, + extra={'category': 'integration:AD', 'status': 'failure'} + ) + continue if project_obj.pi == user_obj: + failed_user_removals += [ + f"{user_obj.username} is the PI of {project_obj.title}" + ] + logger.warning( + "attempted PI removal via ProjectUserRemovalForm. request_user=%s,member=%s,project=%s", + request.user, user_form_data['username'], project_obj.title, + extra={'category': 'integration:AD', 'status': 'failure'} + ) continue project_user_obj = project_obj.projectuser_set.get(user=user_obj) @@ -892,27 +913,33 @@ def post(self, request, *args, **kwargs): user_name=user_obj.username, group_name=project_obj.title ) logger.info( - "ColdFront user %s removed AD User for %s from AD Group for %s", - self.request.user, user_obj.username, project_obj.title, + "AD Group member removed/deactivated. request_user=%s,member=%s,group=%s,primary_group=%s", + self.request.user, user_obj.username, project_obj.title, user_form_data['primary_group'], extra={'category': 'integration:AD', 'status': 'success'} ) except Exception as e: failed_user_removals += [f"could not remove user {user_obj}: {e}"] logger.exception( - "Coldfront user %s could NOT remove AD User for %s from AD Group for %s: %s", + "Failed AD Group member removal. request_user=%s,member=%s,group=%s,primary_group=%s,error=%s", self.request.user, user_obj.username, project_obj.title, + user_form_data['primary_group'], e, extra={'category': 'integration:AD', 'status': 'failure'} ) continue - project_user_obj.status = projectuser_status_removed + if user_form_data['primary_group']: + project_user_obj.status = projectuser_status_deactivated + action = 'deactivated' + else: + project_user_obj.status = projectuser_status_removed + action = 'removed' project_user_obj.save() logger.info( - "User %s removed from project %s by %s", - user_obj.username, project_obj.title, request.user, + "User %s %s from project %s by %s", + user_obj.username, action, project_obj.title, request.user, extra={'category': 'database_change:ProjectUser', 'status': 'success'} ) diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index a52e334fef..9d728d794c 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -109,7 +109,10 @@ def add_user_to_group(sender, **kwargs): @receiver(project_preremove_projectuser) def remove_member_from_group(sender, **kwargs): ldap_conn = LDAPConn() - ldap_conn.remove_member_from_group(kwargs['user_name'], kwargs['group_name']) + if kwargs.get('primary_group', False): + ldap_conn.remove_member_from_group(kwargs['user_name'], kwargs['group_name']) + else: + ldap_conn.deactivate_user(kwargs['user_name']) if 'coldfront.plugins.sftocf' in import_from_settings('INSTALLED_APPS', []): @receiver(starfish_add_aduser) From 594247f35746346af9ebab63e1f0c913be0fada8 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Thu, 18 Dec 2025 11:08:11 -0800 Subject: [PATCH 09/20] remove newly disabled users from secondary projects and their allocations, add interface to view disabled users in projectuser addition page --- .../templates/project/project_add_users.html | 40 +++++++++++++++++++ coldfront/core/project/views.py | 38 +++++++++++++++--- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/coldfront/core/project/templates/project/project_add_users.html b/coldfront/core/project/templates/project/project_add_users.html index dbc3da18cd..770c0f0312 100644 --- a/coldfront/core/project/templates/project/project_add_users.html +++ b/coldfront/core/project/templates/project/project_add_users.html @@ -44,6 +44,46 @@

Add users to project: {{project.title}}

+ +{% if disabled_users %} +
+

+ The following users, which had this lab's AD Group as their primary group, are currently disabled. + You can reenable them by selecting them and clicking "Reactivate Users" below. +

+
+
+ + + + + + + + + + + + + {% for form in formset %} + + + + + + + + + {% endfor %} + +
UsernameFirst NameLast NameEmailRole
{{ form.selected }}{{ form.username.value }}{{ form.first_name.value }}{{ form.last_name.value }}{{ form.email.value }}{{ form.role.value }}
+ + +
+
+{% endif %} + + {% endblock %} diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index f13aa1712e..bd91f768e4 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -46,6 +46,7 @@ ProjectUserUpdateForm, ProjectReviewEmailForm, ProjectAttributeAddForm, + ProjectReactivateUserForm, ProjectAttributeDeleteForm, ProjectAttributeUpdateForm, ProjectAddUsersToAllocationForm, @@ -554,7 +555,18 @@ def dispatch(self, request, *args, **kwargs): def get_context_data(self, *args, **kwargs): context = super().get_context_data(*args, **kwargs) context['user_search_form'] = UserSearchForm() - context['project'] = Project.objects.get(pk=self.kwargs.get('pk')) + project = Project.objects.get(pk=self.kwargs.get('pk')) + context['project'] = project + disabled_users = project.projectuser_set.filter(status__name='Disabled') + if disabled_users.exists(): + + formset = formset_factory( + ProjectReactivateUserForm, max_num=len(disabled_users) + ) + disabled_formset = formset(initial=disabled_users, prefix='userform') + context['disabled_users'] = disabled_formset + else: + context['disabled_users'] = None return context @@ -933,9 +945,28 @@ def post(self, request, *args, **kwargs): if user_form_data['primary_group']: project_user_obj.status = projectuser_status_deactivated action = 'deactivated' + # change status to "removed" for all other projectusers with this user + ProjectUser.objects.filter( + user=user_obj, + status__name='Active', + project__status__name__in=['Active', 'New'], + ).exclude(project=project_obj).update(status=projectuser_status_removed) + # get allocations to remove user from across all active/new projects + allocations_to_remove_user_from = Allocation.objects.filter( + allocationuser__user=user_obj, + project__projectuser__status__name='Active', + project__projectuser__user=user_obj, + status__name__in=['Active', 'Renewal Requested'], + resources__resource_type__name='Storage' + ).distinct() else: project_user_obj.status = projectuser_status_removed action = 'removed' + # get allocation to remove users from + allocations_to_remove_user_from = project_obj.allocation_set.filter( + status__name__in=['Active', 'Renewal Requested'], + resources__resource_type__name='Storage' + ) project_user_obj.save() logger.info( "User %s %s from project %s by %s", @@ -943,11 +974,6 @@ def post(self, request, *args, **kwargs): extra={'category': 'database_change:ProjectUser', 'status': 'success'} ) - # get allocation to remove users from - allocations_to_remove_user_from = project_obj.allocation_set.filter( - status__name__in=['Active', 'New', 'Renewal Requested'], - resources__resource_type__name='Storage' - ) for allocation in allocations_to_remove_user_from: for alloc_user in allocation.allocationuser_set.filter( user=user_obj, status__name='Active' From 63f54afa8b40cf18fe907df8e567681375241881 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Thu, 18 Dec 2025 11:20:21 -0800 Subject: [PATCH 10/20] add projectreactivateuserform --- coldfront/core/project/forms.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index fdf0c5168f..b309c58c8e 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -46,6 +46,16 @@ class ProjectAddUserForm(forms.Form): selected = forms.BooleanField(initial=False, required=False) +class ProjectReactivateUserForm(forms.Form): + username = forms.CharField(max_length=150, disabled=True) + first_name = forms.CharField(max_length=150, required=False, disabled=True) + last_name = forms.CharField(max_length=150, required=False, disabled=True) + email = forms.EmailField(max_length=100, required=False, disabled=True) + role = forms.ModelChoiceField( + queryset=ProjectUserRoleChoice.objects.all(), empty_label=None) + selected = forms.BooleanField(initial=False, required=False) + + class ProjectAddUsersToAllocationForm(forms.Form): allocation = forms.MultipleChoiceField( widget=forms.CheckboxSelectMultiple(attrs={'checked': 'checked'}), required=False) @@ -55,7 +65,9 @@ def __init__(self, request_user, project_pk, *args, **kwargs): project_obj = get_object_or_404(Project, pk=project_pk) allocation_query_set = project_obj.allocation_set.filter( - resources__is_allocatable=True, is_locked=False, status__name__in=PENDING_ACTIVE_ALLOCATION_STATUSES).distinct() + resources__is_allocatable=True, is_locked=False, + status__name__in=PENDING_ACTIVE_ALLOCATION_STATUSES + ).distinct() allocation_choices = [( allocation.id, "%s (%s) %s" % ( allocation.get_parent_resource.name, From c1de83ec2c8787802804a50ec5196d7690e51e26 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 22 Dec 2025 11:22:59 -0800 Subject: [PATCH 11/20] improve projectadduserssearchview --- .../templates/project/project_add_users.html | 53 ++++++++++--------- coldfront/core/project/views.py | 20 ++++--- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/coldfront/core/project/templates/project/project_add_users.html b/coldfront/core/project/templates/project/project_add_users.html index 770c0f0312..4a91ad44ee 100644 --- a/coldfront/core/project/templates/project/project_add_users.html +++ b/coldfront/core/project/templates/project/project_add_users.html @@ -53,32 +53,37 @@

Add users to project: {{project.title}}

- - + + {% csrf_token %} +
+ + + + + + + + + + + + {% for form in deactivated_formset %} - - - - - - + + + + + + - - - {% for form in formset %} - - - - - - - - - {% endfor %} - -
UsernameFirst NameLast NameEmailRole
UsernameFirst NameLast NameEmailRole{{ form.selected }}{{ form.username.value }}{{ form.first_name.value }}{{ form.last_name.value }}{{ form.email.value }}{{ form.role.value }}
{{ form.selected }}{{ form.username.value }}{{ form.first_name.value }}{{ form.last_name.value }}{{ form.email.value }}{{ form.role.value }}
- - + {% endfor %} + + + {{ formset.management_form }} + +
{% endif %} diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index bd91f768e4..2f6b895797 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -557,16 +557,22 @@ def get_context_data(self, *args, **kwargs): context['user_search_form'] = UserSearchForm() project = Project.objects.get(pk=self.kwargs.get('pk')) context['project'] = project - disabled_users = project.projectuser_set.filter(status__name='Disabled') - if disabled_users.exists(): - + deactivated_users = project.projectuser_set.filter(status__name='Deactivated') + if deactivated_users.exists(): formset = formset_factory( - ProjectReactivateUserForm, max_num=len(disabled_users) + ProjectReactivateUserForm, max_num=len(deactivated_users) ) - disabled_formset = formset(initial=disabled_users, prefix='userform') - context['disabled_users'] = disabled_formset + deactivated_forms = [{ + 'username': u.user.username, + 'first_name': u.user.first_name, + 'last_name': u.user.last_name, + 'email': u.user.email, + 'role': u.role, + } for u in deactivated_users] + deactivated_formset = formset(initial=deactivated_forms, prefix='userform') + context['deactivated_formset'] = deactivated_formset else: - context['disabled_users'] = None + context['deactivated_formset'] = None return context From 385c264190bbbead34bdf1743f61a6c370588076 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 22 Dec 2025 16:18:52 -0800 Subject: [PATCH 12/20] add user reactivation routine --- .../templates/project/project_add_users.html | 4 +- coldfront/core/project/views.py | 74 ++++++++++++++++--- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/coldfront/core/project/templates/project/project_add_users.html b/coldfront/core/project/templates/project/project_add_users.html index 4a91ad44ee..33fecfea91 100644 --- a/coldfront/core/project/templates/project/project_add_users.html +++ b/coldfront/core/project/templates/project/project_add_users.html @@ -45,14 +45,14 @@

Add users to project: {{project.title}}

-{% if disabled_users %} +{% if deactivated_formset %}

The following users, which had this lab's AD Group as their primary group, are currently disabled. You can reenable them by selecting them and clicking "Reactivate Users" below.

-
+
{% csrf_token %} diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 2f6b895797..20c65d7c38 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -672,12 +672,67 @@ def dispatch(self, request, *args, **kwargs): return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': pk})) return super().dispatch(request, *args, **kwargs) + def reactivate_user(self, project_obj, user_form_data): + """Full organizational logic for user reactivation + - reactivate projectuser entry + x reactivate any other projectusers in active projects that still have this user in AD + (taken care of by ldap sync) + - reactivate any related allocationuser entries in active storage allocations + """ + user_username = user_form_data.get('username') + user_obj = get_user_model().objects.get(username=user_username) + project_obj.projectuser_set.filter( + user=user_obj, + status__name='Deactivated' + ).update(status=ProjectUserStatusChoice.objects.get(name='Active')) + AllocationUser.objects.filter( + user=user_obj, + allocation__project__status__name__in=['Active', 'Renewal Requested'], + allocation__status__name__in=['Active','Pending Deactivation'], + resources__resource_type__name='Storage' + ).distinct().update( + status=AllocationUserStatusChoice.objects.get(name='Active') + ) + def post(self, request, *args, **kwargs): - user_search_string = request.POST.get('q') - search_by = request.POST.get('search_by') pk = self.kwargs.get('pk') - project_obj = get_object_or_404(Project, pk=pk) + deactivated_users = project_obj.projectuser_set.filter(status__name='Deactivated') + projuserstatus_active = ProjectUserStatusChoice.objects.get(name='Active') + if deactivated_users.exists(): + formset = formset_factory( + ProjectReactivateUserForm, max_num=len(deactivated_users) + ) + deactivated_forms = [{ + 'username': u.user.username, + 'first_name': u.user.first_name, + 'last_name': u.user.last_name, + 'email': u.user.email, + 'role': u.role, + } for u in deactivated_users] + deactivated_formset = formset( + request.POST, initial=deactivated_forms, prefix='userform') + # if any have been selected for reactivation, reactivate them + if deactivated_formset.is_valid() and any( + form.cleaned_data.get('selected') for form in deactivated_formset + ): + reactivated_users_count = 0 + for form in deactivated_formset: + user_form_data = form.cleaned_data + if user_form_data['selected']: + self.reactivate_user(project_obj, user_form_data) + reactivated_users_count += 1 + if reactivated_users_count: + messages.success( + request, + f'Reactivated {reactivated_users_count} users in project.' + ) + # otherwise, proceed to normal user addition flow + + + + user_search_string = request.POST.get('q') + search_by = request.POST.get('search_by') users_to_exclude = [ u.user.username @@ -703,7 +758,6 @@ def post(self, request, *args, **kwargs): added_users_count = 0 if formset.is_valid() and allocation_form.is_valid(): - projuserstatus_active = ProjectUserStatusChoice.objects.get(name='Active') allocuser_status_active = AllocationUserStatusChoice.objects.get( name='Active' ) @@ -951,12 +1005,6 @@ def post(self, request, *args, **kwargs): if user_form_data['primary_group']: project_user_obj.status = projectuser_status_deactivated action = 'deactivated' - # change status to "removed" for all other projectusers with this user - ProjectUser.objects.filter( - user=user_obj, - status__name='Active', - project__status__name__in=['Active', 'New'], - ).exclude(project=project_obj).update(status=projectuser_status_removed) # get allocations to remove user from across all active/new projects allocations_to_remove_user_from = Allocation.objects.filter( allocationuser__user=user_obj, @@ -965,6 +1013,12 @@ def post(self, request, *args, **kwargs): status__name__in=['Active', 'Renewal Requested'], resources__resource_type__name='Storage' ).distinct() + # change status to "removed" for all other projectusers with this user + ProjectUser.objects.filter( + user=user_obj, + status__name='Active', + project__status__name__in=['Active', 'New'], + ).exclude(project=project_obj).update(status=projectuser_status_removed) else: project_user_obj.status = projectuser_status_removed action = 'removed' From 26377d47a05bbb3d6e1759d3c10a2f6a05503b15 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 22 Dec 2025 17:03:40 -0800 Subject: [PATCH 13/20] change deactivated projectuser allocation reactivation --- coldfront/core/project/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 20c65d7c38..a2dbc00983 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -689,6 +689,8 @@ def reactivate_user(self, project_obj, user_form_data): user=user_obj, allocation__project__status__name__in=['Active', 'Renewal Requested'], allocation__status__name__in=['Active','Pending Deactivation'], + allocation__project__projectuser__user=user_obj, + allocation__project__projectuser__status__name='Active', resources__resource_type__name='Storage' ).distinct().update( status=AllocationUserStatusChoice.objects.get(name='Active') From 5a2b314d6470f4535da4ef3cdc029ac612e358ff Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 22 Dec 2025 19:41:36 -0800 Subject: [PATCH 14/20] add reactivate_user ldap signal --- coldfront/core/project/signals.py | 6 ++++++ coldfront/core/project/views.py | 30 +++++++++++++++++++----------- coldfront/plugins/ldap/signals.py | 24 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/coldfront/core/project/signals.py b/coldfront/core/project/signals.py index c7982c2720..08a3840972 100644 --- a/coldfront/core/project/signals.py +++ b/coldfront/core/project/signals.py @@ -1,5 +1,9 @@ +import logging import django.dispatch +log = logging.getLogger(__name__) + + project_create = django.dispatch.Signal() #providing_args=["project_title"] project_post_create = django.dispatch.Signal() @@ -11,6 +15,8 @@ project_preremove_projectuser = django.dispatch.Signal() #providing_args=["user_name", "group_name"] +project_reactivate_projectuser = django.dispatch.Signal() + project_filter_users_to_remove = django.dispatch.Signal() #providing_args=["project_user_list"] # return tuple of (no_removal, can_remove) diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index a2dbc00983..d1df48e41e 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -33,6 +33,7 @@ from coldfront.core.project.signals import ( project_filter_users_to_remove, project_preremove_projectuser, + project_reactivate_projectuser, project_make_projectuser, project_create, project_post_create @@ -676,7 +677,7 @@ def reactivate_user(self, project_obj, user_form_data): """Full organizational logic for user reactivation - reactivate projectuser entry x reactivate any other projectusers in active projects that still have this user in AD - (taken care of by ldap sync) + (taken care of by ldap signal) - reactivate any related allocationuser entries in active storage allocations """ user_username = user_form_data.get('username') @@ -685,6 +686,7 @@ def reactivate_user(self, project_obj, user_form_data): user=user_obj, status__name='Deactivated' ).update(status=ProjectUserStatusChoice.objects.get(name='Active')) + project_reactivate_projectuser.send(sender=self.__class__, user=user_obj) AllocationUser.objects.filter( user=user_obj, allocation__project__status__name__in=['Active', 'Renewal Requested'], @@ -1007,20 +1009,25 @@ def post(self, request, *args, **kwargs): if user_form_data['primary_group']: project_user_obj.status = projectuser_status_deactivated action = 'deactivated' - # get allocations to remove user from across all active/new projects + # change status to "removed" for all other projectusers with this user + secondary_projectusers = ProjectUser.objects.filter( + user=user_obj, + status__name='Active', + project__status__name__in=['Active', 'New'], + ).exclude(project=project_obj) + secondary_projectusers.update(status=projectuser_status_removed) + # get allocations to remove user from in projects where they have been removed allocations_to_remove_user_from = Allocation.objects.filter( allocationuser__user=user_obj, - project__projectuser__status__name='Active', + project__projectuser__status__name='Removed', project__projectuser__user=user_obj, status__name__in=['Active', 'Renewal Requested'], resources__resource_type__name='Storage' ).distinct() - # change status to "removed" for all other projectusers with this user - ProjectUser.objects.filter( - user=user_obj, - status__name='Active', - project__status__name__in=['Active', 'New'], - ).exclude(project=project_obj).update(status=projectuser_status_removed) + secondary_projects = ','.join( + [sp.project.title for sp in secondary_projectusers] + ) + extra_log = f',secondary_projects="{secondary_projects}"' else: project_user_obj.status = projectuser_status_removed action = 'removed' @@ -1029,10 +1036,11 @@ def post(self, request, *args, **kwargs): status__name__in=['Active', 'Renewal Requested'], resources__resource_type__name='Storage' ) + extra_log = '' project_user_obj.save() logger.info( - "User %s %s from project %s by %s", - user_obj.username, action, project_obj.title, request.user, + "User %s from project. requesting_user=%s,project_user=%s,project=%s%s", + action, request.user, user_obj.username, project_obj.title, extra_log, extra={'category': 'database_change:ProjectUser', 'status': 'success'} ) diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 9d728d794c..3d3f4a8436 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -9,6 +9,7 @@ project_make_projectuser, project_create, project_post_create, + project_reactivate_projectuser, ) from coldfront.core.project.models import ( ProjectUserRoleChoice, @@ -47,6 +48,29 @@ def identify_ad_group(sender, **kwargs): raise ValueError(f"issue retrieving pi's ifxuser entry: {e}") from e return ifx_pi +@receiver(project_reactivate_projectuser) +def reactivate_user(user): + """Reactivate a user in LDAP""" + ldap_conn = LDAPConn() + ldap_conn.reactivate_user(user.username) + ldap_user = ldap_conn.return_user_by_name(user.username) + user_group_names = [group.split(',')[0].replace('CN=', '') for group in ldap_user['memberOf']] + projectuser_entries = ProjectUser.objects.filter( + user=user, + project__title__in=user_group_names, + project__status__name='Active', + status__name__in=['Removed', 'Deactivated'], + ) + projectuser_entries.update( + status=ProjectUserStatusChoice.objects.get(name='Active') + ) + projects = ', '.join([pu.project.title for pu in projectuser_entries]) + logger.info( + 'Reactivated AD user and related ProjectUsers. user=%s, projects=%s', + user.username, projects, + extra={ 'category': 'ldap:User', 'status': 'success' } + ) + @receiver(project_post_create) def update_new_project(sender, **kwargs): """Update the new project using the AD group information""" From b84c61abb05ed68e036db2dc50d58e743a3f0a34 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 22 Dec 2025 19:43:46 -0800 Subject: [PATCH 15/20] add kwarg handling to signals call --- coldfront/plugins/ldap/signals.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 3d3f4a8436..797e6f7c32 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -49,8 +49,9 @@ def identify_ad_group(sender, **kwargs): return ifx_pi @receiver(project_reactivate_projectuser) -def reactivate_user(user): +def reactivate_user(sender, **kwargs): """Reactivate a user in LDAP""" + user = kwargs['user'] ldap_conn = LDAPConn() ldap_conn.reactivate_user(user.username) ldap_user = ldap_conn.return_user_by_name(user.username) From 1148191b3cfe9bdbea49ce74ea1b4da3c9d9febd Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Tue, 23 Dec 2025 01:22:33 -0800 Subject: [PATCH 16/20] user reactivation view/form/signal fixes --- coldfront/core/project/forms.py | 2 +- .../core/project/templates/project/project_add_users.html | 4 ++-- coldfront/core/project/views.py | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/coldfront/core/project/forms.py b/coldfront/core/project/forms.py index b309c58c8e..e29b76d766 100644 --- a/coldfront/core/project/forms.py +++ b/coldfront/core/project/forms.py @@ -52,7 +52,7 @@ class ProjectReactivateUserForm(forms.Form): last_name = forms.CharField(max_length=150, required=False, disabled=True) email = forms.EmailField(max_length=100, required=False, disabled=True) role = forms.ModelChoiceField( - queryset=ProjectUserRoleChoice.objects.all(), empty_label=None) + queryset=ProjectUserRoleChoice.objects.all(), required=False, empty_label=None) selected = forms.BooleanField(initial=False, required=False) diff --git a/coldfront/core/project/templates/project/project_add_users.html b/coldfront/core/project/templates/project/project_add_users.html index 33fecfea91..6316481af3 100644 --- a/coldfront/core/project/templates/project/project_add_users.html +++ b/coldfront/core/project/templates/project/project_add_users.html @@ -79,9 +79,9 @@

Add users to project: {{project.title}}

{% endfor %}
- {{ formset.management_form }} + {{ deactivated_formset.management_form }}
diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index d1df48e41e..37e9b3547c 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -568,7 +568,7 @@ def get_context_data(self, *args, **kwargs): 'first_name': u.user.first_name, 'last_name': u.user.last_name, 'email': u.user.email, - 'role': u.role, + 'role': u.role.name, } for u in deactivated_users] deactivated_formset = formset(initial=deactivated_forms, prefix='userform') context['deactivated_formset'] = deactivated_formset @@ -712,7 +712,7 @@ def post(self, request, *args, **kwargs): 'first_name': u.user.first_name, 'last_name': u.user.last_name, 'email': u.user.email, - 'role': u.role, + 'role': u.role.name, } for u in deactivated_users] deactivated_formset = formset( request.POST, initial=deactivated_forms, prefix='userform') @@ -725,6 +725,9 @@ def post(self, request, *args, **kwargs): user_form_data = form.cleaned_data if user_form_data['selected']: self.reactivate_user(project_obj, user_form_data) + messages.success( + request, f'Reactivated {user_form_data.user.username}.' + ) reactivated_users_count += 1 if reactivated_users_count: messages.success( @@ -732,6 +735,7 @@ def post(self, request, *args, **kwargs): f'Reactivated {reactivated_users_count} users in project.' ) # otherwise, proceed to normal user addition flow + return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': pk})) From e4d452558ada24ed4451567300807c7b4d809b39 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Tue, 23 Dec 2025 01:52:37 -0800 Subject: [PATCH 17/20] add error raising to user deactivation and reactivation --- coldfront/plugins/ldap/utils.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index 566fb2cced..499a25bd23 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -259,6 +259,11 @@ def deactivate_user(self, username): 'userAccountControl': [(MODIFY_REPLACE, ['514'])] } ) + if not result: + reason = self.conn.last_error + logger.error('Failed to deactivate user %s: %s', username, reason) + raise LDAPException(f'Failed to deactivate user {username}: {reason}') + return result def reactivate_user(self, username): @@ -271,6 +276,10 @@ def reactivate_user(self, username): 'userAccountControl': [(MODIFY_REPLACE, ['512'])] } ) + if not result: + reason = self.conn.last_error + logger.error('Failed to deactivate user %s: %s', username, reason) + raise LDAPException(f'Failed to reactivate user {username}: {reason}') return result def users_in_primary_group(self, usernames, groupname): From cc3b4ae902cad02c50d31ac0128248e8468c57f6 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 5 Jan 2026 14:37:35 -0800 Subject: [PATCH 18/20] improve project reactivation logging --- coldfront/plugins/ldap/signals.py | 2 +- coldfront/plugins/ldap/utils.py | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 797e6f7c32..c91d54dcaf 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -62,10 +62,10 @@ def reactivate_user(sender, **kwargs): project__status__name='Active', status__name__in=['Removed', 'Deactivated'], ) + projects = ', '.join([pu.project.title for pu in projectuser_entries]) projectuser_entries.update( status=ProjectUserStatusChoice.objects.get(name='Active') ) - projects = ', '.join([pu.project.title for pu in projectuser_entries]) logger.info( 'Reactivated AD user and related ProjectUsers. user=%s, projects=%s', user.username, projects, diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index 499a25bd23..128b2a571d 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -263,6 +263,10 @@ def deactivate_user(self, username): reason = self.conn.last_error logger.error('Failed to deactivate user %s: %s', username, reason) raise LDAPException(f'Failed to deactivate user {username}: {reason}') + logger.info( + 'Deactivated user %s (dn: %s)', username, user_dn, + extra={ 'category': 'integration:AD' } + ) return result @@ -294,7 +298,9 @@ def users_in_primary_group(self, usernames, groupname): try: users.append(self.return_user_by_name(user, attributes=attrs)) except ValueError: - logger.warning('user %s not found in LDAP when checking primary group membership', user) + logger.warning( + 'user %s not found in LDAP when checking primary group membership', user + ) return [ u['sAMAccountName'][0] for u in users if u['gidNumber'] == group['gidNumber'] ] From 58102fc7aa129497e9a918b1f979507098134abb Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 5 Jan 2026 16:55:25 -0800 Subject: [PATCH 19/20] fix formset bug --- coldfront/core/project/views.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 37e9b3547c..2c45a0755f 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -570,7 +570,7 @@ def get_context_data(self, *args, **kwargs): 'email': u.user.email, 'role': u.role.name, } for u in deactivated_users] - deactivated_formset = formset(initial=deactivated_forms, prefix='userform') + deactivated_formset = formset(initial=deactivated_forms, prefix='reactivateuserform') context['deactivated_formset'] = deactivated_formset else: context['deactivated_formset'] = None @@ -693,7 +693,7 @@ def reactivate_user(self, project_obj, user_form_data): allocation__status__name__in=['Active','Pending Deactivation'], allocation__project__projectuser__user=user_obj, allocation__project__projectuser__status__name='Active', - resources__resource_type__name='Storage' + allocation__resources__resource_type__name='Storage' ).distinct().update( status=AllocationUserStatusChoice.objects.get(name='Active') ) @@ -704,7 +704,7 @@ def post(self, request, *args, **kwargs): deactivated_users = project_obj.projectuser_set.filter(status__name='Deactivated') projuserstatus_active = ProjectUserStatusChoice.objects.get(name='Active') if deactivated_users.exists(): - formset = formset_factory( + deactivated_formset = formset_factory( ProjectReactivateUserForm, max_num=len(deactivated_users) ) deactivated_forms = [{ @@ -714,8 +714,8 @@ def post(self, request, *args, **kwargs): 'email': u.user.email, 'role': u.role.name, } for u in deactivated_users] - deactivated_formset = formset( - request.POST, initial=deactivated_forms, prefix='userform') + deactivated_formset = deactivated_formset( + request.POST, initial=deactivated_forms, prefix='reactivateuserform') # if any have been selected for reactivation, reactivate them if deactivated_formset.is_valid() and any( form.cleaned_data.get('selected') for form in deactivated_formset @@ -726,7 +726,7 @@ def post(self, request, *args, **kwargs): if user_form_data['selected']: self.reactivate_user(project_obj, user_form_data) messages.success( - request, f'Reactivated {user_form_data.user.username}.' + request, f'Reactivated {user_form_data["username"]}.' ) reactivated_users_count += 1 if reactivated_users_count: @@ -850,10 +850,20 @@ def post(self, request, *args, **kwargs): user=user_obj, status=allocuser_status_active, ) - allocation_activate_user.send( - sender=self.__class__, - allocation_user_pk=allocation_user_obj.pk, - ) + try: + allocation_activate_user.send( + sender=self.__class__, + allocation_user_pk=allocation_user_obj.pk, + ) + except Exception as e: + logger.exception( + "user added to project but not allocation. user=%s,project=%s,allocation_resource=%s,error=%s", + user_obj.username, project_obj.title, + allocation.get_parent_resource.name, e + ) + errors.append( + f"User {user_obj.username} was added to the project but an error occurred when activating them in allocation for {allocation.get_parent_resource.name}: {e}" + ) if errors: for error in errors: messages.error(request, error) From 1e37afa993537839099a06c9588bdae0bf59a848 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Mon, 5 Jan 2026 17:30:30 -0800 Subject: [PATCH 20/20] enable simultaneous addition and reactivation --- coldfront/core/project/views.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 2c45a0755f..275b9238d6 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -734,9 +734,6 @@ def post(self, request, *args, **kwargs): request, f'Reactivated {reactivated_users_count} users in project.' ) - # otherwise, proceed to normal user addition flow - return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': pk})) - user_search_string = request.POST.get('q') @@ -750,8 +747,10 @@ def post(self, request, *args, **kwargs): combined_user_search_obj = CombinedUserSearch( user_search_string, search_by, users_to_exclude ) - - context = combined_user_search_obj.search() + try: + context = combined_user_search_obj.search() + except AttributeError: + return HttpResponseRedirect(reverse('project-detail', kwargs={'pk': pk})) matches = context.get('matches') for match in matches: