From 467dea1315fbaf8d09ccbba292cd0bcc60d9f3ab Mon Sep 17 00:00:00 2001 From: Fringg Date: Wed, 4 Mar 2026 17:21:14 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20review=20findings=20=E2=80=94=20exceptio?= =?UTF-8?q?n=20chaining,=20redundant=20unquote,=20validator=20tightening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `from exc` to IntegrityError raise for consistent exception chaining - Remove redundant unquote() in validate_telegram_init_data (parse_qsl already decodes) - Tighten model_validator to require all 3 widget fields (id, auth_date, hash) together - Extract _MAX_CLOCK_SKEW_SECONDS constant replacing magic number -300 - Use logger.exception() instead of logger.error(exc_info=True) in 2 places --- app/cabinet/auth/telegram_auth.py | 12 ++++++++---- app/cabinet/routes/account_linking.py | 10 ++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/cabinet/auth/telegram_auth.py b/app/cabinet/auth/telegram_auth.py index c057eb84..19d750f4 100644 --- a/app/cabinet/auth/telegram_auth.py +++ b/app/cabinet/auth/telegram_auth.py @@ -5,11 +5,15 @@ import hmac import json from datetime import UTC, datetime from typing import Any -from urllib.parse import parse_qsl, unquote +from urllib.parse import parse_qsl from app.config import settings +# Maximum allowed clock skew (seconds) for auth_date — tolerates minor drift between Telegram servers and ours. +_MAX_CLOCK_SKEW_SECONDS = 300 + + def validate_telegram_login_widget(data: dict[str, Any], max_age_seconds: int = 86400) -> bool: """ Validate Telegram Login Widget data. @@ -36,7 +40,7 @@ def validate_telegram_login_widget(data: dict[str, Any], max_age_seconds: int = try: auth_time = datetime.fromtimestamp(int(auth_date), tz=UTC) age = (datetime.now(UTC) - auth_time).total_seconds() - if age > max_age_seconds or age < -300: + if age > max_age_seconds or age < -_MAX_CLOCK_SKEW_SECONDS: return False except (ValueError, TypeError, OSError): return False @@ -83,7 +87,7 @@ def validate_telegram_init_data(init_data: str, max_age_seconds: int = 86400) -> try: auth_time = datetime.fromtimestamp(int(auth_date), tz=UTC) age = (datetime.now(UTC) - auth_time).total_seconds() - if age > max_age_seconds or age < -300: + if age > max_age_seconds or age < -_MAX_CLOCK_SKEW_SECONDS: return None except (ValueError, TypeError, OSError): return None @@ -105,7 +109,7 @@ def validate_telegram_init_data(init_data: str, max_age_seconds: int = 86400) -> # Parse user data from the validated data user_data_str = parsed.get('user') if user_data_str: - user_data = json.loads(unquote(user_data_str)) + user_data = json.loads(user_data_str) return user_data return parsed diff --git a/app/cabinet/routes/account_linking.py b/app/cabinet/routes/account_linking.py index 1d63156f..69fcc7c1 100644 --- a/app/cabinet/routes/account_linking.py +++ b/app/cabinet/routes/account_linking.py @@ -113,6 +113,8 @@ class LinkTelegramRequest(BaseModel): raise ValueError('Provide either init_data or Login Widget fields, not both') if not has_init and not has_widget: raise ValueError('Provide either init_data or Login Widget fields (id, auth_date, hash)') + if has_widget and not (self.id is not None and self.auth_date is not None and self.hash is not None): + raise ValueError('Login Widget mode requires id, auth_date, and hash fields') return self @@ -498,12 +500,12 @@ async def link_telegram( try: await db.commit() - except IntegrityError: + except IntegrityError as exc: await db.rollback() raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail='This Telegram account was just linked to another user', - ) + ) from exc logger.info( 'Telegram linked to account', @@ -616,7 +618,7 @@ async def execute_merge_endpoint( except Exception as exc: await db.rollback() await restore_merge_token(merge_token, consumed) - logger.error('Merge execution failed', exc_info=True) + logger.exception('Merge execution failed') raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail='Account merge failed due to an internal error', @@ -635,7 +637,7 @@ async def execute_merge_endpoint( auth_response = await _create_auth_response(merged_user, db) await _store_refresh_token(db, merged_user.id, auth_response.refresh_token, device_info='merge') except Exception as exc: - logger.error('Failed to create auth tokens after merge', exc_info=True) + logger.exception('Failed to create auth tokens after merge') raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail='Merge succeeded but failed to create new session',