Repository navigation
feat(web): optional web login and API tokens, off by default (stacked on #674) - #683
Conversation
…heck) The web interface had no CSRF protection, on the reasoning that anyone who can forge a request on the LAN can also send it directly. That misses the browser as a confused deputy: any website a LAN user opens can make their browser POST a plain HTML form to http://<pi>:5000. CORS does not stop that request, only hides its answer, and /api/v3/system/action accepted form bodies, so a hostile page could reboot or power off the Pi, pull code, or reach any other mutating route. - web_interface/origin_guard.py: an app-wide before_request hook refuses POST/PUT/PATCH/DELETE whose Origin (or, without one, Referer) is not the host the request was addressed to, and Origin "null", with 403 CROSS_SITE_REQUEST. Requests with neither header (curl, Home Assistant, the MQTT bridge) are not from a browser and pass. Host and port are compared, not the scheme, so a TLS proxy that passes Host through works; X-Forwarded-Host is not trusted (no ProxyFix). - /api/v3/system/action refuses a non-JSON body (415) unless HX-Request is set; every caller in the interface already sends JSON. - app.py comment states the real threat model; SECURITY.md, REST_API_REFERENCE.md, WEB_INTERFACE_GUIDE.md and CHANGELOG updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The claimed origin is attacker-chosen, so the refusal reason in the response names only which header failed; the values are logged instead. Clears Codacy's directly-returned-format-string finding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
Stacked on #674 (the Origin guard, which stays always on). - web_interface/auth.py: one before_request hook. With no password stored it returns immediately, so nothing changes. With one, every page and API route needs a login session (signed with a stored key, 30 days, survives restarts, rotated on password change) or an API token sent as "Authorization: Bearer". Page loads redirect to /login, HTMX requests get HX-Redirect, API calls get 401 JSON. Open regardless: loopback requests without proxy headers, the Wi-Fi setup flow in AP mode, static files, captive-portal probes, and a status-only /api/v3/health. - Password hashed with werkzeug; tokens stored as SHA-256 and shown once; all in the web_auth section of config_secrets.json. Config GET endpoints, the raw JSON editor and raw secrets save never expose or overwrite it; web_auth added to CORE_SECRETS_KEYS so orphan cleanup keeps it. - Wrong passwords rate-limited per address through flask-limiter (failures only), on /login and the change/disable routes. - /api/v3/auth/{status,password,disable,tokens} and a Security section on the General tab; Log out in the header; fetch() wrapper follows 401s to the login page. - scripts/reset_web_password.py: lost-password recovery. - MQTT bridge: optional ledmatrix_api_token (write-only in the Tools tab). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 36 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (23)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 1652 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…only the site - A portless Host now matches the default port of either the browser's scheme or Flask's, so nginx terminating TLS in front of a plain-http upstream (Origin https://pi.example -> 443, Flask sees http -> 80) no longer refuses every legitimate write. A non-default port still has to match exactly. - The refusal log records only scheme://host[:port] of Origin/Referer, never a Referer's path or query (which can carry tokens), and repr()s the path. - Docs: forward $http_host, not $host (nginx's $host drops the port). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aude/web-optional-auth Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Semgrep's directly-returned-format-string rule read the helper as a Flask route returning a formatted string. Same output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- safe_next: a path must start with a single '/'; '//', any backslash and any control character (DEL included) are refused both as sent and once percent-decoded, so '/%2F%2Fhost', '/%5Chost' and '/%09/host' fall back to '/'. The result is rebuilt behind a constant '/'. Tests cover protocol-relative, backslash, scheme, javascript: and encoded variants, on POST and on the signed-in GET redirect. - AuthError carries a fixed user_message; routes return that, never str(e). - Login-setting and API-token log lines name the event, the token name and id only; a test asserts no password, token or token hash is ever logged. An unusable stored hash logs only the exception type. - reset_web_password.py: nothing read from the secrets file is printed (test added); variables no longer named after secrets. - TOKEN_PREFIX marked nosec B105; the test app's stub page no longer echoes its URL segment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aude/web-optional-auth Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # SECURITY.md # docs/REST_API_REFERENCE.md # web_interface/app.py
app.css no longer holds the utility classes (they moved to the generated tailwind.css), and login.html (#683) links only app.css, so the login page rendered almost unstyled once both PRs were on main. Link tailwind.css before app.css, as base.html does, and extend the load-order test to every template that links app.css. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… CLI (#685) Replaces the hand-written Tailwind subset in app.css with a real, purged Tailwind build: scripts/build_css.py runs the pinned, SHA-256-checked standalone Tailwind CLI (no Node), the generated tailwind.css and plugin-frame.css are committed, and CI fails when they are stale. The Pi never builds anything. The login page (#683) now links tailwind.css too, and the load-order test covers every template that links app.css. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - src/display_controller.py: _display_once keeps the watchdog beat and #677's per-call display() timing (note_display_duration). - docs/REST_API_REFERENCE.md: health documents both checks.display_loop and #683's minimal unauthenticated answer, which a stalled loop still degrades. - CHANGELOG.md: both sides' Unreleased entries kept. Adds a test that a stale heartbeat turns the minimal (not logged in) /api/v3/health answer to degraded without leaking checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - CHANGELOG.md: kept both; "Update channels" first, then main's sections. - docs/ARCHITECTURE.md: the verifier paragraph now says both things -- the heartbeat check from #687 and the return to old_ref from this branch. - web_interface/blueprints/pages_v3.py: the General partial gets both update_channel_status (this branch) and web_login (#683). Follow-ups for main's changes: - #688: the channel API test fixture stubs api_v3.plugin_catalog, not the retired plugin_manager. - #683: test_web_auth pins that GET/POST /system/update-channel need login from the LAN when login is on (the default rule; mutation-checked by exempting the endpoints). - #685: scripts/build_css.py regenerates identical CSS; --check passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#684) Adds auto_update.channel: stable follows the newest vX.Y.Z release tag (detached HEAD; pre-releases and other tags ignored), beta follows main as before. Nothing ever moves a device backwards: a checkout newer than the newest release keeps following main (or stays put when detached) until a release contains its commit. Legacy configs migrate to stable when they reach a release. Update Code, the weekly updater's preflight, and the verifier's rollback (back to old_ref: branch or detached release) all honour the channel. General tab Update Channel select, GET/POST /api/v3/system/update-channel, release-aware Overview banner and Tools git panel. New installs default to stable. Rig fix (ledpi): /system/check-update reports update_available: false when the channel's action is none (a detached HEAD newer than the newest release), matching Update Code; the Tools panel no longer calls every detached HEAD "a release". Merged with main through #687 (heartbeat verifier, #683 login, #688 plugin_catalog, #685 Tailwind build). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What this does
The owner's decision: the Origin guard stays always on, and authentication is optional and off by default. A device that never sets a password behaves exactly as before: the hook returns before doing anything, no cookie is issued, and every route answers as it does today.
Optional password (General tab → new Security section)
generate_password_hash) in theweb_authsection ofconfig/config_secrets.json, written throughConfigManager.save_raw_file_contentlike every other secret (atomic, secrets file mode). The store reads that file directly, cached on its mtime, so aweb_authkey smuggled intoconfig.jsonmeans nothing./loginpage styled like the rest of the UI (sameapp.css, Font Awesome, theme). Log out button in the header when signed in. Change password and turn login off both require the current password.secret_keyis random per process). The key is replaced on every password change, which signs every other browser out; the browser that changed it stays in. 30-day lifetime,SameSite=Lax,HttpOnly.5 per minute; 30 per hourper address, counting only failures (deduct_when), onPOST /loginand on the change/disable routes (wrong current password). Once over the limit, even the right password waits.sudo python3 scripts/reset_web_password.pyremoves the password (tokens kept unless--revoke-tokens); the running service notices on the next request, no restart. Or open the UI from the Pi itself.API tokens
lmx_+ 43 chars) is shown once; only its SHA-256 is stored (safe for a 256-bit random value and cheap enough to check on every request; the password uses werkzeug). Sent asAuthorization: Bearer <token>. A token bypasses the session requirement, but cannot change login settings or mint tokens (403TOKEN_NOT_ALLOWED).ledmatrix_api_token(orLEDMATRIX_MQTT_LEDMATRIX_API_TOKEN, or a write-only field in the Tools tab), sent as a Bearer header. Only needed when the bridge runs on another machine; on the Pi it is a loopback request.Exemptions while login is on
X-Forwarded-For/X-Real-IP/Forwarded/X-Forwarded-Host(otherwise a reverse proxy on the Pi would make everyone local; documented that such a proxy must addX-Forwarded-For).is_ap_mode_active()(hostapd or the nmcli AP, cached 30s). Only the Wi-Fi setup flow is opened:/setup,GET /wifi/status,GET /wifi/scan,POST /wifi/connect(exactly whatcaptive_setup.htmlcalls). The hook is registered aftercaptive_portal_redirect, so unknown paths still land on/setup;/loginand/logoutwere added to the captive allow-list so the full UI can still be logged into in AP mode.staticendpoint), the captive probe URLs,/favicon.ico,/login./api/v3/healthstays open but, to a caller that is not logged in, answers only{"status": "healthy" | "degraded"}.Enforcement
before_requesthook (web_interface/auth.py), no per-route decorators. Unauthenticated page navigations → 302 to/login?next=...(nextrestricted to local paths); HTMX requests → 401 withHX-Redirect; everything else → 401 JSON (AUTH_REQUIRED/INVALID_TOKEN) withWWW-Authenticate: BearerandX-LEDMatrix-Login. A smallfetch()wrapper inbase.htmlfollows that header to the login page when a session runs out mid-use.api_v3/web_login.pyon the shared blueprint, so they are in the URL-map snapshot and thedescribe_exception(e)rule (save failures return it;test_web_error_detailpasses).Keeping the hash in
GET /config/mainandGET /config/secretsleaveweb_authout entirely; so does the raw JSON editor partial.POST /config/raw/secretsignores an incomingweb_authand keeps the stored one;POST /config/maindrops aweb_authkey.web_authjoinsCORE_SECRETS_KEYS, because orphaned-plugin cleanup would otherwise have deleted it as an unknown plugin (and reconciliation's system-key list now uses that constant).Found in passing (separate task, not fixed here)
flask-limiter enforces a decorated limit inside the wrapper it returns, so
app.py's existinglimiter.limit("200 per minute")(stream_stats)(and the two other SSE routes) limit nothing. This PR installs its wrappers intoapp.view_functionsso the login limit really applies; the SSE fix is left for its own PR.Tests
test/test_web_auth.py(46 tests): off by default and nothing changes (pages, partials, API, no cookie, full health, real app wiring and hook order); enabling redirects page loads to/login, HTMX getsHX-Redirect, API gets 401 JSON, POSTs aren't run; login works,nextcannot leave the site, logout, session survives a restart, password change signs other browsers out, current password required; wrong passwords rate-limited (429 even for the right one, per address, successes not counted, settings routes too); token grants API access, revoked/made-up token refused, only a hash stored, tokens can't manage auth; AP mode exempts only the setup flow and only in AP mode; localhost exempt, proxied loopback not; static open; health minimal; hash/token hash/cookie key/token never in/config/main,/config/secrets,/auth/*or the raw JSON editor; raw secrets save can't overwrite;/config/maincan't plant one; orphan cleanup keeps it; reset script (clears password, keeps tokens and other secrets, seen live without restart,--revoke-tokens, no-op writes nothing); MQTT bridge sends its token; bridge settings keep it write-only.test/fixtures/api_v3_url_map.jsonregenerated (6 new/api/v3/auth/*entries).pytest -m "not hardware" test/ --ignore=test/plugins, Windows, Python 3.14, Pillow 12.3.0 in both runs, flask-limiter 3.12 in a venv for both): baselineorigin/claude/web-origin-checkin a separate worktree 59 failed / 6 errors / 6098 passed; this branch 58 failed / 6 errors / 6144 passed. No new FAILED/ERROR IDs; the one difference istest_sync_manager.py::TestRealSocketHandshake::test_leader_and_follower_negotiate_over_real_sockets, failing only in the baseline (real-socket timing). Without flask-limiter installed, the 3 rate-limit tests skip.test/jsunit and DOM suites (REQUIRE_DOM=1) against the real app: all 14 pass, includingtest_tools_sections.js(MQTT bridge form).web_interface.applocally in the browser (loopback treated as non-local for the run): Security section set a password and stayed signed in; token created and shown once; curl: no creds → 401 JSON, token → 200, navigation → 302/login, HTMX → 401 +HX-Redirect, health → status only, static → 200,/config/mainand/config/secretswithoutweb_auth, token → 403 on token creation; Log out → login page; wrong password → error, right → back in; 5 failures → 429 "Too many wrong passwords"; fetch() 401 → wrapper went to the login page;reset_web_password.pyturned login off live.Still needs a hardware check
/setup, networks scan, connect works, and the full UI asks for the password. Covered by unit tests with a stubbedis_ap_mode_active, not exercised on hardware.Open questions
/api/v3/backup/export) still includeconfig_secrets.jsonwhole, so a backup carries the password hash and token hashes (restoring it restores the login). That is the file-level backup contract, and the download itself needs a login when auth is on; say if you'd rather stripweb_authfrom backups.🤖 Generated with Claude Code