security: gate Beatify admin behind Home Assistant login (#998)#1007
security: gate Beatify admin behind Home Assistant login (#998)#1007mholzi wants to merge 2 commits into
Conversation
Admin/host control was reachable by anyone who could reach Home Assistant. /beatify/admin even embedded the live admin token into the page for any visitor. This makes every host action require a logged-in HA user; players still join /beatify/play with no auth. Server: - requires_auth=True on start-game, start-gameplay, end-game, rematch-game, force-reset, preview-lights, tts-test, lights, capabilities - AdminView no longer embeds the admin token (_inject_admin_token removed) - BeatifyAdminView gated by HA auth; retired the per-game admin_token check - WS admin_connect and is_admin joins validate an HA access token Frontend: - new ha-auth.js: HA OAuth2 client (login redirect, refresh, authed fetch) - admin console requires HA login on load; player page only when the host role is claimed - admin/player/wizard/lights/tts calls carry the HA token Pre-release for testing — the OAuth flow needs verification on a live HA instance. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…#998) rc3 failed to load — server/stats_views.py still imported _verify_admin_token from server/base.py, which was removed when the per-game admin-token mechanism was retired. ImportError aborted the whole integration setup. StatsView (/beatify/api/stats) now uses requires_auth=True instead of the old "admin token when a game is active" check (#386). No Beatify page fetches this endpoint, so gating it breaks nothing. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a significant security improvement by migrating from a per-game admin token system to native Home Assistant authentication. The changes include the introduction of a new OAuth2 client helper (ha-auth.js) to handle Home Assistant login flows, updating admin REST endpoints and WebSocket handlers to require valid Home Assistant access tokens, and removing the legacy admin token injection mechanism. I have reviewed the implementation and provided feedback on improving error logging in the authentication helper and simplifying the URL-encoded body construction in the frontend auth client.
| except Exception: # noqa: BLE001 — any decode/validation error means "no" | ||
| return False |
There was a problem hiding this comment.
While catching a broad Exception is a sound defensive strategy in this security context to ensure the function fails closed, swallowing the exception without logging can make debugging very difficult if an unexpected error occurs during token validation (e.g., due to a change in Home Assistant core). It would be beneficial to log the exception to aid in troubleshooting potential issues.
| except Exception: # noqa: BLE001 — any decode/validation error means "no" | |
| return False | |
| except Exception as e: # noqa: BLE001 — any decode/validation error means "no" | |
| _LOGGER.warning("HA token validation failed unexpectedly: %s", e) | |
| return False |
| var body = Object.keys(params) | ||
| .map(function (k) { | ||
| return encodeURIComponent(k) + '=' + encodeURIComponent(params[k]); | ||
| }) | ||
| .join('&'); |
There was a problem hiding this comment.
The manual construction of the x-www-form-urlencoded body can be simplified by using the standard URLSearchParams API. This makes the code more concise and less prone to encoding errors.
var body = new URLSearchParams(params).toString();1 participant
Admin/host control was reachable by anyone who could reach Home
Assistant. /beatify/admin even embedded the live admin token into the
page for any visitor. This makes every host action require a logged-in
HA user; players still join /beatify/play with no auth.
Server:
rematch-game, force-reset, preview-lights, tts-test, lights, capabilities
Frontend:
host role is claimed
Pre-release for testing — the OAuth flow needs verification on a live
HA instance.
Co-Authored-By: Claude Opus 4.7 (1M context) [email protected]