From 1ca3472195e5c9a181f27689efaa9da298f92e2d Mon Sep 17 00:00:00 2001 From: JeanMarie Mariadassou Date: Fri, 6 Oct 2023 10:41:17 -0700 Subject: [PATCH 1/4] First working version of staff admin screens --- backend/audit/admin.py | 20 ++++++ ...uditchecklist_cognizant_agency_and_more.py | 22 ++++++ backend/audit/models.py | 4 +- backend/support/admin.py | 10 +-- backend/users/admin.py | 56 ++++++++++++++- .../migrations/0002_staffuser_staffuserlog.py | 68 +++++++++++++++++++ backend/users/models.py | 58 ++++++++++++++++ backend/users/signals.py | 6 +- 8 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py create mode 100644 backend/users/migrations/0002_staffuser_staffuserlog.py diff --git a/backend/audit/admin.py b/backend/audit/admin.py index 291a18719c..362d63e181 100644 --- a/backend/audit/admin.py +++ b/backend/audit/admin.py @@ -4,7 +4,27 @@ class SACAdmin(admin.ModelAdmin): + def has_module_permission(self, request, obj=None): + return request.user.is_staff + + def has_view_permission(self, request, obj=None): + return request.user.is_staff + list_display = ("id", "report_id") + fields = ( + "report_id", + ( + "Cog_Agency", + "OSight_Agency", + ), + "general_information", + ) + + def Cog_Agency(self, obj): + return obj.cognizant_agency + + def OSight_Agency(self, obj): + return obj.oversight_agency class AccessAdmin(admin.ModelAdmin): diff --git a/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py b/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py new file mode 100644 index 0000000000..4e108ceb54 --- /dev/null +++ b/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.5 on 2023-10-06 16:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("audit", "0003_alter_singleauditchecklist_data_source_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="singleauditchecklist", + name="cognizant_agency", + field=models.CharField(blank=True, max_length=2, null=True), + ), + migrations.AlterField( + model_name="singleauditchecklist", + name="oversight_agency", + field=models.CharField(blank=True, max_length=2, null=True), + ), + ] diff --git a/backend/audit/models.py b/backend/audit/models.py index 0a0f2e84e1..cd5dc9ecbc 100644 --- a/backend/audit/models.py +++ b/backend/audit/models.py @@ -390,13 +390,13 @@ class STATUS: ) cognizant_agency = models.CharField( - "Agency assigned to this large submission. Computed when the submisson is finalized, but may be overridden", + # "Agency assigned to this large submission. Computed when the submisson is finalized, but may be overridden", max_length=2, blank=True, null=True, ) oversight_agency = models.CharField( - "Agency assigned to this not so large submission. Computed when the submisson is finalized", + # "Agency assigned to this not so large submission. Computed when the submisson is finalized", max_length=2, blank=True, null=True, diff --git a/backend/support/admin.py b/backend/support/admin.py index 1d8a776349..59905d733e 100644 --- a/backend/support/admin.py +++ b/backend/support/admin.py @@ -5,9 +5,11 @@ class SupportAdmin(admin.ModelAdmin): + def has_module_permission(self, request, obj=None): + return request.user.is_staff + def has_view_permission(self, request, obj=None): - if request.user.is_staff: - return True + return request.user.is_staff def has_delete_permission(self, request, obj=None): return False @@ -22,12 +24,12 @@ class CognizantBaselineAdmin(SupportAdmin): "ein", "dbkey", "is_active", + "source", ] list_filter = [ "is_active", - "uei", + "source", "cognizant_agency", - "ein", ] def has_change_permission(self, request, obj=None): diff --git a/backend/users/admin.py b/backend/users/admin.py index f5bccb6d3f..079c46070d 100644 --- a/backend/users/admin.py +++ b/backend/users/admin.py @@ -1,5 +1,59 @@ from django.contrib import admin -from .models import UserProfile +from .models import StaffUserLog, UserProfile, StaffUser admin.site.register(UserProfile) + + +@admin.register(StaffUserLog) +class StaffUserLogAdmin(admin.ModelAdmin): + list_display = [ + "staff_email", + "added_by_email", + "date_added", + "removed_by_email", + "date_removed", + ] + + def has_add_permission(self, request, obj=None): + return False + + def has_change_permission(self, request, obj=None): + return False + + def has_delete_permission(self, request, obj=None): + return False + + +@admin.register(StaffUser) +class StaffUserAdmin(admin.ModelAdmin): + list_display = [ + "staff_email", + "added_by_email", + "date_added", + ] + fields = [ + "staff_email", + ] + + def save_model(self, request, obj, form, change): + obj.added_by_email = request.user.email + super().save_model(request, obj, form, change) + + def delete_model(self, request, obj): + StaffUserLog( + staff_email=obj.staff_email, + added_by_email=obj.added_by_email, + date_added=obj.date_added, + removed_by_email=request.user.email, + ).save() + super().delete_model(request, obj) + + def has_module_permission(self, request, obj=None): + return request.user.is_staff + + def has_view_permission(self, request, obj=None): + return request.user.is_staff + + def has_change_permission(self, request, obj=None): + return False diff --git a/backend/users/migrations/0002_staffuser_staffuserlog.py b/backend/users/migrations/0002_staffuser_staffuserlog.py new file mode 100644 index 0000000000..e8fd3fcf33 --- /dev/null +++ b/backend/users/migrations/0002_staffuser_staffuserlog.py @@ -0,0 +1,68 @@ +# Generated by Django 4.2.5 on 2023-10-06 16:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("users", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="StaffUser", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("staff_email", models.EmailField(max_length=254)), + ( + "date_added", + models.DateTimeField(auto_now_add=True, verbose_name="Date Added"), + ), + ( + "added_by_email", + models.EmailField(max_length=254, verbose_name="Email of Adder"), + ), + ], + ), + migrations.CreateModel( + name="StaffUserLog", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("staff_email", models.EmailField(max_length=254)), + ( + "date_added", + models.DateTimeField(auto_now_add=True, verbose_name="Date Added"), + ), + ( + "added_by_email", + models.EmailField(max_length=254, verbose_name="Email of Adder"), + ), + ( + "date_removed", + models.DateTimeField( + auto_now_add=True, verbose_name="Date removed" + ), + ), + ( + "removed_by_email", + models.EmailField(max_length=254, verbose_name="Email of Remover"), + ), + ], + ), + ] diff --git a/backend/users/models.py b/backend/users/models.py index ca988a8792..1db2f3191f 100644 --- a/backend/users/models.py +++ b/backend/users/models.py @@ -16,3 +16,61 @@ class UserProfile(models.Model): def __str__(self): return self.user.email + + +class StaffUser(models.Model): + """ + List of email addresses maintained by a superuser using an admin screen. + When a login email matches an entry here, the corresponding user is upgraded to a staff user. + Should have only one active row per staff_email. + """ + + staff_email = models.EmailField() + date_added = models.DateTimeField( + auto_now_add=True, + verbose_name="Date Added", + ) + added_by_email = models.EmailField( + verbose_name="Email of Adder", + ) + + def save(self, *args, **kwargs): + try: + user = User.objects.get(email=self.staff_email) + user.is_staff = True + user.save() + except User.DoesNotExist: + pass # silently ignore. User may be created later. + super(StaffUser, self).save(*args, **kwargs) + + def delete(self, *args, **kwargs): + try: + user = User.objects.get(email=self.staff_email) + user.is_staff = False + user.save() + except User.DoesNotExist: + pass # silently ignore. Nothing to do. + super(StaffUser, self).delete(*args, **kwargs) + + +class StaffUserLog(models.Model): + """ + Tracks removals from StaffUser + A row is inserted whenever StaffUser is deleted in admin.py + """ + + staff_email = models.EmailField() + date_added = models.DateTimeField( + auto_now_add=True, + verbose_name="Date Added", + ) + added_by_email = models.EmailField( + verbose_name="Email of Adder", + ) + date_removed = models.DateTimeField( + auto_now_add=True, + verbose_name="Date removed", + ) + removed_by_email = models.EmailField( + verbose_name="Email of Remover", + ) diff --git a/backend/users/signals.py b/backend/users/signals.py index 14a93f8969..b89a979f64 100644 --- a/backend/users/signals.py +++ b/backend/users/signals.py @@ -2,7 +2,7 @@ from django.db.models.signals import post_save from django.dispatch import receiver -from .models import UserProfile +from .models import UserProfile, StaffUser User = get_user_model() @@ -11,3 +11,7 @@ def create_user_profile(sender, instance, created, **kwargs): if created: UserProfile.objects.get_or_create(user=instance) + staff_emails = StaffUser.objects.all().values("staff_email") + if instance.email in staff_emails: + instance.is_staff = True + instance.save() From f28df4c25ddab652722145b9ca6e5d3a751b639c Mon Sep 17 00:00:00 2001 From: JeanMarie Mariadassou Date: Fri, 6 Oct 2023 12:22:03 -0700 Subject: [PATCH 2/4] Include cog_over filter in SAC --- backend/audit/admin.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/backend/audit/admin.py b/backend/audit/admin.py index 362d63e181..909f2b851a 100644 --- a/backend/audit/admin.py +++ b/backend/audit/admin.py @@ -10,22 +10,26 @@ def has_module_permission(self, request, obj=None): def has_view_permission(self, request, obj=None): return request.user.is_staff - list_display = ("id", "report_id") + list_display = ( + "id", + "report_id", + "cognizant_agency", + "oversight_agency", + ) + list_filter = [ + "cognizant_agency", + "oversight_agency", + ] + fields = ( "report_id", ( - "Cog_Agency", - "OSight_Agency", + "cognizant_agency", + "oversight_agency", ), "general_information", ) - def Cog_Agency(self, obj): - return obj.cognizant_agency - - def OSight_Agency(self, obj): - return obj.oversight_agency - class AccessAdmin(admin.ModelAdmin): """ From 4fbe405f26204fab253bc48adafc51c648cce457 Mon Sep 17 00:00:00 2001 From: JeanMarie Mariadassou Date: Wed, 11 Oct 2023 14:49:39 -0700 Subject: [PATCH 3/4] Implement tests --- ...uditchecklist_cognizant_agency_and_more.py | 18 ++- backend/audit/models.py | 6 +- backend/support/test_admin.py | 103 ++++++++++++++++++ backend/users/admin.py | 6 + backend/users/signals.py | 4 +- 5 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 backend/support/test_admin.py diff --git a/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py b/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py index 4e108ceb54..8455400a6d 100644 --- a/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py +++ b/backend/audit/migrations/0004_alter_singleauditchecklist_cognizant_agency_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.5 on 2023-10-06 16:37 +# Generated by Django 4.2.5 on 2023-10-11 15:05 from django.db import migrations, models @@ -12,11 +12,23 @@ class Migration(migrations.Migration): migrations.AlterField( model_name="singleauditchecklist", name="cognizant_agency", - field=models.CharField(blank=True, max_length=2, null=True), + field=models.CharField( + blank=True, + help_text="Agency assigned to this large submission. Computed when the submisson is finalized, but may be overridden", + max_length=2, + null=True, + verbose_name="Cog Agency", + ), ), migrations.AlterField( model_name="singleauditchecklist", name="oversight_agency", - field=models.CharField(blank=True, max_length=2, null=True), + field=models.CharField( + blank=True, + help_text="Agency assigned to this not so large submission. Computed when the submisson is finalized", + max_length=2, + null=True, + verbose_name="OSight Agency", + ), ), ] diff --git a/backend/audit/models.py b/backend/audit/models.py index cd5dc9ecbc..201127af78 100644 --- a/backend/audit/models.py +++ b/backend/audit/models.py @@ -390,16 +390,18 @@ class STATUS: ) cognizant_agency = models.CharField( - # "Agency assigned to this large submission. Computed when the submisson is finalized, but may be overridden", + help_text="Agency assigned to this large submission. Computed when the submisson is finalized, but may be overridden", max_length=2, blank=True, null=True, + verbose_name="Cog Agency", ) oversight_agency = models.CharField( - # "Agency assigned to this not so large submission. Computed when the submisson is finalized", + help_text="Agency assigned to this not so large submission. Computed when the submisson is finalized", max_length=2, blank=True, null=True, + verbose_name="OSight Agency", ) def validate_full(self): diff --git a/backend/support/test_admin.py b/backend/support/test_admin.py new file mode 100644 index 0000000000..644b1e842c --- /dev/null +++ b/backend/support/test_admin.py @@ -0,0 +1,103 @@ +from django.test import TestCase, Client +from django.contrib.auth import get_user_model +from django.urls import reverse + +from users.models import StaffUser, StaffUserLog + +User = get_user_model() + + +class StaffUserAdminTest(TestCase): + def setUp(self): + self.client = Client() + self.superuser = User.objects.create_superuser( + username="roor", + password="testpassword", + email="root@test.com", + ) + self.user = User( + username="testuser", password="testpassword", email="testemail@test.com" + ) + self.url_prefix = "admin:users_staffuser" + + def test_make_user_staff_then_create_user(self): + # first test that a super user can add a staff user + self.client.force_login(self.superuser) + self.client.post( + reverse(f"{self.url_prefix}_add"), + data={"staff_email": self.user.email}, + ) + staff = StaffUser.objects.filter(staff_email=self.user.email) + self.assertEqual(len(staff), 1) + self.assertEqual(staff.first().added_by_email, self.superuser.email) + + # test that when the user is now created, they have is_staff set to True + User.objects.create_user( + username=self.user.username, + password=self.user.password, + email=self.user.email, + ) + added_users = User.objects.filter(email=self.user.email) + self.assertEqual(len(added_users), 1) + added_user = added_users.first() + self.assertTrue(added_user.is_staff) + + # test that a staff user can view the list + self.client.force_login(added_user) + response = self.client.get(reverse(f"{self.url_prefix}_changelist")) + self.assertContains(response, added_user.email) + + # test that a staff user cannot add to the lisy + response = self.client.post( + reverse(f"{self.url_prefix}_add"), + data={"staff_email": "another@test.gov"}, + ) + self.assertGreaterEqual(response.status_code, 400) + + def test_create_user_then_make_user_staff(self): + User.objects.create_user( + username=self.user.username, + password=self.user.password, + email=self.user.email, + ) + self.client.force_login(self.superuser) + self.client.post( + reverse(f"{self.url_prefix}_add"), + data={"staff_email": self.user.email}, + ) + added_users = User.objects.filter(email=self.user.email) + self.assertEqual(len(added_users), 1) + added_user = added_users.first() + self.assertTrue(added_user.is_staff) + + def test_make_user_nonstaff(self): + User.objects.create_user( + username=self.user.username, + password=self.user.password, + email=self.user.email, + ) + self.client.force_login(self.superuser) + self.client.post( + reverse(f"{self.url_prefix}_add"), + data={"staff_email": self.user.email}, + ) + staff = StaffUser.objects.get(staff_email=self.user.email) + + response = self.client.post( + reverse(f"{self.url_prefix}_delete", kwargs={"object_id": staff.pk}) + # reverse(f"{self.url_prefix}_changelist"), + # data={"action": "delete_selected", "_selected_action": [staff.pk]}, + ) + csrf_token = response.context["csrf_token"] + post_data = { + "csrfmiddlewaretoken": str(csrf_token), + "post": "yes", # This indicates confirmation of deletion + } + self.client.post( + reverse(f"{self.url_prefix}_delete", kwargs={"object_id": staff.pk}), + data=post_data, + ) + staff = StaffUser.objects.filter(staff_email=self.user.email) + self.assertEqual(len(staff), 0) + staff_logs = StaffUserLog.objects.filter(staff_email=self.user.email) + self.assertEqual(len(staff_logs), 1) diff --git a/backend/users/admin.py b/backend/users/admin.py index 079c46070d..d554a0b090 100644 --- a/backend/users/admin.py +++ b/backend/users/admin.py @@ -57,3 +57,9 @@ def has_view_permission(self, request, obj=None): def has_change_permission(self, request, obj=None): return False + + def has_add_permission(self, request, obj=None): + return request.user.is_superuser + + def has_delete_permission(self, request, obj=None): + return request.user.is_superuser diff --git a/backend/users/signals.py b/backend/users/signals.py index b89a979f64..06d3104878 100644 --- a/backend/users/signals.py +++ b/backend/users/signals.py @@ -11,7 +11,9 @@ def create_user_profile(sender, instance, created, **kwargs): if created: UserProfile.objects.get_or_create(user=instance) - staff_emails = StaffUser.objects.all().values("staff_email") + staff_emails = list( + StaffUser.objects.all().values_list("staff_email", flat=True) + ) if instance.email in staff_emails: instance.is_staff = True instance.save() From cee11900f132b8de4166c1f5edacc9014140dd17 Mon Sep 17 00:00:00 2001 From: JeanMarie Mariadassou Date: Wed, 11 Oct 2023 17:32:47 -0700 Subject: [PATCH 4/4] Fix linting issue with static passwords --- backend/support/admin.py | 8 +++----- backend/support/test_admin.py | 8 ++++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/backend/support/admin.py b/backend/support/admin.py index 59905d733e..6d1012f701 100644 --- a/backend/support/admin.py +++ b/backend/support/admin.py @@ -93,7 +93,7 @@ def change_view(self, request, object_id, form_url="", extra_context=None): extra_context["show_save"] = False extra_context[ "show_save_and_add_another" - ] = False # this not works if has_add_permision is True + ] = False # this does not work if has_add_permision is True return super().change_view(request, object_id, extra_context=extra_context) def save_model(self, request, obj, form, change): @@ -102,9 +102,7 @@ def save_model(self, request, obj, form, change): super().save_model(request, obj, form, change) def has_change_permission(self, request, obj=None): - if request.user.is_staff: - return True + return request.user.is_staff def has_add_permission(self, request, obj=None): - if request.user.is_staff: - return True + return request.user.is_staff diff --git a/backend/support/test_admin.py b/backend/support/test_admin.py index 644b1e842c..537cc09d81 100644 --- a/backend/support/test_admin.py +++ b/backend/support/test_admin.py @@ -1,3 +1,6 @@ +import hashlib + + from django.test import TestCase, Client from django.contrib.auth import get_user_model from django.urls import reverse @@ -5,6 +8,7 @@ from users.models import StaffUser, StaffUserLog User = get_user_model() +PASSWORD = hashlib.sha256("password".encode()).hexdigest() class StaffUserAdminTest(TestCase): @@ -12,11 +16,11 @@ def setUp(self): self.client = Client() self.superuser = User.objects.create_superuser( username="roor", - password="testpassword", + password=PASSWORD, email="root@test.com", ) self.user = User( - username="testuser", password="testpassword", email="testemail@test.com" + username="testuser", password=PASSWORD, email="testemail@test.com" ) self.url_prefix = "admin:users_staffuser"