Conversation
…insights, efficiency metrics, and improved readability
… back for 409 responses
There was a problem hiding this comment.
Pull Request Overview
This PR introduces comprehensive authentication endpoints and infrastructure for the ClashKing application, establishing user registration, login, OAuth integration, and session management capabilities.
- Adds complete user authentication system with email verification and password reset functionality
- Implements Discord OAuth integration for social login
- Creates new database collections and client instances for authentication data
- Updates utility functions and dependencies across multiple modules
Reviewed Changes
Copilot reviewed 49 out of 95 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/utils.py | Adds coc client, auth collections, and utility functions for ID generation |
| utils/time.py | Creates comprehensive time handling utilities for Discord timestamps and season calculations |
| utils/security_middleware.py | Implements JWT authentication middleware and user validation |
| utils/password_validator.py | Provides password strength validation and input sanitization |
| utils/email_service.py | Establishes email verification and password reset functionality |
| utils/database.py | Creates new database client structure with additional collections |
| utils/config.py | Extends configuration with authentication, encryption, and email settings |
| templates/tickets.html | Updates ticketing interface with modern flat design |
| routers/v2/war/* | Implements comprehensive war statistics and analysis endpoints |
| routers/v2/player/* | Creates player data and analytics endpoints |
| routers/v2/exports/* | Adds Excel export functionality for war statistics |
| routers/v2/server/* | Implements server settings management |
| routers/v2/search/* | Creates search endpoints for clans and players |
| routers/v2/link/* | Adds basic linking endpoints |
| routers/v2/raid/* | Creates raid weekend data structure |
|
|
||
|
|
||
| def remove_id_fields(data): | ||
| return json_loads(json_util.dumps(data)) |
There was a problem hiding this comment.
The remove_id_fields function now returns early on line 305 without executing the original logic below. This creates unreachable code that should be removed to avoid confusion.
| try: | ||
| await db_client.app_email_verifications.create_index("email_hash") | ||
| except Exception: | ||
| # Index might already exist, that's okay | ||
| pass | ||
|
|
||
| # Index for fast lookup by verification token | ||
| try: | ||
| await db_client.app_email_verifications.create_index("verification_token") | ||
| except Exception: | ||
| # Index might already exist, that's okay | ||
| pass | ||
|
|
||
| # TTL index for automatic cleanup of expired documents | ||
| try: | ||
| await db_client.app_email_verifications.create_index( | ||
| "expires_at", | ||
| expireAfterSeconds=0 # MongoDB will delete when expires_at < current time | ||
| ) | ||
| except Exception: | ||
| # Index might already exist, that's okay | ||
| pass |
There was a problem hiding this comment.
The create_verification_indexes function uses empty except blocks to silently ignore index creation failures. Consider logging these exceptions or using more specific exception handling to distinguish between expected duplicate index errors and actual problems.
| try: | |
| await db_client.app_email_verifications.create_index("email_hash") | |
| except Exception: | |
| # Index might already exist, that's okay | |
| pass | |
| # Index for fast lookup by verification token | |
| try: | |
| await db_client.app_email_verifications.create_index("verification_token") | |
| except Exception: | |
| # Index might already exist, that's okay | |
| pass | |
| # TTL index for automatic cleanup of expired documents | |
| try: | |
| await db_client.app_email_verifications.create_index( | |
| "expires_at", | |
| expireAfterSeconds=0 # MongoDB will delete when expires_at < current time | |
| ) | |
| except Exception: | |
| # Index might already exist, that's okay | |
| pass | |
| try: | |
| await db_client.app_email_verifications.create_index("email_hash") | |
| except pymongo.errors.OperationFailure as e: | |
| if "already exists" in str(e): | |
| # Index might already exist, that's okay | |
| pass | |
| else: | |
| sentry_sdk.capture_exception(e, tags={"index": "email_hash"}) | |
| raise | |
| # Index for fast lookup by verification token | |
| try: | |
| await db_client.app_email_verifications.create_index("verification_token") | |
| except pymongo.errors.OperationFailure as e: | |
| if "already exists" in str(e): | |
| # Index might already exist, that's okay | |
| pass | |
| else: | |
| sentry_sdk.capture_exception(e, tags={"index": "verification_token"}) | |
| raise | |
| # TTL index for automatic cleanup of expired documents | |
| try: | |
| await db_client.app_email_verifications.create_index( | |
| "expires_at", | |
| expireAfterSeconds=0 # MongoDB will delete when expires_at < current time | |
| ) | |
| except pymongo.errors.OperationFailure as e: | |
| if "already exists" in str(e): | |
| # Index might already exist, that's okay | |
| pass | |
| else: | |
| sentry_sdk.capture_exception(e, tags={"index": "expires_at"}) | |
| raise |
| detail="Password must contain at least one digit" | ||
| ) | ||
|
|
||
| if not re.search(r'[!@#$%^&*(),.?":{}|<>]', password): |
There was a problem hiding this comment.
The special character regex contains unescaped characters that could be interpreted as regex metacharacters. Consider escaping the special characters or using a character class to avoid potential regex parsing issues.
| if not re.search(r'[!@#$%^&*(),.?":{}|<>]', password): | |
| if not re.search(r'[!@#\$%\^&\*\(\),\.\?":\{\}\|<>]', password): |
| # Handle season filtering | ||
| if filter and filter.season: | ||
| try: | ||
| # Parse season string (format: "YYYY-MM") |
There was a problem hiding this comment.
The season filtering logic uses month - 1 when calling coc.utils.get_season_start(), but the comment indicates the season format is "YYYY-MM". This offset could cause incorrect season boundary calculations and should be verified against the expected season format.
routers/v2/player/utils.py
Outdated
| else: | ||
| season["season_stars_distribution_attacks"][2] += 1 |
There was a problem hiding this comment.
The star distribution logic in the legend stats processing uses a catch-all else clause that assigns all non-matching trophy values to 2-star category. This could mask data quality issues and should be more explicit about handling edge cases.
| else: | |
| season["season_stars_distribution_attacks"][2] += 1 | |
| else: | |
| raise ValueError(f"Unexpected attack trophy value: {trophies}. Expected values: 5-15, 16-32, or 40.") |
| # Encryption/Decryption/Hashing/Token | ||
| cipher = Fernet(ENCRYPTION_KEY) | ||
| pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") | ||
| oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") |
There was a problem hiding this comment.
Creating the Fernet cipher instance at module level could cause application startup failures if ENCRYPTION_KEY is not properly set. Consider initializing this within a function or adding proper error handling with a meaningful error message.
| # Encryption/Decryption/Hashing/Token | |
| cipher = Fernet(ENCRYPTION_KEY) | |
| pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") | |
| oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") | |
| # Encryption/Decryption/Hashing/Token | |
| pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") | |
| oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") | |
| def get_cipher(self): | |
| if not self.ENCRYPTION_KEY: | |
| raise ValueError("ENCRYPTION_KEY is not set. Please configure it in the environment variables.") | |
| try: | |
| return Fernet(self.ENCRYPTION_KEY) | |
| except Exception as e: | |
| raise ValueError(f"Failed to initialize Fernet cipher: {e}") |
|
|
The preview deployment is ready. 🟢 Open Preview | Open Build Logs Last updated at: 2025-08-10 22:51:00 CET |
|
The preview deployment is ready. 🟢 Open Preview | Open Build Logs Last updated at: 2025-09-28 02:54:16 CET |
No description provided.