From c44cac2e9a3ff56f0df922a11e7e60c6ebdd7b2c Mon Sep 17 00:00:00 2001
From: SergeantPanda <chester.cullen@gmail.com>
Date: Thu, 25 Jun 2026 11:27:57 -0500
Subject: [PATCH] fix(recording playback): Complete authentication handling for
 DVR recordings by allowing JWT tokens via query parameters for both native
 video and HLS segments. Updated API views to support token propagation in
 redirects and playlist rewrites. Enhanced tests to validate new
 authentication behavior across endpoints.

---
 CHANGELOG.md                                  |   3 +-
 apps/accounts/authentication.py               |   3 +-
 apps/channels/api_urls.py                     |   6 +-
 apps/channels/api_views.py                    |  34 +++-
 .../tests/test_recording_playback_auth.py     | 152 ++++++++++++++++--
 frontend/src/WebSocket.jsx                    |   2 +-
 6 files changed, 175 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c5312ff9..bbfe159f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -54,8 +54,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-- **Completed DVR recordings no longer return 403 when played in the browser.** In-progress recordings used hls.js, which injects the JWT via `xhrSetup`. Completed recordings switch to the `/file/` endpoint with native `<video src>`, which cannot send `Authorization` headers. Recording playback now accepts `?token=` query-param JWT (matching VOD streams), and the floating video player appends the token when assigning native `<video src>` URLs.
+- **DVR recording playback auth is complete for native video, HLS segments, and redirects.** Completed recordings use `/file/` with native `<video src>`, which cannot send `Authorization` headers. Playback accepts `?token=` query-param JWT (matching VOD streams), and the floating video player appends the token when assigning native URLs. The explicit HLS URL route (`/api/channels/recordings/.../hls/...`) now registers the same authenticators as the ViewSet `@action` handlers—previously only header JWT applied on that path, so `?token=` failed for native HLS clients. When the request used `?token=`, rewritten playlist segment URLs and `/file/`↔`/hls/` redirects preserve the token; hls.js clients that authenticate via `Authorization` are unchanged. `QueryParamJWTAuthentication` reads `request.query_params` on DRF requests.
 - **In-progress DVR playback no longer jumps to the live edge.** The floating video player still opens in-progress recordings at the start of the seekable range, but hls.js was configured with `liveMaxLatencyDurationCount: 10`, which forced the playhead forward to "now" shortly after playback began. Live-edge sync is now disabled for recording HLS so users can watch, pause, and scrub from the beginning while the recording continues. (Fixes #1329)
+- **`refresh_single_m3u_account` no longer re-raises after setting account ERROR.** Matches `refresh_epg_data`: the account status and UI already reflect the failure; Celery no longer marks the task FAILED after the terminal error state is persisted.
 - **M3U refresh recovers DB connections and account status after failures.** Celery workers now call `close_old_connections()` before and after every task; `refresh_single_m3u_account` also resets its DB connection at task start and retries account loads once after a connection reset (avoids opaque `list index out of range` errors from poisoned worker connections). Accounts leave `fetching`/`parsing` for a terminal `error` or `success` state when refresh aborts. Error-path status updates use `QuerySet.update()` on a clean connection instead of `save()` on a connection that may already be INTRANS after `refresh_m3u_groups` or batch ORM failures. Failed group downloads now set `error` instead of returning silently. Refresh start replaces stale `last_message` text. `refresh_account_profiles` releases DB connections after each profile failure. (Fixes #1338)
 - **EPG refresh recovers DB connections and source status after failures.** `refresh_epg_data` now mirrors the M3U hardening: connection reset at task start/end, retried source load after a poisoned-worker connection reset, `QuerySet.update()` for error status on a clean connection, and a `finally` guard that moves sources stuck in `fetching`/`parsing` to `error`. Programme index builds are skipped when programme parsing fails. `parse_channels_only` now persists the error status when a missing file has no URL to refetch.
 - **M3U `custom_properties` JSON normalization.** Legacy rows and some API writes could store a JSON-encoded string instead of an object in `custom_properties`, causing refresh, profile sync, and auto-sync to fail with `'str' object has no attribute 'get'`. M3U account, profile, and group-relation models normalize on `save()`; group settings bulk writes and read/merge paths use shared helpers in `core.utils` without re-parsing dict values.
diff --git a/apps/accounts/authentication.py b/apps/accounts/authentication.py
index e8551e5d..c472028f 100644
--- a/apps/accounts/authentication.py
+++ b/apps/accounts/authentication.py
@@ -93,7 +93,8 @@ class QueryParamJWTAuthentication(JWTAuthentication):
     where the browser cannot set Authorization headers (e.g. <video src>)."""
 
     def authenticate(self, request):
-        raw_token = request.GET.get('token')
+        params = getattr(request, "query_params", request.GET)
+        raw_token = params.get("token")
         if not raw_token:
             return None
         try:
diff --git a/apps/channels/api_urls.py b/apps/channels/api_urls.py
index ff00e0f7..036bc50d 100644
--- a/apps/channels/api_urls.py
+++ b/apps/channels/api_urls.py
@@ -13,6 +13,7 @@ from .api_views import (
     UpdateChannelMembershipAPIView,
     BulkUpdateChannelMembershipAPIView,
     RecordingViewSet,
+    RECORDING_PLAYBACK_AUTHENTICATORS,
     RecurringRecordingRuleViewSet,
     GetChannelStreamsAPIView,
     GetChannelStreamStatsAPIView,
@@ -53,7 +54,10 @@ urlpatterns = [
     path('recordings/bulk-delete-upcoming/', BulkDeleteUpcomingRecordingsAPIView.as_view(), name='bulk_delete_upcoming_recordings'),
     path(
         'recordings/<int:pk>/hls/<path:seg_path>',
-        RecordingViewSet.as_view({'get': 'hls'}),
+        RecordingViewSet.as_view(
+            {'get': 'hls'},
+            authentication_classes=RECORDING_PLAYBACK_AUTHENTICATORS,
+        ),
         name='recording-hls',
     ),
     path('dvr/comskip-config/', ComskipConfigAPIView.as_view(), name='comskip_config'),
diff --git a/apps/channels/api_views.py b/apps/channels/api_views.py
index ce1a61f3..fd8a4758 100644
--- a/apps/channels/api_views.py
+++ b/apps/channels/api_views.py
@@ -14,6 +14,7 @@ from django.db import connection, transaction
 from django.db.models import Count, F, Prefetch
 from django.db.models import Q
 import os, json, requests, logging, mimetypes, threading, time
+from urllib.parse import urlencode
 from datetime import timedelta
 from django.utils.http import http_date
 from apps.accounts.permissions import (
@@ -3248,6 +3249,24 @@ RECORDING_PLAYBACK_AUTHENTICATORS = [
 ]
 
 
+def _recording_auth_query_suffix(request):
+    """Suffix for rewritten recording URLs when auth used ?token= (native <video>).
+
+    hls.js clients authenticate via Authorization on each XHR and do not need
+    tokens embedded in playlist segment lines.
+    """
+    from rest_framework.request import Request as DRFRequest
+
+    if isinstance(request, DRFRequest):
+        params = request.query_params
+    else:
+        params = request.GET
+    token = params.get("token")
+    if not token:
+        return ""
+    return "?" + urlencode({"token": token})
+
+
 class RecordingViewSet(viewsets.ModelViewSet):
     queryset = Recording.objects.all()
     serializer_class = RecordingSerializer
@@ -3347,7 +3366,7 @@ class RecordingViewSet(viewsets.ModelViewSet):
             if hls_dir and os.path.isdir(hls_dir):
                 hls_url = request.build_absolute_uri(
                     f"/api/channels/recordings/{pk}/hls/index.m3u8"
-                )
+                ) + _recording_auth_query_suffix(request)
                 return HttpResponseRedirect(hls_url)
             if not file_path or not os.path.exists(file_path):
                 raise Http404("Recording file not found")
@@ -3440,9 +3459,10 @@ class RecordingViewSet(viewsets.ModelViewSet):
             cp = recording.custom_properties or {}
             file_path = cp.get("file_path")
             if seg_path.endswith(".m3u8") and file_path and os.path.exists(file_path) and os.path.getsize(file_path) > 0:
-                return HttpResponseRedirect(
-                    request.build_absolute_uri(f"/api/channels/recordings/{pk}/file/")
-                )
+                file_url = request.build_absolute_uri(
+                    f"/api/channels/recordings/{pk}/file/"
+                ) + _recording_auth_query_suffix(request)
+                return HttpResponseRedirect(file_url)
             raise Http404("HLS content not available for this recording")
 
         # Security: prevent path traversal outside the HLS directory
@@ -3455,16 +3475,18 @@ class RecordingViewSet(viewsets.ModelViewSet):
             raise Http404(f"HLS file not found: {seg_path}")
 
         if seg_path.endswith(".m3u8"):
-            # Rewrite relative segment lines to absolute URLs through this API
+            # Rewrite relative segment lines to absolute URLs through this API.
+            # Propagate ?token= only for native <video> clients (see helper).
             base_url = request.build_absolute_uri(
                 f"/api/channels/recordings/{pk}/hls/"
             )
+            auth_suffix = _recording_auth_query_suffix(request)
             lines = []
             with open(requested) as _f:
                 for line in _f:
                     stripped = line.strip()
                     if stripped and not stripped.startswith("#"):
-                        lines.append(f"{base_url}{stripped}\n")
+                        lines.append(f"{base_url}{stripped}{auth_suffix}\n")
                     else:
                         lines.append(line)
             return HttpResponse("".join(lines), content_type="application/x-mpegURL")
diff --git a/apps/channels/tests/test_recording_playback_auth.py b/apps/channels/tests/test_recording_playback_auth.py
index f5c65136..87cc251f 100644
--- a/apps/channels/tests/test_recording_playback_auth.py
+++ b/apps/channels/tests/test_recording_playback_auth.py
@@ -1,13 +1,16 @@
 """Tests for DVR recording playback authentication (file/hls endpoints)."""
 import os
+import shutil
 import tempfile
+from types import SimpleNamespace
+from unittest.mock import patch
 
 from django.test import TestCase, override_settings
 from django.utils import timezone
-from rest_framework.test import APIRequestFactory
+from rest_framework.test import APIClient
 from rest_framework_simplejwt.tokens import RefreshToken
 
-from apps.channels.api_views import RecordingViewSet
+from apps.channels.api_views import _recording_auth_query_suffix
 from apps.channels.models import Channel, Recording
 
 
@@ -19,20 +22,27 @@ def _make_admin():
         username="recording_playback_admin",
         defaults={"user_level": User.UserLevel.ADMIN},
     )
+    user.user_level = User.UserLevel.ADMIN
     user.set_password("pass")
     user.save()
     return user
 
 
 @override_settings(ALLOWED_HOSTS=["testserver"])
+@patch("apps.channels.api_views.network_access_allowed", return_value=True)
 class RecordingPlaybackAuthTests(TestCase):
     def setUp(self):
         self.channel = Channel.objects.create(channel_number=42, name="Playback Auth Channel")
         self.user = _make_admin()
-        self.factory = APIRequestFactory()
+        self.client = APIClient()
         self.tmp = tempfile.NamedTemporaryFile(suffix=".mkv", delete=False)
         self.tmp.write(b"\x00" * 1024)
         self.tmp.close()
+        self.hls_dir = tempfile.mkdtemp(prefix="dvr_playback_auth_hls_")
+        with open(os.path.join(self.hls_dir, "index.m3u8"), "w", encoding="utf-8") as playlist:
+            playlist.write("#EXTM3U\n#EXTINF:4.0,\nseg_00001.ts\n")
+        with open(os.path.join(self.hls_dir, "seg_00001.ts"), "wb") as segment:
+            segment.write(b"\x00" * 188)
         now = timezone.now()
         self.recording = Recording.objects.create(
             channel=self.channel,
@@ -48,24 +58,136 @@ class RecordingPlaybackAuthTests(TestCase):
     def tearDown(self):
         if os.path.exists(self.tmp.name):
             os.unlink(self.tmp.name)
-
-    def _file(self, *, token=None):
-        url = f"/api/channels/recordings/{self.recording.id}/file/"
-        if token:
-            url = f"{url}?token={token}"
-        request = self.factory.get(url)
-        view = RecordingViewSet.as_view({"get": "file"})
-        return view(request, pk=self.recording.id)
+        if os.path.isdir(self.hls_dir):
+            shutil.rmtree(self.hls_dir, ignore_errors=True)
 
     @staticmethod
     def _jwt_for(user):
         return str(RefreshToken.for_user(user).access_token)
 
-    def test_file_requires_authentication(self):
-        response = self._file()
+    def test_file_requires_authentication(self, _mock_network):
+        response = self.client.get(
+            f"/api/channels/recordings/{self.recording.id}/file/"
+        )
         self.assertEqual(response.status_code, 403)
 
-    def test_file_accepts_jwt_query_param(self):
+    def test_file_accepts_jwt_query_param(self, _mock_network):
         token = self._jwt_for(self.user)
-        response = self._file(token=token)
+        response = self.client.get(
+            f"/api/channels/recordings/{self.recording.id}/file/",
+            {"token": token},
+        )
         self.assertEqual(response.status_code, 200)
+
+    def test_file_redirect_to_hls_preserves_token(self, _mock_network):
+        pending = os.path.join(self.hls_dir, "pending.mkv")
+        now = timezone.now()
+        in_progress = Recording.objects.create(
+            channel=self.channel,
+            start_time=now,
+            end_time=now,
+            custom_properties={
+                "status": "recording",
+                "_hls_dir": self.hls_dir,
+                "file_path": pending,
+            },
+        )
+        token = self._jwt_for(self.user)
+        response = self.client.get(
+            f"/api/channels/recordings/{in_progress.id}/file/",
+            {"token": token},
+        )
+        self.assertEqual(response.status_code, 302)
+        self.assertIn("token=", response["Location"])
+        self.assertIn("/hls/index.m3u8", response["Location"])
+
+    def test_hls_playlist_rewrites_segments_with_token_when_present(self, _mock_network):
+        now = timezone.now()
+        hls_rec = Recording.objects.create(
+            channel=self.channel,
+            start_time=now,
+            end_time=now,
+            custom_properties={
+                "status": "recording",
+                "_hls_dir": self.hls_dir,
+            },
+        )
+        token = self._jwt_for(self.user)
+        response = self.client.get(
+            f"/api/channels/recordings/{hls_rec.id}/hls/index.m3u8",
+            {"token": token},
+        )
+        self.assertEqual(response.status_code, 200)
+        body = response.content.decode("utf-8")
+        self.assertIn("token=", body)
+        self.assertIn("seg_00001.ts", body)
+
+    def test_hls_playlist_omits_token_when_not_in_request(self, _mock_network):
+        now = timezone.now()
+        hls_rec = Recording.objects.create(
+            channel=self.channel,
+            start_time=now,
+            end_time=now,
+            custom_properties={
+                "status": "recording",
+                "_hls_dir": self.hls_dir,
+            },
+        )
+        token = self._jwt_for(self.user)
+        response = self.client.get(
+            f"/api/channels/recordings/{hls_rec.id}/hls/index.m3u8",
+            HTTP_AUTHORIZATION=f"Bearer {token}",
+        )
+        self.assertEqual(response.status_code, 200)
+        body = response.content.decode("utf-8")
+        self.assertIn("seg_00001.ts", body)
+        self.assertNotIn("token=", body)
+
+    def test_hls_segment_accepts_jwt_query_param(self, _mock_network):
+        now = timezone.now()
+        hls_rec = Recording.objects.create(
+            channel=self.channel,
+            start_time=now,
+            end_time=now,
+            custom_properties={
+                "status": "recording",
+                "_hls_dir": self.hls_dir,
+            },
+        )
+        token = self._jwt_for(self.user)
+        response = self.client.get(
+            f"/api/channels/recordings/{hls_rec.id}/hls/seg_00001.ts",
+            {"token": token},
+        )
+        self.assertEqual(response.status_code, 200)
+
+    def test_hls_redirect_to_file_preserves_token(self, _mock_network):
+        now = timezone.now()
+        hls_rec = Recording.objects.create(
+            channel=self.channel,
+            start_time=now,
+            end_time=now,
+            custom_properties={
+                "status": "completed",
+                "file_path": self.tmp.name,
+                "file_name": "test.mkv",
+            },
+        )
+        token = self._jwt_for(self.user)
+        response = self.client.get(
+            f"/api/channels/recordings/{hls_rec.id}/hls/index.m3u8",
+            {"token": token},
+        )
+        self.assertEqual(response.status_code, 302)
+        self.assertIn("token=", response["Location"])
+        self.assertIn("/file/", response["Location"])
+
+
+class RecordingAuthQuerySuffixTests(TestCase):
+    def test_empty_when_no_token(self):
+        request = SimpleNamespace(GET={})
+        self.assertEqual(_recording_auth_query_suffix(request), "")
+
+    def test_includes_token_when_present(self):
+        request = SimpleNamespace(GET={"token": "abc123"})
+        self.assertEqual(_recording_auth_query_suffix(request), "?token=abc123")
diff --git a/frontend/src/WebSocket.jsx b/frontend/src/WebSocket.jsx
index 4cbb3d89..89b1a1ef 100644
--- a/frontend/src/WebSocket.jsx
+++ b/frontend/src/WebSocket.jsx
@@ -203,7 +203,7 @@ export const WebsocketProvider = ({ children }) => {
                 notifications.update({
                   id,
                   title: 'Commercials removed',
-                  message: `${title} — kept ${parsedEvent.data.segments_kept} segments`,
+                  message: `${title}: kept ${parsedEvent.data.segments_kept} segments`,
                   color: 'green.5',
                   loading: false,
                   autoClose: 4000,