feat(sources): pre-flight credential verify on backfill/recovery arm — #703 step 2
Before arming a deep walk on a native-ingester platform (Patreon — where
verify is one cheap API page), POST /sources/{id}/backfill {start|recover}
runs the shared verify_source_credential first and REFUSES (409 + reason)
only on a definitive rejection (verify→False, e.g. expired cookies). It
proceeds on valid (True) or inconclusive (None — a network blip must not
block). Gated to native platforms: gallery-dl verify is a slow --simulate
subprocess, too heavy for an arm action. The credential read happens in a
session that's CLOSED before the verify network call (no held conn).
Frontend: onBackfill/onRecover now read e.body.detail (ApiError carries the
reason in .body, not .detail) so the rejection text surfaces in the toast.
Tests: arm blocked on rejection (409, source not armed), proceeds on
inconclusive, stop never pre-flights, gallery-dl platform skips pre-flight.
An autouse fixture stubs verify to 'valid' for the existing backfill
endpoint tests so they stay network-free.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -130,6 +130,15 @@ async def set_backfill(source_id: int):
|
|||||||
current pHash threshold; 'stop' cancels either back to tick mode. Returns the
|
current pHash threshold; 'stop' cancels either back to tick mode. Returns the
|
||||||
updated source dict (incl. backfill_state / backfill_chunks /
|
updated source dict (incl. backfill_state / backfill_chunks /
|
||||||
backfill_bypass_seen)."""
|
backfill_bypass_seen)."""
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from ..services.credential_service import CredentialService
|
||||||
|
from ..services.download_backends import (
|
||||||
|
uses_native_ingester,
|
||||||
|
verify_source_credential,
|
||||||
|
)
|
||||||
|
from .credentials import _get_crypto
|
||||||
|
|
||||||
payload = await request.get_json(silent=True) or {}
|
payload = await request.get_json(silent=True) or {}
|
||||||
action = payload.get("action", "start")
|
action = payload.get("action", "start")
|
||||||
if action not in ("start", "stop", "recover"):
|
if action not in ("start", "stop", "recover"):
|
||||||
@@ -137,6 +146,37 @@ async def set_backfill(source_id: int):
|
|||||||
"invalid_action",
|
"invalid_action",
|
||||||
detail="action must be 'start', 'stop', or 'recover'",
|
detail="action must be 'start', 'stop', or 'recover'",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Pre-flight (plan #703 #2): before arming a deep walk on a native-ingester
|
||||||
|
# platform (where verify is one cheap API page), refuse if the credential is
|
||||||
|
# DEFINITIVELY rejected — don't burn chunks against expired cookies. Proceed
|
||||||
|
# on valid OR inconclusive (a network blip shouldn't block). Gated to native
|
||||||
|
# platforms: gallery-dl verify is a slow --simulate subprocess, too heavy for
|
||||||
|
# an arm action. The credential read happens in a session that's CLOSED
|
||||||
|
# before the verify network call (don't hold a DB conn across the request).
|
||||||
|
if action in ("start", "recover"):
|
||||||
|
async with get_session() as session:
|
||||||
|
rec = await SourceService(session).get(source_id)
|
||||||
|
if rec is None:
|
||||||
|
return _bad("not_found", status=404)
|
||||||
|
native = uses_native_ingester(rec.platform)
|
||||||
|
if native:
|
||||||
|
cred = CredentialService(session, _get_crypto())
|
||||||
|
cookies_path = await cred.get_cookies_path(rec.platform)
|
||||||
|
auth_token = await cred.get_token(rec.platform)
|
||||||
|
if native:
|
||||||
|
ok, message = await verify_source_credential(
|
||||||
|
platform=rec.platform,
|
||||||
|
url=rec.url,
|
||||||
|
artist_slug=rec.artist_slug,
|
||||||
|
config_overrides=rec.config_overrides or {},
|
||||||
|
cookies_path=str(cookies_path) if cookies_path else None,
|
||||||
|
auth_token=auth_token,
|
||||||
|
images_root=Path("/images"),
|
||||||
|
)
|
||||||
|
if ok is False:
|
||||||
|
return _bad("credential_rejected", detail=message, status=409)
|
||||||
|
|
||||||
async with get_session() as session:
|
async with get_session() as session:
|
||||||
try:
|
try:
|
||||||
svc = SourceService(session)
|
svc = SourceService(session)
|
||||||
|
|||||||
@@ -550,7 +550,7 @@ async function onBackfill(source) {
|
|||||||
await store.loadAll()
|
await store.loadAll()
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
toast({
|
toast({
|
||||||
text: `Backfill ${running ? 'stop' : 'start'} failed: ${e?.detail || e?.message || e}`,
|
text: `Backfill ${running ? 'stop' : 'start'} failed: ${e?.body?.detail || e?.message || e}`,
|
||||||
type: 'error',
|
type: 'error',
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -566,7 +566,7 @@ async function onRecover(source) {
|
|||||||
await store.loadAll()
|
await store.loadAll()
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
toast({
|
toast({
|
||||||
text: `Recovery start failed: ${e?.detail || e?.message || e}`,
|
text: `Recovery start failed: ${e?.body?.detail || e?.message || e}`,
|
||||||
type: 'error',
|
type: 'error',
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,20 @@ from backend.app.models import Artist, Source
|
|||||||
pytestmark = pytest.mark.integration
|
pytestmark = pytest.mark.integration
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _stub_preflight_verify(monkeypatch):
|
||||||
|
"""Backfill/recover arms run a pre-flight credential verify (plan #703 #2)
|
||||||
|
which for Patreon would hit the network. Default it to 'valid' so the
|
||||||
|
endpoint tests stay network-free; the dedicated pre-flight tests override
|
||||||
|
this with a rejection/inconclusive verdict."""
|
||||||
|
from backend.app.services import download_backends as db_mod
|
||||||
|
|
||||||
|
async def _ok(**kwargs):
|
||||||
|
return (True, "Credentials valid")
|
||||||
|
|
||||||
|
monkeypatch.setattr(db_mod, "verify_source_credential", _ok)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
async def artist(db):
|
async def artist(db):
|
||||||
a = Artist(name="Alice", slug="alice")
|
a = Artist(name="Alice", slug="alice")
|
||||||
@@ -275,3 +289,92 @@ async def test_backfill_endpoint_recover_arms_bypass(client, artist, db):
|
|||||||
stopped_body = await stopped.get_json()
|
stopped_body = await stopped.get_json()
|
||||||
assert stopped_body["backfill_state"] is None
|
assert stopped_body["backfill_state"] is None
|
||||||
assert stopped_body["backfill_bypass_seen"] is False
|
assert stopped_body["backfill_bypass_seen"] is False
|
||||||
|
|
||||||
|
|
||||||
|
# --- Plan #703 #2: pre-flight credential verify on backfill/recover arm -----
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_arm_blocked_when_credential_rejected(client, artist, db, monkeypatch):
|
||||||
|
"""A definitively-rejected credential refuses the arm (409 + reason) instead
|
||||||
|
of starting a doomed walk; the source is NOT armed."""
|
||||||
|
from backend.app.services import download_backends as db_mod
|
||||||
|
|
||||||
|
async def _reject(**kwargs):
|
||||||
|
return (False, "Patreon rejected the credential — cookies expired")
|
||||||
|
monkeypatch.setattr(db_mod, "verify_source_credential", _reject)
|
||||||
|
|
||||||
|
src = Source(
|
||||||
|
artist_id=artist.id, platform="patreon",
|
||||||
|
url="https://patreon.com/alice-preflight", enabled=True,
|
||||||
|
)
|
||||||
|
db.add(src)
|
||||||
|
await db.commit()
|
||||||
|
sid = src.id
|
||||||
|
|
||||||
|
resp = await client.post(f"/api/sources/{sid}/backfill", json={"action": "recover"})
|
||||||
|
assert resp.status_code == 409
|
||||||
|
assert "expired" in ((await resp.get_json()).get("detail") or "")
|
||||||
|
|
||||||
|
# Not armed.
|
||||||
|
one = await (await client.get(f"/api/sources/{sid}")).get_json()
|
||||||
|
assert one["backfill_state"] is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_arm_proceeds_when_credential_inconclusive(client, artist, db, monkeypatch):
|
||||||
|
"""An inconclusive verify (network blip / drift) must NOT block the arm."""
|
||||||
|
from backend.app.services import download_backends as db_mod
|
||||||
|
|
||||||
|
async def _inconclusive(**kwargs):
|
||||||
|
return (None, "couldn't verify (network)")
|
||||||
|
monkeypatch.setattr(db_mod, "verify_source_credential", _inconclusive)
|
||||||
|
|
||||||
|
src = Source(
|
||||||
|
artist_id=artist.id, platform="patreon",
|
||||||
|
url="https://patreon.com/alice-incon", enabled=True,
|
||||||
|
)
|
||||||
|
db.add(src)
|
||||||
|
await db.commit()
|
||||||
|
|
||||||
|
resp = await client.post(f"/api/sources/{src.id}/backfill", json={"action": "start"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert (await resp.get_json())["backfill_state"] == "running"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_stop_never_pre_flights(client, artist, db, monkeypatch):
|
||||||
|
from backend.app.services import download_backends as db_mod
|
||||||
|
|
||||||
|
async def _boom(**kwargs):
|
||||||
|
raise AssertionError("stop must not pre-flight verify")
|
||||||
|
monkeypatch.setattr(db_mod, "verify_source_credential", _boom)
|
||||||
|
|
||||||
|
src = Source(
|
||||||
|
artist_id=artist.id, platform="patreon",
|
||||||
|
url="https://patreon.com/alice-stop", enabled=True,
|
||||||
|
)
|
||||||
|
db.add(src)
|
||||||
|
await db.commit()
|
||||||
|
resp = await client.post(f"/api/sources/{src.id}/backfill", json={"action": "stop"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_gallery_dl_platform_arm_skips_pre_flight(client, artist, db, monkeypatch):
|
||||||
|
"""Pre-flight is gated to native-ingester platforms (cheap verify); a
|
||||||
|
gallery-dl source arms without the slow --simulate probe."""
|
||||||
|
from backend.app.services import download_backends as db_mod
|
||||||
|
|
||||||
|
async def _boom(**kwargs):
|
||||||
|
raise AssertionError("gallery-dl arm must not pre-flight verify")
|
||||||
|
monkeypatch.setattr(db_mod, "verify_source_credential", _boom)
|
||||||
|
|
||||||
|
src = Source(
|
||||||
|
artist_id=artist.id, platform="subscribestar",
|
||||||
|
url="https://www.subscribestar.com/x", enabled=True,
|
||||||
|
)
|
||||||
|
db.add(src)
|
||||||
|
await db.commit()
|
||||||
|
resp = await client.post(f"/api/sources/{src.id}/backfill", json={"action": "start"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
|||||||
Reference in New Issue
Block a user