From b987663333e68fe3c99e4e007df66f27c8b3f9be Mon Sep 17 00:00:00 2001
From: Wes Appler <145372368+wes-otf@users.noreply.github.com>
Date: Wed, 17 Apr 2024 09:59:13 -0400
Subject: [PATCH] Make `is_staff` true when a user is Staff Admin (#3841)

Fixes #3782.

Rather than having `is_staff` set to true when a user's
email is the org's domain, this sets it true when a user is Staff Admin
or a Superuser. Also includes a migration to handle existing cases where
Staff Admins are not `is_staff`.

Co-authored-by: Sandeep Chauhan <sandeepsajan0@gmail.com>
Co-authored-by: Saurabh Kumar <theskumar@users.noreply.github.com>
---
 .../funds/applicationsubmission_detail.html   |  3 ++-
 .../users/migrations/0024_update_is_staff.py  | 22 +++++++++++++++++++
 hypha/apply/users/pipeline.py                 | 13 -----------
 hypha/apply/users/utils.py                    | 19 ++++++++++++++++
 hypha/apply/users/wagtail_hooks.py            |  7 +++++-
 hypha/settings/base.py                        |  1 -
 6 files changed, 49 insertions(+), 16 deletions(-)
 create mode 100644 hypha/apply/users/migrations/0024_update_is_staff.py
 delete mode 100644 hypha/apply/users/pipeline.py

diff --git a/hypha/apply/funds/templates/funds/applicationsubmission_detail.html b/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
index 7021a81bb..798291630 100644
--- a/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
+++ b/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
@@ -59,7 +59,8 @@
                     >
                         {% trans "Activity feed" %}
                     </a>
-                    {% if request.user.is_apply_staff_admin %}
+                    {# is_staff is only used for Django Admin. #}
+                    {% if request.user.is_staff %}
                         <a class="tab__item tab__item--right js-tabs-off" href="{% url 'admin:activity_event_changelist' %}?submission__id={{ object.id }}">
                             {% trans "View message log" %}
                         </a>
diff --git a/hypha/apply/users/migrations/0024_update_is_staff.py b/hypha/apply/users/migrations/0024_update_is_staff.py
new file mode 100644
index 000000000..09501e7a6
--- /dev/null
+++ b/hypha/apply/users/migrations/0024_update_is_staff.py
@@ -0,0 +1,22 @@
+# Generated by Django 4.2.11 on 2024-03-28 19:18
+
+from django.db import migrations
+from django.contrib.auth.models import Group
+
+from hypha.apply.users.groups import TEAMADMIN_GROUP_NAME
+from hypha.apply.users.utils import update_is_staff
+
+
+def migrate_is_staff(apps, schema_editor):
+    group = Group.objects.get(name=TEAMADMIN_GROUP_NAME)
+    users = group.user_set.all()
+
+    [update_is_staff(None, user) for user in users]
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("users", "0023_merge_0021_groupdesc_0022_confirmaccesstoken"),
+    ]
+
+    operations = [migrations.RunPython(migrate_is_staff)]
diff --git a/hypha/apply/users/pipeline.py b/hypha/apply/users/pipeline.py
deleted file mode 100644
index 7a0d230a1..000000000
--- a/hypha/apply/users/pipeline.py
+++ /dev/null
@@ -1,13 +0,0 @@
-from django.conf import settings
-from django.contrib.auth.models import Group
-
-from hypha.apply.users.groups import STAFF_GROUP_NAME
-
-
-def make_otf_staff(backend, user, response, *args, **kwargs):
-    _, email_domain = user.email.split("@")
-    if email_domain in settings.STAFF_EMAIL_DOMAINS:
-        staff_group = Group.objects.get(name=STAFF_GROUP_NAME)
-        user.groups.add(staff_group)
-        # Required in order to allow access to django admin, no other functional use
-        user.is_staff = True
diff --git a/hypha/apply/users/utils.py b/hypha/apply/users/utils.py
index 6f544ea31..fa0988f74 100644
--- a/hypha/apply/users/utils.py
+++ b/hypha/apply/users/utils.py
@@ -174,3 +174,22 @@ def generate_numeric_token(length=6):
     We use this formatting to allow leading 0s.
     """
     return get_random_string(length, allowed_chars=string.digits)
+
+
+def update_is_staff(request, user):
+    """Determine if the user should have `is_staff`
+
+    Django Admin is the only use for `is_staff`
+
+    Args:
+        request: The edit request object (unused)
+        user: [`User`][hypha.apply.users.models.User] to modify
+
+    """
+
+    if not user.is_staff and user.is_apply_staff_admin:
+        user.is_staff = True
+        user.save()
+    elif user.is_staff and not user.is_apply_staff_admin:
+        user.is_staff = False
+        user.save()
diff --git a/hypha/apply/users/wagtail_hooks.py b/hypha/apply/users/wagtail_hooks.py
index aa2ffb2e8..74797343e 100644
--- a/hypha/apply/users/wagtail_hooks.py
+++ b/hypha/apply/users/wagtail_hooks.py
@@ -5,7 +5,7 @@ from wagtail.models import Site
 from hypha.apply.activity.messaging import MESSAGES, messenger
 
 from .admin_views import CustomGroupViewSet, CustomUserIndexView
-from .utils import send_activation_email
+from .utils import send_activation_email, update_is_staff
 
 
 @hooks.register("register_admin_urls")
@@ -47,3 +47,8 @@ def notify_after_edit_user(request, user):
             source=user,
             roles=roles,
         )
+
+
+# Handle setting of `is_staff` after updating a user
+hooks.register("after_create_user", update_is_staff)
+hooks.register("after_edit_user", update_is_staff)
diff --git a/hypha/settings/base.py b/hypha/settings/base.py
index ff0c798c3..30a456282 100644
--- a/hypha/settings/base.py
+++ b/hypha/settings/base.py
@@ -378,7 +378,6 @@ SOCIAL_AUTH_PIPELINE = (
     "social_core.pipeline.social_auth.associate_user",
     "social_core.pipeline.social_auth.load_extra_data",
     "social_core.pipeline.user.user_details",
-    "hypha.apply.users.pipeline.make_otf_staff",
 )
 
 # NH3 Settings
-- 
GitLab