Decomission is_staff flag

apps/accounts/admin.py

@@ -102,7 +102,8 @@ class CustomUserAdmin(UserAdmin):
 
     model = User
     search_fields = ("username", "email")
-    list_display = ["username", "email", "last_login", "is_staff"]
+    list_display = ["username", "email", "last_login"]
+    list_filter = ("is_superuser", "is_active", "groups")
     readonly_fields = ("managed_providers", "managed_datacenters")
 
     def get_queryset(self, request, *args, **kwargs):
@@ -169,7 +170,7 @@ def get_fieldsets(self, request, obj=None, *args, **kwargs):
         staff_fieldsets = (
             "User status and group membership",
             {
-                "fields": ("is_active", "is_staff", "groups"),
+                "fields": ("is_active", "groups"),
             },
         )
 
@@ -197,7 +198,6 @@ def get_fieldsets(self, request, obj=None, *args, **kwargs):
                     {
                         "fields": (
                             "is_active",
-                            "is_staff",
                             "is_superuser",
                             "groups",
                         ),
@@ -775,7 +775,7 @@ def get_queryset(self, request, *args, **kwargs):
             "services",
         ).annotate(models.Count("greencheckip"))
 
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             # filter for non-archived providers & those the current user has permissions to
             qs = qs.filter(archived=False).filter(
                 id__in=[p.id for p in request.user.hosting_providers]
@@ -845,7 +845,7 @@ def services(self, obj):
 
     def get_readonly_fields(self, request, obj=None):
         read_only = super().get_readonly_fields(request, obj)
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             read_only.append("partner")
         if obj is not None:
             read_only.append("created_by")
@@ -882,9 +882,11 @@ def get_inlines(self, request, obj):
 
     def _changeform_view(self, request, object_id, form_url, extra_context):
         """Include whether current user is staff, so it can be picked up by a form"""
+        # TODO: clarify why the "is_staff" flag is passed to the form
+        # and until then, use the value from is_admin
         if request.method == "POST":
             post = request.POST.copy()
-            post["is_staff"] = request.user.is_staff
+            post["is_staff"] = request.user.is_admin
             request.POST = post
 
         extra_context = extra_context or {}
@@ -1054,7 +1056,7 @@ def get_queryset(self, request, *args, **kwargs):
 
     def get_readonly_fields(self, request, obj=None):
         read_only = super().get_readonly_fields(request, obj)
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             read_only.append("showonwebsite")
         if obj is not None:
             read_only.append("created_by")

apps/accounts/management/commands/migrate_permissions.py

@@ -28,9 +28,6 @@ def handle(self, *args, **options):
             for role in roles:
                 group, created = GROUPS[role]
                 user.groups.add(group)
-                if role == "ROLE_ADMIN":
-                    user.is_staff = True
-                    user.is_superuser = True
                 user.is_active = True
                 user.save()
         self.stdout.write(self.style.SUCCESS("Migrating permissions completed!"))

apps/accounts/migrations/0058_alter_user_is_staff.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.19 on 2023-07-06 13:00
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("accounts", "0057_remove_user_hostingprovider_fk"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="is_staff",
+            field=models.BooleanField(
+                default=False,
+                help_text="Django way of designating if the user can log into the admin. This flag is ignored in GWF Admin",
+                verbose_name="staff status",
+            ),
+        ),
+    ]

apps/accounts/models/user.py

@@ -46,7 +46,7 @@ class User(AbstractBaseUser, PermissionsMixin, GuardianUserMixin):
     is_staff = models.BooleanField(
         "staff status",
         default=False,
-        help_text="Designates whether the user can log into this admin site.",
+        help_text="Django way of designating if the user can log into the admin. This flag is ignored in GWF Admin",
     )
     is_active = models.BooleanField(
         "active",

apps/greencheck/admin.py

@@ -81,7 +81,7 @@ def get_fieldsets(self, request, obj=None):
             "status",
         )
 
-        if request.user.is_staff:
+        if request.user.is_admin:
             fields = fields + ("approval",)
 
         fieldsets = ((None, {"fields": fields}),)
@@ -90,7 +90,7 @@ def get_fieldsets(self, request, obj=None):
     def get_readonly_fields(self, request, obj):
         """Non staff user should only be able to read the fields"""
         read_only = super().get_readonly_fields(request, obj)
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             read_only = ("asn",) + read_only
         return read_only
 
@@ -150,7 +150,7 @@ def get_fieldsets(self, request, obj=None):
             "status",
         )
 
-        if request.user.is_staff:
+        if request.user.is_admin:
             fields = fields + ("approval",)
 
         fieldsets = ((None, {"fields": fields}),)
@@ -159,7 +159,7 @@ def get_fieldsets(self, request, obj=None):
     def get_readonly_fields(self, request, obj):
         """Non staff user should only be able to read the fields"""
         read_only = super().get_readonly_fields(request, obj)
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             read_only = ("ip_start", "ip_end") + read_only
         return read_only
 
@@ -232,7 +232,7 @@ def get_actions(self, request):
 
         # only staff users should be able to to bulk updates
 
-        if request.user and request.user.is_staff:
+        if request.user and request.user.is_admin:
             return default_actions
         else:
             del default_actions["approve_selected"]
@@ -276,7 +276,7 @@ def get_queryset(self, request):
         qs = qs.select_related("hostingprovider")
 
         # only show a normal user's own requests
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             res = qs.filter(hostingprovider__in=request.user.hosting_providers)
             return res
 
@@ -310,7 +310,7 @@ def get_queryset(self, request):
         qs = qs.select_related("hostingprovider")
 
         # only show a normal user's own requests
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             res = qs.filter(hostingprovider__in=request.user.hosting_providers)
             return res
 
@@ -354,7 +354,7 @@ def get_actions(self, request):
         default_actions = super().get_actions(request)
 
         # only staff users should be able to to bulk updates
-        if request.user and request.user.is_staff:
+        if request.user and request.user.is_admin:
             return default_actions
         else:
             del default_actions["allocate_to_provider"]
@@ -496,7 +496,7 @@ def get_queryset(self, request):
         qs = qs.select_related("hostingprovider")
 
         # only show a normal user's own requests
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             res = qs.filter(hostingprovider__in=request.user.hosting_providers)
             return res
 
@@ -549,7 +549,7 @@ def get_queryset(self, request):
         qs = qs.select_related("hostingprovider")
 
         # only show a normal user's own requests
-        if not request.user.is_staff:
+        if not request.user.is_admin:
             res = qs.filter(hostingprovider__in=request.user.hosting_providers)
             return res
 

apps/greencheck/tests/test_admin.py

@@ -6,12 +6,13 @@
 
 
 class TestGreencheckIpApproveAdmin:
-    def test_get_actions_staff_user(self, db, rf, sample_hoster_user, hosting_provider):
+    def test_get_actions_staff_user(
+        self, db, rf, greenweb_staff_user, hosting_provider
+    ):
         """Check that a user with the correct privileges can"""
 
         hosting_provider.save()
-        sample_hoster_user.is_staff = True
-        sample_hoster_user.save()
+        greenweb_staff_user.save()
 
         gcip_admin = gc_admin.GreencheckIpApproveAdmin(
             models.GreencheckIp, admin_site.greenweb_admin
@@ -21,7 +22,7 @@ def test_get_actions_staff_user(self, db, rf, sample_hoster_user, hosting_provid
             "greenweb_admin:greencheck_greencheckipapprove_changelist"
         )
         request = rf.get(ip_range_listing_path)
-        request.user = sample_hoster_user
+        request.user = greenweb_staff_user
 
         actions = gcip_admin.get_actions(request)
 
@@ -48,12 +49,11 @@ def test_get_actions_non_staff_user(
 
         assert "approve_selected" not in actions
 
-    def test_get_queryset_staff(
-        self, db, rf, sample_hoster_user, hosting_provider, green_ip
+    def test_get_queryset_admin(
+        self, db, rf, greenweb_staff_user, hosting_provider, green_ip
     ):
         hosting_provider.save()
-        sample_hoster_user.is_staff = True
-        sample_hoster_user.save()
+        greenweb_staff_user.save()
 
         provider = gc_factories.HostingProviderFactory()
         gip = gc_factories.GreenIpFactory(hostingprovider=provider)
@@ -67,7 +67,7 @@ def test_get_queryset_staff(
             "greenweb_admin:greencheck_greencheckipapprove_changelist"
         )
         request = rf.get(ip_range_listing_path)
-        request.user = sample_hoster_user
+        request.user = greenweb_staff_user
 
         qs = gcip_admin.get_queryset(request)
 
@@ -98,7 +98,7 @@ def test_get_queryset_non_staff(
         request = rf.get(ip_range_listing_path)
         request.user = hosting_provider_with_sample_user.users.first()
 
-        assert not request.user.is_staff
+        assert not request.user.is_admin
 
         qs = gcip_admin.get_queryset(request)
 

templates/admin/base.html

@@ -94,7 +94,7 @@ <h1 id="site-name">
                 {% if site_url %}
                     <a href="{{ site_url }}">{% trans 'View site' %}</a> /
                 {% endif %}
-                {% if user.is_active and user.is_staff %}
+                {% if user.is_active and user.is_admin %}
                     {% url 'django-admindocs-docroot' as docsroot %}
                     {% if docsroot %}
                         <a href="{{ docsroot }}">{% trans 'Documentation' %}</a> /