Null-password (SSO/LDAP-only) accounts: bcrypt throw → 500 on /api/auth/login, leaky 401 + brute-force-limiter evasion on /mcp #70

New Issue

Ghost · 2026-06-21T02:03:57+03:00

Ghost commented

2026-06-21 02:03:57 +03:00

Кратко

Аккаунты без локального пароля (users.password IS NULL — SSO / LDAP-only пользователи) при попытке аутентификации по паролю приводят к исключению внутри bcrypt вместо чистого отказа в доступе.

verifyUserCredentials() пропускает такого пользователя через guard и передаёт null в bcrypt.compare(), который реджектит промис с Error: data and hash arguments required. Это не HttpException, поэтому:

на стандартном POST /api/auth/login — необработанная ошибка → HTTP 500 (а не 401);
на /mcp (HTTP Basic) — ошибка ловится и оборачивается в UnauthorizedException с протекающим сообщением ("data and hash arguments required") и, главное, обходит brute-force-лимитер (см. ниже).

Severity: low (это не обход аутентификации — войти под SSO-only аккаунтом по паролю всё равно нельзя). Но это дефект обработки ошибок + side-channel: оракул для перечисления SSO-only аккаунтов и уклонение от счётчика перебора.

Найдено в ходе security-review (находка #15).

Первопричина

apps/server/src/database/types/db.d.ts:376 — колонка пароля nullable:

password: string | null;

apps/server/src/core/auth/services/auth.service.ts:87-115 — guard покрывает только отсутствующего/деактивированного пользователя, но не null-пароль:

async verifyUserCredentials(loginDto, workspaceId): Promise<User> {
  const user = await this.userRepo.findByEmail(loginDto.email, workspaceId, {
    includePassword: true,
  });

  const errorMessage = CREDENTIALS_MISMATCH_MESSAGE;
  if (!user || isUserDisabled(user)) {            // <-- null-пароль сюда НЕ попадает
    await comparePasswordHash(loginDto.password, DUMMY_PASSWORD_HASH);
    throw new UnauthorizedException(errorMessage);
  }

  const isPasswordMatch = await comparePasswordHash(
    loginDto.password,
    user.password,                                 // <-- user.password === null для SSO/LDAP-only
  );
  ...
}

isUserDisabled() (common/helpers/utils.ts) проверяет только deactivatedAt/deletedAt, пароль не смотрит — значит включённый SSO-only пользователь проходит guard и доходит до comparePasswordHash(pw, null).

comparePasswordHash (common/helpers/utils.ts:14) — тонкая обёртка над нативным bcrypt (^6.0.0):

export async function comparePasswordHash(plainPassword: string, passwordHash: string): Promise<boolean> {
  return bcrypt.compare(plainPassword, passwordHash);
}

Проверено фактическое поведение нативного bcrypt:

$ node -e "require('bcrypt').compare('x', null).catch(e=>console.log(e.message))"
data and hash arguments required

Глобального exception-фильтр�� в apps/server/src/main.ts нет, поэтому на стандартном login-пути реджект становится дефолтным 500.

Затронутые поверхности

POST /api/auth/login → auth.controller.ts:103 → authService.login() → verifyUserCredentials(). Нет try/catch и нет глобального фильтра → HTTP 500 Internal Server Error для SSO/LDAP-only аккаунта (вместо 401). Это и есть «500 вместо 401».
/mcp HTTP Basic → integrations/mcp/mcp-auth.helpers.ts:577-610. Реджект ловится в catch и ре-кидается как UnauthorizedException(err.message):
- сообщение "data and hash arguments required" уходит клиенту → info-leak / оракул: по нему (и по поведению лимитера) SSO-only аккаунт отличим от обычного «неверный пароль»;
- isCredentialsFailure(err) возвращает false (сообщение ≠ CREDENTIALS_MISMATCH_MESSAGE), поэтому deps.limiter.recordFailure(...) НЕ вызывается → попытки против SSO-only аккаунта не учитываются brute-force-лимитером (обход счётчика).

Воспроизведение

Завести/иметь пользователя с password = NULL (SSO/LDAP-only).
POST /api/auth/login c его email и любым паролем → 500 (ожидается 401 с обычным сообщением).
Через /mcp с Authorization: Basic <email:любой_пароль> → 401 с телом "data and hash arguments required", при этом счётчик перебора по этому email/IP не растёт.

Предлагаемый фикс

Трактовать null/пустой user.password так же, как отсутствующего пользователя — выполнить dummy-сравнение (для паритета по времени) и бросить тот же унифицированный credentials-error:

// auth.service.ts, verifyUserCredentials
if (!user || isUserDisabled(user) || !user.password) {
  // SSO/LDAP-only accounts have no local password hash: never feed null to
  // bcrypt (it rejects), and keep timing/branch identical to the wrong-user path
  // so the credentials error is uniform (recognised by isCredentialsFailure on /mcp).
  await comparePasswordHash(loginDto.password, DUMMY_PASSWORD_HASH);
  throw new UnauthorizedException(errorMessage);
}

Эффект:

стандартный login отдаёт чистый 401 (нет 500);
/mcp отдаёт унифицированное сообщение CREDENTIALS_MISMATCH_MESSAGE → isCredentialsFailure снова true → brute-force-лимитер корректно инкрементируется, оракул закрыт;
тайминг-паритет сохранён (ровно одно bcrypt-сравнение на всех ветках).

Дополнительно проверить родственный путь changePassword() (auth.service.ts:167-173), где user.password тоже передаётся в comparePasswordHash после guard if (!user || isUserDisabled(user)) — для SSO-only аккаунта там та же проблема.

Желательно покрыть тестом в verify-user-credentials.contract.spec.ts (вернуть унифицированный 401 для password === null, без throw из bcrypt).

## Кратко Аккаунты без локального пароля (`users.password IS NULL` — SSO / LDAP-only пользователи) при попытке аутентификации по паролю приводят к **исключению внутри bcrypt** вместо чистого отказа в доступе. `verifyUserCredentials()` пропускает такого пользователя через guard и передаёт `null` в `bcrypt.compare()`, который **реджектит** промис с `Error: data and hash arguments required`. Это не `HttpException`, поэтому: - на стандартном `POST /api/auth/login` — необработанная ошибка → **HTTP 500** (а не 401); - на `/mcp` (HTTP Basic) — ошибка ловится и оборачивается в `UnauthorizedException` с **протекающим сообщением** (`"data and hash arguments required"`) и, главное, **обходит brute-force-лимитер** (см. ниже). Severity: **low** (это не обход аутентификации — войти под SSO-only аккаунтом по паролю всё равно нельзя). Но это дефект обработки ошибок + side-channel: оракул для перечисления SSO-only аккаунтов и уклонение от счётчика перебора. Найдено в ходе security-review (находка #15). ## Первопричина `apps/server/src/database/types/db.d.ts:376` — колонка пароля nullable: ```ts password: string | null; ``` `apps/server/src/core/auth/services/auth.service.ts:87-115` — guard покрывает только отсутствующего/деактивированного пользователя, но **не** `null`-пароль: ```ts async verifyUserCredentials(loginDto, workspaceId): Promise<User> { const user = await this.userRepo.findByEmail(loginDto.email, workspaceId, { includePassword: true, }); const errorMessage = CREDENTIALS_MISMATCH_MESSAGE; if (!user || isUserDisabled(user)) { // <-- null-пароль сюда НЕ попадает await comparePasswordHash(loginDto.password, DUMMY_PASSWORD_HASH); throw new UnauthorizedException(errorMessage); } const isPasswordMatch = await comparePasswordHash( loginDto.password, user.password, // <-- user.password === null для SSO/LDAP-only ); ... } ``` `isUserDisabled()` (`common/helpers/utils.ts`) проверяет только `deactivatedAt`/`deletedAt`, пароль не смотрит — значит включённый SSO-only пользователь проходит guard и доходит до `comparePasswordHash(pw, null)`. `comparePasswordHash` (`common/helpers/utils.ts:14`) — тонкая обёртка над нативным `bcrypt` (`^6.0.0`): ```ts export async function comparePasswordHash(plainPassword: string, passwordHash: string): Promise<boolean> { return bcrypt.compare(plainPassword, passwordHash); } ``` Проверено фактическое поведение нативного bcrypt: ``` $ node -e "require('bcrypt').compare('x', null).catch(e=>console.log(e.message))" data and hash arguments required ``` Глобального exception-фильтр�� в `apps/server/src/main.ts` нет, поэтому на стандартном login-пути реджект становится дефолтным 500. ## Затронутые поверхности 1. **`POST /api/auth/login`** → `auth.controller.ts:103` → `authService.login()` → `verifyUserCredentials()`. Нет try/catch и нет глобального фильтра → **HTTP 500 Internal Server Error** для SSO/LDAP-only аккаунта (вместо 401). Это и есть «500 вместо 401». 2. **`/mcp` HTTP Basic** → `integrations/mcp/mcp-auth.helpers.ts:577-610`. Реджект ловится в catch и ре-кидается как `UnauthorizedException(err.message)`: - сообщение `"data and hash arguments required"` уходит клиенту → **info-leak / оракул**: по нему (и по поведению лимитера) SSO-only аккаунт отличим от обычного «неверный пароль»; - `isCredentialsFailure(err)` возвращает **false** (сообщение ≠ `CREDENTIALS_MISMATCH_MESSAGE`), поэтому `deps.limiter.recordFailure(...)` НЕ вызывается → попытки против SSO-only аккаунта **не учитываются brute-force-лимитером** (обход счётчика). ## Воспроизведение 1. Завести/иметь пользователя с `password = NULL` (SSO/LDAP-only). 2. `POST /api/auth/login` c его email и любым паролем → **500** (ожидается 401 с обычным сообщением). 3. Через `/mcp` с `Authorization: Basic <email:любой_пароль>` → **401** с телом `"data and hash arguments required"`, при этом счётчик перебора по этому email/IP не растёт. ## Предлагаемый фикс Трактовать `null`/пустой `user.password` так же, как отсутствующего пользователя — выполнить dummy-сравнение (для паритета по времени) и бросить тот же унифицированный credentials-error: ```ts // auth.service.ts, verifyUserCredentials if (!user || isUserDisabled(user) || !user.password) { // SSO/LDAP-only accounts have no local password hash: never feed null to // bcrypt (it rejects), and keep timing/branch identical to the wrong-user path // so the credentials error is uniform (recognised by isCredentialsFailure on /mcp). await comparePasswordHash(loginDto.password, DUMMY_PASSWORD_HASH); throw new UnauthorizedException(errorMessage); } ``` Эффект: - стандартный login отдаёт чистый **401** (нет 500); - `/mcp` отдаёт унифицированное сообщение `CREDENTIALS_MISMATCH_MESSAGE` → `isCredentialsFailure` снова true → brute-force-лимитер корректно инкрементируется, оракул закрыт; - тайминг-паритет сохранён (ровно одно bcrypt-сравнение на всех ветках). Дополнительно проверить родственный путь `changePassword()` (`auth.service.ts:167-173`), где `user.password` тоже передаётся в `comparePasswordHash` после guard `if (!user || isUserDisabled(user))` — для SSO-only аккаунта там та же проблема. Желательно покрыть тестом в `verify-user-credentials.contract.spec.ts` (вернуть унифицированный 401 для `password === null`, без throw из bcrypt).

Ghost added the bug security labels 2026-06-21 02:27:18 +03:00

Ghost referenced this issue

2026-06-21 02:33:12 +03:00

mcp-auth: check-then-act гонка позволяет конкурентным запросам обойти порог brute-force лимитера /mcp #83

Ghost referenced this issue

2026-06-21 02:33:14 +03:00

arch/mcp-auth: устранить дрейф между enforceBasicLoginGate и AuthController.login (общий pre-token гейт или coupling-тест) #91

Ghost referenced this issue from a commit

2026-06-21 03:18:09 +03:00

fix(auth): handle null-password (SSO/LDAP-only) accounts without bcrypt throw (#70)

Ghost referenced this issue

2026-06-21 04:19:31 +03:00