Skip to content

Commit

Permalink
Handle hashing of changed passwords in update_user
Browse files Browse the repository at this point in the history
Update exception in `delete_user` to be more specific
  • Loading branch information
NeonDaniel committed Oct 25, 2024
1 parent ae37559 commit cf33b27
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 4 deletions.
6 changes: 6 additions & 0 deletions neon_users_service/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion neon_users_service/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 7 additions & 2 deletions neon_users_service/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)

Expand All @@ -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):
Expand Down
10 changes: 9 additions & 1 deletion tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 7 additions & 0 deletions tests/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit cf33b27

Please sign in to comment.