fix(downloads): Retry and Re-download take a cookie added after the failure

The login-wall copy tells the user to add their cookies and press Retry, but Retry kept the job on the shared asset the worker never attaches a cookie to, so it failed on the same wall. Resume now moves a pending/errored job onto the asset enqueue would pick with the cookie (never off a file or a download in flight). Re-download salted its fresh asset with the job's previous asset id, which the cookie gate cannot recompute, so even a cookie download rebuilt without the cookie. The rebuild salt is now the new asset's own id (plus the user's cookie salt when one applies), and is_authed_asset recognises it.
2026-09-13 22:31:49 +02:00
parent 18c11ca437
commit 98ec1ffcab
6 changed files with 229 additions and 28 deletions
@@ -18,7 +18,7 @@ from cryptography.fernet import Fernet

 from app import security
 from app.downloads import credentials, formats, service
-from app.models import DownloadCredential, MediaAsset, User
+from app.models import DownloadCredential, DownloadJob, MediaAsset, User


@pytest.fixture(autouse=True)
@@ -189,7 +189,9 @@ class TestResolveCookiefile:
        db.commit()
        spec = formats.normalize({})
        salted = credentials.authed_format_sig(spec, u.id, "facebook")
-        path = credentials.resolve_cookiefile(db, u.id, FB_URL, salted, spec, tmp_path)
+        path = credentials.resolve_cookiefile(
+            db, u.id, FB_URL, self._asset_sig(salted), spec, tmp_path
+        )
        assert path is not None and Path(path).read_text() == COOKIES

    def test_never_attaches_to_a_shared_asset(self, db, tmp_path):
@@ -200,7 +202,12 @@ class TestResolveCookiefile:
        db.commit()
        spec = formats.normalize({})
        shared = formats.format_sig(spec)
-        assert credentials.resolve_cookiefile(db, u.id, FB_URL, shared, spec, tmp_path) is None
+        assert (
+            credentials.resolve_cookiefile(
+                db, u.id, FB_URL, self._asset_sig(shared), spec, tmp_path
+            )
+            is None
+        )

    def test_non_matching_host_gets_no_cookie(self, db, tmp_path):
        u = _user(db)
@@ -210,13 +217,141 @@ class TestResolveCookiefile:
        # A YouTube URL has no supported host_key -> no cookie regardless of what's stored.
        yt = "https://www.youtube.com/watch?v=abc"
        salted = credentials.authed_format_sig(spec, u.id, "facebook")
-        assert credentials.resolve_cookiefile(db, u.id, yt, salted, spec, tmp_path) is None
+        assert (
+            credentials.resolve_cookiefile(db, u.id, yt, self._asset_sig(salted), spec, tmp_path)
+            is None
+        )

    def test_no_credential_gets_no_cookie(self, db, tmp_path):
        u = _user(db)
        spec = formats.normalize({})
        salted = credentials.authed_format_sig(spec, u.id, "facebook")
-        assert credentials.resolve_cookiefile(db, u.id, FB_URL, salted, spec, tmp_path) is None
+        assert (
+            credentials.resolve_cookiefile(
+                db, u.id, FB_URL, self._asset_sig(salted), spec, tmp_path
+            )
+            is None
+        )
+
+
+# --- Retry / Re-download take a cookie added later (DB) -----------------------------------------
+# Measured on dev (E11.5): a Facebook reel failed on the login wall, the user added their cookie as
+# the failure copy tells them to, pressed Retry — and it failed again, because the job still held the
+# shared asset the worker never attaches a cookie to. Re-download forked a salt the gate could not
+# recognise, so even a job downloaded WITH a cookie rebuilt without it.
+
+
+def _held_job(db, u, *, sig, asset_status="error", job_status="error"):
+    a = MediaAsset(
+        source_kind="url", source_ref=FB_URL, format_sig=sig, status=asset_status, ref_count=1
+    )
+    db.add(a)
+    db.flush()
+    j = DownloadJob(
+        user_id=u.id,
+        asset_id=a.id,
+        source_kind="url",
+        source_ref=FB_URL,
+        status=job_status,
+        queue_pos=0,
+        profile_snapshot={},
+    )
+    db.add(j)
+    db.commit()
+    return a, j
+
+
+class TestRequeueTakesTheCookie:
+    spec = formats.normalize({})
+
+    def test_retry_after_adding_a_cookie_moves_the_job_to_the_private_asset(self, db, tmp_path):
+        u = _user(db)
+        shared, j = _held_job(db, u, sig=formats.format_sig(self.spec))
+        credentials.upsert_credential(db, u.id, "facebook", COOKIES)
+        db.commit()
+
+        service.resume(db, j)
+
+        db.expire_all()
+        job = db.get(DownloadJob, j.id)
+        target = db.get(MediaAsset, job.asset_id)
+        assert job.status == "queued" and target.id != shared.id
+        assert target.format_sig == credentials.authed_format_sig(self.spec, u.id, "facebook")
+        assert (target.status, target.ref_count) == ("pending", 1)
+        assert db.get(MediaAsset, shared.id).ref_count == 0  # the old hold went with the move
+        assert credentials.resolve_cookiefile(db, u.id, FB_URL, target, self.spec, tmp_path)
+
+    def test_retry_without_a_cookie_keeps_the_shared_asset(self, db):
+        u = _user(db)
+        shared, j = _held_job(db, u, sig=formats.format_sig(self.spec))
+
+        service.resume(db, j)
+
+        db.expire_all()
+        assert db.get(DownloadJob, j.id).asset_id == shared.id
+        assert db.get(MediaAsset, shared.id).ref_count == 1
+
+    @pytest.mark.parametrize(
+        ("asset_status", "job_status"), [("ready", "error"), ("downloading", "paused")]
+    )
+    def test_retry_never_moves_off_a_file_or_a_download_in_flight(
+        self, db, asset_status, job_status
+    ):
+        u = _user(db)
+        shared, j = _held_job(
+            db,
+            u,
+            sig=formats.format_sig(self.spec),
+            asset_status=asset_status,
+            job_status=job_status,
+        )
+        credentials.upsert_credential(db, u.id, "facebook", COOKIES)
+        db.commit()
+
+        service.resume(db, j)
+
+        db.expire_all()
+        assert db.get(DownloadJob, j.id).asset_id == shared.id
+
+    def test_redownload_with_a_cookie_rebuilds_into_an_asset_the_cookie_goes_with(
+        self, db, tmp_path
+    ):
+        u = _user(db)
+        other = _user(db, "other@example.com")
+        credentials.upsert_credential(db, u.id, "facebook", COOKIES)
+        credentials.upsert_credential(db, other.id, "facebook", COOKIES)
+        authed = credentials.authed_format_sig(self.spec, u.id, "facebook")
+        first, j = _held_job(db, u, sig=authed, asset_status="ready", job_status="done")
+
+        seen = {first.id}
+        for _ in range(2):  # a rebuild of a rebuild is still recognised
+            service.redownload(db, j)
+            db.expire_all()
+            job = db.get(DownloadJob, j.id)
+            target = db.get(MediaAsset, job.asset_id)
+            assert target.id not in seen and target.format_sig != authed
+            seen.add(target.id)
+            assert credentials.resolve_cookiefile(db, u.id, FB_URL, target, self.spec, tmp_path)
+            # Private: another user's cookie is never attached to it.
+            assert not credentials.is_authed_asset(target, self.spec, other.id, FB_URL)
+
+        # A Retry of a failed rebuild keeps the rebuild — it is already the cookie's identity.
+        target.status, job.status = "error", "error"
+        db.commit()
+        service.resume(db, job)
+        db.expire_all()
+        assert db.get(DownloadJob, j.id).asset_id == target.id
+
+    def test_a_rebuild_made_without_a_cookie_is_no_cookie_identity(self, db, tmp_path):
+        u = _user(db)
+        _, j = _held_job(db, u, sig=formats.format_sig(self.spec))
+        service.redownload(db, j)
+        credentials.upsert_credential(db, u.id, "facebook", COOKIES)
+        db.commit()
+
+        db.expire_all()
+        target = db.get(MediaAsset, db.get(DownloadJob, j.id).asset_id)
+        assert not credentials.is_authed_asset(target, self.spec, u.id, FB_URL)


 # --- API: write-only, never echoes the blob (DB) ----------------------------------------------