Resets User.failed_login_count on password reset from anywhere

api/main/models.py

@@ -372,36 +372,32 @@ def get_description(self):
         )
 
 
-@receiver(post_save, sender=User)
-def user_save(sender, instance, created, **kwargs):
+def block_user_save_email(instance, method, *args, **kwargs):
     # Create random attribute name with static prefix for determining if this is the root trigger of
     # this signal
     attr_prefix = "_saving_"
     random_attr = f"{attr_prefix}{''.join(random.sample(string.ascii_lowercase, 16))}"
 
+    # Adds random attribute to suppress email from save during creation, then removes it
+    setattr(instance, random_attr, True)
+    getattr(instance, method)(*args, **kwargs)
+    delattr(instance, random_attr)
+
+
+@receiver(post_save, sender=User)
+def user_save(sender, instance, created, **kwargs):
     if os.getenv("COGNITO_ENABLED") == "TRUE":
         if created:
-            # Adds random attribute to suppress email from save during creation, then removes it
-            setattr(instance, random_attr, True)
-            instance.move_to_cognito()
-            delattr(instance, random_attr)
+            block_user_save_email(instance, "move_to_cognito")
         else:
             TatorCognito().update_attributes(instance)
     if created:
         if instance.username:
             instance.username = instance.username.strip()
-
-            # Adds random attribute to suppress email from save during creation, then removes it
-            setattr(instance, random_attr, True)
-            instance.save()
-            delattr(instance, random_attr)
+            block_user_save_email(instance, "save")
         if settings.SAML_ENABLED and not instance.email:
             instance.email = instance.username
-
-            # Adds random attribute to suppress email from save during creation, then removes it
-            setattr(instance, random_attr, True)
-            instance.save()
-            delattr(instance, random_attr)
+            block_user_save_email(instance, "save")
         invites = Invitation.objects.filter(email=instance.email, status="Pending")
         if (invites.count() == 0) and (os.getenv("AUTOCREATE_ORGANIZATIONS")):
             organization = Organization.objects.create(name=f"{instance}'s Team")
@@ -413,49 +409,53 @@ def user_pre_save(sender, instance, **kwargs):
     # Prefix for random attribute name to determine if this is the root trigger of this signal
     attr_prefix = "_saving_"
     user_desc = instance.get_description()
-    is_root = all(not attr.startswith(attr_prefix) for attr in dir(instance))
-    created = not instance.pk
-
-    if created:
-        msg = (
-            f"You are being notified that a new user {instance} (username {instance.username}, "
-            f"email {instance.email}) has been added to the Tator deployment with the following "
-            f"attributes:\n\n{user_desc}"
-        )
-        is_monitored = True
-    else:
-        msg = (
-            f"You are being notified that an existing user {instance} been modified with the "
-            f"following values:\n\n{user_desc}"
-        )
-
-        # Only send an email if this is the root `post_save` trigger, i.e. does not have a random
-        # attribute added to it, and is a modification of a monitored field.
-        original_instance = type(instance).objects.get(pk=instance.id)
-        monitored_fields = [
-            "username",
-            "first_name",
-            "last_name",
-            "email",
-            "is_staff",
-            "profile",
-            "password",
-        ]
-        is_monitored = any(
-            getattr(instance, fieldname, None) != getattr(original_instance, fieldname, None)
-            for fieldname in monitored_fields
-        )
+    if all(not attr.startswith(attr_prefix) for attr in dir(instance)):
+        created = not instance.pk
+        if created:
+            msg = (
+                f"You are being notified that a new user {instance} (username {instance.username}, "
+                f"email {instance.email}) has been added to the Tator deployment with the "
+                f"following attributes:\n\n{user_desc}"
+            )
+            is_monitored = True
+            password_modified = False
+        else:
+            msg = (
+                f"You are being notified that an existing user {instance} been modified with the "
+                f"following values:\n\n{user_desc}"
+            )
 
-    if is_root and is_monitored:
-        logger.info(msg)
-        email_service = get_email_service()
-        if email_service:
-            email_service.email_staff(
-                sender=settings.TATOR_EMAIL_SENDER,
-                title=f"{'Created' if created else 'Modified'} user",
-                text=msg,
+            # Only send an email if this is the root `pre_save` trigger, i.e. does not have a random
+            # attribute added to it, and is a modification of a monitored field.
+            original_instance = type(instance).objects.get(pk=instance.id)
+            monitored_fields = [
+                "username",
+                "first_name",
+                "last_name",
+                "email",
+                "is_staff",
+                "profile",
+                "password",
+            ]
+            password_modified = instance.password != original_instance.password
+            is_monitored = password_modified or any(
+                getattr(instance, fieldname, None) != getattr(original_instance, fieldname, None)
+                for fieldname in monitored_fields
             )
 
+        if is_monitored:
+            if password_modified:
+                instance.last_failed_login = 0
+                block_user_save_email(instance, "save")
+            logger.info(msg)
+            email_service = get_email_service()
+            if email_service:
+                email_service.email_staff(
+                    sender=settings.TATOR_EMAIL_SENDER,
+                    title=f"{'Created' if created else 'Modified'} user",
+                    text=msg,
+                )
+
 
 @receiver(post_delete, sender=User)
 def user_post_delete(sender, instance, **kwargs):

api/main/notify.py

@@ -12,38 +12,35 @@
 
 
 class Notify:
+    @staticmethod
     def notification_enabled():
         """Returns true if notification is enabled"""
         return settings.TATOR_SLACK_TOKEN and settings.TATOR_SLACK_CHANNEL
 
-    def notify_admin_msg(msg):
+    @classmethod
+    def notify_admin_msg(cls, msg):
         """Sends a given message to administrators"""
         try:
-            if Notify.notification_enabled():
+            if cls.notification_enabled():
                 client = slack.WebClient(token=settings.TATOR_SLACK_TOKEN)
                 response = client.chat_postMessage(channel=settings.TATOR_SLACK_CHANNEL, text=msg)
-                if response["ok"]:
-                    return True
-                else:
-                    return False
+                return bool(response["ok"])
         except:
-            logger.warning("Slack Comms failed")
+            logger.warning("Slack Comms failed", exc_info=True)
 
         return False
 
-    def notify_admin_file(title, content):
+    @classmethod
+    def notify_admin_file(cls, title, content):
         """Send a given file to administrators"""
         try:
-            if Notify.notification_enabled():
+            if cls.notification_enabled():
                 client = slack.WebClient(token=settings.TATOR_SLACK_TOKEN)
                 response = client.files_upload(
                     channels=settings.TATOR_SLACK_CHANNEL, content=content, title=title
                 )
-                if response["ok"]:
-                    return True
-                else:
-                    return False
+                return bool(response["ok"])
         except:
-            logger.warning("Slack Comms failed")
+            logger.warning("Slack Comms failed", exc_info=True)
 
         return False

api/main/rest/media.py

@@ -32,7 +32,6 @@
 )
 from ..schema import MediaListSchema, MediaDetailSchema, parse
 from ..schema.components import media as media_schema
-from ..notify import Notify
 from ..download import download_file
 from ..store import get_tator_store, get_storage_lookup
 from ..cache import TatorCache

api/main/rest/transcode.py

@@ -15,7 +15,6 @@
 from ..models import Media
 from ..schema import TranscodeListSchema
 from ..schema import TranscodeDetailSchema
-from ..notify import Notify
 
 from .media import _create_media
 from ._util import url_to_key

api/main/views.py

@@ -7,23 +7,17 @@
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponse
 from django.http import JsonResponse
-from rest_framework.authtoken.models import Token
 from django.contrib.auth.models import AnonymousUser
 from django.conf import settings
 
 from django.template.response import TemplateResponse
 from rest_framework.authentication import TokenAuthentication
-import yaml
 
 from .models import Project
 from .models import Membership
-from .models import Affiliation
-from .models import Invitation
-from .models import User
 from .notify import Notify
 from .cache import TatorCache
 
-import os
 import logging
 
 import sys
@@ -34,10 +28,7 @@
 
 
 def check_login(request):
-    if request.user.is_authenticated:
-        return JsonResponse({"is_authenticated": True})
-    else:
-        return JsonResponse({"is_authenticated": False})
+    return JsonResponse({"is_authenticated": bool(request.user.is_authenticated)})
 
 
 class LoginRedirect(View):
@@ -65,7 +56,7 @@ def get_context_data(self, **kwargs):
 
         # Check if user is part of project.
         if not project.has_user(self.request.user.pk):
-            raise PermissionDenied
+            raise PermissionDenied(f"User {self.request.user} does not have access to {project.id}")
         return context
 
 
@@ -132,16 +123,14 @@ def dispatch(self, request, *args, **kwargs):
         user = request.user
         if isinstance(user, AnonymousUser):
             try:
-                (user, token) = TokenAuthentication().authenticate(request)
-            except Exception as e:
+                user, _ = TokenAuthentication().authenticate(request)
+            except Exception:
                 msg = "*Security Alert:* "
                 msg += f"Bad credentials presented for '{original_url}' ({user})"
                 Notify.notify_admin_msg(msg)
-                logger.warn(msg)
+                logger.warn(msg, exc_info=True)
                 return HttpResponse(status=403)
 
-        filename = os.path.basename(original_url)
-
         project = None
         try:
             comps = original_url.split("/")
@@ -150,21 +139,19 @@ def dispatch(self, request, *args, **kwargs):
                 project_id = comps[3]
             project = Project.objects.get(pk=project_id)
             authorized = validate_project(user, project)
-        except Exception as e:
-            logger.info(f"ERROR: {e}")
+        except Exception:
+            logger.info("Could not validate project access", exc_info=True)
             authorized = False
 
         if authorized:
             return HttpResponse(status=200)
-        else:
-            # Files that aren't in the whitelist or database are forbidden
-            msg = f"({user}/{user.id}): "
-            msg += f"Attempted to access unauthorized file '{original_url}'"
-            msg += f". "
-            msg += f"Does not have access to '{project}'"
-            Notify.notify_admin_msg(msg)
-            return HttpResponse(status=403)
 
+        # Files that aren't in the whitelist or database are forbidden
+        msg = (
+            f"({user}/{user.id}): Attempted to access unauthorized file '{original_url}'; does not "
+            f"have access to '{project}'"
+        )
+        Notify.notify_admin_msg(msg)
         return HttpResponse(status=403)
 
 
@@ -183,23 +170,18 @@ def dispatch(self, request, *args, **kwargs):
         user = request.user
         if isinstance(user, AnonymousUser):
             try:
-                (user, token) = TokenAuthentication().authenticate(request)
-            except Exception as e:
-                msg = "*Security Alert:* "
-                msg += f"Bad credentials presented for '{original_url}'"
+                user, _ = TokenAuthentication().authenticate(request)
+            except Exception:
+                msg = f"*Security Alert:* Bad credentials presented for '{original_url}'"
                 Notify.notify_admin_msg(msg)
                 return HttpResponse(status=403)
 
         if user.is_staff:
             return HttpResponse(status=200)
-        else:
-            # Files that aren't in the whitelist or database are forbidden
-            msg = f"({user}/{user.id}): "
-            msg += f"Attempted to access unauthorized URL '{original_url}'"
-            msg += f"."
-            Notify.notify_admin_msg(msg)
-            return HttpResponse(status=403)
 
+        # Files that aren't in the whitelist or database are forbidden
+        msg = f"({user}/{user.id}): Attempted to access unauthorized URL '{original_url}'."
+        Notify.notify_admin_msg(msg)
         return HttpResponse(status=403)
 
 
@@ -214,11 +196,10 @@ def ErrorNotifierView(request, code, message, details=None):
 
     # Generate slack message
     if Notify.notification_enabled():
-        msg = f"{request.get_host()}:"
-        msg += f" ({request.user}/{request.user.id})"
-        msg += f" caused {code} at {request.get_full_path()}"
+        user = request.user
+        msg = f"{request.get_host()}: ({user}/{user.id}) caused {code} at {request.get_full_path()}"
         if details:
-            Notify.notify_admin_file(msg, msg + "\n" + details)
+            Notify.notify_admin_file(msg, f"{msg}\n{details}")
         else:
             if code == 404 and isinstance(request.user, AnonymousUser):
                 logger.warn(msg)