Lock accounts after repeated failed logins, make polling safe against empty results and overload, and verify controller TLS by default

README.md

@@ -491,8 +491,13 @@ own left-hand sub-tab strip:
   reset a user's password, filter/sort the list, and mark one active admin
   as the **default admin** (the star icon): when every auth method is
   disabled, the app skips the login page and signs everyone in as that
-  account, so there's always a way in.
-- **Auth** — toggle local username/password auth, set the session timeout,
+  account, so there's always a way in. A local account is locked for 30
+  minutes after repeated failed logins and, if it fails again after that,
+  until an admin unlocks it (unlock icon; `scripts/unlock_user.py` from the
+  host if the only admin is locked).
+- **Auth** — toggle local username/password auth, set the session timeout
+  how many failed logins lock an account (default 3) and the window they
+  count within (default 24 hours),
   and configure SAML 2.0 SSO (paste IdP metadata XML to auto-fill Entity
   ID/SSO URL/certificate, or fill them in by hand; the ACS URL and SP
   metadata link are derived from **Base URL** on the General tab, so set
@@ -513,7 +518,7 @@ own left-hand sub-tab strip:
 
 - **Storage** — pktWiFi is SQLite-only; there's no analytical-backend
   picker here (unlike some sibling apps). What you do configure is
-  **retention**: days to keep resolved alert events, and days to keep raw
+  **retention**: days to keep resolved alert events, client events and raw
   RF metric history, plus a **Run Cleanup Now** button
   (`POST /api/system/cleanup`) to apply the current thresholds immediately
   instead of waiting for the once-daily scheduled pass.

app/alerts/cleanup.py

@@ -1,9 +1,9 @@
 """
-Alert event + RF-metrics history auto-cleanup.
+Alert event, client event + RF-metrics history auto-cleanup.
 
-Runs once per day. Deletes resolved alert_events and radio_metrics rows
-older than the configured retention windows (defaults: 90 days for alert
-events, 30 days for RF metric history).
+Runs once per day. Deletes resolved alert_events, client_events and
+radio_metrics rows older than the configured retention windows (defaults:
+90 days for alert events, 90 for client events, 30 for RF metric history).
 """
 from __future__ import annotations
 
@@ -52,7 +52,7 @@ async def _run_loop(self) -> None:
 
 
 async def run_cleanup_now(db_path: str) -> dict:
-    """Delete resolved alert_events and radio_metrics rows past their retention
+    """Delete resolved alert_events, client_events and radio_metrics rows past their retention
     window. Shared by the scheduled loop and the manual "Run Cleanup Now" button."""
     async with aiosqlite.connect(db_path) as db:
         async def _setting(key: str, default: int) -> int:
@@ -67,6 +67,7 @@ async def _setting(key: str, default: int) -> int:
 
         alert_retention_days = await _setting("alert_event_retention_days", 90)
         metrics_retention_days = await _setting("radio_metrics_retention_days", 30)
+        client_event_retention_days = await _setting("client_event_retention_days", 90)
 
         result = await db.execute(
             "DELETE FROM alert_events WHERE resolved = 1 AND created_at < datetime('now', ?)",
@@ -80,16 +81,25 @@ async def _setting(key: str, default: int) -> int:
         )
         metrics_deleted = result.rowcount
 
+        result = await db.execute(
+            "DELETE FROM client_events WHERE ts < datetime('now', ?)",
+            (f"-{client_event_retention_days} days",),
+        )
+        client_events_deleted = result.rowcount
+
         await db.commit()
 
-    if alerts_deleted or metrics_deleted:
+    if alerts_deleted or metrics_deleted or client_events_deleted:
         log.info(
             f"Cleanup: removed {alerts_deleted} resolved alerts (>{alert_retention_days}d), "
-            f"{metrics_deleted} radio_metrics rows (>{metrics_retention_days}d)"
+            f"{metrics_deleted} radio_metrics rows (>{metrics_retention_days}d), "
+            f"{client_events_deleted} client events (>{client_event_retention_days}d)"
         )
     return {
         "alerts_deleted": alerts_deleted,
         "metrics_deleted": metrics_deleted,
+        "client_events_deleted": client_events_deleted,
         "alert_retention_days": alert_retention_days,
         "metrics_retention_days": metrics_retention_days,
+        "client_event_retention_days": client_event_retention_days,
     }

app/api/auth.py

@@ -11,6 +11,7 @@
 from pydantic import BaseModel
 
 from app.auth.local import verify_password, create_access_token, create_refresh_token, decode_refresh_token
+from app.auth import lockout
 from app.auth import saml as saml_auth
 from app.database import get_db
 from app.dependencies import cookie_secure
@@ -31,20 +32,39 @@ class TokenResponse(BaseModel):
 
 # -- Local auth ------------------------------------------------------------------
 
+def _locked(state: dict) -> HTTPException:
+    # 423 rather than 401, so a client can tell "locked" from "wrong password".
+    if state["permanent"]:
+        detail = "This account is locked after repeated failed logins. Contact an administrator to unlock it."
+    else:
+        detail = f"This account is locked after repeated failed logins. Try again after {state['until']} UTC."
+    return HTTPException(status_code=status.HTTP_423_LOCKED, detail=detail)
+
+
 @router.post("/login", response_model=TokenResponse)
 async def login(body: LoginRequest, request: Request, response: Response, db: aiosqlite.Connection = Depends(get_db)):
     async with db.execute(
-        "SELECT id, hashed_password, role, is_active FROM users WHERE username = ? OR email = ?",
+        f"SELECT id, hashed_password, role, is_active, {lockout.LOCK_COLUMNS} FROM users WHERE username = ? OR email = ?",
         (body.username, body.username),
     ) as cur:
         user = await cur.fetchone()
 
     if not user or not user["is_active"]:
         raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials")
 
+    # Refused before the password is looked at, so a locked account cannot be
+    # guessed at and a correct password does not get past the lock.
+    state = lockout.describe(user)
+    if state["locked"]:
+        raise _locked(state)
+
     if not user["hashed_password"] or not verify_password(body.password, user["hashed_password"]):
+        state = await lockout.record_failure(db, user["id"])
+        if state["locked"]:
+            raise _locked(state)
         raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials")
 
+    await lockout.record_success(db, user["id"])
     await db.execute("UPDATE users SET last_login = datetime('now'), auth_provider = 'local' WHERE id = ?", (user["id"],))
     await db.commit()
 

app/api/collectors.py

@@ -111,37 +111,30 @@ async def delete_collector(collector_id: int, user: AdminUser, db: aiosqlite.Con
 
 @router.post("/{collector_id}/poll-now")
 async def poll_now(collector_id: int, user: AdminUser, db: aiosqlite.Connection = Depends(get_db)):
+    """Poll now and store the result, exactly as a scheduled poll does — the
+    same code, the same empty-result protection, the same concurrency cap."""
     async with db.execute("SELECT * FROM collectors WHERE id = ?", (collector_id,)) as cur:
         row = await cur.fetchone()
     if not row:
         raise HTTPException(status_code=404, detail="Collector not found")
 
-    from app.wifi.collectors.registry import get_collector_instance
-    from app.wifi.poll_engine import resolve_credential
+    from app.wifi import poll_engine
+    engine = poll_engine.PollEngine._instance
+    if engine is not None and not engine.try_claim(collector_id):
+        raise HTTPException(status_code=409, detail="This collector is already being polled — try again in a moment")
     try:
-        config = await resolve_credential(db, decrypt_config(row["config_json"]))
-    except ValueError as exc:
-        raise HTTPException(status_code=400, detail=str(exc))
-    collector = get_collector_instance(row["collector_type"], config)
-    if collector is None:
-        raise HTTPException(status_code=400, detail="Collector type is not implemented yet")
-    try:
-        result = await collector.poll()
-    except Exception as exc:
-        # Some exceptions (e.g. httpx.ConnectTimeout) have an empty str() —
-        # always include the exception type name so the user never sees a
-        # blank error message.
-        detail = f"{type(exc).__name__}: {exc}" if str(exc) else type(exc).__name__
-        await db.execute(
-            "UPDATE collectors SET status = 'error', last_error = ?, last_poll_at = datetime('now') WHERE id = ?",
-            (detail, collector_id),
-        )
-        await db.commit()
-        raise HTTPException(status_code=502, detail=f"Poll failed: {detail}")
-
-    await db.execute(
-        "UPDATE collectors SET status = 'ok', last_error = NULL, last_poll_at = datetime('now') WHERE id = ?",
-        (collector_id,),
-    )
-    await db.commit()
+        await db.execute(f"PRAGMA busy_timeout={poll_engine._DB_BUSY_TIMEOUT_MS}")
+        try:
+            if engine is not None:
+                async with engine.slots:
+                    result = await poll_engine.poll_and_store(db, row)
+            else:
+                result = await poll_engine.poll_and_store(db, row)
+        except poll_engine.PollFailed as exc:
+            if exc.config:
+                raise HTTPException(status_code=400, detail=exc.detail)
+            raise HTTPException(status_code=502, detail=f"Poll failed: {exc.detail}")
+    finally:
+        if engine is not None:
+            engine.release(collector_id)
     return {"status": "ok", "access_points": len(result.access_points), "clients": len(result.clients)}

app/api/credentials.py

@@ -133,7 +133,7 @@ class CredentialTestRequest(BaseModel):
     target_url: str | None = None   # userpass / api_key+unifi: controller URL
     vendor: str | None = None       # api_key: 'unifi' | 'meraki'
     udm: bool = False               # userpass: UniFi OS console (UDM/Cloud Gateway)
-    verify_tls: bool = False
+    verify_tls: bool = True
     host: str | None = None         # snmp_v2c/v3: device IP
     port: int = 161
 

app/api/users.py

@@ -9,6 +9,7 @@
 
 from app.database import get_db
 from app.dependencies import CurrentUser, AdminUser
+from app.auth import lockout
 from app.auth.local import hash_password, verify_password
 
 router = APIRouter()
@@ -39,6 +40,9 @@ class ResetPasswordRequest(BaseModel):
 
 
 def _user_out(row) -> dict:
+    # Rows from the list query carry the lock columns; others (create, the
+    # suite-token /me) do not, and are simply not locked.
+    lock = lockout.describe(row) if "temp_locked" in row.keys() else {"locked": False, "permanent": False, "until": None}
     return {
         "id": row["id"],
         "username": row["username"],
@@ -50,6 +54,9 @@ def _user_out(row) -> dict:
         "created_at": row["created_at"],
         "last_login": row["last_login"],
         "has_password": bool(row["hashed_password"]) if "hashed_password" in row.keys() else True,
+        "is_locked": lock["locked"],
+        "lock_permanent": lock["permanent"],
+        "locked_until": lock["until"],
     }
 
 
@@ -61,16 +68,16 @@ async def get_me(user: CurrentUser, db: aiosqlite.Connection = Depends(get_db)):
             "is_active": True, "auth_provider": "suite", "created_at": user["created_at"],
             "last_login": None, "has_password": False,
         }
-    async with db.execute("SELECT * FROM users WHERE id = ?", (user["id"],)) as cur:
+    async with db.execute(f"SELECT *, {lockout.TEMP_LOCKED} FROM users WHERE id = ?", (user["id"],)) as cur:
         row = await cur.fetchone()
     return _user_out(row)
 
 
 @router.get("")
 async def list_users(user: AdminUser, db: aiosqlite.Connection = Depends(get_db)):
     async with db.execute(
-        "SELECT id, username, email, role, is_active, is_default_admin, auth_provider, created_at, last_login "
-        "FROM users ORDER BY username"
+        f"SELECT id, username, email, role, is_active, is_default_admin, auth_provider, created_at, last_login, "
+        f"{lockout.LOCK_COLUMNS} FROM users ORDER BY username"
     ) as cur:
         rows = await cur.fetchall()
     return [_user_out(r) for r in rows]
@@ -120,7 +127,7 @@ async def update_user(user_id: int, body: UpdateUserRequest, user: AdminUser, db
         await db.commit()
     except aiosqlite.IntegrityError:
         raise HTTPException(status_code=409, detail="Username or email already exists")
-    async with db.execute("SELECT * FROM users WHERE id = ?", (user_id,)) as cur:
+    async with db.execute(f"SELECT *, {lockout.TEMP_LOCKED} FROM users WHERE id = ?", (user_id,)) as cur:
         row = await cur.fetchone()
     return _user_out(row)
 
@@ -137,6 +144,14 @@ async def reset_user_password(user_id: int, body: ResetPasswordRequest, user: Ad
     await db.commit()
 
 
+@router.post("/{user_id}/unlock", status_code=204)
+async def unlock_user(user_id: int, user: AdminUser, db: aiosqlite.Connection = Depends(get_db)):
+    """Admin-only: lift a failed-login lockout, temporary or permanent, and
+    clear the failure count and the record of earlier lockouts."""
+    if not await lockout.unlock(db, user_id):
+        raise HTTPException(status_code=404, detail="User not found")
+
+
 @router.patch("/{user_id}/set-default-admin", status_code=204)
 async def set_default_admin(user_id: int, user: AdminUser, db: aiosqlite.Connection = Depends(get_db)):
     """Mark this user as the account auto-logged-in when every auth method is disabled.

app/auth/lockout.py

@@ -0,0 +1,114 @@
+"""
+Failed-login lockout for local accounts.
+
+Failed password attempts against an existing account count up to a limit (the
+`login_max_failed_attempts` setting, default 3) within a window (the
+`login_failure_window_hours` setting, default 24): failures older than the
+window no longer count. Reaching the limit locks the account for 30 minutes. After that lock ends, reaching the limit again
+locks it permanently, until an admin unlocks it. A successful login clears both
+the failure count and the record of earlier lockouts.
+
+While an account is locked the password is not checked at all, so a locked
+account cannot be guessed at.
+"""
+from __future__ import annotations
+
+import json
+import logging
+
+import aiosqlite
+
+log = logging.getLogger("pktwifi.auth")
+
+LOCK_MINUTES = 30
+DEFAULT_MAX_ATTEMPTS = 3
+_MAX_ATTEMPTS_CEILING = 100
+DEFAULT_WINDOW_HOURS = 24
+_WINDOW_HOURS_CEILING = 8760
+
+# Selected alongside the user row so the lock state comes from the database's
+# own clock — locked_until is written with datetime('now', ...), and comparing
+# it in Python would mix two clocks and two time zones.
+TEMP_LOCKED = "(locked_until IS NOT NULL AND locked_until > datetime('now')) AS temp_locked"
+LOCK_COLUMNS = f"is_locked, locked_until, {TEMP_LOCKED}"
+
+
+def describe(row) -> dict:
+    """Lock state of a user row selected with LOCK_COLUMNS."""
+    permanent = bool(row["is_locked"])
+    temporary = bool(row["temp_locked"]) and not permanent
+    return {
+        "locked": permanent or temporary,
+        "permanent": permanent,
+        "until": row["locked_until"] if temporary else None,
+    }
+
+
+async def _int_setting(db: aiosqlite.Connection, key: str, default: int, ceiling: int) -> int:
+    async with db.execute("SELECT value FROM settings WHERE key = ?", (key,)) as cur:
+        row = await cur.fetchone()
+    if not row:
+        return default
+    try:
+        value = int(json.loads(row[0]))
+    except (ValueError, TypeError):
+        return default
+    return value if 1 <= value <= ceiling else default
+
+
+async def max_attempts(db: aiosqlite.Connection) -> int:
+    return await _int_setting(db, "login_max_failed_attempts", DEFAULT_MAX_ATTEMPTS, _MAX_ATTEMPTS_CEILING)
+
+
+async def window_hours(db: aiosqlite.Connection) -> int:
+    return await _int_setting(db, "login_failure_window_hours", DEFAULT_WINDOW_HOURS, _WINDOW_HOURS_CEILING)
+
+
+async def record_failure(db: aiosqlite.Connection, user_id: int) -> dict:
+    """Count one failed attempt and lock the account if that reaches the limit.
+    One UPDATE does the counting and the locking, so two attempts arriving
+    together cannot both read the same count. Returns the lock state after."""
+    limit = await max_attempts(db)
+    window = f"-{await window_hours(db)} hours"
+    # Failures older than the window count as none; `prior` is what is left.
+    prior = "(CASE WHEN last_failed_login IS NULL OR last_failed_login < datetime('now', :window) THEN 0 ELSE failed_login_count END)"
+    await db.execute(
+        f"""UPDATE users SET
+              failed_login_count = CASE WHEN {prior} + 1 >= :limit THEN 0 ELSE {prior} + 1 END,
+              lockout_count = CASE WHEN {prior} + 1 >= :limit
+                                   THEN lockout_count + 1 ELSE lockout_count END,
+              is_locked = CASE WHEN {prior} + 1 >= :limit AND lockout_count + 1 >= 2
+                               THEN 1 ELSE is_locked END,
+              locked_until = CASE WHEN {prior} + 1 >= :limit AND lockout_count + 1 < 2
+                                  THEN datetime('now', '+{LOCK_MINUTES} minutes') ELSE locked_until END,
+              last_failed_login = datetime('now')
+            WHERE id = :id""",
+        {"limit": limit, "window": window, "id": user_id},
+    )
+    await db.commit()
+    async with db.execute(f"SELECT username, {LOCK_COLUMNS} FROM users WHERE id = ?", (user_id,)) as cur:
+        row = await cur.fetchone()
+    state = describe(row)
+    if state["permanent"]:
+        log.warning(f"Account '{row['username']}' locked permanently after repeated failed logins — an admin must unlock it")
+    elif state["locked"]:
+        log.warning(f"Account '{row['username']}' locked for {LOCK_MINUTES} minutes after {limit} failed logins")
+    return state
+
+
+async def record_success(db: aiosqlite.Connection, user_id: int) -> None:
+    await db.execute(
+        "UPDATE users SET failed_login_count = 0, lockout_count = 0, locked_until = NULL, "
+        "last_failed_login = NULL WHERE id = ?",
+        (user_id,),
+    )
+
+
+async def unlock(db: aiosqlite.Connection, user_id: int) -> bool:
+    cur = await db.execute(
+        "UPDATE users SET failed_login_count = 0, lockout_count = 0, locked_until = NULL, is_locked = 0, "
+        "last_failed_login = NULL WHERE id = ?",
+        (user_id,),
+    )
+    await db.commit()
+    return cur.rowcount > 0