Add User Service class #28
Conversation
|
@sentry review |
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
|
|
||
| def register(self, username, password): | ||
| if self.db.get_user(username): | ||
| raise Exception("User already exists") | ||
| hashed = hashlib.md5(password.encode()).hexdigest() | ||
| self.db.save_user(username, hashed) |
There was a problem hiding this comment.
Add input validation for username and password. Empty or None values should be handled gracefully before processing.
| return False | ||
| hashed = hashlib.md5(password.encode()).hexdigest() |
There was a problem hiding this comment.
Same MD5 security issue here. Also, when using bcrypt, you need to use bcrypt.checkpw() for password verification instead of direct comparison.
|
|
||
| def login(self, username, password): | ||
| user = self.db.get_user(username) | ||
| if not user: | ||
| return False | ||
| hashed = hashlib.md5(password.encode()).hexdigest() | ||
| if hashed == user["password"]: | ||
| token = self._generate_token(username) | ||
| return token |
There was a problem hiding this comment.
Add input validation for login parameters and improve return value consistency. Consider returning a structured response instead of mixed boolean/string types.
|
|
||
| def _generate_token(self, username): | ||
| raw = f"{username}:{time.time()}" |
There was a problem hiding this comment.
Security Issue: This token generation is not secure. SHA1 with timestamp is predictable and vulnerable. Use a cryptographically secure random token generator or JWT with proper signing.
|
|
||
| def delete_user(self, username): | ||
| try: | ||
| self.db.delete(username) | ||
| self.logger.info("User deleted") | ||
| except: |
There was a problem hiding this comment.
Poor Exception Handling: Bare except clause catches all exceptions, making debugging difficult. Be specific about exceptions and provide meaningful error messages. Also, the success case doesn't return anything.
|
|
||
| class UserService: | ||
| def __init__(self, db): | ||
| self.db = db |
There was a problem hiding this comment.
Consider adding type hints and docstrings to improve code documentation and maintainability. This helps with IDE support and makes the API clearer for other developers.
|
@sentry review |
|
🔒 GenAI Consent Required To enable PR review and test generation via Prevent, an organization admin needs to:
Once enabled, you can re-trigger this review by commenting. |
No description provided.