From cf33b2712ae545d77bf3116a214e54751c62e406 Mon Sep 17 00:00:00 2001
From: Daniel McKnight <daniel@neon.ai>
Date: Fri, 25 Oct 2024 16:51:33 -0700
Subject: [PATCH] Handle hashing of changed passwords in `update_user` Update
 exception in `delete_user` to be more specific

---
 neon_users_service/exceptions.py |  6 ++++++
 neon_users_service/models.py     |  2 +-
 neon_users_service/service.py    |  9 +++++++--
 tests/test_models.py             | 10 +++++++++-
 tests/test_service.py            |  7 +++++++
 5 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/neon_users_service/exceptions.py b/neon_users_service/exceptions.py
index 38f82a1..3eaf415 100644
--- a/neon_users_service/exceptions.py
+++ b/neon_users_service/exceptions.py
@@ -11,6 +11,12 @@ class UserNotFoundError(Exception):
     """
 
 
+class UserNotMatchedError(Exception):
+    """
+    Raised when two `User` objects are expected to match and do not.
+    """
+
+
 class ConfigurationError(KeyError):
     """
     Raised when service configuration is not valid.
diff --git a/neon_users_service/models.py b/neon_users_service/models.py
index 1cb203a..38a026c 100644
--- a/neon_users_service/models.py
+++ b/neon_users_service/models.py
@@ -121,7 +121,7 @@ class TokenConfig(BaseModel):
 
 class User(BaseModel):
     username: str
-    password_hash: Optional[str]
+    password_hash: Optional[str] = None
     user_id: str = Field(default_factory=lambda: str(uuid4()))
     created_timestamp: int = Field(default_factory=lambda: round(time()))
     neon: NeonUserConfig = NeonUserConfig()
diff --git a/neon_users_service/service.py b/neon_users_service/service.py
index 985a61e..f5a79d3 100644
--- a/neon_users_service/service.py
+++ b/neon_users_service/service.py
@@ -5,7 +5,9 @@
 from typing import Optional
 from ovos_config import Configuration
 from neon_users_service.databases import UserDatabase
-from neon_users_service.exceptions import ConfigurationError, AuthenticationError, UserNotFoundError
+from neon_users_service.exceptions import (ConfigurationError,
+                                           AuthenticationError,
+                                           UserNotMatchedError)
 from neon_users_service.models import User
 
 
@@ -88,10 +90,13 @@ def update_user(self, user: User) -> User:
         @param user: The updated user object to update in the database
         @retruns: User object as it exists in the database, after updating
         """
+        # Create a copy to prevent modifying the input object
+        user = copy(user)
         if not user.password_hash:
             raise ValueError("Supplied user password is empty")
         if not isinstance(user.tokens, list):
             raise ValueError("Supplied tokens configuration is not a list")
+        user.password_hash = self._ensure_hashed(user.password_hash)
         # This will raise a `UserNotFound` exception if the user doesn't exist
         return self.database.update_user(user)
 
@@ -104,7 +109,7 @@ def delete_user(self, user: User) -> User:
         """
         db_user = self.database.read_user_by_id(user.user_id)
         if db_user != user:
-            raise UserNotFoundError(user)
+            raise UserNotMatchedError(user)
         return self.database.delete_user(user.user_id)
 
     def shutdown(self):
diff --git a/tests/test_models.py b/tests/test_models.py
index a9230fa..3b80ec8 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -2,7 +2,7 @@
 
 from pydantic import ValidationError
 from datetime import date
-from neon_users_service.models import NeonUserConfig, TokenConfig, User
+from neon_users_service.models import NeonUserConfig, TokenConfig, User, MQRequest
 
 
 class TestModelValidation(TestCase):
@@ -75,6 +75,14 @@ def test_user_model(self):
         self.assertNotEqual(default_user, duplicate_user)
         self.assertEqual(default_user.tokens, duplicate_user.tokens)
 
+    def test_mq_request_model(self):
+        valid_model = MQRequest(operation="create", username="test_user")
+        self.assertIsInstance(valid_model, MQRequest)
+        with self.assertRaises(ValidationError):
+            MQRequest(operation="get", username="test")
+        with self.assertRaises(ValidationError):
+            MQRequest(operation="delete", username="test_user", user="test_user")
+
 
 class TestEnumClasses(TestCase):
     def test_access_roles(self):
diff --git a/tests/test_service.py b/tests/test_service.py
index 84dc168..45c01b7 100644
--- a/tests/test_service.py
+++ b/tests/test_service.py
@@ -107,6 +107,13 @@ def test_update_user(self):
         updated_user = service.update_user(updated_user)
         self.assertEqual(updated_user.password_hash, user_1.password_hash)
 
+        # Input plaintext passwords should be hashed
+        updated_user.password_hash = "test"
+        new_updated_user = service.update_user(updated_user)
+        self.assertNotEqual(updated_user.password_hash,
+                            new_updated_user.password_hash)
+        self.assertEqual(new_updated_user.password_hash, user_1.password_hash)
+
         # Invalid token values
         updated_user.tokens = None
         with self.assertRaises(ValueError):