diff --git a/coldfront/core/project/signals.py b/coldfront/core/project/signals.py index e0563ba5d..c7982c272 100644 --- a/coldfront/core/project/signals.py +++ b/coldfront/core/project/signals.py @@ -1,11 +1,16 @@ import django.dispatch - project_create = django.dispatch.Signal() #providing_args=["project_title"] project_post_create = django.dispatch.Signal() + #providing_args=["project_obj"] + +project_make_projectuser = django.dispatch.Signal() + #providing_args=["user_name", "group_name"] + +project_preremove_projectuser = django.dispatch.Signal() + #providing_args=["user_name", "group_name"] -project_activate_user = django.dispatch.Signal() - #providing_args=["project_user_pk"] -project_remove_user = django.dispatch.Signal() - #providing_args=["project_user_pk"] +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/tests/test_views.py b/coldfront/core/project/tests/test_views.py index 04e0519a3..b8873dacd 100644 --- a/coldfront/core/project/tests/test_views.py +++ b/coldfront/core/project/tests/test_views.py @@ -1,7 +1,8 @@ import logging -from django.test import TestCase, tag +from django.test import TestCase, tag, override_settings from django.urls import reverse +from unittest.mock import patch from coldfront.core.test_helpers import utils from coldfront.core.test_helpers.factories import ( @@ -12,7 +13,11 @@ ProjectStatusChoiceFactory, ProjectAttributeTypeFactory, ) -from coldfront.core.project.models import Project, ProjectUserStatusChoice +from coldfront.core.project.models import ( + Project, ProjectUser, + ProjectUserRoleChoice, + ProjectUserStatusChoice +) logging.disable(logging.CRITICAL) @@ -161,7 +166,6 @@ def test_project_attribute_create_post_required_values(self): def test_project_attribute_create_value_type_match(self): """ProjectAttributeCreate correctly flags value-type mismatch""" - self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') # test that value must be numeric if proj_attr_type is string @@ -351,7 +355,7 @@ def test_projectnotecreateview_access(self): self.project_access_tstbase(self.url) -class ProjectAddUsersSearchView(ProjectViewTestBase): +class ProjectAddUsersSearchViewTest(ProjectViewTestBase): """Tests for ProjectAddUsersSearchView""" def setUp(self): """set up users and project for testing""" @@ -365,6 +369,50 @@ def test_projectadduserssearchview_access(self): utils.test_user_cannot_access(self, self.proj_datamanager, self.url)# data manager cannot access utils.test_user_cannot_access(self, self.proj_allocation_user, self.url)# user cannot access +class ProjectAddUsersViewTest(ProjectViewTestBase): + """Tests for ProjectAddUsersView""" + def setUp(self): + """set up users and project for testing""" + self.url = reverse('project-add-users', kwargs={'pk': self.project.pk}) + self.form_data = { + 'q': self.nonproj_allocation_user.username, + 'search_by': 'username_only', + 'userform-TOTAL_FORMS': '1', + 'userform-INITIAL_FORMS': '0', + 'userform-MIN_NUM_FORMS': '0', + 'userform-MAX_NUM_FORMS': '1', + 'userform-0-selected': 'on', + 'userform-0-role': ProjectUserRoleChoice.objects.get(name='User').pk, + 'allocationform-allocation': [self.proj_allocation.pk] + } + + @override_settings(PLUGIN_LDAP=True) + def test_projectaddusers_ldapsignalfail_messages(self): + """Test the messages displayed when the add user signal fails""" + self.client.force_login(self.pi_user) + + @patch('coldfront.core.project.signals.project_make_projectuser.send') + def test_projectaddusers_form_validation(self, mock_signal): + """Test that the formset and allocation form are validated correctly""" + self.client.force_login(self.proj_accessmanager) + mock_signal.return_value = None + # Prepare form data for adding a user + response = self.client.post(self.url, data=self.form_data) + self.assertEqual(response.url, reverse('project-detail', kwargs={'pk': self.project.pk})) + self.assertEqual(response.status_code, 302) + # Check that user was added + self.assertTrue(ProjectUser.objects.filter(project=self.project, user=self.nonproj_allocation_user).exists()) + + @patch('coldfront.core.project.signals.project_make_projectuser.send') + def test_projectaddusers_signal_fail(self, mock_signal): + """Test that the add users form fails when the signal sent to LDAP fails""" + self.client.force_login(self.proj_accessmanager) + mock_signal.side_effect = Exception("LDAP error occurred") + # Prepare form data for adding a user + response = self.client.post(self.url, data=self.form_data, follow=True) + self.assertContains(response, 'LDAP error occurred') + self.assertContains(response, 'Added 0 users') + class ProjectUserDetailViewTest(ProjectViewTestBase): """Tests for ProjectUserDetailView""" diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index c95b996cd..0b742687c 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -29,7 +29,13 @@ allocation_remove_user, allocation_activate_user, ) -from coldfront.core.project.signals import project_create, project_post_create +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.grant.models import Grant from coldfront.core.project.forms import ( ProjectReviewForm, @@ -68,9 +74,6 @@ if 'django_q' in settings.INSTALLED_APPS: from django_q.tasks import Task -if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - from coldfront.plugins.ldap.utils import LDAPConn - ALLOCATION_ENABLE_ALLOCATION_RENEWAL = import_from_settings( 'ALLOCATION_ENABLE_ALLOCATION_RENEWAL', True) ALLOCATION_DEFAULT_ALLOCATION_LENGTH = import_from_settings( @@ -609,10 +612,7 @@ def post(self, request, *args, **kwargs): allocation_form = ProjectAddUsersToAllocationForm( request.user, project_obj.pk, request.POST, prefix='allocationform' ) - added_users_count = 0 - if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - ldap_conn = LDAPConn() if formset.is_valid() and allocation_form.is_valid(): projuserstatus_active = ProjectUserStatusChoice.objects.get(name='Active') @@ -649,28 +649,19 @@ def post(self, request, *args, **kwargs): role_choice = user_form_data.get('role') - if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - try: - ldap_conn.add_user_to_group( - user_obj.username, project_obj.title, - ) - logger.info( - "P678: Coldfront user %s added AD User for %s to AD Group %s", - self.request.user, - user_obj.username, - project_obj.title, - ) - except Exception as e: - error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}\nPlease contact Coldfront administration for further assistance." - logger.error( - "P685: user %s could not add AD user of %s to AD Group of %s: %s", - self.request.user, user_obj, project_obj.title, e - ) - errors.append(error) - continue - success_msg = f"User {user_obj} added by {request.user} to AD Group for {project_obj.title}" - logger.info(success_msg) - successes.append(success_msg) + try: + project_make_projectuser.send( + sender=self.__class__, + user_name=user_obj.username, group_name=project_obj.title + ) + except Exception as e: + error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}\nPlease contact Coldfront administration for further assistance." + logger.exception('P646: %s', e) + errors.append(error) + continue + success_msg = f"User {user_obj} added by {request.user} to AD Group for {project_obj.title}" + logger.info(success_msg) + successes.append(success_msg) # Is the user already in the project? project_obj.projectuser_set.update_or_create( @@ -680,7 +671,6 @@ def post(self, request, *args, **kwargs): 'status': projuserstatus_active, } ) - added_users_count += 1 for allocation in Allocation.objects.filter( pk__in=allocation_form_data @@ -757,18 +747,19 @@ def get_users_to_remove(self, project_obj): def get(self, request, *args, **kwargs): pk = self.kwargs.get('pk') project_obj = get_object_or_404(Project, pk=pk) - users_to_remove = self.get_users_to_remove(project_obj) - users_no_removal = None + users_list = self.get_users_to_remove(project_obj) # if ldap is activated, prevent selection of users with project corresponding to primary group - if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - - usernames = [u['username'] for u in users_to_remove] - ldap_conn = LDAPConn() - users_main_group = ldap_conn.users_in_primary_group( - usernames, project_obj.title) - ingroup = lambda u: u['username'] in users_main_group - users_no_removal, users_to_remove = sort_by(users_to_remove, ingroup, how="condition") + signal_response = project_filter_users_to_remove.send( + sender=self.__class__, users_to_remove=users_list, project=project_obj + ) + if signal_response: + user_categories = signal_response[0][1] + users_no_removal = user_categories[0] + users_to_remove = user_categories[1] + else: + users_no_removal = [] + users_to_remove = users_list context = {} @@ -789,15 +780,14 @@ def post(self, request, *args, **kwargs): 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 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - - usernames = [u['username'] for u in users_to_remove] - ldap_conn = LDAPConn() - users_main_group = ldap_conn.users_in_primary_group( - usernames, project_obj.title) - ingroup = lambda u: u['username'] in users_main_group - users_no_removal, users_to_remove = sort_by(users_to_remove, ingroup, how="condition") - + signal_response = project_filter_users_to_remove.send( + sender=self.__class__, users_to_remove=users_to_remove, project=project_obj + ) + if signal_response: + user_categories = signal_response[0][1] + users_to_remove = user_categories[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') @@ -822,30 +812,30 @@ def post(self, request, *args, **kwargs): project_user_obj = project_obj.projectuser_set.get(user=user_obj) - if 'coldfront.plugins.ldap' in settings.INSTALLED_APPS: - try: - ldap_conn.remove_member_from_group( - user_obj.username, project_obj.title, - ) - logger.info( - "P835: Coldfront user %s removed AD User for %s from AD Group for %s", - self.request.user, - user_obj.username, - project_obj.title, - ) - except Exception as e: - messages.error( - request, - f"could not remove user {user_obj}: {e}" - ) - logger.error( - "P846: Coldfront user %s could NOT remove AD User for %s from AD Group for %s: %s", - self.request.user, - user_obj.username, - project_obj.title, - e - ) - continue + try: + project_preremove_projectuser.send( + sender=self.__class__, + user_name=user_obj.username, group_name=project_obj.title + ) + logger.info( + "P802: Coldfront user %s removed AD User for %s from AD Group for %s", + self.request.user, + user_obj.username, + project_obj.title, + ) + except Exception as e: + messages.error( + request, + f"could not remove user {user_obj}: {e}" + ) + logger.exception( + "P802: Coldfront user %s could NOT remove AD User for %s from AD Group for %s: %s", + self.request.user, + user_obj.username, + project_obj.title, + e + ) + continue project_user_obj.status = projectuser_status_removed project_user_obj.save() diff --git a/coldfront/core/resource/management/commands/add_resource_defaults.py b/coldfront/core/resource/management/commands/add_resource_defaults.py index ee0cf09af..d159b9703 100644 --- a/coldfront/core/resource/management/commands/add_resource_defaults.py +++ b/coldfront/core/resource/management/commands/add_resource_defaults.py @@ -83,27 +83,28 @@ def handle(self, *args, **options): ('Tier 3', 'Attic Storage - Tape', True, storage_tier, None, 20, True, True), ('holylfs04/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True), ('holylfs05/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True), + ('holylfs06/tier0', 'Holyoke data center lustre storage', True, storage, 'Tier 0', 1, True, True), ('nesetape/tier3', 'Cold storage for past projects', True, storage, 'Tier 3', 20, True, True), ('holy-isilon/tier1', 'Tier1 storage with snapshots and disaster recovery copy', True, storage, 'Tier 1', 1, True, True), ('bos-isilon/tier1', 'Tier1 storage for on-campus storage mounting', True, storage, 'Tier 1', 1, True, True), ('holystore01/tier0', 'Luster storage under Tier0', True, storage, 'Tier 0', 1, True, True), - ('b-nfs02-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs03-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs04-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs05-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs06-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs07-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs08-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('b-nfs09-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs11-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs12-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs13-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs14-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs15-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs16-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs17-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs18-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), - ('h-nfs19-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, False, True), + ('b-nfs02-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs03-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs04-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs05-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs06-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs07-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs08-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('b-nfs09-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs11-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs12-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs13-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs14-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs15-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs16-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs17-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs18-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), + ('h-nfs19-p/tier2', 'Tier2 CEPH storage', True, storage, 'Tier 2', 1, True, True), ('boslfs02', 'complimentary lab storage', True, storage, 'Tier 0', 1, False, False), ('holylabs', 'complimentary lab storage', True, storage, 'Tier 0', 1, False, False), ): diff --git a/coldfront/plugins/ldap/utils.py b/coldfront/plugins/ldap/utils.py index 0e7eeb9ae..2afa305e2 100644 --- a/coldfront/plugins/ldap/utils.py +++ b/coldfront/plugins/ldap/utils.py @@ -5,7 +5,6 @@ import operator from functools import reduce -from coldfront.core import field_of_science from django.db.models import Q from django.dispatch import receiver from django.utils import timezone @@ -14,7 +13,13 @@ 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_create, project_post_create +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 ) @@ -179,11 +184,6 @@ def return_group_by_name(self, groupname, return_as='dict', attributes=ALL_ATTRI raise ValueError("no groups returned") return group[0] - def add_user_to_group(self, user_name, group_name): - group = self.return_group_by_name(group_name) - user = self.return_user_by_name(user_name) - self.add_member_to_group(user, group) - def add_group_to_group(self, group_name, parent_group_name): group = self.return_group_by_name(group_name) parent_group = self.return_group_by_name(parent_group_name) @@ -198,8 +198,9 @@ def add_member_to_group(self, member, group): except Exception as e: logger.exception("Error encountered while adding user to group: %s", e) raise LDAPUserAdditionError("Error adding user to group.") - if not self.member_in_group(member_dn, group_dn): + if not self.member_in_group(member_dn, group_dn) or not result: raise LDAPUserAdditionError("Member not successfully added to group.") + logger.info('user %s added to AD group %s', member_dn, group_dn) return result def remove_member_from_group(self, user_name, group_name): @@ -219,8 +220,9 @@ def remove_member_from_group(self, user_name, group_name): except Exception as e: logger.exception("Error encountered while removing user from group: %s", e) raise LDAPUserRemovalError("Error removing user from group.") - if self.member_in_group(user_dn, group_dn): + if self.member_in_group(user_dn, group_dn) or not result: raise LDAPUserRemovalError("Member not successfully removed from group.") + logger.info('user %s removed from AD group %s', user_dn, group_dn) return result def users_in_primary_group(self, usernames, groupname): @@ -714,3 +716,25 @@ def update_new_project(sender, **kwargs): role=ProjectUserRoleChoice.objects.get(name=role_name), status=ProjectUserStatusChoice.objects.get(name='Active'), ) + +@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) + ingroup = lambda u: u['username'] in users_main_group + users_no_removal, users_to_remove = sort_by(users_to_remove, ingroup, how="condition") + return (users_no_removal, users_to_remove) + +@receiver(project_make_projectuser) +def add_user_to_group(sender, **kwargs): + ldap_conn = LDAPConn() + group = ldap_conn.return_group_by_name(kwargs['group_name']) + user = ldap_conn.return_user_by_name(kwargs['user_name']) + ldap_conn.add_member_to_group(user, group) + +@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']) diff --git a/coldfront/plugins/system_monitor/utils.py b/coldfront/plugins/system_monitor/utils.py index f07003b98..c4d7a50c9 100644 --- a/coldfront/plugins/system_monitor/utils.py +++ b/coldfront/plugins/system_monitor/utils.py @@ -1,7 +1,7 @@ import re import requests -from bs4 import BeautifulSoup +from beautifulsoup4 import BeautifulSoup from coldfront.core.utils.common import import_from_settings