From 392b5e4a53aace725794c7bbabf9e95fb4e1b9c5 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Mon, 14 Sep 2026 01:53:04 +0200 Subject: fix(hub): a per-account sign-in lockout, and a reviewed unauthenticated surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passphrase sign-in locks per username: after `login.max_failures` wrong passphrases (default 4) the name is refused with `429 account_locked` and a `Retry-After` for `login.lockout_minutes` (default 60), without the passphrase being checked. Both numbers are instance policy an admin sets from the panel; zero failures turns it off. The per-IP limit bounds one address, and IPv6 gives every subscriber a /64 of them — an online guess targets an account, so the account is what is counted. - Counted by the name as typed, existing or not, so `login` stays uniform (M1). The key is a hash: people type passphrases into the username field. - The attempt is taken before the check in one `INSERT … ON CONFLICT DO UPDATE … WHERE … RETURNING`, so a concurrent burst gets no more than the limit. - Sign-in, passphrase change and account deletion count on the same row; the last had no rate limit at all. - A lockout refuses passphrase sign-in and nothing else: sessions, renewal and device sign-in continue, and a reset code clears it (AV26). A session learns its own lockout from `/v1/users/me`, and the passphrase change checks it before re-wrapping any node's bundle — the hub accepts the new passphrase only after the nodes have it. The SPA now shows what the hub said. `loginAndRecover` threw "Login failed: {json}", so `email_verification_required` never matched and was never shown; the passphrase-change form rendered no error at all in its first phase. The unauthenticated surface, reviewed route by route: - No `/docs`, `/redoc` or `/openapi.json`, in the code. The Caddyfile hid them on meshbay.org only; a packaged hub behind any other proxy published all three. - The node socket's first message must arrive within ten seconds. It is accepted before anyone is known, and an unbounded read is a connection any stranger holds for free. - `/v1/relays` answers 503 behind `relay.RELAYS_ENABLED`, as federation does: nothing in the tree calls it and two of its routes take no account. - `test_unauthenticated_surface.py` walks every route and fails on one without an authentication dependency that is not listed with its reason. Verified in Chrome against a local hub: the lockout and wrong-passphrase messages, the admin section saving both lockout and mail limits, and the passphrase change refused while locked. Not verified in Firefox (a running instance blocks the headless one), nor the upsert's concurrency on PostgreSQL. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LcF3QKWii7uQ2kSyXErzCt --- packages/meshbay-hub/src/meshbay_hub/api/users.py | 73 ++++++++++++++++++----- 1 file changed, 59 insertions(+), 14 deletions(-) (limited to 'packages/meshbay-hub/src/meshbay_hub/api/users.py') diff --git a/packages/meshbay-hub/src/meshbay_hub/api/users.py b/packages/meshbay-hub/src/meshbay_hub/api/users.py index 7cebd91..2a6baf0 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/users.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/users.py @@ -14,7 +14,7 @@ from pydantic import BaseModel, field_validator from sqlalchemy import delete, func, select, update from sqlalchemy.ext.asyncio import AsyncSession -from meshbay_hub import hub_settings, mail +from meshbay_hub import hub_settings, login_throttle, mail from meshbay_hub.api.deps import get_current_user, require_user_scope from meshbay_hub.api.middleware import limiter from meshbay_hub.api.netutil import client_ip @@ -296,6 +296,31 @@ async def verify_email( return {"status": "verified"} +async def _take_login_attempt(db: AsyncSession, username: str) -> None: + """Spend one passphrase attempt for `username`, or refuse with 429. + + Every path that checks a passphrase goes through here first — `login` and + `change_password` alike, because a lockout on one door is not a lockout. + """ + allowed, retry_after = await login_throttle.reserve(db, username) + if not allowed: + raise HTTPException(status_code=429, detail="account_locked", + headers={"Retry-After": str(retry_after)}) + + +async def _login_failed(db: AsyncSession, username: str, ip: str, + user_id: str | None = None) -> None: + """Record a wrong passphrase and answer 401. Always raises.""" + db.add(IPLog(user_id=user_id, event="login_fail", ip_address=ip, detail=username)) + if await login_throttle.is_now_locked(db, username): + # Once, on the failure that spent the last attempt — so the logs tab + # shows when a name was locked, not every refusal after it. + db.add(IPLog(user_id=user_id, event="login_locked", ip_address=ip, + detail=username)) + await db.commit() + raise HTTPException(status_code=401, detail="Invalid credentials") + + @router.post("/login") @limiter.limit("10/minute") async def login( @@ -303,38 +328,39 @@ async def login( request: Request, db: AsyncSession = Depends(get_db), ): - result = await db.execute( - select(User).where(User.username == body.username)) - user = result.scalar_one_or_none() - ip = client_ip(request) if not body.auth_key and not body.password: raise HTTPException(status_code=401, detail="No credentials provided") + # Before the account is even looked up: an unknown name spends attempts and + # locks exactly like a real one, so neither answer tells them apart (M1). + await _take_login_attempt(db, body.username) + + result = await db.execute( + select(User).where(User.username == body.username)) + user = result.scalar_one_or_none() + if not user: - db.add(IPLog(event="login_fail", ip_address=ip, detail=body.username)) - await db.commit() - raise HTTPException(status_code=401, detail="Invalid credentials") + await _login_failed(db, body.username, ip) if user.pw_version >= 3: # New scheme: verify auth_key if not body.auth_key or not verify_password( body.auth_key, user.pw_hash, user.pw_salt, version=user.pw_version ): - db.add(IPLog(event="login_fail", ip_address=ip, detail=body.username)) - await db.commit() - raise HTTPException(status_code=401, detail="Invalid credentials") + await _login_failed(db, body.username, ip, user.id) else: # Legacy scheme: need raw password if not body.password: + # Nothing was checked, so nothing was guessed. + await login_throttle.release(db, body.username) + await db.commit() raise HTTPException(status_code=401, detail="auth_upgrade_required") if not verify_password( body.password, user.pw_hash, user.pw_salt, version=user.pw_version ): - db.add(IPLog(event="login_fail", ip_address=ip, detail=body.username)) - await db.commit() - raise HTTPException(status_code=401, detail="Invalid credentials") + await _login_failed(db, body.username, ip, user.id) # Migrate to new scheme if auth_key provided alongside password if body.auth_key: new_hash, new_salt = hash_password(body.auth_key) @@ -348,6 +374,11 @@ async def login( user.pw_salt = new_salt user.pw_version = 2 + # The passphrase was right, whatever the account's status turns out to be. + await login_throttle.clear(db, body.username) + + if user.status != "active": + await db.commit() if user.status == "pending": raise HTTPException(status_code=403, detail="email_verification_required") if user.status != "active": @@ -624,6 +655,7 @@ async def token_refresh( @router.get("/me") async def get_current_user_info( current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), ): email = "" try: @@ -636,6 +668,12 @@ async def get_current_user_info( "email": email, "role": current_user.role, "status": current_user.status, + # Seconds left on a sign-in lockout, 0 when there is none. Told to the + # account's own session only, so it reveals nothing about anyone else. + # A passphrase change re-wraps every node's bundle *before* the hub + # accepts the new passphrase, and must not start while the hub would + # then refuse it. + "passphrase_locked_for": await login_throttle.locked_for(db, current_user.username), } @@ -846,10 +884,12 @@ async def change_password( current_user: User = Depends(require_user_scope), db: AsyncSession = Depends(get_db), ): + await _take_login_attempt(db, current_user.username) if not verify_password(body.old_auth_key, current_user.pw_hash, current_user.pw_salt, current_user.pw_version): raise HTTPException(status_code=403, detail="Current passphrase does not match") + await login_throttle.clear(db, current_user.username) if body.new_auth_key == body.old_auth_key: raise HTTPException(status_code=400, detail="New passphrase must differ from the current one") @@ -1049,6 +1089,9 @@ async def password_reset( update(RefreshToken).where(RefreshToken.user_id == user.id) .values(revoked=True)) await db.execute(delete(UserDevice).where(UserDevice.user_id == user.id)) + # A code sent to the address on file is a stronger proof than a passphrase, + # and it is the way out of a lockout somebody else caused. + await login_throttle.clear(db, user.username) db.add(IPLog(user_id=user.id, event="password_reset", ip_address=client_ip(request))) await db.commit() @@ -1286,9 +1329,11 @@ async def delete_own_account( borrowed laptop or a session left open. Same value as at sign-in, so the hub still never sees the passphrase itself. """ + await _take_login_attempt(db, current_user.username) if not verify_password(body.auth_key, current_user.pw_hash, current_user.pw_salt, current_user.pw_version): raise HTTPException(status_code=403, detail="Passphrase does not match") + await login_throttle.clear(db, current_user.username) return await erase_account(db, current_user) -- cgit v1.2.3