Fix/magic link bot auth#24
Merged
Merged
Conversation
added 6 commits
May 27, 2026 18:52
…ot to an account by adding X-Bot-Secret to the header
… into fix/magic-link-bot-auth
Contributor
Руководство для ревьюераДобавляет HMAC-проверку общего секрета бота для генерации Telegram magic-link, прокидывает секрет бота через конфиг и Telegram‑клиент, ужесточает контроль доступа к API генерации magic-link и обновляет зависимости для поддержки инструментов работы с коммитами. Диаграмма последовательности генерации Telegram magic-link с проверкой X-Bot-SecretsequenceDiagram
actor TgUser
participant TgBot as TgClient
participant Api as create_telegram_magic_link
participant TgService as TgLinkService
participant Config as BOT_SECRET
participant DB as AsyncSession
TgUser->>TgBot: generate_magic_token(tg_id)
TgBot->>TgBot: BOT_SECRET
TgBot->>Api: GET /auth/telegram/generate-link/{tg_id}\nX-Bot-Secret: BOT_SECRET
Api->>Api: request.headers.get(X-Bot-Secret)
Api->>TgService: verify_bot_secret_key(bot_secret_header)
TgService->>Config: BOT_SECRET
TgService-->>Api: bool
Api->>TgService: check_telegram_connection(telegram_id, db)
TgService->>DB: select(User)
TgService-->>Api: User | None
Api->>TgService: save_link_token(token, expires_at, db, telegram_id)
TgService->>DB: insert(MagicToken)
TgService-->>Api: None
Api-->>TgBot: { token }
TgBot-->>TgUser: magic token status
Изменения на уровне файлов
Подсказки и командыВзаимодействие с Sourcery
Настройка работыЗайдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideAdds HMAC-based verification of a shared bot secret for Telegram magic link generation, wires the bot secret through config and the Telegram client, tightens access control on the magic link API, and updates dependencies to support commit tooling. Sequence diagram for Telegram magic link generation with X-Bot-Secret verificationsequenceDiagram
actor TgUser
participant TgBot as TgClient
participant Api as create_telegram_magic_link
participant TgService as TgLinkService
participant Config as BOT_SECRET
participant DB as AsyncSession
TgUser->>TgBot: generate_magic_token(tg_id)
TgBot->>TgBot: BOT_SECRET
TgBot->>Api: GET /auth/telegram/generate-link/{tg_id}\nX-Bot-Secret: BOT_SECRET
Api->>Api: request.headers.get(X-Bot-Secret)
Api->>TgService: verify_bot_secret_key(bot_secret_header)
TgService->>Config: BOT_SECRET
TgService-->>Api: bool
Api->>TgService: check_telegram_connection(telegram_id, db)
TgService->>DB: select(User)
TgService-->>Api: User | None
Api->>TgService: save_link_token(token, expires_at, db, telegram_id)
TgService->>DB: insert(MagicToken)
TgService-->>Api: None
Api-->>TgBot: { token }
TgBot-->>TgUser: magic token status
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Привет! Я нашёл 2 проблемы и оставил несколько общих замечаний:
- Обращение к BOT_SECRET через os.environ["BOT_SECRET"] вызовет KeyError на этапе импорта, если переменная отсутствует, что помешает приложению стартовать; имеет смысл использовать os.getenv с явной валидацией при запуске, чтобы ошибки конфигурации обрабатывались контролируемым образом.
- Логика валидации X-Bot-Secret сейчас дублируется между API-слоем и сервисом (извлечение заголовка + verify_bot_secret_key); стоит инкапсулировать это в FastAPI dependency или middleware, чтобы все маршруты, требующие bot secret, использовали одну и ту же проверку и обработку ошибок.
- Некоторые новые сообщения логгера довольно подробные и используют logger.info для потенциально высокочастотных или низкоуровневых проверок (например, наличие заголовка и сравнение ключей); стоит понизить часть таких логов до debug, чтобы избежать шума в логах в продакшене.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessing BOT_SECRET via os.environ["BOT_SECRET"] will raise a KeyError at import time if the variable is missing, which prevents the app from starting; consider using os.getenv with explicit validation during startup so configuration errors are handled in a controlled way.
- The X-Bot-Secret validation logic is now duplicated between the API layer and the service (header extraction + verify_bot_secret_key); consider encapsulating this into a FastAPI dependency or middleware so all routes that require the bot secret can share the same check and error handling.
- Some of the new log messages are quite verbose and use logger.info for what might be high-frequency or low-level checks (e.g., header presence and key comparison); consider downgrading some of these to debug to avoid noisy logs in production.
## Individual Comments
### Comment 1
<location path="src/domain/services/tg_link_service.py" line_range="13-14" />
<code_context>
class TgLinkService:
+ @staticmethod
+ def verify_bot_secret_key(bot_secret_header: str):
+ """
+ Сравниваем секретный ключ с полученным ключем в заголовке.
+ Устранена возможность timig атаки.
</code_context>
<issue_to_address>
**nitpick (typo):** Docstring and log message contain typos that may hinder future maintenance/searchability
In `verify_bot_secret_key`, please fix typos in the docstring/log messages: `timig` → `timing`, `ключем` → `ключом`, `Не верный` → `Неверный` to keep security-related text clear and searchable.
Suggested implementation:
```python
def verify_bot_secret_key(bot_secret_header: str):
"""
Сравниваем секретный ключ с полученным ключом в заголовке.
Устранена возможность timing-атаки.
"""
```
```python
if not result:
logger.warning("Неверный ключ в X-Bot-Secret. Возможна попытка атаки!")
```
</issue_to_address>
### Comment 2
<location path="src/presentation/api/v1/auth/tg_link.py" line_range="39-44" />
<code_context>
+ logger.info("Сравнение ключей...")
+ verify_key = TgLinkService.verify_bot_secret_key(bot_secret_header)
+
+ if not verify_key:
+ logger.info("Предоставленый ключ не совпадает с секретным ключем")
+ raise NoAccess("Недостаточно прав для доступа к этому ресурсу")
+
</code_context>
<issue_to_address>
**nitpick (typo):** Typographical issues in the log message for invalid bot secret
Please correct the Russian typos in the log message: `Предоставленый` → `Предоставленный`, `ключем` → `ключом` to keep logs clear and consistent.
```suggestion
logger.info("Сравнение ключей...")
verify_key = TgLinkService.verify_bot_secret_key(bot_secret_header)
if not verify_key:
logger.info("Предоставленный ключ не совпадает с секретным ключом")
raise NoAccess("Недостаточно прав для доступа к этому ресурсу")
```
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- Accessing BOT_SECRET via os.environ["BOT_SECRET"] will raise a KeyError at import time if the variable is missing, which prevents the app from starting; consider using os.getenv with explicit validation during startup so configuration errors are handled in a controlled way.
- The X-Bot-Secret validation logic is now duplicated between the API layer and the service (header extraction + verify_bot_secret_key); consider encapsulating this into a FastAPI dependency or middleware so all routes that require the bot secret can share the same check and error handling.
- Some of the new log messages are quite verbose and use logger.info for what might be high-frequency or low-level checks (e.g., header presence and key comparison); consider downgrading some of these to debug to avoid noisy logs in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessing BOT_SECRET via os.environ["BOT_SECRET"] will raise a KeyError at import time if the variable is missing, which prevents the app from starting; consider using os.getenv with explicit validation during startup so configuration errors are handled in a controlled way.
- The X-Bot-Secret validation logic is now duplicated between the API layer and the service (header extraction + verify_bot_secret_key); consider encapsulating this into a FastAPI dependency or middleware so all routes that require the bot secret can share the same check and error handling.
- Some of the new log messages are quite verbose and use logger.info for what might be high-frequency or low-level checks (e.g., header presence and key comparison); consider downgrading some of these to debug to avoid noisy logs in production.
## Individual Comments
### Comment 1
<location path="src/domain/services/tg_link_service.py" line_range="13-14" />
<code_context>
class TgLinkService:
+ @staticmethod
+ def verify_bot_secret_key(bot_secret_header: str):
+ """
+ Сравниваем секретный ключ с полученным ключем в заголовке.
+ Устранена возможность timig атаки.
</code_context>
<issue_to_address>
**nitpick (typo):** Docstring and log message contain typos that may hinder future maintenance/searchability
In `verify_bot_secret_key`, please fix typos in the docstring/log messages: `timig` → `timing`, `ключем` → `ключом`, `Не верный` → `Неверный` to keep security-related text clear and searchable.
Suggested implementation:
```python
def verify_bot_secret_key(bot_secret_header: str):
"""
Сравниваем секретный ключ с полученным ключом в заголовке.
Устранена возможность timing-атаки.
"""
```
```python
if not result:
logger.warning("Неверный ключ в X-Bot-Secret. Возможна попытка атаки!")
```
</issue_to_address>
### Comment 2
<location path="src/presentation/api/v1/auth/tg_link.py" line_range="39-44" />
<code_context>
+ logger.info("Сравнение ключей...")
+ verify_key = TgLinkService.verify_bot_secret_key(bot_secret_header)
+
+ if not verify_key:
+ logger.info("Предоставленый ключ не совпадает с секретным ключем")
+ raise NoAccess("Недостаточно прав для доступа к этому ресурсу")
+
</code_context>
<issue_to_address>
**nitpick (typo):** Typographical issues in the log message for invalid bot secret
Please correct the Russian typos in the log message: `Предоставленый` → `Предоставленный`, `ключем` → `ключом` to keep logs clear and consistent.
```suggestion
logger.info("Сравнение ключей...")
verify_key = TgLinkService.verify_bot_secret_key(bot_secret_header)
if not verify_key:
logger.info("Предоставленный ключ не совпадает с секретным ключом")
raise NoAccess("Недостаточно прав для доступа к этому ресурсу")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Coverage Report for CI Build 26639560946Coverage decreased (-0.2%) to 28.072%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions54 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
Coverage Report for CI Build 26639796309Coverage decreased (-0.2%) to 28.072%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions54 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Обеспечена безопасность генерации magic-ссылок для Telegram с помощью общего секрета бота и улучшено логирование, связанное с созданием ссылок.
New Features:
X-Bot-Secretдля запросов на генерацию Telegram magic-ссылок с использованием настраиваемого значенияBOT_SECRET.TgLinkServiceи повторное использование её в клиенте Telegram при обращении к API.Bug Fixes:
Enhancements:
Build:
commitizenи связанных библиотек) вrequirements.txt.Original summary in English
Summary by Sourcery
Secure Telegram magic link generation behind a shared bot secret and improve logging around link creation.
New Features:
Bug Fixes:
Enhancements:
Build: