fix(trade-in): усилить валидацию /lead и почистить контракт (#2376)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 12s
CI / changes (pull_request) Successful in 9s
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 58s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 12s
CI / changes (pull_request) Successful in 9s
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 58s
- [MED] phone: маска r"^[+]?[\d\s().-]{5,32}$" делала цифры опциональными
("(()) -- .." проходило) — добавлен field_validator, требующий 10-15
реальных цифр (re.sub(r"\D","")), формат-толерантность сохранена.
- [MED] source: убран мёртвый литерал "landing" из Literal — воронка
недостижима (rbac_guard закрывает /lead 401 без X-Authenticated-User,
публичного лендинг-роута нет), контракт больше не обещает невозможное.
- [LOW] 152-ФЗ: захват client_ip (X-Forwarded-For -> peer) и
_CONSENT_POLICY_VERSION в audit-лог; live-схема trade_in_leads не имеет
колонок под IP/policy/consent-snapshot -> TODO на follow-up миграцию
(out of scope), durable proof-of-consent gap задокументирован.
Тесты: +digit-free/too-few-digits phone 422, +source=landing 422,
estimate-тест переведён на source=result. 9/9 pass, ruff clean.
This commit is contained in:
parent
0f4ed2f6e9
commit
0583fb0efd
2 changed files with 79 additions and 6 deletions
|
|
@ -1,6 +1,6 @@
|
||||||
"""Trade-in lead capture endpoint (issue #2376, sub-issue родителя #1971).
|
"""Trade-in lead capture endpoint (issue #2376, sub-issue родителя #1971).
|
||||||
|
|
||||||
POST /api/v1/trade-in/lead — контактная заявка с результата оценки (или лендинга):
|
POST /api/v1/trade-in/lead — контактная заявка с результата оценки:
|
||||||
телефон + явное согласие на обработку персональных данных. Persist в
|
телефон + явное согласие на обработку персональных данных. Persist в
|
||||||
trade_in_leads. Notification (Telegram/email) — вне scope: нет существующей
|
trade_in_leads. Notification (Telegram/email) — вне scope: нет существующей
|
||||||
SMTP/Telegram интеграции в коде (подтверждено при разборе issue), только
|
SMTP/Telegram интеграции в коде (подтверждено при разборе issue), только
|
||||||
|
|
@ -10,11 +10,12 @@ persist + log; `notified_at` в таблице зарезервирован по
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import re
|
||||||
from typing import Annotated, Any, Literal
|
from typing import Annotated, Any, Literal
|
||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field, field_validator
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
|
|
@ -28,13 +29,36 @@ router = APIRouter()
|
||||||
# скобки/дефисы/точки, 5-32 символа. Полная нормализация в E.164 — вне scope MVP,
|
# скобки/дефисы/точки, 5-32 символа. Полная нормализация в E.164 — вне scope MVP,
|
||||||
# см. #2376 DoD ("простая regex, не EmailStr-подобное").
|
# см. #2376 DoD ("простая regex, не EmailStr-подобное").
|
||||||
_PHONE_PATTERN = r"^[+]?[\d\s().-]{5,32}$"
|
_PHONE_PATTERN = r"^[+]?[\d\s().-]{5,32}$"
|
||||||
|
# Маска выше делает цифры ОПЦИОНАЛЬНЫМИ: "(()) -- .." её проходит (0 цифр).
|
||||||
|
# Поэтому дополнительно требуем правдоподобное число реальных цифр. RU-мобильный =
|
||||||
|
# 11 цифр; берём лениентный диапазон 10-15 (нац. номер без/с кодом страны).
|
||||||
|
_PHONE_MIN_DIGITS = 10
|
||||||
|
_PHONE_MAX_DIGITS = 15
|
||||||
|
|
||||||
|
# Версия политики обработки ПДн (152-ФЗ), под которую собрано согласие. Пока пишется
|
||||||
|
# только в audit-лог: в trade_in_leads НЕТ колонок под policy_version / client_ip /
|
||||||
|
# снимок текста согласия (проверено live: \d trade_in_leads, 2026-07-12). Полноценный
|
||||||
|
# proof-of-consent требует follow-up миграции — см. TODO у INSERT ниже.
|
||||||
|
_CONSENT_POLICY_VERSION = "2026-07"
|
||||||
|
|
||||||
|
|
||||||
class TradeInLeadInput(BaseModel):
|
class TradeInLeadInput(BaseModel):
|
||||||
phone: str = Field(min_length=5, max_length=32, pattern=_PHONE_PATTERN)
|
phone: str = Field(min_length=5, max_length=32, pattern=_PHONE_PATTERN)
|
||||||
estimate_id: UUID | None = None
|
estimate_id: UUID | None = None
|
||||||
consent: Literal[True]
|
consent: Literal[True]
|
||||||
source: Literal["result", "landing"] = "result"
|
# "landing"-воронка недостижима: /api/v1/trade-in/lead закрыт rbac_guard
|
||||||
|
# (main.py — путь не в _PUBLIC_PATHS => 401 без X-Authenticated-User), а
|
||||||
|
# публичного лендинг-роута нет. Убрали мёртвый литерал, чтобы контракт не
|
||||||
|
# обещал невозможную воронку (#2376). Вернуть, если появится public-роут.
|
||||||
|
source: Literal["result"] = "result"
|
||||||
|
|
||||||
|
@field_validator("phone")
|
||||||
|
@classmethod
|
||||||
|
def _phone_has_enough_digits(cls, value: str) -> str:
|
||||||
|
digits = len(re.sub(r"\D", "", value))
|
||||||
|
if not (_PHONE_MIN_DIGITS <= digits <= _PHONE_MAX_DIGITS):
|
||||||
|
raise ValueError(f"phone must contain {_PHONE_MIN_DIGITS}-{_PHONE_MAX_DIGITS} digits")
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
@router.post("/lead")
|
@router.post("/lead")
|
||||||
|
|
@ -53,7 +77,21 @@ async def create_trade_in_lead(
|
||||||
raise HTTPException(status_code=404, detail="estimate not found")
|
raise HTTPException(status_code=404, detail="estimate not found")
|
||||||
|
|
||||||
user_agent = request.headers.get("user-agent")
|
user_agent = request.headers.get("user-agent")
|
||||||
|
# 152-ФЗ audit trail: реальный клиентский IP из X-Forwarded-For (его ставит
|
||||||
|
# фронтящий Caddy), fallback — прямой peer. Пишем только в лог: durable-колонки
|
||||||
|
# под IP в trade_in_leads нет (см. TODO у INSERT).
|
||||||
|
client_ip = request.headers.get("x-forwarded-for")
|
||||||
|
if client_ip:
|
||||||
|
client_ip = client_ip.split(",")[0].strip()
|
||||||
|
elif request.client is not None:
|
||||||
|
client_ip = request.client.host
|
||||||
|
|
||||||
|
# TODO(152-ФЗ proof-of-consent): trade_in_leads хранит согласие как голый boolean
|
||||||
|
# + created_at + user_agent. Для полноценного доказательства согласия нужна
|
||||||
|
# follow-up миграция с колонками client_ip (inet), consent_policy_version (text),
|
||||||
|
# consent_text_snapshot (text) — тогда client_ip/_CONSENT_POLICY_VERSION пойдут в
|
||||||
|
# строку, а не только в лог. Live-схема на 2026-07-12 таких колонок НЕ имеет
|
||||||
|
# (out of scope этого PR).
|
||||||
row = (
|
row = (
|
||||||
db.execute(
|
db.execute(
|
||||||
text(
|
text(
|
||||||
|
|
@ -78,10 +116,12 @@ async def create_trade_in_lead(
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"trade_in_lead saved id=%s estimate_id=%s source=%s",
|
"trade_in_lead saved id=%s estimate_id=%s source=%s ip=%s policy=%s",
|
||||||
row["id"],
|
row["id"],
|
||||||
payload.estimate_id,
|
payload.estimate_id,
|
||||||
payload.source,
|
payload.source,
|
||||||
|
client_ip,
|
||||||
|
_CONSENT_POLICY_VERSION,
|
||||||
)
|
)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,8 @@
|
||||||
- happy path: consent=True + валидный телефон -> 200, INSERT + commit
|
- happy path: consent=True + валидный телефон -> 200, INSERT + commit
|
||||||
- consent=False -> 422 (Literal[True] guard)
|
- consent=False -> 422 (Literal[True] guard)
|
||||||
- невалидный формат телефона -> 422
|
- невалидный формат телефона -> 422
|
||||||
|
- телефон без цифр / слишком мало цифр -> 422 (digit-guard, #2376 hardening)
|
||||||
|
- source="landing" (мёртвая воронка) -> 422 (литерал убран из схемы)
|
||||||
- estimate_id, которого нет в trade_in_estimates -> 404
|
- estimate_id, которого нет в trade_in_estimates -> 404
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
@ -121,10 +123,41 @@ def test_lead_with_known_estimate_id_200(client: TestClient, db: MagicMock) -> N
|
||||||
"phone": "+79123456789",
|
"phone": "+79123456789",
|
||||||
"consent": True,
|
"consent": True,
|
||||||
"estimate_id": estimate_id,
|
"estimate_id": estimate_id,
|
||||||
"source": "landing",
|
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
params = db.execute.call_args.args[1]
|
params = db.execute.call_args.args[1]
|
||||||
assert params["estimate_id"] == estimate_id
|
assert params["estimate_id"] == estimate_id
|
||||||
assert params["source"] == "landing"
|
assert params["source"] == "result"
|
||||||
|
|
||||||
|
|
||||||
|
def test_lead_digit_free_phone_422(client: TestClient, db: MagicMock) -> None:
|
||||||
|
# "(()) -- .." проходит regex-маску (только +/скобки/дефисы/точки/пробелы),
|
||||||
|
# но содержит 0 цифр -> должно отклоняться digit-валидатором.
|
||||||
|
r = client.post(
|
||||||
|
"/api/v1/trade-in/lead",
|
||||||
|
json={"phone": "(()) -- ..", "consent": True},
|
||||||
|
)
|
||||||
|
assert r.status_code == 422, r.text
|
||||||
|
assert not db.execute.called
|
||||||
|
|
||||||
|
|
||||||
|
def test_lead_too_few_digits_phone_422(client: TestClient, db: MagicMock) -> None:
|
||||||
|
# 6 цифр < минимума 10 -> 422 (маску проходит по длине, digit-guard режет).
|
||||||
|
r = client.post(
|
||||||
|
"/api/v1/trade-in/lead",
|
||||||
|
json={"phone": "123-456", "consent": True},
|
||||||
|
)
|
||||||
|
assert r.status_code == 422, r.text
|
||||||
|
assert not db.execute.called
|
||||||
|
|
||||||
|
|
||||||
|
def test_lead_source_landing_rejected_422(client: TestClient, db: MagicMock) -> None:
|
||||||
|
# "landing"-воронка недостижима (rbac_guard закрывает /lead) — литерал убран
|
||||||
|
# из схемы, поэтому явный source="landing" теперь 422.
|
||||||
|
r = client.post(
|
||||||
|
"/api/v1/trade-in/lead",
|
||||||
|
json={"phone": "+79123456789", "consent": True, "source": "landing"},
|
||||||
|
)
|
||||||
|
assert r.status_code == 422, r.text
|
||||||
|
assert not db.execute.called
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue