diff --git a/app/blueprints/admin.py b/app/blueprints/admin.py index 2d02b46..4043e62 100644 --- a/app/blueprints/admin.py +++ b/app/blueprints/admin.py @@ -1585,22 +1585,27 @@ def maintenance_set_interval(): @login_required @admin_required def maintenance_reset_token(player_id: int): - """Reset a single player's authentication token.""" + """Request that a single player re-authenticate. + + The token is not replaced here (see ``Player.request_reauth``): the player + is flagged, told to authenticate again on its next heartbeat, and receives + a fresh token at that point. + """ try: player = Player.query.get_or_404(player_id) - player.rotate_auth_token(force_reauth=True) + player.request_reauth() db.session.commit() log_action('info', - f'Auth token reset for player "{player.name}" ' + f'Re-authentication requested for player "{player.name}" ' f'(ID: {player.id}) by {current_user.username}') - flash(f'Token reset for "{player.name}". It will re-authenticate on its ' - f'next heartbeat.', 'success') + flash(f'"{player.name}" will be asked to re-authenticate on its next ' + f'heartbeat and then receive a new token.', 'success') except Exception as exc: db.session.rollback() - log_action('error', f'Error resetting auth token: {exc}') - flash('Error resetting the authentication token.', 'danger') + log_action('error', f'Error requesting token reset: {exc}') + flash('Error requesting the re-authentication.', 'danger') return redirect(url_for('admin.maintenance')) @@ -1609,23 +1614,23 @@ def maintenance_reset_token(player_id: int): @login_required @admin_required def maintenance_reset_all_tokens(): - """Reset the authentication token of every player.""" + """Request that every player re-authenticate (see ``request_reauth``).""" try: players = Player.query.all() for player in players: - player.rotate_auth_token(force_reauth=True) + player.request_reauth() db.session.commit() log_action('info', - f'Auth tokens reset for all {len(players)} player(s) by ' - f'{current_user.username}') - flash(f'Token reset for {len(players)} player(s). They will ' - f're-authenticate on their next heartbeat.', 'success') + f'Re-authentication requested for all {len(players)} ' + f'player(s) by {current_user.username}') + flash(f'{len(players)} player(s) will be asked to re-authenticate on ' + f'their next heartbeat and then receive new tokens.', 'success') except Exception as exc: db.session.rollback() - log_action('error', f'Error resetting all auth tokens: {exc}') - flash('Error resetting the authentication tokens.', 'danger') + log_action('error', f'Error requesting all token resets: {exc}') + flash('Error requesting the re-authentications.', 'danger') return redirect(url_for('admin.maintenance')) diff --git a/app/blueprints/api.py b/app/blueprints/api.py index 5e825c0..f6836de 100644 --- a/app/blueprints/api.py +++ b/app/blueprints/api.py @@ -115,8 +115,12 @@ def player_needs_reauth(player: Player): * an explicit reset flag (``auth_reset_required``) set from the admin maintenance page, individually or for every player; and * the optional automatic expiry interval configured on that page — a token - older than ``reset_interval_days`` is rotated and the player is asked to - authenticate again. + older than ``reset_interval_days`` asks the player to authenticate again. + + Both are *requests*, not enforcement: the flag is raised here and the token + is only replaced once the player actually re-authenticates (see + ``Player.mark_authenticated``). That keeps a player which cannot handle the + request working instead of silently losing playlist sync. Args: player: The player sending the heartbeat. @@ -139,9 +143,8 @@ def player_needs_reauth(player: Player): return False, None if datetime.utcnow() - issued >= timedelta(days=interval_days): - # Expire the token now: rotate it and flag the player, so the response - # below tells the client to restart authentication. - player.rotate_auth_token(force_reauth=True) + # Flag the player; the replacement token is issued when it re-auths. + player.request_reauth() db.session.commit() return True, 'token_expired' @@ -199,10 +202,12 @@ def authenticate_player(): log_action('warning', f'Failed authentication attempt for hostname: {hostname}') return jsonify({'error': 'Invalid credentials'}), 401 - # Update player status and stamp the token. Marking the player as - # authenticated clears any pending re-authentication request set from the - # admin maintenance page, so a player that just re-authenticated stops - # being told to authenticate again. + # Update player status and finalise the token. If a re-authentication had + # been requested (admin reset or expiry interval), this is where the old + # token is retired: mark_authenticated() issues a brand-new one. The value + # is returned below as `auth_code`, so the player picks it up here. A + # player that re-authenticated with no request pending simply gets its + # issue time refreshed. player.mark_authenticated() player.update_status('online') db.session.commit() @@ -554,11 +559,15 @@ def receive_player_feedback(): return jsonify({'error': 'Invalid quickconnect code'}), 403 # ------------------------------------------------------------------ - # Authentication-token reset. + # Re-authentication request. + # + # When an admin requests a token reset (or an automatic reset interval + # elapses), the heartbeat is answered with 401 + auth_required so the + # client restarts its authentication flow. The player's existing token + # is still valid at this point — it is replaced only once the client + # actually re-authenticates — so a player that does not understand this + # response keeps syncing instead of breaking. # - # When the admin maintenance page resets a player's token (or an - # automatic reset interval has elapsed), the heartbeat is answered with - # 401 + auth_required so the client restarts its authentication flow. # This runs before any feedback is recorded: the heartbeat is not a # meaningful status update while the client is being asked to # re-authenticate. @@ -571,7 +580,7 @@ def receive_player_feedback(): 'success': False, 'auth_required': True, 'reason': reauth_reason, - 'message': 'Authentication token reset. Please re-authenticate.', + 'message': 'Re-authentication required. Please authenticate again.', }), 401 # Create feedback record diff --git a/app/blueprints/players.py b/app/blueprints/players.py index a6cb1ed..8c079cb 100644 --- a/app/blueprints/players.py +++ b/app/blueprints/players.py @@ -300,10 +300,12 @@ def regenerate_auth_code(player_id: int): try: player = Player.query.get_or_404(player_id) - # Issue a new auth code and stamp the issue time so the maintenance - # page's reset interval measures age from this point. A manual - # regenerate keeps its previous behaviour (no forced re-auth). - new_auth_code = player.rotate_auth_token(force_reauth=False) + # Issue a new auth code immediately and stamp the issue time so the + # maintenance page's reset interval measures age from this point. The + # admin is shown the new value, so an immediate swap is intended here — + # unlike the maintenance page, which only asks the player to re-auth. + new_auth_code = player.issue_new_token() + player.auth_reset_required = False db.session.commit() log_action('info', f'Auth code regenerated for player "{player.name}" (ID: {player_id})') diff --git a/app/models/player.py b/app/models/player.py index e08a6f1..99447a4 100644 --- a/app/models/player.py +++ b/app/models/player.py @@ -41,8 +41,10 @@ class Player(db.Model): # Authentication token lifecycle. ``auth_token_issued_at`` records when the # current ``auth_code`` was handed out, so the maintenance page can expire - # tokens by age. ``auth_reset_required`` is set when the token has been - # invalidated; the next heartbeat then tells the player to re-authenticate. + # tokens by age. ``auth_reset_required`` is set when a re-authentication has + # been requested (by an admin, or by the automatic interval); the next + # heartbeat then tells the player to authenticate again, and the token is + # replaced at the moment it does. See :meth:`request_reauth`. auth_token_issued_at = db.Column(db.DateTime, nullable=True) auth_reset_required = db.Column(db.Boolean, default=False, nullable=False) @@ -126,35 +128,52 @@ class Player(db.Model): from app.extensions import bcrypt return bcrypt.check_password_hash(self.quickconnect_code, code) - def rotate_auth_token(self, force_reauth: bool = True) -> str: - """Issue a brand new authentication token for this player. - - Rotating the token invalidates any token the player currently holds: - endpoints guarded by ``verify_player_auth`` will start returning 401 - for the old value. Combined with ``auth_reset_required`` this makes the - player restart its authentication flow on the next heartbeat. - - Args: - force_reauth: When True (default) also flag the player so the next - heartbeat asks it to re-authenticate. - + def issue_new_token(self) -> str: + """Replace the authentication token immediately. + + The old token stops working at once, so this is only safe when the + caller can hand the new value straight to the player — i.e. during + authentication itself, or when an admin explicitly asks for a new code + to be shown in the UI. To *ask* a running player to re-authenticate, + use :meth:`request_reauth` instead. + Returns: The newly generated auth code. """ import secrets self.auth_code = secrets.token_urlsafe(16) self.auth_token_issued_at = datetime.utcnow() - if force_reauth: - self.auth_reset_required = True return self.auth_code - + + def request_reauth(self) -> None: + """Ask the player to re-authenticate on its next heartbeat. + + Deliberately *deferred*: the current token is left valid and only the + ``auth_reset_required`` flag is set. The heartbeat then answers + ``401 {auth_required: true}``, and when the player re-authenticates a + brand-new token is issued (:meth:`mark_authenticated`), at which point + the old one finally stops working. + + Why not rotate at once: the token guards the playlist endpoints, so + rotating it immediately would stop a player from syncing content the + moment the admin pressed the button — before the player ever had the + chance to re-authenticate. Deferring makes the operation non-breaking + for players that do not (yet) react to the request. + """ + self.auth_reset_required = True + def mark_authenticated(self) -> None: """Record a successful authentication. - - Stamps the token issue time and clears any pending reset request, so a - player that re-authenticates stops being told to authenticate again. + + Stamps the token issue time and clears any pending reset request. If a + reset *was* pending, this is the point where the old token dies: a + brand-new token is generated (available afterwards as ``auth_code``) + so the caller's response hands the player a fresh credential. """ - self.auth_token_issued_at = datetime.utcnow() + if self.auth_reset_required: + self.issue_new_token() + else: + self.auth_token_issued_at = datetime.utcnow() self.auth_reset_required = False def auth_token_age_days(self) -> Optional[float]: diff --git a/app/templates/admin/maintenance.html b/app/templates/admin/maintenance.html index 3771174..da51c5a 100644 --- a/app/templates/admin/maintenance.html +++ b/app/templates/admin/maintenance.html @@ -27,6 +27,23 @@ color: #92400e; } + /* Informational note (e.g. the "this is a request, not a forced logout" + explanation), themed so it stays readable in both modes. */ + .maint-note { + padding: 10px 14px; + border-radius: 8px; + font-size: 0.875rem; + background: #e7f3ff; + border-left: 4px solid #0066cc; + color: #0c5460; + } + + body.dark-mode .maint-note { + background: #1e3a5f; + border-left-color: #90cdf4; + color: #cbd5e1; + } + body.dark-mode .maint-badge-pending { background: #4a3800; color: #fbbf24; @@ -93,9 +110,16 @@

🔑 Player Authentication Tokens

-

- Resetting a token invalidates the player's current credentials. On its next - heartbeat the player is told to authenticate again and receives a fresh token. +

+ Requests that a player authenticate again. The player is told on its next + heartbeat and receives a fresh token when it does — its current token stays + valid until then, so playlist syncing is not interrupted. +

+

+ ℹ️ This asks the player to re-authenticate; it is not a forced logout. A player + whose software does not yet handle the request keeps working on its existing + token and simply stays awaiting re-auth below. Players do not need + a password — the quick-connect code they already hold is enough.

- +
@@ -169,7 +193,7 @@ {% if player.auth_reset_required %} - pending + awaiting re-auth {% else %} — {% endif %} @@ -178,9 +202,9 @@
+ onsubmit="return confirm('Ask {{ player.name }} to re-authenticate?');"> + style="padding: 4px 10px; font-size: 12px;">🔁 Re-auth
{% if player.auth_reset_required %}
>A: POST /api/auth/player {hostname, quickconnect_code} + A-->>P: {auth_code, player_id, playlist_id, orientation} + Note over P: stores auth_code (player_auth.json) + P->>A: GET /api/playlists/ (Bearer auth_code) + A-->>P: playlist + playlist_version + loop refresh poll + P->>A: GET /api/playlist-version/ (Bearer auth_code) + A-->>P: {playlist_version, content_count} + end + loop heartbeat (~15s) + P->>A: POST /api/player-feedback {hostname, status, ...} + A-->>P: {success: true} + end +``` + +### Which endpoints use the token + +| Endpoint | Auth | +|---|---| +| `POST /api/auth/player` | hostname + password **or** quickconnect code (no token) | +| `GET /api/playlists?hostname=&quickconnect_code=` | quickconnect code (no token) | +| `GET /api/playlists/` | **Bearer `auth_code`** | +| `GET /api/playlist-version/` | **Bearer `auth_code`** | +| `POST /api/player-edit-media` | **Bearer `auth_code`** | +| `POST /api/player-feedback` (heartbeat) | none — the player is looked up by hostname | + +Because the token guards the playlist endpoints, **an invalid token stops +content sync**, not just auth. + +--- + +## 2. Re-authentication request (what changed on the server) + +An admin can request re-authentication from **Admin → Maintenance → Player +Authentication Tokens**, either per player or for all players. There is also an +optional automatic interval (days) after which the request fires on its own. + +A re-authentication request is **deferred** — the server does *not* invalidate +the current token: + +```mermaid +sequenceDiagram + participant Admin + participant A as DigiServer + participant P as Player + Admin->>A: request re-auth + A->>A: auth_reset_required = true (token unchanged) + P->>A: POST /api/player-feedback + A-->>P: 401 {auth_required: true, reason: "token_reset"} + Note over P: ⚠️ CLIENT ACTION REQUIRED HERE + P->>A: POST /api/auth/player {hostname, quickconnect_code} + A->>A: issue NEW token, clear flag + A-->>P: {auth_code: NEW} + Note over P: store new auth_code, retry requests + P->>A: GET /api/playlists/ (Bearer NEW) ✅ +``` + +Key points: + +* Until the player re-authenticates, its **old token remains valid** — so a + player that ignores the request keeps syncing content (no outage). +* The new token is issued **during** `POST /api/auth/player`, and returned in + the `auth_code` field of the response, exactly like first-time setup. +* The player needs **no password**: the `quickconnect_key` already in + `app_config.json` is sufficient. +* No SSH, restart, or redeploy is required. +* The admin UI shows the player as **awaiting re-auth** until this completes; + the admin can also **Cancel** the request. + +--- + +## 3. What the player must implement + +Today, if the client never handles the request, nothing breaks — but the reset +never happens (the player keeps its old token forever and the admin UI stays on +"awaiting re-auth"). + +To make the reset effective, the player client should: + +1. **Detect the request.** On the heartbeat response, treat + `HTTP 401` with `auth_required: true` as "re-authenticate", e.g.: + + ```python + resp = requests.post(f"{base}/api/player-feedback", json=payload, timeout=10) + if resp.status_code == 401: + body = resp.json() if resp.content else {} + if body.get("auth_required"): + reauthenticate(reason=body.get("reason")) # "token_reset" | "token_expired" + return + ``` + +2. **Also handle 401 on the token-guarded calls.** If the token is ever + rejected by `/api/playlists/` or `/api/playlist-version/`, run the + same re-authentication path. This makes the client self-healing even if the + heartbeat was missed. + +3. **Re-authenticate using the stored quick-connect code:** + + ```python + def reauthenticate(): + cfg = load_app_config() # config/app_config.json + r = requests.post(f"{base}/api/auth/player", json={ + "hostname": cfg["screen_name"], + "quickconnect_code": cfg["quickconnect_key"], + }, timeout=15) + r.raise_for_status() + save_auth(r.json()["auth_code"]) # player_auth.json + # then resume: fetch playlist + version with the new token + ``` + +4. **Retry the failed request** with the new token so sync resumes seamlessly. + +5. **Be idempotent / back off.** While a request is pending the heartbeat keeps + answering `401`; avoid a tight retry loop (e.g. re-authenticate at most once + per N seconds, or immediately then resume the normal heartbeat cadence). + +### Recommended: also add `re-auth` to the edit-media path + +`POST /api/player-edit-media` is token-guarded too, so the same 401 → +re-authenticate → retry wrapper should apply there. + +--- + +## 4. Manual alternative (no client change needed) + +Redeploying the player via **Admin → Build Player Files / Deploy** rewrites +`config/app_config.json` and deletes `player_auth.json`, which forces a clean +authentication on next start. This is the deterministic path and is what the +maintenance reset is intended to replace (it is remote and non-disruptive).