Development

coldfront/config/base.py

@@ -51,7 +51,6 @@
     'django_tables2',
     'table',
     'rest_framework_datatables',
-    'rest_framework',
     'easy_pdf',
 ]
 
@@ -148,19 +147,6 @@
     },
 ]
 
-REST_FRAMEWORK = {
-    'DEFAULT_RENDERER_CLASSES': (
-        'rest_framework.renderers.JSONRenderer',
-        'rest_framework.renderers.BrowsableAPIRenderer',
-        'rest_framework_datatables.renderers.DatatablesRenderer',
-    ),
-    'DEFAULT_FILTER_BACKENDS': (
-        'rest_framework_datatables.filters.DatatablesFilterBackend',
-    ),
-    'DEFAULT_PAGINATION_CLASS': 'rest_framework_datatables.pagination.DatatablesPageNumberPagination',
-    'PAGE_SIZE': 500,
-}
-
 # Add local site templates files if set
 SITE_TEMPLATES = ENV.str('SITE_TEMPLATES', default='')
 if len(SITE_TEMPLATES) > 0:

coldfront/config/plugins/api.py

@@ -1,15 +1,15 @@
 from coldfront.config.base import INSTALLED_APPS
 
 INSTALLED_APPS += [
-        'django_filters',
-        'coldfront.plugins.api'
-        ]
+    'django_filters',
+    'rest_framework',
+    'coldfront.plugins.api',
+]
 
 REST_FRAMEWORK = {
     'DEFAULT_AUTHENTICATION_CLASSES': (
         'rest_framework.authentication.TokenAuthentication',
         'rest_framework.authentication.SessionAuthentication',
-        'rest_framework.authentication.BasicAuthentication',
     ),
     'DEFAULT_PERMISSION_CLASSES': [
         'rest_framework.permissions.IsAuthenticated'

coldfront/core/project/models.py

@@ -9,6 +9,7 @@
 from model_utils.models import TimeStampedModel
 from simple_history.models import HistoricalRecords
 
+from ifxuser.models import Organization
 from coldfront.core.field_of_science.models import FieldOfScience
 from coldfront.core.utils.common import import_from_settings
 
@@ -214,6 +215,17 @@ def user_permissions(self, user):
 
         if self.pi.id == user.id:
             permissions.append(ProjectPermission.PI)
+
+        # if the user is an approver in a department connected to the project,
+        # give them user permissions
+        departments = Organization.objects.filter(
+            org_tree='Research Computing Storage Billing'
+        )
+        proj_departments = [d for d in departments if self in d.get_projects()]
+        for department in proj_departments:
+            if user in department.useraffiliation_set.filter(role='approver'):
+                permissions.append(ProjectPermission.USER)
+
         return permissions
 
     def has_perm(self, user, perm):

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)

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"""

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,18 @@ 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:
+        signal_response = project_filter_users_to_remove.send(
+            sender=self.__class__, users_to_remove=users_list, project=project_obj
+        )
+        if signal_response:
+            users_to_remove = signal_response[0][1]
+        else:
+            users_to_remove = users_list
 
-            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")
+        users_no_removal = [u for u in users_list if u not in users_to_remove]
 
         context = {}
 
@@ -789,20 +779,19 @@ 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:
+            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')
 
         remove_users_count = 0
+        failed_user_removals = []
 
         if formset.is_valid():
             projectuser_status_removed = ProjectUserStatusChoice.objects.get(
@@ -822,30 +811,27 @@ 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(
+                            "P815: 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:
+                        failed_user_removals += [f"could not remove user {user_obj}: {e}"]
+                        logger.exception(
+                            "P815: 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()
@@ -866,6 +852,8 @@ def post(self, request, *args, **kwargs):
                             )
                     remove_users_count += 1
             user_pl = 'user' if remove_users_count == 1 else 'users'
+            for fail in failed_user_removals:
+                messages.error(request, fail)
             messages.success(
                 request, f'Removed {remove_users_count} {user_pl} from project.'
             )