lekss361/gendesign

Fork 0

feat(tradein): cian_session cookies management + admin endpoints (Wave 4 Worker A) #457

Merged

lekss361 merged 2 commits from feat/tradein-cian-session-cookies into main

2026-05-23 13:22:01 +00:00

lekss361 commented

2026-05-23 13:17:40 +00:00

Owner

Summary

Stage 4 / Wave 4 of CianScraper v1. Cookie management infrastructure для Cian Valuation Calculator auth (Stage 7 dependency).

Files

File	Purpose
`app/services/cian_session.py` (NEW)	load/save/verify encrypted Cian cookies — `save_session()`, `load_session()`, `verify_session()`, `mark_session_invalid()`
`app/api/v1/admin.py` (MOD)	+ 2 endpoints (preserve existing)
`app/core/config.py` (MOD)	+ `cookie_encryption_key` setting (env `COOKIE_ENCRYPTION_KEY`)
`tests/test_cian_session.py` (NEW)	16 unit tests
`scripts/upload-cian-cookies.sh` (NEW)	CLI helper для upload flow

Endpoints added

Method	Path	Purpose
POST	`/api/v1/admin/scrape/cian/upload-cookies`	Filter recognized cookies → verify auth → `pgp_sym_encrypt` UPSERT
GET	`/api/v1/admin/scrape/cian/test-auth`	Load session + reverify → `{authenticated, userId, reason}`

Both behind existing require_admin X-Admin-Token middleware.

CIAN_REQUIRED_COOKIES (finalized)

_ym_uid, _ym_d, _ym_isad, _cian_visitor_session_id, _cian_app_session_id, _cian_uid, _ga, tlsr_id, cf_clearance, session-id, anti_bot, csrftoken

Encryption

pgp_sym_encrypt via settings.cookie_encryption_key (env COOKIE_ENCRYPTION_KEY)
Storage: cian_session_cookies table (migration 027 — already в prod)
TTL default 30 days; auto-detect expiry через state.user.isAuthenticated check
Если key empty → endpoints return 503 encryption_key_not_configured (graceful degradation)

Reused existing admin.py patterns

Router: router = APIRouter() (unchanged)
Auth: dependencies=[Depends(require_admin)] — matches existing POST /scrape pattern
DB: Annotated[Session, Depends(get_db)] — same as geocode_missing

Tests

16/16 tests pass
Coverage: cookies set, SQL containment checks (pgp_sym_encrypt, CAST AS bigint, ON CONFLICT), commit assertions, params, verify_session paths (no state / isAuthenticated=false / authenticated → state)

Ruff

New files clean. Pre-existing E501 в admin.py не тронуты.

Refs

decisions/CianScraper_v1_Implementation_Plan.md sec "🔥 Cookie auth для Cian Valuation Calculator" + Stage 4
decisions/Schema_Cian_SERP_Inventory.md sec 24 (auth requirements)
SQL migration 027 (cian_session_cookies table)

Test plan

16 unit tests pass
Ruff clean
Post-deploy: set COOKIE_ENCRYPTION_KEY env → POST cookies → GET test-auth returns {authenticated:true, userId:...}
Stage 7 (cian_valuation.py) unblocked — load_session() доступна

## Summary **Stage 4 / Wave 4** of CianScraper v1. Cookie management infrastructure для Cian Valuation Calculator auth (Stage 7 dependency). ## Files | File | Purpose | |---|---| | `app/services/cian_session.py` (NEW) | load/save/verify encrypted Cian cookies — `save_session()`, `load_session()`, `verify_session()`, `mark_session_invalid()` | | `app/api/v1/admin.py` (MOD) | + 2 endpoints (preserve existing) | | `app/core/config.py` (MOD) | + `cookie_encryption_key` setting (env `COOKIE_ENCRYPTION_KEY`) | | `tests/test_cian_session.py` (NEW) | 16 unit tests | | `scripts/upload-cian-cookies.sh` (NEW) | CLI helper для upload flow | ## Endpoints added | Method | Path | Purpose | |---|---|---| | POST | `/api/v1/admin/scrape/cian/upload-cookies` | Filter recognized cookies → verify auth → `pgp_sym_encrypt` UPSERT | | GET | `/api/v1/admin/scrape/cian/test-auth` | Load session + reverify → `{authenticated, userId, reason}` | Both behind existing `require_admin` X-Admin-Token middleware. ## CIAN_REQUIRED_COOKIES (finalized) `_ym_uid`, `_ym_d`, `_ym_isad`, `_cian_visitor_session_id`, `_cian_app_session_id`, `_cian_uid`, `_ga`, `tlsr_id`, `cf_clearance`, `session-id`, `anti_bot`, `csrftoken` ## Encryption - `pgp_sym_encrypt` via `settings.cookie_encryption_key` (env `COOKIE_ENCRYPTION_KEY`) - Storage: `cian_session_cookies` table (migration 027 — already в prod) - TTL default 30 days; auto-detect expiry через `state.user.isAuthenticated` check - Если key empty → endpoints return `503 encryption_key_not_configured` (graceful degradation) ## Reused existing admin.py patterns - Router: `router = APIRouter()` (unchanged) - Auth: `dependencies=[Depends(require_admin)]` — matches existing `POST /scrape` pattern - DB: `Annotated[Session, Depends(get_db)]` — same as `geocode_missing` ## Tests - 16/16 tests pass - Coverage: cookies set, SQL containment checks (`pgp_sym_encrypt`, `CAST AS bigint`, `ON CONFLICT`), commit assertions, params, `verify_session` paths (no state / `isAuthenticated=false` / authenticated → state) ## Ruff New files clean. Pre-existing E501 в admin.py не тронуты. ## Refs - `decisions/CianScraper_v1_Implementation_Plan.md` sec "🔥 Cookie auth для Cian Valuation Calculator" + Stage 4 - `decisions/Schema_Cian_SERP_Inventory.md` sec 24 (auth requirements) - SQL migration 027 (`cian_session_cookies` table) ## Test plan - [x] 16 unit tests pass - [x] Ruff clean - [ ] Post-deploy: set COOKIE_ENCRYPTION_KEY env → POST cookies → GET test-auth returns `{authenticated:true, userId:...}` - [ ] Stage 7 (cian_valuation.py) unblocked — `load_session()` доступна

lekss361 added 2 commits 2026-05-23 13:17:41 +00:00

feat(tradein): cian_detail.py — detail page scraper (88 fields + BTI + priceChanges, Wave 4 Worker B) f6ece38619

- fetch_detail(): curl_cffi AsyncSession → HTML → extract_state(mfe='frontend-offer-card', key='defaultState')
- DetailEnrichment dataclass: cian_id, windows_view_type, wcs counts, ceiling_height, repair_type, views_total/today, bti_data, price_changes, agent_profile, newbuilding_data, raw_offer/sister_states
- extract_all_states() sister container stash: bti (houseData), newObject, plus raw dump for future extraction
- save_detail_enrichment(): UPDATE listings (all 019 cols present), INSERT offer_price_history (ON CONFLICT DO NOTHING), UPSERT agents + link agent_id_fk, psycopg v3 CAST(:x AS type) everywhere
- Tests: 19 tests (unit helpers + async mocked fetch_detail: no-state, 403, core fields, BTI sister state, newbuilding link)
- Ruff clean, asyncio_mode=auto

feat(tradein): cian_session cookies management + admin endpoints (Wave 4 Worker A) 34e8c72e6c

- New: app/services/cian_session.py — load/save/verify encrypted Cian cookies
  (pgp_sym_encrypt via COOKIE_ENCRYPTION_KEY, psycopg v3, httpx)
- Modified: app/api/v1/admin.py — POST /scrape/cian/upload-cookies +
  GET /scrape/cian/test-auth (behind existing require_admin X-Admin-Token)
- Modified: app/core/config.py — add cookie_encryption_key setting (env COOKIE_ENCRYPTION_KEY)
- New: tests/test_cian_session.py — 16 unit tests (16/16 passed)
- New: scripts/upload-cian-cookies.sh — CLI helper for cookie upload flow

lekss361 merged commit 2dd76ea5e8 into main

2026-05-23 13:22:01 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 13:22:01 +00:00

feat(tradein): cian_session cookies management + admin endpoints (Wave 4 Worker A) (#457)

lekss361 commented

2026-05-23 13:22:49 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE with minor follow-ups.

Deep review summary

Status: APPROVE (squash merged, commit 2dd76ea5)
Files reviewed: 7 (P0: 1 SQL-equivalent service, P1: 2 service+admin, P2: 1 config, P3: 3 tests+CLI)
LOC: +1206/-0

Critical concern resolved: `require_admin` is alive and well

PR #437 / #442 context (removed AdminTokenAuth from 43 endpoints) applies to main backend/app/, NOT to tradein-mvp/backend/app/. This is a separate standalone trade-in MVP with its own auth: require_admin is locally defined in tradein-mvp/backend/app/api/v1/admin.py:31-42 and gates on settings.admin_token env var. Existing POST /scrape + POST /geocode-missing use same pattern. New endpoints are consistent.

Verified

Migration 027 (cian_session_cookies) exists in repo (tradein-mvp/backend/data/sql/027_cian_session_cookies.sql) with pgcrypto extension + correct schema (account_user_id PK, cookies_encrypted bytea, expires_at_estimate, last_invalid_at, last_used_at, notes).
All listings columns referenced by cian_detail.save_detail_enrichment exist in 019_listings_alter_cian.sql (windows_view_type, separate_wcs_count, combined_wcs_count, ceiling_height, repair_type, views_total, views_today).
offer_price_history table + agents table + listings.agent_id_fk exist (022, 023).
extract_state / extract_all_states functions exist in cian_state_parser.py.
curl_cffi, httpx, pytest-asyncio (with asyncio_mode = "auto") already in pyproject.toml.
Deploy workflow .forgejo/workflows/deploy-tradein.yml triggers on tradein-mvp/backend/**.
SQL uses parameterized :key (not string interpolation) — encryption key not injected into SQL text.
Logger never prints raw cookies / key — only userId and counts.
CAST(:x AS bigint) everywhere — psycopg v3 compatible.
ON CONFLICT DO UPDATE for save, ON CONFLICT DO NOTHING for price history (correct idempotency).

Discovered scope mismatch (informational)

PR description claims 5 files / 16 tests. Actual: 7 files including cian_detail.py (324 lines, Stage 5) + test_cian_detail.py (324 lines) which were not mentioned in description. These are clean additions (no conflicts, mocked tests pass own coverage), but represents Stage 5 work landing inside a Stage 4 PR. Worth noting for Wave 4 tracker — Stage 5 was bundled, not separate PR.

Minor follow-ups (non-blocking)

cookie_encryption_key: str = "" → consider SecretStr — Pydantic plain str will show in repr(settings) if ever logged. Not critical (key is non-public and admin-only), but Pydantic best practice. File: tradein-mvp/backend/app/core/config.py:33.
Empty cleaned check is weak — upload-cookies accepts payload with even a single recognized cookie (e.g. only _ym_uid) and proceeds to verify_session(). Auth will fail there gracefully (no cf_clearance → 403/captcha from Cian), so safe — but might be clearer to reject upfront if cf_clearance / _cian_uid missing. Optional.
No rate limit on /test-auth — admin endpoint, but each call hits Cian. Document admin abuse risk in runbook. Optional.
CLI helper upload-cian-cookies.sh reads TRADEIN_ADMIN_TOKEN from env first then .env.runtime — correct, no leak via argv.
Encryption test gap — no roundtrip test (encrypt → decrypt → equal). All current tests mock SQL execution. Real validation will happen post-deploy via manual verify step.
test_save_session_passes_correct_params (test_cian_session.py:79) — defensive param extraction tries multiple patterns (call_args[0][1] vs call_args[1].get("params")). Brittle but harmless. Could simplify after first CI run confirms which path.
Key rotation undocumented — if COOKIE_ENCRYPTION_KEY changes, existing rows in cian_session_cookies become unrecoverable. Suggest decisions/Cookie_Encryption_Strategy_May23.md documenting: key generation, rotation flow (re-upload all cookies after rotation), recovery path.

Post-merge manual verification (per PR test plan)

CI: deploy-tradein.yml should trigger on paths: tradein-mvp/backend/** — verify green.
Pytest: 16 tests in test_cian_session.py + 12 tests in test_cian_detail.py should all pass.
VPS env: add COOKIE_ENCRYPTION_KEY=$(openssl rand -base64 32) to .env.runtime → restart tradein-backend.
Export Cian cookies via Chrome DevTools → run scripts/upload-cian-cookies.sh cookies.json → expect {"ok": true, "userId": N, "cookieCount": M}.
GET /api/v1/admin/scrape/cian/test-auth → expect {"authenticated": true, "userId": N}.
Stage 7 (cian_valuation.py) can now consume cian_session_svc.load_session(db).

Risk / reversibility

Risk: Low. New code is isolated (cian_session.py, cian_detail.py); admin.py modifications are append-only; no existing logic touched.
Reversibility: Easy revert — migration 027 is CREATE TABLE IF NOT EXISTS (idempotent, harmless if rolled back).
Merge window: Anytime — no breaking changes, dormant until Stage 7 wires up.

Merged via deep-code-reviewer — verdict APPROVE with minor follow-ups. ## Deep review summary **Status**: APPROVE (squash merged, commit `2dd76ea5`) **Files reviewed**: 7 (P0: 1 SQL-equivalent service, P1: 2 service+admin, P2: 1 config, P3: 3 tests+CLI) **LOC**: +1206/-0 ### Critical concern resolved: `require_admin` is alive and well PR #437 / #442 context (removed `AdminTokenAuth` from 43 endpoints) applies to **main `backend/app/`**, NOT to `tradein-mvp/backend/app/`. This is a **separate standalone trade-in MVP** with its own auth: `require_admin` is locally defined in `tradein-mvp/backend/app/api/v1/admin.py:31-42` and gates on `settings.admin_token` env var. Existing `POST /scrape` + `POST /geocode-missing` use same pattern. New endpoints are consistent. ### Verified - Migration 027 (`cian_session_cookies`) exists in repo (`tradein-mvp/backend/data/sql/027_cian_session_cookies.sql`) with `pgcrypto` extension + correct schema (account_user_id PK, cookies_encrypted bytea, expires_at_estimate, last_invalid_at, last_used_at, notes). - All listings columns referenced by `cian_detail.save_detail_enrichment` exist in 019_listings_alter_cian.sql (`windows_view_type`, `separate_wcs_count`, `combined_wcs_count`, `ceiling_height`, `repair_type`, `views_total`, `views_today`). - `offer_price_history` table + `agents` table + `listings.agent_id_fk` exist (022, 023). - `extract_state` / `extract_all_states` functions exist in `cian_state_parser.py`. - `curl_cffi`, `httpx`, `pytest-asyncio` (with `asyncio_mode = "auto"`) already in `pyproject.toml`. - Deploy workflow `.forgejo/workflows/deploy-tradein.yml` triggers on `tradein-mvp/backend/**`. - SQL uses parameterized `:key` (not string interpolation) — encryption key not injected into SQL text. - Logger never prints raw cookies / key — only `userId` and counts. - `CAST(:x AS bigint)` everywhere — psycopg v3 compatible. - `ON CONFLICT DO UPDATE` for save, `ON CONFLICT DO NOTHING` for price history (correct idempotency). ### Discovered scope mismatch (informational) PR description claims 5 files / 16 tests. Actual: **7 files including `cian_detail.py` (324 lines, Stage 5) + `test_cian_detail.py` (324 lines)** which were not mentioned in description. These are clean additions (no conflicts, mocked tests pass own coverage), but represents Stage 5 work landing inside a Stage 4 PR. Worth noting for Wave 4 tracker — Stage 5 was bundled, not separate PR. ### Minor follow-ups (non-blocking) 1. **`cookie_encryption_key: str = ""` → consider `SecretStr`** — Pydantic plain str will show in `repr(settings)` if ever logged. Not critical (key is non-public and admin-only), but Pydantic best practice. File: `tradein-mvp/backend/app/core/config.py:33`. 2. **Empty `cleaned` check is weak** — `upload-cookies` accepts payload with even a single recognized cookie (e.g. only `_ym_uid`) and proceeds to `verify_session()`. Auth will fail there gracefully (no `cf_clearance` → 403/captcha from Cian), so safe — but might be clearer to reject upfront if `cf_clearance` / `_cian_uid` missing. Optional. 3. **No rate limit on `/test-auth`** — admin endpoint, but each call hits Cian. Document admin abuse risk in runbook. Optional. 4. **CLI helper `upload-cian-cookies.sh`** reads `TRADEIN_ADMIN_TOKEN` from env first then `.env.runtime` — correct, no leak via argv. 5. **Encryption test gap** — no roundtrip test (encrypt → decrypt → equal). All current tests mock SQL execution. Real validation will happen post-deploy via manual verify step. 6. **`test_save_session_passes_correct_params`** (test_cian_session.py:79) — defensive param extraction tries multiple patterns (`call_args[0][1]` vs `call_args[1].get("params")`). Brittle but harmless. Could simplify after first CI run confirms which path. 7. **Key rotation undocumented** — if `COOKIE_ENCRYPTION_KEY` changes, existing rows in `cian_session_cookies` become unrecoverable. Suggest `decisions/Cookie_Encryption_Strategy_May23.md` documenting: key generation, rotation flow (re-upload all cookies after rotation), recovery path. ### Post-merge manual verification (per PR test plan) 1. CI: `deploy-tradein.yml` should trigger on `paths: tradein-mvp/backend/**` — verify green. 2. Pytest: 16 tests in `test_cian_session.py` + 12 tests in `test_cian_detail.py` should all pass. 3. VPS env: add `COOKIE_ENCRYPTION_KEY=$(openssl rand -base64 32)` to `.env.runtime` → restart tradein-backend. 4. Export Cian cookies via Chrome DevTools → run `scripts/upload-cian-cookies.sh cookies.json` → expect `{"ok": true, "userId": N, "cookieCount": M}`. 5. `GET /api/v1/admin/scrape/cian/test-auth` → expect `{"authenticated": true, "userId": N}`. 6. Stage 7 (`cian_valuation.py`) can now consume `cian_session_svc.load_session(db)`. ### Risk / reversibility - **Risk**: Low. New code is isolated (`cian_session.py`, `cian_detail.py`); admin.py modifications are append-only; no existing logic touched. - **Reversibility**: Easy revert — migration 027 is `CREATE TABLE IF NOT EXISTS` (idempotent, harmless if rolled back). - **Merge window**: Anytime — no breaking changes, dormant until Stage 7 wires up.

lekss361 referenced this pull request

2026-05-23 13:26:02 +00:00

feat(tradein): cian_detail.py — detail page scraper (88 fields + BTI + priceChanges, Wave 4 Worker B retry) #463

lekss361 referenced this pull request

2026-05-23 13:28:44 +00:00

feat(tradein): Stage 4a — API endpoints for Avito enrichment + IMV benchmark UI #460

lekss361 referenced this pull request

2026-05-23 13:35:04 +00:00

feat(tradein): Stage 4a — API endpoints for Avito enrichment + IMV benchmark UI #460

lekss361 referenced this pull request

2026-05-23 13:38:27 +00:00

feat(tradein): cian_valuation.py — auth-required Calculator scraper + 24h cache #465