feat: add vk_id app for VK authentication#73
feat: add vk_id app for VK authentication#73LovichLevich wants to merge 2 commits intohexlet-volunteers:mainfrom
Conversation
maxjamchuk
left a comment
maxjamchuk
left a comment
There was a problem hiding this comment.
Отличная работа, но необходимы несколько изменений.
Главное поправить CAUTION: добавить отсутствующие vk_start_auth / vk_draft_login (или привести urls.py в соответствие), и завершить OAuth-флоу.
Затем пройтись по WARNING: не логировать state целиком и сделать конфиг VK более “fail-fast”.
| from app.services.auth.vk_id import views | ||
|
|
||
| urlpatterns = [ | ||
| path("", views.vk_draft_login, name="vk_login"), |
There was a problem hiding this comment.
Caution
вы вызываете views.vk_draft_login, но в views.py этих функций в диффе нет. В результате импорт/роутинг упадут при старте Django (AttributeError при доступе к атрибутам модуля).
Вероятно вы просто забыли добавить эти изменения в коммит.
|
|
||
| urlpatterns = [ | ||
| path("", views.vk_draft_login, name="vk_login"), | ||
| path("start/", views.vk_start_auth, name="vk_start"), |
There was a problem hiding this comment.
Caution
вы вызываете views.vk_start_auth, но в views.py этих функций в диффе нет. В результате импорт/роутинг упадут при старте Django (AttributeError при доступе к атрибутам модуля).
Вероятно вы просто забыли добавить эти изменения в коммит.
There was a problem hiding this comment.
Caution
нужно либо добавить недостающие функции в app/services/auth/vk_id/views.py, либо убрать эти URL’ы.
There was a problem hiding this comment.
Caution
отсутствуют vk_start_auth и vk_draft_login, хотя они используются в app/services/auth/vk_id/urls.py (см. выше).
| session_state = request.session.get("vk_state") | ||
|
|
||
| if not state or state != session_state: | ||
| logger.warning(f"Invalid state. Expected: {session_state}, Got: {state}") |
There was a problem hiding this comment.
Warning
validate_state() логирует ожидаемый и пришедший state целиком
state — это анти-CSRF токен. В логах (особенно централизованных) лучше не светить такие значения. Минимум: логировать факт несовпадения без значений, или маскировать (abcd…wxyz).
There was a problem hiding this comment.
Tip
стиль получения env-переменных сейчас смешанный (os.environ.get и os.getenv, плюс кавычки одинарные/двойные). Это не баг, но лучше привести к одному стилю по проекту для читаемости.
| EMAIL_HOST_PASSWORD = os.environ.get("EMAIL_HOST_PASSWORD", "") | ||
| EMAIL_TIMEOUT = int(os.environ.get("EMAIL_TIMEOUT", 10)) | ||
|
|
||
| VK_CLIENT_ID = os.getenv('VK_CLIENT_ID', '') |
There was a problem hiding this comment.
Warning
пустые дефолты для критичных VK-полей могут скрывать проблему конфигурации до первого запроса.
| EMAIL_TIMEOUT = int(os.environ.get("EMAIL_TIMEOUT", 10)) | ||
|
|
||
| VK_CLIENT_ID = os.getenv('VK_CLIENT_ID', '') | ||
| VK_CLIENT_SECRET = os.getenv('VK_CLIENT_SECRET', '') |
There was a problem hiding this comment.
Warning
пустые дефолты для критичных VK-полей могут скрывать проблему конфигурации до первого запроса.
|
|
||
| VK_CLIENT_ID = os.getenv('VK_CLIENT_ID', '') | ||
| VK_CLIENT_SECRET = os.getenv('VK_CLIENT_SECRET', '') | ||
| VK_REDIRECT_URI = os.getenv('VK_REDIRECT_URI', '') No newline at end of file |
There was a problem hiding this comment.
Warning
пустые дефолты для критичных VK-полей могут скрывать проблему конфигурации до первого запроса.
There was a problem hiding this comment.
Привести к единому стилю и добавить явную проверку/валидацию VK-конфига (или в settings, или в коде перед запросами).
3 participants
Нужно зарегистрировать приложение в VK, получить ключи и указать url redirect