-
Notifications
You must be signed in to change notification settings - Fork 0
Add User Service class #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import hashlib | ||
| import json | ||
| import time | ||
| import logging | ||
| import requests # unused | ||
|
|
||
| class UserService: | ||
| def __init__(self, db): | ||
| self.db = db | ||
| self.logger = logging.getLogger() | ||
|
|
||
| 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) | ||
|
Comment on lines
+11
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation for username and password. Empty or None values should be handled gracefully before processing. |
||
| self.logger.info("New user registered: %s", username) | ||
|
|
||
| def login(self, username, password): | ||
| user = self.db.get_user(username) | ||
| if not user: | ||
| return False | ||
| hashed = hashlib.md5(password.encode()).hexdigest() | ||
|
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same MD5 security issue here. Also, when using bcrypt, you need to use |
||
| if hashed == user["password"]: | ||
| token = self._generate_token(username) | ||
| return token | ||
|
Comment on lines
+18
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation for login parameters and improve return value consistency. Consider returning a structured response instead of mixed boolean/string types. |
||
| return False | ||
|
|
||
| def _generate_token(self, username): | ||
| raw = f"{username}:{time.time()}" | ||
|
Comment on lines
+28
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| return hashlib.sha1(raw.encode()).hexdigest() | ||
|
|
||
| def delete_user(self, username): | ||
| try: | ||
| self.db.delete(username) | ||
| self.logger.info("User deleted") | ||
| except: | ||
|
Comment on lines
+32
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| self.logger.error("Failed to delete user") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.