From 52dc04c1acebfda230ba321a43b41e3b91caf41d Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 7 Jul 2023 11:01:15 -0400 Subject: [PATCH 1/3] Expose users in django admin for CISA Analysts while hiding uuids --- docs/django-admin/roles.md | 3 +- src/registrar/admin.py | 21 +++++++- src/registrar/fixtures.py | 1 + src/registrar/tests/common.py | 44 ++++++++++++----- src/registrar/tests/test_admin.py | 80 +++++++++++++++++++++++-------- 5 files changed, 112 insertions(+), 37 deletions(-) diff --git a/docs/django-admin/roles.md b/docs/django-admin/roles.md index 431380300..ab4867184 100644 --- a/docs/django-admin/roles.md +++ b/docs/django-admin/roles.md @@ -17,4 +17,5 @@ Staff auditlog | log entry | can view log entry registrar | contact | can view contact registrar | domain application | can change domain application -registrar | domain | can view domain \ No newline at end of file +registrar | domain | can view domain +registrar | user | can view user \ No newline at end of file diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7a3647582..784da0564 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,6 +1,6 @@ import logging from django.contrib import admin, messages -from django.contrib.auth.admin import UserAdmin +from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse @@ -93,12 +93,29 @@ class UserContactInline(admin.StackedInline): model = models.Contact -class MyUserAdmin(UserAdmin): +class MyUserAdmin(BaseUserAdmin): """Custom user admin class to use our inlines.""" inlines = [UserContactInline] + def get_list_display(self, request): + if not request.user.is_superuser: + # Customize the list display for staff users + return ("email", "first_name", "last_name", "is_staff", "is_superuser") + else: + # Use the default list display for non-staff users + return super().get_list_display(request) + + def get_fieldsets(self, request, obj=None): + if not request.user.is_superuser: + # If the user doesn't have permission to change the model, + # show a read-only fieldset + return ((None, {"fields": []}),) + + # If the user has permission to change the model, show all fields + return super().get_fieldsets(request, obj) + class HostIPInline(admin.StackedInline): diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index b47ed4aef..35a72ab15 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -85,6 +85,7 @@ class UserFixture: "permissions": ["change_domainapplication"], }, {"app_label": "registrar", "model": "domain", "permissions": ["view_domain"]}, + {"app_label": "registrar", "model": "user", "permissions": ["view_user"]}, ] @classmethod diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index c89f36563..a613ba648 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -88,6 +88,37 @@ class MockSESClient(Mock): self.EMAILS_SENT.append({"args": args, "kwargs": kwargs}) +def mock_user(): + """A simple user.""" + user_kwargs = dict( + id=4, + first_name="Rachid", + last_name="Mrad", + ) + mock_user, _ = User.objects.get_or_create(**user_kwargs) + return mock_user + + +def create_superuser(self): + User = get_user_model() + p = "adminpass" + return User.objects.create_superuser( + username="superuser", + email="admin@example.com", + password=p, + ) + + +def create_user(self): + User = get_user_model() + p = "userpass" + return User.objects.create_user( + username="staffuser", + email="user@example.com", + password=p, + ) + + def completed_application( has_other_contacts=True, has_current_website=True, @@ -157,16 +188,3 @@ def completed_application( application.alternative_domains.add(alt) return application - - -def mock_user(): - """A simple user.""" - user_kwargs = dict( - id=4, - first_name="Rachid", - last_name="Mrad", - ) - - user, _ = User.objects.get_or_create(**user_kwargs) - - return user diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index d5396a829..0b3b6bc1e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,8 +1,8 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin +from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin from registrar.models import DomainApplication, DomainInformation, User -from .common import completed_application, mock_user +from .common import completed_application, mock_user, create_superuser, create_user from django.contrib.auth import get_user_model from django.conf import settings @@ -14,21 +14,9 @@ class TestDomainApplicationAdmin(TestCase): def setUp(self): self.site = AdminSite() self.factory = RequestFactory() - self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) - self.client = Client(HTTP_HOST="localhost:8080") - username = "admin" - first_name = "First" - last_name = "Last" - email = "info@example.com" - p = "adminpassword" - User = get_user_model() - self.superuser = User.objects.create_superuser( - username=username, - first_name=first_name, - last_name=last_name, - email=email, - password=p, - ) + # self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) + # self.client = Client(HTTP_HOST="localhost:8080") + # self.superuser = create_superuser(self) @boto3_mocking.patching def test_save_model_sends_submitted_email(self): @@ -179,10 +167,23 @@ class TestDomainApplicationAdmin(TestCase): DomainInformation.objects.get(id=application.pk).delete() application.delete() + def tearDown(self): + DomainApplication.objects.all().delete() + User.objects.all().delete() + + +class ListHeaderAdminTest(TestCase): + def setUp(self): + self.site = AdminSite() + self.factory = RequestFactory() + self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) + self.client = Client(HTTP_HOST="localhost:8080") + self.superuser = create_superuser(self) + def test_changelist_view(self): # Have to get creative to get past linter - p = "adminpassword" - self.client.login(username="admin", password=p) + p = "adminpass" + self.client.login(username="superuser", password=p) # Mock a user user = mock_user() @@ -240,7 +241,44 @@ class TestDomainApplicationAdmin(TestCase): ) def tearDown(self): - # delete any applications too - DomainApplication.objects.all().delete() User.objects.all().delete() self.superuser.delete() + + +class MyUserAdminTest(TestCase): + def setUp(self): + admin_site = AdminSite() + self.admin = MyUserAdmin(model=get_user_model(), admin_site=admin_site) + + def test_list_display_without_username(self): + request = self.client.request().wsgi_request + request.user = create_user(self) + + list_display = self.admin.get_list_display(request) + expected_list_display = ( + "email", + "first_name", + "last_name", + "is_staff", + "is_superuser", + ) + + self.assertEqual(list_display, expected_list_display) + self.assertNotIn("username", list_display) + + def test_get_fieldsets_superuser(self): + request = self.client.request().wsgi_request + request.user = create_superuser(self) + fieldsets = self.admin.get_fieldsets(request) + expected_fieldsets = super(MyUserAdmin, self.admin).get_fieldsets(request) + self.assertEqual(fieldsets, expected_fieldsets) + + def test_get_fieldsets_non_superuser(self): + request = self.client.request().wsgi_request + request.user = create_user(self) + fieldsets = self.admin.get_fieldsets(request) + expected_fieldsets = ((None, {"fields": []}),) + self.assertEqual(fieldsets, expected_fieldsets) + + def tearDown(self): + User.objects.all().delete() From a92ca84ccb7520a86897b40022231e2578942257 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 7 Jul 2023 11:09:15 -0400 Subject: [PATCH 2/3] cleanup --- src/registrar/tests/test_admin.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 0b3b6bc1e..cd499e306 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -14,9 +14,6 @@ class TestDomainApplicationAdmin(TestCase): def setUp(self): self.site = AdminSite() self.factory = RequestFactory() - # self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) - # self.client = Client(HTTP_HOST="localhost:8080") - # self.superuser = create_superuser(self) @boto3_mocking.patching def test_save_model_sends_submitted_email(self): From 616b9345f1df6e077f2a8bba40b58d685e0fd34d Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 13 Jul 2023 13:15:07 -0400 Subject: [PATCH 3/3] Remove unecessary self argument --- src/registrar/admin.py | 6 +++--- src/registrar/tests/common.py | 4 ++-- src/registrar/tests/test_admin.py | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 784da0564..5108cfd8e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -103,9 +103,9 @@ class MyUserAdmin(BaseUserAdmin): if not request.user.is_superuser: # Customize the list display for staff users return ("email", "first_name", "last_name", "is_staff", "is_superuser") - else: - # Use the default list display for non-staff users - return super().get_list_display(request) + + # Use the default list display for non-staff users + return super().get_list_display(request) def get_fieldsets(self, request, obj=None): if not request.user.is_superuser: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a613ba648..4359fc454 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -99,7 +99,7 @@ def mock_user(): return mock_user -def create_superuser(self): +def create_superuser(): User = get_user_model() p = "adminpass" return User.objects.create_superuser( @@ -109,7 +109,7 @@ def create_superuser(self): ) -def create_user(self): +def create_user(): User = get_user_model() p = "userpass" return User.objects.create_user( diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index cd499e306..db875324e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -175,7 +175,7 @@ class ListHeaderAdminTest(TestCase): self.factory = RequestFactory() self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) self.client = Client(HTTP_HOST="localhost:8080") - self.superuser = create_superuser(self) + self.superuser = create_superuser() def test_changelist_view(self): # Have to get creative to get past linter @@ -249,7 +249,7 @@ class MyUserAdminTest(TestCase): def test_list_display_without_username(self): request = self.client.request().wsgi_request - request.user = create_user(self) + request.user = create_user() list_display = self.admin.get_list_display(request) expected_list_display = ( @@ -265,14 +265,14 @@ class MyUserAdminTest(TestCase): def test_get_fieldsets_superuser(self): request = self.client.request().wsgi_request - request.user = create_superuser(self) + request.user = create_superuser() fieldsets = self.admin.get_fieldsets(request) expected_fieldsets = super(MyUserAdmin, self.admin).get_fieldsets(request) self.assertEqual(fieldsets, expected_fieldsets) def test_get_fieldsets_non_superuser(self): request = self.client.request().wsgi_request - request.user = create_user(self) + request.user = create_user() fieldsets = self.admin.get_fieldsets(request) expected_fieldsets = ((None, {"fields": []}),) self.assertEqual(fieldsets, expected_fieldsets)