Changed permission flow for mixins / Made logging a bit more detailed

src/registrar/views/domain.py

@@ -83,10 +83,10 @@ class DomainOrgNameAddressView(DomainPermissionView, FormMixin):
         # Q: Is there a more efficent way to do this?
         # It would be ideal if we didn't have to repeat this.
         if self.request.user.is_staff or self.request.user.is_superuser:
-            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info, changes
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         # superclass has the redirect
@@ -134,8 +134,9 @@ class DomainAuthorizingOfficialView(DomainPermissionView, FormMixin):
 
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         # superclass has the redirect
@@ -207,8 +208,9 @@ class DomainNameserversView(DomainPermissionView, FormMixin):
 
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: formset.cleaned_data[field] for field in formset.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         # superclass has the redirect
@@ -254,8 +256,9 @@ class DomainYourContactInformationView(DomainPermissionView, FormMixin):
 
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         # superclass has the redirect
@@ -306,8 +309,9 @@ class DomainSecurityEmailView(DomainPermissionView, FormMixin):
 
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         # superclass has the redirect
@@ -388,7 +392,7 @@ class DomainAddUserView(DomainPermissionView, FormMixin):
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, obj=self.get_object()
             )
         return redirect(self.get_success_url())
 
@@ -414,8 +418,9 @@ class DomainAddUserView(DomainPermissionView, FormMixin):
 
         if self.request.user.is_staff or self.request.user.is_superuser:
             # if they are editing from an '/admin' redirect, log their actions
+            changes = {field: form.cleaned_data[field] for field in form.changed_data}
             self.log_analyst_form_actions(
-                self.form_class.__name__, self.get_object().domain_info
+                self.form_class.__name__, self.get_object().domain_info, changes, obj=self.get_object()
             )
 
         return redirect(self.get_success_url())

src/registrar/views/utility/mixins.py

@@ -36,27 +36,55 @@ class DomainPermission(PermissionsLoginMixin):
         if not self.request.user.is_authenticated:
             return False
 
+        if self.request.user.is_restricted():
+            return False
+
         pk = self.kwargs["pk"]
         # If pk is none then something went very wrong...
         if pk is None:
             raise ValueError("Primary key is None")
 
+        # ticket 806
+        if self.can_access_other_user_domains(pk):
+            return True
+
         # user needs to have a role on the domain,
         # and user cannot be restricted
-        if (
+        if not UserDomainRole.objects.filter(
-            UserDomainRole.objects.filter(
             user=self.request.user, domain__id=pk
-            ).exists()
+        ).exists():
-            and not self.request.user.is_restricted()
-        ):
-            return True
-        elif self.request.user.is_restricted():
             return False
 
-        # ticket 806
+        # if we need to check more about the nature of role, do it here.
-        requested_domain = None
+        return True
-        if DomainInformation.objects.filter(id=pk).exists():
+
-            requested_domain = DomainInformation.objects.get(id=pk)
+    def can_access_other_user_domains(self, pk):
+        """Checks to see if an authorized user (staff or superuser)
+        can access a domain that they did not create or was invited to.
+        """
+
+        # Check if the user is permissioned...
+        user_is_analyst_or_superuser = (
+            self.request.user.is_staff or self.request.user.is_superuser
+        )
+        logger.debug(f"is auth {user_is_analyst_or_superuser}")
+
+        if not user_is_analyst_or_superuser:
+            return False
+
+        # Check if the user is attempting a valid edit action.
+        # In other words, if the analyst/admin did not click
+        # the 'Manage Domain' button in /admin,
+        # then they cannot access this page.
+        session = self.request.session
+        can_do_action = (
+            "analyst_action" in session
+            and "analyst_action_location" in session
+            and session["analyst_action_location"] == pk
+        )
+        logger.debug(f"can do {can_do_action}")
+        if not can_do_action:
+            return False
 
         # Analysts may manage domains, when they are in these statuses:
         valid_domain_statuses = [
@@ -64,38 +92,24 @@ class DomainPermission(PermissionsLoginMixin):
             DomainApplication.IN_REVIEW,
             DomainApplication.REJECTED,
             DomainApplication.ACTION_NEEDED,
+            # Edge case - some domains do not have
+            # a status or DomainInformation... aka a status of 'None'.
+            # It is necessary to access those to correct errors.
+            None,
         ]
 
-        # Check if the user is permissioned...
+        requested_domain = None
-        user_is_analyst_or_superuser = (
+        if DomainInformation.objects.filter(id=pk).exists():
-            self.request.user.is_staff or self.request.user.is_superuser
+            requested_domain = DomainInformation.objects.get(id=pk)
-        )
 
-        session = self.request.session
+        if not requested_domain.domain_application.status in valid_domain_statuses:
-        # Check if the user is attempting a valid edit action.
-        can_do_action = (
-            "analyst_action" in session
-            and "analyst_action_location" in session
-            and session["analyst_action_location"] == pk
-        )
-
-        # Edge case - some domains do not have
-        # a status or DomainInformation... aka a status of 'None'
-        # This checks that it has a status, before checking if it does
-        # Otherwise, analysts can edit these domains
-        if requested_domain is not None:
-            can_do_action = (
-                can_do_action
-                and requested_domain.domain_application.status in valid_domain_statuses
-            )
-        # If the valid session keys exist, if the user is permissioned,
-        # and if its in a valid status
-        if can_do_action and user_is_analyst_or_superuser:
-            return True
-
-        # if we need to check more about the nature of role, do it here.
             return False
 
+        # Valid session keys exist,
+        # the user is permissioned,
+        # and it is in a valid status
+        return True
+
 
 class DomainApplicationPermission(PermissionsLoginMixin):
 

src/registrar/views/utility/permission_views.py

@@ -3,9 +3,9 @@
 import abc  # abstract base class
 
 from django.views.generic import DetailView, DeleteView, TemplateView
-
+from django.contrib.contenttypes.models import ContentType
 from registrar.models import Domain, DomainApplication, DomainInvitation
-
+from django.contrib.admin.models import LogEntry, CHANGE
 
 from .mixins import (
     DomainPermission,
@@ -48,7 +48,7 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC):
         return context
 
     def log_analyst_form_actions(
-        self, form_class_name, printable_object_info, changes=None
+        self, form_class_name, printable_object_info, changes=None, obj=None
     ):
         """Generates a log for when key 'analyst_action' exists on the session.
         Follows this format:
@@ -72,12 +72,20 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC):
             # or 'copy', for instance.
             match action:
                 case "edit":
-                    # Q: do we want to be logging on every changed field?
+                    if obj is not None:
-                    # I could see that becoming spammy log-wise,
+                        content_type = ContentType.objects.get_for_model(obj)
-                    # but it may also be important.
+                        LogEntry.objects.log_action(
+                            user_id=self.request.user.id,
+                            content_type_id=content_type.pk,
+                            object_id=obj.id,
+                            object_repr=str(obj),
+                            action_flag=CHANGE,
+                        )
+
                     if changes is not None:
-                        # Logs every change made to the domain field
+                        # Logs every change made to the domain field.
-                        # noqa for readability/format
+                        # noqa for readability/format.
+                        # Used to manually capture changes, if need be.
                         for field, new_value in changes.items():
                             logger.info(
                                 f"""