feat(auth): add email verification flow - #53
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements email verification functionality for the authentication system by integrating with NotifyLinc for email delivery and adding database support for verification tokens.
- Adds email verification token storage and
/verify-emailendpoint - Integrates NotifyLinc service for sending verification emails
- Updates user registration flow to generate and send verification tokens
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| backend/auth/requirements.txt | Adds requests library dependency for HTTP communication |
| backend/auth/main.py | Implements email verification database model, helper functions, and API endpoint |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| from datetime import datetime, timedelta | ||
| from typing import Dict, Any, Optional, List, Union | ||
|
|
||
| import requests |
There was a problem hiding this comment.
[nitpick] The requests import should be grouped with other third-party imports. Move this import to be with the other third-party library imports around line 18-21 for better organization.
| } | ||
| headers = {"X-MCP-Task": "email"} | ||
| try: | ||
| response = requests.post(NOTIFYLINC_URL, json=payload, headers=headers, timeout=5) |
There was a problem hiding this comment.
The error handling in the except block logs a generic error message. Consider logging more specific information about the failure, such as the HTTP status code or response content when available.
| return db_user | ||
|
|
||
| @app.get("/verify-email") | ||
| async def verify_email(token: str, db: Session = Depends(get_db)): |
There was a problem hiding this comment.
The endpoint lacks rate limiting protection. Consider adding rate limiting to prevent brute force attacks on email verification tokens.
| user.is_verified = True | ||
| db.delete(record) | ||
| db.commit() |
There was a problem hiding this comment.
If the database commit fails after setting user.is_verified = True, the user will be marked as verified but the verification record won't be deleted, potentially allowing token reuse. Use a database transaction to ensure atomicity.
| user.is_verified = True | |
| db.delete(record) | |
| db.commit() | |
| # Use a transaction to ensure atomicity | |
| with db.begin(): | |
| user.is_verified = True | |
| db.delete(record) |
Summary
/verify-emailendpointTesting
python -m py_compile backend/auth/main.pynpm test(fails: Missing script "test" in frontend workspace)npm run linthttps://chatgpt.com/codex/tasks/task_e_68b307dc5cfc83318b9d074efa56abb7