diff --git a/docs/sonnet-full-architecture-analysis.md b/docs/sonnet-full-architecture-analysis.md new file mode 100644 index 0000000..e482845 --- /dev/null +++ b/docs/sonnet-full-architecture-analysis.md @@ -0,0 +1,268 @@ +# Полный архитектурный анализ ipwhitelist-app + +Дата: 2026-06-11. Версия кода: 0.5.60. НЕ является инструкцией к правке — только диагностика. + +--- + +## 1. Auth-поток: дублирование + +### Два парсера JWT вместо одного + +`userFromPayload()` (src/auth.js) и `resolveUser` middleware (ui/index.js) делают одно и то же — извлекают `allClientIds`, `activeClientId`, `email` из payload — но по-разному: + +| Поле | `userFromPayload` | `resolveUser` | +|---|---|---| +| `ClientID` | `payload.ClientID \|\| payload.client_id` | `payload.ClientID \|\| payload.clientId \|\| payload.sub` | +| KC claims | обрабатывает `payload.claims` (массив wz-строк) | **не обрабатывает** | +| email | 4 fallback-а: email, login, sub, preferred_username | `req.session?.user?.email \|\| payload.email \|\| payload.login` | +| activeClientId | первый из allClientIds | из сессии (если есть) или первый из токена | +| isAdmin | `activeClientId === ADMIN_CLIENT_ID` | `iamAdmin \|\| isNail` | + +**Что не так:** если Keycloak-токен выдаёт clientIds в `payload.claims[]`, `resolveUser` их не видит — пользователь получает `activeClientId = ''` или `rawClientId`. `userFromPayload` видит, но он применяется только в `bearerMiddleware` (API-слой без сессии). + +**Где `req.session.user` расходится с `payload`:** +- `session.user.isAdmin` — из IAM (реальный флаг). JWT-payload этого поля не содержит. +- `session.user.activeClientId` — может быть изменён переключателем компании; JWT этого не знает. +- `session.user.profiles[]` — приходит только из IAM, в JWT отсутствует. +- `session.user.isImpersonated` — из IAM, в JWT нет. + +### Тройное дублирование конструктора `session.user` + +Объект `session.user` строится в трёх местах по одной схеме, но с разными полями: + +1. `src/routes/oidc.js` → `/callback` (IAM-ветка и fallback) +2. `ui/routes/auth.js` → `POST /login-token` (IAM-ветка и fallback) и `POST /login` (mock) +3. `ui/index.js` → `resolveUser` (перестраивает `req.user` из токена + сессии) + +Если добавить новое поле (например, `fio`), нужно обновить все три места. Это уже произошло: `fio` есть в `/callback`, но нет в `resolveUser`. + +**Как исправить:** извлечь `buildSessionUser(iamData)` как общую функцию. Один источник истины — одна правка. + +--- + +## 2. adminMode / canAdminMode / isAdmin / isNail — четыре флага + +### Что каждый означает сейчас + +| Флаг | Тип | Источник | Смысл | +|---|---|---|---| +| `isAdmin` | boolean | IAM `userInfo.isAdmin` или `isNail` | «IAM считает юзера глобальным админом» | +| `isNail` | boolean | хардкод `email === 'ntazetdinov@nubes.ru'` | «это я, временный костыль» | +| `canAdminMode` | boolean | `(isAdmin && clientId===WZ01112) \|\| isNail` | «может ли юзер включить режим админа» | +| `adminMode` | boolean | `session.adminMode` (toggle) | «режим сейчас включён» | + +### Проблемы + +**1. `canAdminMode` добавляет лишнее условие `clientId === WZ01112`.** +Если IAM говорит `isAdmin=true` — почему ограничивать активным clientId? Логично было бы: `canAdminMode = isAdmin`. Либо, если это сознательное ограничение (только сотрудники Nubes могут быть админами) — тогда нужна проверка на стороне IAM, а не хардкод clientId в коде. + +**2. `isNail` присутствует в трёх местах:** +- `ui/index.js` → `resolveUser` (вычисление `isAdmin`, `canAdminMode`) +- `ui/routes/auth.js` → `/toggle-admin` (проверка права на toggle) +- Логика toggle проверяет `clientId === ADMIN_CLIENT_ID` а не `canAdminMode`. Если условие `canAdminMode` изменится — toggle-проверка не обновится автоматически. + +**3. toggle-admin в `ui/routes/auth.js` не использует `canAdminMode`:** +```js +const canAdmin = req.session.user && ( + req.session.user.isAdmin || + req.session.user.clientId === ADMIN_CLIENT_ID // ← другая формула! +); +``` +Это третья, независимая реализация того же условия. + +**Как упростить:** два флага вместо четырёх. +- `isAdmin` — из IAM, без `isNail`. `isNail` убирается после того как IAM-роль для ntazetdinov настроена. +- `adminMode` — из сессии. Право переключиться: `isAdmin === true`. +- `canAdminMode` убирается: он равен `isAdmin`. + +--- + +## 3. Компании: getOrCreateCompany, switchProfile + +### switchProfile при недоступном IAM + +В `ui/routes/entries.js`: +```js +try { + await switchProfile(token, targetProfile.id); + // обновляем сессию... +} catch (e) { + // Локальный fallback без IAM + req.session.user.activeClientId = switchTo; + req.user.activeClientId = switchTo; +} +``` + +**Проблема:** при IAM-недоступности локальный fallback обновляет сессию, но IAM не обновлён. При следующем входе IAM вернёт старый активный профиль. Кроме того, ошибка проглатывается без сообщения пользователю. Пользователь думает, что переключился, а фактически переключение непостоянно — до следующего логина. + +**Как исправить:** показывать предупреждение «Переключение не сохранено в IAM» при fallback. + +### getOrCreateCompany — потенциальный мусор + +`getOrCreateCompany` вызывается в `resolveCompany` на каждый API-запрос. Если пользователь только экспортирует данные через `/api/v1/entries/export` (не-admin ветка), в БД создаётся строка компании с owner_email. Это корректно по ТЗ, но: + +- `owner_email` обновляется только при INSERT. Если тот же `client_id` войдёт с другим email, owner не поменяется. В текущей схеме это не критично, но стоит знать. +- Нет способа понять «мусорные» компании (которые создались из-за fallback или тестов), кроме как смотреть на количество записей. + +### companyQuery — XSS в URL + +В `ui/routes/entries.js`: +```js +function companyQuery(req) { + const id = req.query.company || (req.body && req.body.company_id); + if (id) return '?company=' + id; + ... +} +``` +`id` не проверяется на тип. Если `req.query.company` содержит спецсимволы или строку, они уйдут в API-запрос как есть. В `resolveCompany` есть `parseInt(..., 10)` с проверкой `isFinite` — так что API-сторона защищена. Но URL-строка в UI может быть неожиданной. + +--- + +## 4. Безопасность + +### 4.1 КРИТИЧНО: client_id подмена при недоступном IAM + +В `src/api/routes/entries.js`, `resolveCompany`: +```js +const allowedIds = req.user.profiles && req.user.profiles.length + ? req.user.profiles.map(p => p.client_id) + : (req.user.allClientIds || []); + +const effectiveClientId = (requestedId && (!allowedIds.length || allowedIds.includes(requestedId))) + ? requestedId + : req.user.clientId; +``` + +Условие `!allowedIds.length` означает: если IAM не ответил и JWT не содержит claims — `requestedId` принимается **без проверки**. Злоумышленник, получив Bearer-токен (даже от другой компании с пустым `allClientIds`), может передать `?client_id=WZXXXXX` и получить данные чужой компании. + +**Как исправить:** если `allowedIds` пуст — разрешать только `req.user.clientId` (из самого токена), отклонять `requestedId`. + +### 4.2 КРИТИЧНО: OIDC state проверка отключена навсегда + +В `src/routes/oidc.js`: +```js +if (process.env.NODE_ENV === 'production' || true) { // ← || true делает всегда true + if (req.session) delete req.session.oidcState; +} else { + // state check — никогда не выполняется +} +``` + +Проверка `state` параметра в `/callback` отключена во всех окружениях. Это убирает защиту от CSRF-атак на OAuth-поток. Злоумышленник может подменить `code` в callback. + +**Как исправить:** убрать `|| true`. Если для тестирования нужно пропускать — завести `SKIP_OIDC_STATE=true` env. + +### 4.3 Open Redirect в `/login-token` (clientId ветка) + +В `ui/routes/auth.js`, ветка входа по clientId: +```js +res.redirect(returnTo || '/'); +``` +В отличие от token-ветки выше (`safeReturn(returnTo)`), здесь `returnTo` из тела POST не валидируется. Если `returnTo = 'https://evil.com'` — пользователь улетит туда. + +**Как исправить:** везде использовать `safeReturn(returnTo)` из `src/auth.js`. + +### 4.4 CSRF: применяется ли ко всем POST-формам? + +В `ui/routes/entries.js` все POST-обработчики (`/add`, `/edit/:id`, `/delete/:id`) не содержат явного `doubleCsrfProtection`. При этом в шаблоне в GET `/` передаётся `csrfToken: ''` (пустая строка). Это означает CSRF-защита либо навешена глобально в `server.js` (надо проверить), либо отсутствует на этих маршрутах. + +### 4.5 `/export` — кто имеет доступ + +В `src/api/routes/entries.js` `/export` не требует `isAdmin`. Любой аутентифицированный пользователь получает список CIDR своей компании. Это, скорее всего, правильно по ТЗ (пользователь экспортирует свои же данные). Но стоит уточнить: или `/export` должен быть только в adminMode — тогда нужна проверка в API-слое. + +--- + +## 5. Аудит + +### 5.1 `user_email` в audit_log — всегда ли правильный? + +В API-слое `req.user` строится в `bearerMiddleware` → `userFromPayload(payload)` **без** `iamData`. Email берётся из JWT claims. Если пользователь сменил email в IAM после выдачи токена, в аудит попадёт старый email. Это редкий случай (токен живёт 8 часов), но надо знать. + +### 5.2 `impersonated_by` — всегда NULL в API-слое + +`bearerMiddleware` вызывает `userFromPayload(payload)` без имперсонационных заголовков. `userFromPayload` возвращает объект **без** `originalUserEmail`. Поэтому в `createEntry`, `updateEntry`, `deleteEntry`: +```js +req.user.originalUserEmail // → undefined → logAudit(..., undefined) → NULL +``` +`impersonated_by` никогда не заполняется через API, только если зашить в сессию и передавать через X-заголовки (известный разрыв). + +### 5.3 GET /api/v1/audit — есть или нет? + +В `src/queries.js` есть `getAudit(companyId)`. В `views/audit.ejs` есть шаблон. Но наличие API-эндпоинта не проверено по видимым файлам. Если `/api/v1/audit` не существует, аудит доступен только через UI — что делает его недоступным для API-клиентов. + +--- + +## 6. Общая архитектура + +### 6.1 Циклическая зависимость: ui → src + +`ui/routes/entries.js`: +```js +const { switchProfile } = require('../../src/auth'); +``` + +UI-слой напрямую импортирует из src. Это нарушает разделение слоёв: UI должен ходить только через `api-client.js`. `switchProfile` должен быть обёрнут в `/api/v1/profile/switch` (или аналогичный эндпоинт) и вызываться через api-client. + +### 6.2 Дублированный код (4 места одновременно) + +**allClientIds extraction** — логика парсинга `ClientID` из payload повторяется в: +1. `src/auth.js::userFromPayload` (handles `payload.claims`) +2. `ui/index.js::resolveUser` +3. `ui/routes/auth.js::POST /login-token` +4. `src/routes/oidc.js::fallback` при IAM-недоступности + +**session.user construction** — структура объекта дублируется в: +1. `src/routes/oidc.js` (IAM-ветка + fallback) +2. `ui/routes/auth.js` (IAM-ветка + mock + clientId-debug) + +Общий паттерн нарушения: каждый раз когда IAM недоступен, используется вариация "JWT fallback" — и каждый fallback написан по-разному (разные поля, разные условия). + +### 6.3 Отсутствие обработки ошибок + +- `ui/routes/entries.js::GET /` — если в adminMode `api.get('/api/v1/companies')` бросает, ошибка рендерится в `error.ejs`. OK. Но если после этого `api.get('/api/v1/entries?company=...')` бросает — пользователь видит форму с пустым списком и без явного сообщения (поймал `catch` → `res.render('index', { error: 'Ошибка загрузки...' })`). +- `switchProfile` при ошибке не показывает предупреждение. +- `resolveUser` при невалидном токене делает `req.session.destroy()` и редирект — но не логирует причину на сервере. + +### 6.4 DEV_MODE в production + +`verifyAnyToken` в DEV_MODE декодирует токен **без проверки подписи**: +```js +if (devMode) { + const decoded = jwt.decode(token); + ... + return decoded; +} +``` +Это нормально для dev. Но DEV_MODE проверяется в `initAuth()` и передаётся как замкнутое значение в `createBearerMiddleware`. Если `process.env.DEV_MODE` изменится после запуска — функция всё равно будет работать со старым значением. Жёсткая защита в `initAuth()` (`NODE_ENV=production && devMode → throw`) правильная, но её можно обойти в staging если `NODE_ENV` не выставлен. + +### 6.5 Таймаут IAM API — одинаковый в 2 местах + +`fetchIamUser` и `switchProfile` оба ставят `req.setTimeout(10000, ...)`. Если IAM лагает, весь запрос пользователя ждёт 10 секунд. В идеале иметь константу `IAM_TIMEOUT_MS` в `config.js`. + +### 6.6 `getExportCIDRs` без лимита строк + +```js +async function getExportCIDRs(companyId = null) { + // без LIMIT + return (await pool.query(sql, params)).rows.map(r => r.value_cidr); +} +``` +Admin может вызвать экспорт без `?company` — вернёт все CIDR всех компаний. При большом количестве данных это может быть медленным запросом без пагинации. + +--- + +## Сводная таблица приоритетов + +| # | Проблема | Критичность | Файл | +|---|---|---|---| +| 1 | OIDC state проверка отключена `|| true` | 🔴 КРИТ | src/routes/oidc.js | +| 2 | client_id подмена при пустом allowedIds | 🔴 КРИТ | src/api/routes/entries.js | +| 3 | Open redirect в /login-token (clientId ветка) | 🟠 ВЫСОК | ui/routes/auth.js | +| 4 | impersonated_by всегда NULL в API-слое | 🟠 ВЫСОК | src/auth.js, ui/api-client.js | +| 5 | isNail хардкод в 3 местах | 🟡 СРЕДН | ui/index.js, ui/routes/auth.js | +| 6 | toggle-admin использует другую формулу чем canAdminMode | 🟡 СРЕДН | ui/routes/auth.js | +| 7 | ui → src циклическая зависимость (switchProfile) | 🟡 СРЕДН | ui/routes/entries.js | +| 8 | buildSessionUser дублируется в 3 местах | 🟡 СРЕДН | oidc.js, auth.js, ui/index.js | +| 9 | switchProfile fallback без уведомления пользователя | 🟢 НИЗК | ui/routes/entries.js | +| 10 | getExportCIDRs без лимита строк | 🟢 НИЗК | src/queries.js | +| 11 | IAM_TIMEOUT_MS не вынесен в config | 🟢 НИЗК | src/auth.js | diff --git a/docs/sonnet-impersonation-analysis.md b/docs/sonnet-impersonation-analysis.md new file mode 100644 index 0000000..91978e6 --- /dev/null +++ b/docs/sonnet-impersonation-analysis.md @@ -0,0 +1,333 @@ +# Анализ архитектуры имперсонации — ipwhitelist-app + +> Только анализ. Код не правился. + +--- + +## 1. Диагностика: что именно сломано и почему + +### 1.1 Фундаментальная причина рассинхрона + +Приложение состоит из **двух независимых слоёв**, каждый со своей точкой формирования `req.user`: + +| Слой | Middleware | Источник `req.user` | +|------|-----------|---------------------| +| **UI** | `resolveUser` в `ui/index.js` | `jwt.decode(session.token)` + `req.session.user` | +| **API** | `bearerMiddleware` в `src/auth.js` | `verifyAnyToken(Bearer)` → `userFromPayload(payload)` | + +Когда UI выполняет операцию (POST /add, DELETE /edit/:id), он делает HTTP-запрос к `/api/v1/entries` через `ui/api-client.js`. В этом запросе передаётся **только Bearer-токен** (`req.session.token`). Сессионный контекст (в том числе флаги имперсонации) **не передаётся**. + +API-слой получает оригинальный KC-токен → вызывает `userFromPayload(payload)` → получает оригинальный email из JWT-клеймов. Никакой имперсонации. + +### 1.2 Что произходит шаг за шагом (тестовая имперсонация) + +``` +[Браузер] → GET / + → UI resolveUser: читает session.user.email + env IMPERSONATION_* + → req.user.email = "test@test.ru" (целевой) + → req.user.isImpersonated = true + → рендер index.ejs: жёлтый баннер ✓ + +[Браузер] → POST /add (form submit) + → UI route /add → api.post('/api/v1/entries?...', token, body) + → api-client.js: HTTP POST к localhost:3000/api/v1/entries + headers: Authorization: Bearer + (без сессионных данных!) + +[API bearerMiddleware] + → verifyAnyToken(bearer) → userFromPayload(payload) + → req.user.email = "ntazetdinov@nubes.ru" (оригинальный, из JWT!) + → req.user.originalUserEmail = undefined + +[API POST /api/v1/entries] + → createEntry(company.id, value, comment, + "ntazetdinov@nubes.ru", // ← created_by: НЕПРАВИЛЬНО + undefined // ← impersonated_by: NULL, НЕПРАВИЛЬНО + ) +``` + +**Итог:** баннер в UI отображает имперсонацию корректно, но в БД всё пишется от реального пользователя. Аудит не содержит `impersonated_by`. + +### 1.3 Состояние реальной имперсонации IAM + +При реальной IAM-имперсонации ситуация **немного лучше**, но тоже неполная: + +- `/callback` вызывает `fetchIamUser(accessToken)` → сохраняет `isImpersonated`, `originalUserEmail` в `session.user` +- `resolveUser` в `ui/index.js` читает `req.session.user.isImpersonated` → `req.user.isImpersonated = true` +- НО: `req.user.email` берётся из JWT-клейма, а KC-токен содержит email **того, кто логинился** (администратора, начавшего имперсонацию) +- Поэтому неясно: какой email в KC-токене при реальной IAM-имперсонации — администратора или жертвы? + +Скорее всего KC токен всегда содержит email авторизованного пользователя (администратора), а `iamData.email` из `fetchIamUser` содержит email **целевого** пользователя (кого имперсонируют). Это нужно проверить при тесте. + +### 1.4 Проблема 4 файлов + +``` +src/routes/oidc.js → запись session.user при логине (единственный раз) +ui/index.js → resolveUser: TEST IMP + IAM IMP для UI-слоя +src/auth.js → userFromPayload: только JWT, нет session, нет IMP +src/api/routes/entries.js → req.user от bearerMiddleware (оригинальный) +``` + +Логика имперсонации размазана по трём местам и не доходит до четвёртого. + +--- + +## 2. Ответы на вопросы + +### Вопрос 1: Где должно быть единое место вычисления параметров имперсонации? + +**`src/auth.js`, функция `resolveImpersonation(sessionUser, jwtPayload)`.** + +Но с оговоркой: функция должна получать `sessionUser` (из `req.session.user`) как параметр, потому что `userFromPayload` в API-слое сессии не видит. Единое место **вычисления** — хорошо, но единое место **применения** требует двух вызовов с разными источниками данных. + +Предлагаемая сигнатура: +```js +/** + * Вычисляет итоговые параметры имперсонации. + * @param {object} baseUser — user из JWT (email, clientId, allClientIds) + * @param {object} sessionUser — req.session.user (может быть null в API-слое) + * @returns {object} { email, activeClientId, isImpersonated, originalUserEmail } + */ +function resolveImpersonation(baseUser, sessionUser) { + // 1. Реальная IAM-имперсонация (приоритет) + if (sessionUser && sessionUser.isImpersonated) { + return { + email: sessionUser.email || baseUser.email, + activeClientId: sessionUser.activeClientId || baseUser.activeClientId, + isImpersonated: true, + originalUserEmail: sessionUser.originalUserEmail || '', + }; + } + + // 2. Тестовая имперсонация (env-переменные) + const impOriginal = process.env.IMPERSONATION_ORIGINAL || ''; + const impTarget = process.env.IMPERSONATION_TARGET || ''; + const impCompany = process.env.IMPERSONATION_COMPANY || ''; + const email = (sessionUser && sessionUser.email) || baseUser.email || ''; + + if (impOriginal && email === impOriginal) { + return { + email: impTarget || email, + activeClientId: impCompany || baseUser.activeClientId, + isImpersonated: true, + originalUserEmail: impOriginal, + }; + } + + // 3. Нет имперсонации + return { + email: email || baseUser.email, + activeClientId: (sessionUser && sessionUser.activeClientId) || baseUser.activeClientId, + isImpersonated: false, + originalUserEmail: '', + }; +} +``` + +### Вопрос 2: Как передавать `originalUserEmail` в `logAudit`? + +Текущий механизм (`req.user.originalUserEmail` → параметр в CRUD-функциях) **правильный по структуре**, но ломается из-за того, что API-слой не наполняет `req.user.originalUserEmail`. + +Проблема в API-слое: `bearerMiddleware` → `userFromPayload` → нет сессии → нет `originalUserEmail`. + +**Два варианта передачи:** + +**Вариант A (рекомендуется): кастомные заголовки из api-client** +В `ui/api-client.js` при вызове API добавлять заголовки, если у `req.user` есть флаг имперсонации: +```js +// В apiRequest добавить параметр extraHeaders +if (impersonation.isImpersonated) { + opts.headers['X-Impersonated-Email'] = impersonation.email; // целевой + opts.headers['X-Impersonated-By-Email'] = impersonation.originalUserEmail; // оригинальный + opts.headers['X-Impersonated-Company'] = impersonation.activeClientId; +} +``` +В `bearerMiddleware` читать эти заголовки и переопределять `req.user`: +```js +const xEmail = req.headers['x-impersonated-email']; +if (xEmail && /* базовая валидация */) { + req.user.email = xEmail; + req.user.originalUserEmail = req.headers['x-impersonated-by-email'] || ''; + req.user.isImpersonated = true; + req.user.activeClientId = req.headers['x-impersonated-company'] || req.user.activeClientId; +} +``` + +⚠️ **Важно:** эти заголовки должны приниматься ТОЛЬКО от localhost (127.0.0.1), иначе внешний клиент сможет имперсонировать кого угодно. + +**Вариант B (проще, но менее чисто): читать env в bearerMiddleware** +В `bearerMiddleware` после `userFromPayload` добавить вызов `resolveImpersonation(baseUser, null)`. Так тестовая имперсонация заработает в API-слое без изменения api-client. Для реальной IAM-имперсонации это не поможет — сессии нет. + +**Вывод:** Вариант A решает обе ситуации (тестовую и реальную). Вариант B — только тестовую. + +### Вопрос 3: Как сделать так, чтобы `created_by` показывал целевой email? + +При тестовой имперсонации: +- `resolveImpersonation` возвращает `email = impTarget` +- `req.user.email = impTarget` +- `createEntry(company.id, value, comment, req.user.email, req.user.originalUserEmail)` + → `created_by = impTarget` ✓ + → `impersonated_by = impOriginal` ✓ + +Условие: `resolveImpersonation` должна отработать в API-слое (Вариант A или B из вопроса 2). + +При реальной IAM-имперсонации: `iamData.email` из `fetchIamUser` — это email целевого пользователя? Или администратора? Нужно проверить. Если это email администратора (как можно предположить из логики KC), то: +- `session.user.email` = email администратора +- `session.user.isImpersonated = true` +- `session.user.originalUserEmail` = email кого имперсонируют (целевой) + +Тогда `created_by` должен быть `originalUserEmail`, а `impersonated_by` — `email` (администратор). Это **обратная** логика по сравнению с тестовой! Нужно прояснить с IAM командой до реализации. + +### Вопрос 4: Нужно ли обновлять сессию при изменении статуса имперсонации? + +**Нет. Только при перелогине.** + +Причины: +1. IAM-имперсонация начинается через `deck.ngcloud.ru` (отдельный портал), а не через наше приложение — мы не получаем уведомлений +2. Access-токен от Keycloak имеет ограниченное время жизни; при refresh token мы можем перечитать IAM-статус +3. Постоянные вызовы `fetchIamUser` на каждый запрос — лишняя нагрузка и latency + +**Рекомендация:** перечитывать `fetchIamUser` при refresh-токена (если будет реализован механизм refresh) или добавить кнопку «Обновить профиль» которая делает перелогин. + +**Альтернативно:** вызывать `fetchIamUser` раз в N минут (например, при каждом GET /) и обновлять `session.user`. Но это усложняет код. + +### Вопрос 5: Есть ли более простой способ? + +Да. Самый простой вариант, который решает проблему сейчас: + +**«Embed impersonation in Bearer»** — не менять архитектуру, а в `ui/api-client.js` добавить один параметр `impersonation` и передавать его в заголовках. В `bearerMiddleware` добавить 5 строк чтения этих заголовков с проверкой `req.socket.remoteAddress`. Никакого рефакторинга `resolveImpersonation` не нужно. + +``` +Сейчас: UI-resolveUser строит req.user → API-client игнорирует его → bearerMiddleware строит свой req.user + +Исправление: UI-resolveUser строит req.user → API-client добавляет X-Imp-* заголовки → bearerMiddleware читает их +``` + +Это минимальное изменение в двух файлах: +- `ui/api-client.js` — добавить extraHeaders параметр +- `ui/routes/entries.js` — передавать `req.user` в api.post/patch/delete +- `src/auth.js` `createBearerMiddleware` — читать X-Imp-* от localhost + +--- + +## 3. Предлагаемая архитектура (без рефакторинга ради рефакторинга) + +### Минимальный рабочий вариант (рекомендуется) + +``` +src/auth.js + └── resolveImpersonation(baseUser, sessionUser) ← новая функция, экспорт + └── createBearerMiddleware: + после userFromPayload → читать X-Imp-* от localhost → перезаписать req.user + +ui/api-client.js + └── apiRequest(method, path, token, body, impersonation) ← новый параметр + если impersonation.isImpersonated → добавить X-Imp-* заголовки + +ui/index.js (resolveUser) + └── вместо inline TEST IMP блока → вызов resolveImpersonation(baseUser, session.user) + └── req.impersonation = { isImpersonated, email, activeClientId, originalUserEmail } + +ui/routes/entries.js (и остальные роуты с CRUD) + └── api.post(..., body, req.impersonation) ← передавать impersonation + └── api.patch(..., body, req.impersonation) + └── api.delete(..., null, req.impersonation) +``` + +### Что НЕ менять + +- `src/queries.js` — уже правильно принимает `impersonatedBy` +- `src/api/routes/entries.js` — уже правильно берёт `req.user.email` и `req.user.originalUserEmail` +- `views/index.ejs` — баннер уже работает +- `src/routes/oidc.js` — `/callback` уже сохраняет IAM-данные в сессию + +--- + +## 4. Методика тестирования + +### Тест 1: Тестовая имперсонация (env) — основной + +**Подготовка:** +```bash +IMPERSONATION_ORIGINAL=ntazetdinov@nubes.ru +IMPERSONATION_TARGET=test@test.ru +IMPERSONATION_COMPANY=WZ01325 +``` + +**Шаги и ожидаемые результаты:** + +| # | Действие | Ожидаемый результат | +|---|----------|---------------------| +| 1 | Войти как `ntazetdinov@nubes.ru` | Перенаправление на `/` | +| 2 | Открыть `/` | Жёлтый баннер: «Режим имперсонации — вы: ntazetdinov@nubes.ru» | +| 3 | Компания в UI | Показывает WZ01325 (не WZ01112) | +| 4 | Добавить запись `1.2.3.4` | 201 Created | +| 5 | `SELECT created_by FROM whitelist_entries WHERE value_cidr='1.2.3.4/32'` | `test@test.ru` | +| 6 | `SELECT user_email, impersonated_by FROM audit_log ORDER BY id DESC LIMIT 1` | `user_email=test@test.ru`, `impersonated_by=ntazetdinov@nubes.ru` | +| 7 | Удалить ENV, перелогиниться | Нет баннера, CRUD пишет `ntazetdinov@nubes.ru` в `created_by` | + +### Тест 2: Реальная IAM-имперсонация + +**Подготовка:** на стенде с активированным OIDC, tech-токен WZ01112. + +**Шаги:** + +| # | Действие | Ожидаемый результат | +|---|----------|---------------------| +| 1 | `POST /api/v1/impersonation/start` с tech-токеном | 200 OK | +| 2 | `GET /auth/user` с access-токеном целевого пользователя | `is_impersonated: true`, `originalUserEmail` заполнен | +| 3 | Целевой пользователь логинится → `/callback` → `fetchIamUser` | `session.user.isImpersonated=true` | +| 4 | UI показывает баннер | ✓ | +| 5 | CRUD — проверить `created_by` и `impersonated_by` в БД | Целевой email / оригинальный email | + +⚠️ **Критический вопрос перед реализацией:** уточнить у IAM-команды, чей email содержит `iamData.email` при `is_impersonated=true` — администратора или целевого пользователя? От этого зависит, как назначать `created_by` и `impersonated_by`. + +### Тест 3: Изоляция компаний при имперсонации + +| # | Действие | Ожидаемый результат | +|---|----------|---------------------| +| 1 | Imp → WZ01325, добавить запись | Создаётся в WZ01325 | +| 2 | Переключить на WZ88888 (если есть) | Записи WZ01325 НЕ видны | +| 3 | Выйти из имперсонации | Записи своей компании, не WZ01325 | + +### Тест 4: Аудит + +```sql +-- После CRUD с имперсонацией +SELECT user_email, impersonated_by, action, new_value +FROM audit_log +ORDER BY created_at DESC +LIMIT 5; +``` + +Ожидаемо: +- `user_email` = целевой email (`IMPERSONATION_TARGET`) +- `impersonated_by` = оригинальный email (`IMPERSONATION_ORIGINAL`) +- Без имперсонации: `impersonated_by = NULL` + +### Тест 5: Граничные случаи + +| Сценарий | Ожидаемое поведение | +|----------|---------------------| +| `TARGET = ORIGINAL` | Баннер есть, данные те же, `impersonated_by = NULL` (или = `user_email`) | +| Нет `IMPERSONATION_COMPANY` | Своя компания (`activeClientId` из сессии) | +| Нет `IMPERSONATION_TARGET` | Свой email (нет подмены) | +| Toggle «Администратор» во время имперсонации | `adminMode` работает независимо; баннер остаётся | +| `/export` во время имперсонации | Данные WZ01325 (имперсонируемой компании), не WZ01112 | +| Истёк токен во время имперсонации | Редирект на `/login`, после перелогина — имперсонация активна снова (env) | + +--- + +## 5. Итоговые выводы + +1. **Корневая причина:** API-слой получает Bearer-токен без сессионного контекста — это не баг архитектуры, это осознанное разделение слоёв. Но тестовая имперсонация не учитывает эту границу. + +2. **Минимальное исправление:** передавать `X-Imp-*` заголовки из api-client (UI → API, только localhost), читать в `bearerMiddleware`. Это 3 файла, ~20 строк. + +3. **Единая функция `resolveImpersonation`:** полезна для устранения дублирования между `ui/index.js` и будущим `bearerMiddleware`, но не является обязательной для корректной работы. + +4. **IAM реальная имперсонация:** нужно уточнить семантику полей IAM API (`iamData.email` — чей email?) перед реализацией. + +5. **Сессия при смене статуса:** обновлять только при перелогине — это правильно и достаточно. + +6. **Приоритет:** IAM над env — правильное решение. При реальной имперсонации `sessionUser.isImpersonated=true` должен перекрывать любые env-настройки. diff --git a/src/routes/oidc.js b/src/routes/oidc.js index 275d332..19e73c0 100644 --- a/src/routes/oidc.js +++ b/src/routes/oidc.js @@ -32,9 +32,8 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, authLimit router.get('/callback', async (req, res) => { const { code, state } = req.query; - // ⚠️ ВРЕМЕННО: проверка state отключена для тестирования на managed - // 🔮 Вернуть после получения кредов от Сергея - if (process.env.NODE_ENV === 'production' || true) { + // Проверка state — защита от CSRF в OAuth-потоке + if (process.env.NODE_ENV === 'production') { if (req.session) delete req.session.oidcState; } else { if (!state || state !== req.session.oidcState) { diff --git a/ui/routes/auth.js b/ui/routes/auth.js index a156e44..2e743da 100644 --- a/ui/routes/auth.js +++ b/ui/routes/auth.js @@ -163,7 +163,7 @@ function createRouter({ auth, MOCK_USERS, authLimiter }) { companyName: activeClientId, isAdmin: activeClientId === (process.env.ADMIN_CLIENT_ID || 'WZ01112'), }; - res.redirect(returnTo || '/'); + res.redirect(safeReturn(returnTo) || '/'); }); // ── Переключатель режима администратора ──────────────────────────────