feat(auth): notify-and-approve gate for suspicious logins (LC-587) #555

Merged

nrupard merged 1 commit from feat/LC-587-suspicious-login-approval into main

2026-07-19 02:11:53 +02:00

nrupard commented

2026-07-19 02:07:39 +02:00

Owner

Summary

LC-587: opt-in suspicious-login notify-and-approve gate, the Lets-Chat analog of mokosh PMS-658 adapted to the pure-RP Bunyip SSO callback. When LOGIN_APPROVAL_ENABLED is set, a login that authenticates at the callback but looks suspicious (a new country - reusing the LC-580 geoip signal - and/or a new device) is withheld: a single-use 6-digit code is emailed and an interstitial asks the user to approve the sign-in by entering it. It notifies rather than locks (the PMS-658 decision), and is off by default because it can withhold a login.

Flow

Assessment splices into bunyip_sso::get_callback BEFORE the session is minted (lets-chat has no password re-POST after the LC-22 pure-RP cutover). A cleared login mints the session and records its country/device as the new baseline; a suspicious one is withheld.
The interstitial posts the emailed code to a new POST /auth/bunyip/approve, which mints the session on the correct code or re-renders with an error on a wrong one.
New-device signal: a first-party device_id cookie (SameSite=Lax so it survives the top-level OIDC redirect back to the callback), stored only as a SHA-256 hash. A login presenting no device_id degrades to country-only (fail-open, no over-gating). The first-ever country/device is a silent baseline.

Safety

Fail-open throughout: if the code cannot be issued or delivered (no verified email, no mailer, a DB error) the login proceeds rather than locking the user out of their own account.
Code stored SHA-256-hashed, single-use, 15-minute TTL, 5-attempt cap (then the challenge is burned).
trust_proxy_headers already defaults true, so the real client IP is captured behind Traefik.
Gate off = the LC-580 alert-only behaviour, unchanged.

Storage

Two SQLite tables (login_approvals + user_login_devices), migration 0041_login_approvals.sql. No tenant_id / RLS: each deployment is single-tenant (unlike PMS-658's multi-tenant Postgres).

Tests

db_login_approval.rs (new): flagged -> approved recovery, single-use replay rejection, attempt-cap burns the challenge, device baseline.
login_approval unit tests: 6-digit code shape, SHA-256 hashing, the pure new-device and suspicious-combine rules.
Verified green: fmt + clippy -D warnings on both the standalone and saas feature sets (all targets), plus the new tests, in the pinned rust-builder.

Diff note

As with LC-580, the bulk of the diff is mechanical: every test's inline AppState literal gains login_approval_enabled: false (the repo builds full inline AppState literals per test rather than sharing a helper). The substantive change is ~10 files.

Deploy note

Dormant until a deployment sets LOGIN_APPROVAL_ENABLED=1; the country signal additionally needs IP2LOCATION_DB_PATH (LC-580, provisioned in docker PR #223). README documents both.

Pre-existing (not this PR)

server/tests/last_visited.rs (home_renders_welcome_when_no_cookie, home_welcome_renders_quick_actions) already fail on main (verified by stashing this branch and running them): the i18n t filter does not resolve the welcome copy in the test env. Untouched here; worth a separate un-red-main issue.

#LC-587

## Summary LC-587: opt-in suspicious-login notify-and-approve gate, the Lets-Chat analog of mokosh PMS-658 adapted to the pure-RP Bunyip SSO callback. When `LOGIN_APPROVAL_ENABLED` is set, a login that authenticates at the callback but looks suspicious (a new country - reusing the LC-580 geoip signal - and/or a new device) is withheld: a single-use 6-digit code is emailed and an interstitial asks the user to approve the sign-in by entering it. It notifies rather than locks (the PMS-658 decision), and is off by default because it can withhold a login. ## Flow - Assessment splices into `bunyip_sso::get_callback` BEFORE the session is minted (lets-chat has no password re-POST after the LC-22 pure-RP cutover). A cleared login mints the session and records its country/device as the new baseline; a suspicious one is withheld. - The interstitial posts the emailed code to a new `POST /auth/bunyip/approve`, which mints the session on the correct code or re-renders with an error on a wrong one. - New-device signal: a first-party `device_id` cookie (`SameSite=Lax` so it survives the top-level OIDC redirect back to the callback), stored only as a SHA-256 hash. A login presenting no `device_id` degrades to country-only (fail-open, no over-gating). The first-ever country/device is a silent baseline. ## Safety - Fail-open throughout: if the code cannot be issued or delivered (no verified email, no mailer, a DB error) the login proceeds rather than locking the user out of their own account. - Code stored SHA-256-hashed, single-use, 15-minute TTL, 5-attempt cap (then the challenge is burned). - `trust_proxy_headers` already defaults true, so the real client IP is captured behind Traefik. - Gate off = the LC-580 alert-only behaviour, unchanged. ## Storage Two SQLite tables (`login_approvals` + `user_login_devices`), migration `0041_login_approvals.sql`. No `tenant_id` / RLS: each deployment is single-tenant (unlike PMS-658's multi-tenant Postgres). ## Tests - `db_login_approval.rs` (new): flagged -> approved recovery, single-use replay rejection, attempt-cap burns the challenge, device baseline. - `login_approval` unit tests: 6-digit code shape, SHA-256 hashing, the pure new-device and suspicious-combine rules. - Verified green: `fmt` + `clippy -D warnings` on both the standalone and saas feature sets (all targets), plus the new tests, in the pinned rust-builder. ## Diff note As with LC-580, the bulk of the diff is mechanical: every test's inline `AppState` literal gains `login_approval_enabled: false` (the repo builds full inline `AppState` literals per test rather than sharing a helper). The substantive change is ~10 files. ## Deploy note Dormant until a deployment sets `LOGIN_APPROVAL_ENABLED=1`; the country signal additionally needs `IP2LOCATION_DB_PATH` (LC-580, provisioned in docker PR #223). README documents both. ## Pre-existing (not this PR) `server/tests/last_visited.rs` (`home_renders_welcome_when_no_cookie`, `home_welcome_renders_quick_actions`) already fail on `main` (verified by stashing this branch and running them): the i18n `t` filter does not resolve the welcome copy in the test env. Untouched here; worth a separate un-red-main issue. #LC-587

nrupard added 1 commit

2026-07-19 02:07:39 +02:00

feat(auth): notify-and-approve gate for suspicious logins (LC-587)

check-secrets / Nosey parker (push) Successful in 4s

Details

check-secrets / Kingfisher (push) Successful in 5s

Details

check-secrets / TruffleHog (push) Successful in 6s

Details

check-secrets / TruffleHog (pull_request) Successful in 5s

Details

Check / clippy + fmt + tests (pull_request) Successful in 3m26s

Details

check-secrets / Nosey parker (pull_request) Successful in 4s

Details

check-secrets / Kingfisher (pull_request) Successful in 6s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

add21bb0f9

LC-587 adds the opt-in suspicious-login notify-and-approve gate, the Lets-Chat analog of mokosh PMS-658 adapted to the pure-RP Bunyip SSO callback. When LOGIN_APPROVAL_ENABLED is set, a login that authenticates at the callback but looks suspicious (a new country - reusing the LC-580 geoip signal - and/or a new device) is withheld: a single-use 6-digit code is emailed and an interstitial asks the user to approve the sign-in by entering it. It notifies rather than locks (the PMS-658 decision: a lock frustrates users), and is off by default because it can withhold a login.

Because lets-chat retired local auth in LC-22, there is no password re-POST to hang a second factor off. The gate splices into bunyip_sso::get_callback before the session is minted and completes via a new POST /auth/bunyip/approve. The new-device signal is a first-party device_id cookie (SameSite=Lax so it survives the top-level OIDC redirect back to the callback); only its SHA-256 hash is stored, and a login that presents no device_id degrades to country-only (fail-open, no over-gating). The first-ever country/device is a silent baseline.

Fail-open throughout: if the code cannot be issued or delivered (no verified email, no mailer, a DB error) the login proceeds rather than locking the user out of their own account. The code is stored SHA-256-hashed, single-use, with a 15-minute TTL and a 5-attempt cap. Two SQLite tables back it (login_approvals + user_login_devices) with no tenant_id / RLS, since each deployment is single-tenant (unlike PMS-658's multi-tenant Postgres). Gate off leaves the LC-580 alert-only behaviour unchanged.

A new db_login_approval integration test covers the flagged -> approved recovery, single-use replay rejection, the attempt cap burning the challenge, and the device baseline; unit tests cover the 6-digit code shape, the hashing, and the pure new-device / suspicious-combine rules. As with LC-580 the bulk of the diff is mechanical: every test's inline AppState literal gains login_approval_enabled: false. Verified green (fmt + clippy -D warnings on both standalone and saas feature sets, all targets, plus the new tests) in the pinned rust-builder. README documents LOGIN_APPROVAL_ENABLED and the IP2LOCATION_DB_PATH it relies on for the country signal.

#LC-587

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>