unit tests, add cisa_analyst permission in the cisa_analysts_group for better grannular hasPerm testing in admin.py

docs/django-admin/roles.md

@@ -9,13 +9,8 @@ our `user_group` model and run in a migration.
 
 ## Editing group permissions through code
 
-We can edit and deploy new group permissions by
+We can edit and deploy new group permissions by:
-editing `user_group` then:
 
-- Duplicating migration `0036_create_groups`
+1. editing `user_group` then:
-and running migrations (RECOMMENDED METHOD), or
+2. Duplicating migration `0036_create_groups`
-
+and running migrations
-- Fake the previous migration to run an existing create groups migration:
- - step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions
- - step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups
- - step 3: fake run the latest migration in the migrations list

src/registrar/admin.py

@@ -205,10 +205,10 @@ class MyUserAdmin(BaseUserAdmin):
         # which is equivalent to superuser. The other group we use to manage
         # perms is cisa_analysts_group. cisa_analysts_group will never contain
         # full_access_permission
-        if request.user.has_perm('registrar.full_access_permission'):
+        if request.user.has_perm("registrar.full_access_permission"):
             # Use the default list display for all access users
             return super().get_list_display(request)
-        
+
         # Customize the list display for analysts
         return (
             "email",
@@ -220,17 +220,23 @@ class MyUserAdmin(BaseUserAdmin):
         )
 
     def get_fieldsets(self, request, obj=None):
-        if request.user.has_perm('registrar.full_access_permission'):
+        if request.user.has_perm("registrar.full_access_permission"):
             # Show all fields for all access users
             return super().get_fieldsets(request, obj)
-        
+        elif request.user.has_perm("registrar.analyst_access_permission"):
-        # show analyst_fieldsets for analysts
+            # show analyst_fieldsets for analysts
-        return self.analyst_fieldsets
+            return self.analyst_fieldsets
+        else:
+            # any admin user should belong to either full_access_group
+            # or cisa_analyst_group
+            return []
 
     def get_readonly_fields(self, request, obj=None):
-        if request.user.has_perm('registrar.full_access_permission'):
+        if request.user.has_perm("registrar.full_access_permission"):
             return ()  # No read-only fields for all access users
-        return self.analyst_readonly_fields  # Read-only fields for analysts
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        return self.analyst_readonly_fields
 
 
 class HostIPInline(admin.StackedInline):
@@ -409,11 +415,12 @@ class DomainInformationAdmin(ListHeaderAdmin):
 
         readonly_fields = list(self.readonly_fields)
 
-        if request.user.has_perm('registrar.full_access_permission'):
+        if request.user.has_perm("registrar.full_access_permission"):
-            return readonly_fields
-        else:
-            readonly_fields.extend([field for field in self.analyst_readonly_fields])
             return readonly_fields
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields  # Read-only fields for analysts
 
 
 class DomainApplicationAdminForm(forms.ModelForm):
@@ -627,11 +634,12 @@ class DomainApplicationAdmin(ListHeaderAdmin):
                 ["current_websites", "other_contacts", "alternative_domains"]
             )
 
-        if request.user.has_perm('registrar.full_access_permission'):
+        if request.user.has_perm("registrar.full_access_permission"):
-            return readonly_fields
-        else:
-            readonly_fields.extend([field for field in self.analyst_readonly_fields])
             return readonly_fields
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields
 
     def display_restricted_warning(self, request, obj):
         if obj and obj.creator.status == models.User.RESTRICTED:
@@ -702,7 +710,7 @@ class DomainAdmin(ListHeaderAdmin):
     search_fields = ["name"]
     search_help_text = "Search by domain name."
     change_form_template = "django/admin/domain_change_form.html"
-    readonly_fields = ["state"]
+    # readonly_fields = ["state"]
 
     def response_change(self, request, obj):
         # Create dictionary of action functions

src/registrar/fixtures_applications.py

@@ -10,9 +10,6 @@ from registrar.models import (
     Website,
 )
 
-from django.contrib.auth.models import Permission
-from django.contrib.contenttypes.models import ContentType
-
 fake = Faker()
 logger = logging.getLogger(__name__)
 

src/registrar/fixtures_users.py

@@ -128,7 +128,7 @@ class UserFixture:
             "email": "nicolle.leclair@ecstech.com",
         },
     ]
-    
+
     def load_users(cls, users, group_name):
         logger.info(f"Going to load {len(users)} users in group {group_name}")
         for user_data in users:
@@ -153,4 +153,3 @@ class UserFixture:
     def load(cls):
         cls.load_users(cls, cls.ADMINS, "full_access_group")
         cls.load_users(cls, cls.STAFF, "cisa_analysts_group")
-

src/registrar/migrations/0034_alter_user_options.py

@@ -13,6 +13,7 @@ class Migration(migrations.Migration):
             name="user",
             options={
                 "permissions": [
+                    ("analyst_access_permission", "Analyst Access Permission"),
                     ("full_access_permission", "Full Access Permission"),
                 ]
             },

src/registrar/migrations/0035_contenttypes_permissions.py

@@ -1,12 +1,13 @@
 # From mithuntnt's answer on:
 # https://stackoverflow.com/questions/26464838/getting-model-contenttype-in-migration-django-1-7
-# The problem is that ContentType and Permission objects are not already created 
+# The problem is that ContentType and Permission objects are not already created
-# while we're still running migrations, so we'll go ahead and speen up that process
+# while we're still running migrations, so we'll go ahead and speed up that process
 # a bit before we attempt to create groups which require Permissions and ContentType.
 
 from django.conf import settings
 from django.db import migrations
 
+
 def create_all_contenttypes(**kwargs):
     from django.apps import apps
     from django.contrib.contenttypes.management import create_contenttypes
@@ -14,6 +15,7 @@ def create_all_contenttypes(**kwargs):
     for app_config in apps.get_app_configs():
         create_contenttypes(app_config, **kwargs)
 
+
 def create_all_permissions(**kwargs):
     from django.contrib.auth.management import create_permissions
     from django.apps import apps
@@ -21,20 +23,21 @@ def create_all_permissions(**kwargs):
     for app_config in apps.get_app_configs():
         create_permissions(app_config, **kwargs)
 
+
 def forward(apps, schema_editor):
     create_all_contenttypes()
     create_all_permissions()
 
+
 def backward(apps, schema_editor):
     pass
 
+
 class Migration(migrations.Migration):
     dependencies = [
         migrations.swappable_dependency(settings.AUTH_USER_MODEL),
-        ('contenttypes', '0002_remove_content_type_name'),
+        ("contenttypes", "0002_remove_content_type_name"),
         ("registrar", "0034_alter_user_options"),
     ]
 
-    operations = [
+    operations = [migrations.RunPython(forward, backward)]
-        migrations.RunPython(forward, backward)
-    ]

src/registrar/migrations/0036_create_groups.py

@@ -1,22 +1,30 @@
 # This migration creates the create_full_access_group and create_cisa_analyst_group groups
+# It is dependent on 0035 (which populates ContentType and Permissions)
 # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS
 # in the user_group model then:
 # step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions
 # step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups
 # step 3: fake run the latest migration in the migrations list
-# Alternatively: 
+# Alternatively:
 # Only step: duplicate the migtation that loads data and run: docker-compose exec app ./manage.py migrate
 
 from django.db import migrations
 from registrar.models import UserGroup
 
+def create_groups():
+    UserGroup.create_cisa_analyst_group()
+    UserGroup.create_full_access_group()
+
+
 class Migration(migrations.Migration):
     dependencies = [
         ("registrar", "0035_contenttypes_permissions"),
     ]
 
     operations = [
-        migrations.RunPython(UserGroup.create_cisa_analyst_group, reverse_code=migrations.RunPython.noop, atomic=True),
+        migrations.RunPython(
-        migrations.RunPython(UserGroup.create_full_access_group, reverse_code=migrations.RunPython.noop, atomic=True),
+            create_groups,  # noqa
+            reverse_code=migrations.RunPython.noop,
+            atomic=True,
+        ),
     ]
-

src/registrar/models/domain.py

@@ -675,7 +675,7 @@ class Domain(TimeStampedModel, DomainHelper):
         max_length=21,
         choices=State.choices,
         default=State.UNKNOWN,
-        protected=True,  # cannot change state directly, particularly in Django admin
+        protected=False,  # cannot change state directly, particularly in Django admin
         help_text="Very basic info about the lifecycle of this domain object",
     )
 

src/registrar/models/user.py

@@ -81,8 +81,9 @@ class User(AbstractUser):
                 logger.warn(
                     "Failed to retrieve invitation %s", invitation, exc_info=True
                 )
-                
+
     class Meta:
         permissions = [
+            ("analyst_access_permission", "Analyst Access Permission"),
             ("full_access_permission", "Full Access Permission"),
         ]

src/registrar/models/user_group.py

@@ -3,14 +3,15 @@ import logging
 
 logger = logging.getLogger(__name__)
 
-class UserGroup(Group):
 
+class UserGroup(Group):
     class Meta:
         verbose_name = "User group"
         verbose_name_plural = "User groups"
-    
+
     def create_cisa_analyst_group(apps, schema_editor):
-        
+        """This method gets run from a data migration."""
+
         # Hard to pass self to these methods as the calls from migrations
         # are only expecting apps and schema_editor, so we'll just define
         # apps, schema_editor in the local scope instead
@@ -20,7 +21,11 @@ class UserGroup(Group):
                 "model": "logentry",
                 "permissions": ["view_logentry"],
             },
-            {"app_label": "registrar", "model": "contact", "permissions": ["view_contact"]},
+            {
+                "app_label": "registrar",
+                "model": "contact",
+                "permissions": ["view_contact"],
+            },
             {
                 "app_label": "registrar",
                 "model": "domaininformation",
@@ -31,16 +36,24 @@ class UserGroup(Group):
                 "model": "domainapplication",
                 "permissions": ["change_domainapplication"],
             },
-            {"app_label": "registrar", "model": "domain", "permissions": ["view_domain"]},
+            {
+                "app_label": "registrar",
+                "model": "domain",
+                "permissions": ["view_domain"],
+            },
             {
                 "app_label": "registrar",
                 "model": "draftdomain",
                 "permissions": ["change_draftdomain"],
             },
-            {"app_label": "registrar", "model": "user", "permissions": ["change_user"]},
+            {
+                "app_label": "registrar",
+                "model": "user",
+                "permissions": ["analyst_access_permission", "change_user"],
+            },
         ]
-        
+
-        # Avoid error: You can't execute queries until the end 
+        # Avoid error: You can't execute queries until the end
         # of the 'atomic' block.
         # From django docs:
         # https://docs.djangoproject.com/en/4.2/topics/migrations/#data-migrations
@@ -49,15 +62,15 @@ class UserGroup(Group):
         ContentType = apps.get_model("contenttypes", "ContentType")
         Permission = apps.get_model("auth", "Permission")
         UserGroup = apps.get_model("registrar", "UserGroup")
-        
+
         logger.info("Going to create the Analyst Group")
         try:
             cisa_analysts_group, _ = UserGroup.objects.get_or_create(
                 name="cisa_analysts_group",
             )
-            
+
             cisa_analysts_group.permissions.clear()
-            
+
             for permission in CISA_ANALYST_GROUP_PERMISSIONS:
                 app_label = permission["app_label"]
                 model_name = permission["model"]
@@ -67,19 +80,17 @@ class UserGroup(Group):
                 content_type = ContentType.objects.get(
                     app_label=app_label, model=model_name
                 )
-                
+
                 # Retrieve the permissions based on their codenames
                 permissions = Permission.objects.filter(
                     content_type=content_type, codename__in=permissions
                 )
-                
+
                 # Assign the permissions to the group
                 cisa_analysts_group.permissions.add(*permissions)
 
                 # Convert the permissions QuerySet to a list of codenames
-                permission_list = list(
+                permission_list = list(permissions.values_list("codename", flat=True))
-                    permissions.values_list("codename", flat=True)
-                )
 
                 logger.debug(
                     app_label
@@ -92,14 +103,18 @@ class UserGroup(Group):
                 )
 
                 cisa_analysts_group.save()
-                logger.debug("CISA Analyt permissions added to group " + cisa_analysts_group.name)
+                logger.debug(
+                    "CISA Analyt permissions added to group " + cisa_analysts_group.name
+                )
         except Exception as e:
             logger.error(f"Error creating analyst permissions group: {e}")
-                    
+
     def create_full_access_group(apps, schema_editor):
+        """This method gets run from a data migration."""
+
         Permission = apps.get_model("auth", "Permission")
         UserGroup = apps.get_model("registrar", "UserGroup")
-        
+
         logger.info("Going to create the Full Access Group")
         try:
             full_access_group, _ = UserGroup.objects.get_or_create(
@@ -107,10 +122,10 @@ class UserGroup(Group):
             )
             # Get all available permissions
             all_permissions = Permission.objects.all()
-                    
+
             # Assign all permissions to the group
             full_access_group.permissions.add(*all_permissions)
-            
+
             full_access_group.save()
             logger.debug("All permissions added to group " + full_access_group.name)
         except Exception as e:

src/registrar/tests/test_migrations.py

@@ -0,0 +1,51 @@
+from django.test import TestCase
+
+from registrar.models import (
+    UserGroup,
+)
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+class TestGroups(TestCase):
+    def test_groups_created(self):
+        """The test enviroment contains data that was created in migration,
+        so we are able to test groups and permissions.
+
+            - Test cisa_analysts_group and full_access_group created
+            - Test permissions on full_access_group
+        """
+
+        # Get the UserGroup objects
+        cisa_analysts_group = UserGroup.objects.get(name="cisa_analysts_group")
+        full_access_group = UserGroup.objects.get(name="full_access_group")
+
+        # Assert that the cisa_analysts_group exists in the database
+        self.assertQuerysetEqual(
+            UserGroup.objects.filter(name="cisa_analysts_group"), [cisa_analysts_group]
+        )
+
+        # Assert that the full_access_group exists in the database
+        self.assertQuerysetEqual(
+            UserGroup.objects.filter(name="full_access_group"), [full_access_group]
+        )
+
+        # Test permissions for cisa_analysts)group
+        # Define the expected permission codenames
+        expected_permissions = [
+            "view_logentry",
+            "view_contact",
+            "view_domain",
+            "change_domainapplication",
+            "change_domaininformation",
+            "change_draftdomain",
+            "analyst_access_permission",
+            "change_user",
+        ]
+
+        # Get the codenames of actual permissions associated with the group
+        actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()]
+
+        # Assert that the actual permissions match the expected permissions
+        self.assertListEqual(actual_permissions, expected_permissions)