diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 26002a5e8..98d1cc553 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -145,7 +145,6 @@ class DomainAdmin(ListHeaderAdmin): PLACE_HOLD = "_place_client_hold" EDIT_DOMAIN = "_edit_domain" if PLACE_HOLD in request.POST: - logger.debug("Hit!") try: obj.place_client_hold() except Exception as err: @@ -163,6 +162,9 @@ class DomainAdmin(ListHeaderAdmin): elif EDIT_DOMAIN in request.POST: # We want to know, globally, when an edit action occurs request.session['analyst_action'] = 'edit' + # Restricts this action to this domain only + request.session['analyst_action_location'] = obj.id + return HttpResponseRedirect(reverse('domain', args=(obj.id,))) return super().response_change(request, obj) # Sets domain_id as a context var @@ -171,6 +173,9 @@ class DomainAdmin(ListHeaderAdmin): # If an analyst performed an edit action, # delete the session variable del request.session['analyst_action'] + # delete the associated location + del request.session['analyst_action_location'] + extra_context = extra_context or {} extra_context["domain_id"] = object_id return super().change_view( diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 6274767fe..5eee3e41b 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -79,26 +79,9 @@ class DomainOrgNameAddressView(DomainPermissionView, FormMixin): self.request, "The organization name and mailing address has been updated." ) - # If the user is not privileged, don't do any special checks - if not self.request.user.is_staff and not self.request.user.is_superuser: - # superclass has the redirect - return super().form_valid(form) - - # Otherwise, if they are editing from an '/admin' redirect, log their actions - # Q: do we want to be logging on every changed field? - # I could see that becoming spammy log-wise, but it may also be important. - # To do so, I'd likely have to override some of the save() functionality of ModelForm. - if 'analyst_action' in self.request.session: - action = self.request.session['analyst_action'] - - # Template for future expansion, - # in the event we want more logging granularity. - # Could include things such as 'view' - # or 'copy', for instance. - match action: - case 'edit': - if(self.request.user.is_staff): - logger.info("Analyst {} edited {} for {}".format(self.request.user, type(form_class).__name__, self.get_object().domain_info)) + 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) # superclass has the redirect return super().form_valid(form) @@ -142,6 +125,11 @@ class DomainAuthorizingOfficialView(DomainPermissionView, FormMixin): messages.success( self.request, "The authorizing official for this domain has been updated." ) + + 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) + # superclass has the redirect return super().form_valid(form) @@ -208,6 +196,11 @@ class DomainNameserversView(DomainPermissionView, FormMixin): messages.success( self.request, "The name servers for this domain have been updated." ) + + 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) + # superclass has the redirect return super().form_valid(formset) @@ -248,6 +241,11 @@ class DomainYourContactInformationView(DomainPermissionView, FormMixin): messages.success( self.request, "Your contact information for this domain has been updated." ) + + 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) + # superclass has the redirect return super().form_valid(form) @@ -293,6 +291,11 @@ class DomainSecurityEmailView(DomainPermissionView, FormMixin): messages.success( self.request, "The security email for this domain have been updated." ) + + 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) + # superclass has the redirect return redirect(self.get_success_url()) @@ -368,6 +371,9 @@ class DomainAddUserView(DomainPermissionView, FormMixin): messages.success( self.request, f"Invited {email_address} to this domain." ) + 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) return redirect(self.get_success_url()) def form_valid(self, form): @@ -389,6 +395,11 @@ class DomainAddUserView(DomainPermissionView, FormMixin): pass messages.success(self.request, f"Added user {requested_email}.") + + 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) + return redirect(self.get_success_url()) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index f1acf4265..33d70ae32 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -44,13 +44,13 @@ class DomainPermission(PermissionsLoginMixin): ) # This should never happen in normal flow. - # If it does, then it likely means something bad happened... + # That said, it does need to be raised here. except DomainInformation.DoesNotExist: raise Http404() # Checks if the creator is the user requesting this item user_is_creator: bool = requested_domain.creator.username == self.request.user.username - + # user needs to have a role on the domain if user_is_creator: return True @@ -58,10 +58,18 @@ class DomainPermission(PermissionsLoginMixin): # ticket 806 # Analysts may manage domains, when they are in these statuses: valid_domain_statuses = [DomainApplication.APPROVED, DomainApplication.IN_REVIEW, DomainApplication.REJECTED, DomainApplication.ACTION_NEEDED] + # Check if the user is permissioned... user_is_analyst_or_superuser = self.request.user.is_staff or self.request.user.is_superuser - if user_is_analyst_or_superuser and requested_domain.domain_application.status in valid_domain_statuses: + session = self.request.session + + # Check if the user is attempting a valid edit action. + # If analyst_action is present, analyst_action_location will be present. + # if it isn't, then it either suggests tampering or a larger omnipresent issue with sessions. + can_do_action = 'analyst_action' in session and session['analyst_action_location'] == pk + + if can_do_action and user_is_analyst_or_superuser and requested_domain.domain_application.status in valid_domain_statuses: return True # ticket 796 diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 30a421ce9..faabc171d 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -43,11 +43,31 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC): # Flag to see if an analyst is attempting to make edits if 'analyst_action' in self.request.session: context['analyst_action'] = self.request.session['analyst_action'] - # Clear the session variable after use - # del self.request.session['analyst_action'] return context + def log_analyst_form_actions(self, form_class_name, printable_object_info): + """ Generates a log for when 'analyst_action' exists on the session """ + if 'analyst_action' in self.request.session: + action = self.request.session['analyst_action'] + + user_type = "Analyst" + if(self.request.user.is_superuser): + user_type = "Superuser" + + # Template for potential future expansion, + # in the event we want more logging granularity. + # Could include things such as 'view' + # or 'copy', for instance. + match action: + case 'edit': + # Q: do we want to be logging on every changed field? + # I could see that becoming spammy log-wise, but it may also be important. + # To do so, I'd likely have to override some of the save() functionality of ModelForm. + logger.info(f"{user_type} {self.request.user} edited {form_class_name} in {printable_object_info}") + else: + logger.debug("'analyst_action' does not exist on the session") + # Abstract property enforces NotImplementedError on an attribute. @property @abc.abstractmethod