diff --git a/coldfront/core/project/exceptions.py b/coldfront/core/project/exceptions.py new file mode 100644 index 0000000000..e5247f8b94 --- /dev/null +++ b/coldfront/core/project/exceptions.py @@ -0,0 +1,14 @@ +"""Exceptions raised when integrations (e.g. LDAP) reject a project-user change. + +Plugins that hook into project signals (see coldfront.core.project.signals) should +raise these -- or subclasses of these -- rather than plugin-specific exception types, +so core code can handle the failure without importing from any plugin. +""" + + +class ProjectUserAdditionError(Exception): + """Raised when a user cannot be added to a project by a signal receiver.""" + + +class ProjectUserDeactivatedError(ProjectUserAdditionError): + """Raised when a user cannot be added to a project because their account is deactivated.""" diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index f7f40b13cb..1bf43319bc 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -66,6 +66,7 @@ ) from coldfront.core.project.manager_role_notifications import notify_manager_role_transition from coldfront.core.project.utils import generate_usage_history_graph +from coldfront.core.project.exceptions import ProjectUserDeactivatedError from coldfront.core.publication.models import Publication from coldfront.core.research_output.models import ResearchOutput from coldfront.core.resource.models import ResourceAttribute @@ -825,6 +826,12 @@ def post(self, request, *args, **kwargs): sender=self.__class__, user_name=user_obj.username, group_name=project_obj.title ) + except ProjectUserDeactivatedError: + errors.append( + f"Could not add user {user_obj} to project {project_obj.title}: " + "this user's account is deactivated." + ) + continue except Exception as e: logger.exception( 'AD user addition to group failed.', diff --git a/coldfront/plugins/ldap/signals.py b/coldfront/plugins/ldap/signals.py index 786a181e03..e4664d1fb6 100644 --- a/coldfront/plugins/ldap/signals.py +++ b/coldfront/plugins/ldap/signals.py @@ -19,7 +19,7 @@ ProjectUserStatusChoice, ProjectUser, ) -from coldfront.plugins.ldap.utils import LDAPConn +from coldfront.plugins.ldap.utils import LDAPConn, LDAPUserDeactivatedError if 'sftocf' in import_from_settings('INSTALLED_APPS', []): from sftocf.signals import ( @@ -138,7 +138,19 @@ def filter_project_users_to_remove(sender, **kwargs): @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']) + try: + ldap_conn.add_user_to_group(kwargs['user_name'], kwargs['group_name']) + except LDAPUserDeactivatedError: + logger.warning( + 'Cannot add deactivated user to AD group.', + extra={ + 'category': 'ldap:GroupMembership', + 'status': 'error', + 'user': kwargs['user_name'], + 'group': kwargs['group_name'], + } + ) + raise @receiver(project_preremove_projectuser) def remove_member_from_group(sender, **kwargs): diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index 04f0b62666..97a02dac12 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -32,6 +32,7 @@ ProjectUserStatusChoice, ProjectUser, ) +from coldfront.core.project.exceptions import ProjectUserDeactivatedError logger = logging.getLogger(__name__) @@ -48,6 +49,9 @@ class LDAPException(Exception): class LDAPUserAdditionError(LDAPException): """An exception raised when a user cannot be added to an LDAP Group""" +class LDAPUserDeactivatedError(LDAPUserAdditionError, ProjectUserDeactivatedError): + """An exception raised when a user cannot be added to an LDAP Group because their account is deactivated""" + class LDAPUserRemovalError(LDAPException): """An exception raised when a user cannot be removed from an LDAP Group""" @@ -211,6 +215,10 @@ def return_project_ldap_groups(self, attributes=('sAMAccountName',), groups=('*_ def add_user_to_group(self, user_name, group_name): user = self.return_user_by_name(user_name) + if not user_valid(user): + raise LDAPUserDeactivatedError( + f"Cannot add user {user_name} to group {group_name}: user is deactivated." + ) group = self.return_group_by_name(group_name) self.add_member_to_group(user, group)