From 9ad881ada8bbff98fd2f8f25b885da37f0eb902b Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Wed, 30 Sep 2026 11:03:03 +0200 Subject: [PATCH] feat(magazine): manage OPDS catalogues in the app, env-based login, result paging Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + Manual.md | 12 ++- TODO.md | 7 +- architecture/magazine-sources.md | 14 ++- core/magazine_sources.py | 57 +++++++++-- routes/magazine_sources.py | 58 ++++++++++- static/js/magazine.js | 51 ++++++++++ static/sections/s-magazine.html | 13 +++ static/style.css | 8 ++ tests/magazine.spec.js | 51 ++++++++++ tests/test_magazine_sources.py | 169 +++++++++++++++++++++++++++++++ 11 files changed, 421 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ebbe23..1092d18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi ## [Unreleased] ### Added +- **OPDS catalogues: manage them in the app, log in, and page through results.** A *Your catalogues (OPDS)* panel adds and removes catalogues (validated server-side). A catalogue that needs a login takes a username and the *name* of an environment variable holding the password, which is never stored or shown and only sent to that catalogue's own host. Searches follow the catalogue's own next-page links (up to five result pages, same host only, loop-protected). - **Page overview says what to do.** Every page card now states its status in words, how many articles are on the page, where each article continues from / on to ("continues from p.3", "continues on p.6"), and a **To do** list when the system could not settle the page (approve it; fix the quality findings with the tool that fixes each; link orphan text to an article; generate audio; approve the other articles on a shared page). A summary line counts the pages that need you and *Show only pages that need me* hides the rest. The legend is larger (14 px, 18 px swatches). A page coloured "audio" that still has an unapproved article is now labelled "Partly done" with the reason. - **Hands-free listening.** The article player now registers with the browser's Media Session: title, magazine and first-page artwork on the lock screen / headset / car display, with play, pause, previous/next article, ±15 s and seek, plus a position bar. While driving mode is open and audio is playing the screen is kept awake (Screen Wake Lock), and released on pause or exit; a refused lock never affects playback. - **EPUB 3 with Media Overlays.** *EPUB with audio* in the contents toolbar (and `GET /api/magazine/{id}/epub`) packages the approved articles that have audio into a book whose text highlights as the audio plays. MP3 export now records each sentence's real start and end (`audio/.timings.json`, no text), so new audio highlights exactly; older audio is placed by estimate and the book's description says so. Validated with W3C EPUBCheck 5.2.1 (0 errors, 0 warnings) on a real 12-minute two-article issue. diff --git a/Manual.md b/Manual.md index fdea352..3cb7f04 100644 --- a/Manual.md +++ b/Manual.md @@ -417,9 +417,13 @@ group that explains the minus/plus convention to a screen reader. **Purpose.** Pull a magazine issue into the app without leaving for a browser and uploading by hand. -**Prerequisites.** Internet access for the Internet Archive; for your own library, an OPDS catalogue -(Calibre-Web, Kavita or Komga all serve one) added under `magazine_opds_catalogs` in the app settings as -`[{"name": "NAS", "url": "http://nas.local:8083/opds/"}]`. Catalogues that need a login are not supported yet. +**Prerequisites.** Internet access for the Internet Archive. For your own library, an OPDS catalogue (Calibre-Web, Kavita or +Komga all serve one): open **Your catalogues (OPDS)** in the same panel and add its name and address (for example +`http://nas.local:8083/opds/`). If it needs a login, enter the username and the **name of an environment variable** that holds the +password (for example `NAS_OPDS_PASSWORD`), and set that variable for the app's container; the password itself is never typed +into, stored in, or shown by the app, and it is sent only to that catalogue's own address. The list tells you if the variable is not +set on the server. Up to 10 catalogues. Use an `https://` address for a catalogue reached over the internet: a login to an `http://` +address travels unencrypted. **Steps.** 1. Open Magazines. In *Find an issue online* pick a source, type a title or subject, press Search. @@ -432,7 +436,7 @@ group that explains the minus/plus convention to a screen reader. **Errors.** A source that is down, rate-limiting or returns something other than a PDF gives a plain message under the search box and nothing is created that you must clean up beyond the empty project shown in your library. Lending-only Internet Archive items are never listed. Only one download runs at a time; a PDF larger than -400 MB is refused. +400 MB is refused. A catalogue's results are read across up to five of its result pages, so "next page" of a large library works. **Mobile / accessibility.** The form stacks on a phone; results are a list with labelled Import buttons; status text is a polite live region, so a screen reader hears "N results" and download progress. diff --git a/TODO.md b/TODO.md index 74da19f..4cdccd0 100644 --- a/TODO.md +++ b/TODO.md @@ -722,9 +722,8 @@ casting. The original PDF remains the visual source of truth while approved arti aligns its transcript by estimate and could use the new timing records)* - [x] Add mobile/driving mode with large controls, automatic article advance, and hands-free resume. *(driving mode with large controls, queue auto-advance and resume at the saved position; 2026-09-30: Media Session for lock screen / headset / car controls and artwork, screen wake lock while playing)* -- [~] Add OCR-engine benchmark comparisons and per-page fallback selection. - *(per-page fallback done, now a real two-engine chain: vision OCR escalates when a page needs OCR or scores - poorly, KDL Frontier Parser nano is the vision fallback, tesseract is the last resort; no formal benchmark)* +- [x] Add OCR-engine benchmark comparisons and per-page fallback selection. + *(per-page fallback is a real engine chain: vision OCR escalates when a page needs OCR or scores poorly, KDL nano is the vision fallback, tesseract the last resort; 2026-09-29: `scripts/bench_ocr.py` scores any engine against a native-text PDF's own text layer, first results for tesseract, Unlimited-OCR, docling and PaddleOCR in `architecture/evidence/ocr-bench-2026-09-29/`)* - [ ] **Evaluate additional OCR/vision candidates against the two already wired (baidu/Unlimited-OCR, KDL-Frontier-Parser-nano) and pick the best fit per task** — user-requested 2026-09-14. Goal: low RAM/VRAM footprint with excellent speed and quality, for both page-text transcription and open-ended figure/chart @@ -746,7 +745,7 @@ casting. The original PDF remains the visual source of truth while approved arti ### P2 — Finding magazines to read -- [~] **Browse and search online PDF magazine sources from inside the app** *(done 2026-09-29: Internet Archive + OPDS catalogues, see architecture/magazine-sources.md; still open: publisher free issues/feeds, open-access journals, OPDS login and pagination)*, so an issue can be found and pulled in +- [~] **Browse and search online PDF magazine sources from inside the app** *(done 2026-09-29/30: Internet Archive + OPDS catalogues with in-app management, login via an environment variable, and result paging, see architecture/magazine-sources.md; still open: publisher free issues/feeds, open-access journals)* without leaving for a browser and uploading by hand. Wanted: a catalogue of sources, a search across them, and a one-click "ingest this issue". **Blocked on a source decision, and that decision is the whole task.** The two sites named as examples diff --git a/architecture/magazine-sources.md b/architecture/magazine-sources.md index 3206751..8a1f576 100644 --- a/architecture/magazine-sources.md +++ b/architecture/magazine-sources.md @@ -34,5 +34,17 @@ Every provider failure is a `SourceError` with a user-safe message (unreachable, ## Offline / sync Needs the network only for search and download. No synchronisation state. +## Catalogues, paging and login (2026-09-30) +- `GET/PUT /api/magazine-sources/catalogues` manage the OPDS list, validated at the boundary (<= 10; `http(s)` with a host; no credentials in the + address; a login needs both a username and `password_env`, a capitalised variable name; duplicates and wrong shapes are refused and + change nothing). The GET tells whether the named variable is set, never its value. +- Login is HTTP Basic: username from the settings, password read from the environment variable named by `password_env`. It is sent only + to the catalogue's own host (every hop, including redirects, must match it; a cross-host redirect or `next` link is refused), used for + both search and download, and appears in no response, log line or error message. +- A search walks the catalogue's `rel="next"` links (at most 5 result pages, loop-protected, same host only) until the requested page of 20 + can be filled, since entries without a PDF are dropped. + ## Known limitations -OPDS catalogues requiring login; OPDS pagination beyond the first feed page; no thumbnails shown yet (returned in the API). +No thumbnails shown yet (returned in the API). Basic auth only (no OAuth/cookies). Each search page re-reads the catalogue from its first page +(bounded at 5), there is no result cache. Anyone who can edit the catalogue list can name any environment variable as the password source, +so the list is an administrator setting. diff --git a/core/magazine_sources.py b/core/magazine_sources.py index 984d019..c874e18 100644 --- a/core/magazine_sources.py +++ b/core/magazine_sources.py @@ -73,14 +73,14 @@ def _clip(value, limit: int) -> str: def _get(session: requests.Session, url: str, *, timeout, stream: bool = False, host_ok=None, - accept: str = "*/*") -> requests.Response: + accept: str = "*/*", auth=None) -> requests.Response: """GET with manual, host-checked redirects.""" for _ in range(MAX_REDIRECTS + 1): if host_ok is not None and not host_ok(url): raise SourceError("The source pointed to a different host; refusing to follow it") try: resp = session.get(url, timeout=timeout, stream=stream, allow_redirects=False, - headers={"User-Agent": USER_AGENT, "Accept": accept}) + headers={"User-Agent": USER_AGENT, "Accept": accept}, **({"auth": auth} if auth else {})) except requests.RequestException as e: raise SourceError(f"Could not reach the source ({type(e).__name__})") from e if resp.is_redirect or resp.status_code in (301, 302, 303, 307, 308): @@ -132,7 +132,7 @@ def download_pdf(provider: Provider, item_id: str, dest, *, session: requests.Se session = session or requests.Session() url = provider.resolve_pdf(item_id) resp = _get(session, url, timeout=DOWNLOAD_TIMEOUT, stream=True, host_ok=provider.allowed_host, - accept="application/pdf,*/*") + accept="application/pdf,*/*", auth=getattr(provider, "auth", lambda: None)()) tmp = dest.with_suffix(".part") total = 0 try: @@ -251,14 +251,34 @@ class InternetArchiveProvider: # ── OPDS ───────────────────────────────────────────────────────────────────── class OpdsProvider: - """A user-configured OPDS catalogue (their own Calibre-Web/Kavita/Komga, or any public one).""" + """A user-configured OPDS catalogue (their own Calibre-Web/Kavita/Komga, or any public one). - def __init__(self, key: str, label: str, root_url: str, session: requests.Session | None = None): + Login, when the catalogue needs one, is HTTP Basic with a username from the settings and a password read from + the *environment variable whose name the settings give* (`password_env`): the password itself is never stored + in the settings, the URL, a log line or an error message. It is only ever sent to the catalogue's own host. + """ + MAX_FEED_PAGES = 5 # how many of the catalogue's result pages one search may walk + + def __init__(self, key: str, label: str, root_url: str, session: requests.Session | None = None, + username: str = "", password_env: str = ""): self.key = key self.label = label self._root = root_url self._host = (urlsplit(root_url).hostname or "").lower() self._s = session or requests.Session() + self._username = username + self._password_env = password_env + + def auth(self): + """(user, password) for this catalogue, or None when it is open. Raises SourceError if configured but unset.""" + if not self._username and not self._password_env: + return None + import os + password = os.environ.get(self._password_env or "", "") + if not self._username or not password: + raise SourceError(f"The catalogue \u201c{self.label}\u201d needs a login: set a username and the environment " + f"variable named in password_env ({self._password_env or 'not configured'})") + return (self._username, password) def allowed_host(self, url: str) -> bool: parts = urlsplit(url) @@ -266,7 +286,7 @@ class OpdsProvider: def _feed(self, url: str) -> ET.Element: resp = _get(self._s, url, timeout=SEARCH_TIMEOUT, stream=True, host_ok=self.allowed_host, - accept="application/atom+xml,application/xml,text/xml,*/*") + accept="application/atom+xml,application/xml,text/xml,*/*", auth=self.auth()) try: return parse_xml_safely(_read_limited(resp, MAX_FEED_BYTES)) finally: @@ -286,10 +306,23 @@ class OpdsProvider: q = _clip(query, 200) if not q: raise SourceError("Enter something to search for") + page = max(1, int(page)) url = self._search_url().replace("{searchTerms}", quote(q)) - items = list(self._entries(self._feed(url), url)) - start = (max(1, int(page)) - 1) * PAGE_SIZE - return items[start:start + PAGE_SIZE] + need = page * PAGE_SIZE + items: list[SourceItem] = [] + seen: set[str] = set() + # A catalogue answers a search one result page at a time and points at the next with rel="next". + # Entries without a PDF are dropped, so walk on (bounded) until this page can be filled. + for _ in range(self.MAX_FEED_PAGES): + if not url or url in seen or len(items) >= need: + break + seen.add(url) + root = self._feed(url) + items.extend(self._entries(root, url)) + nxt = next((urljoin(url, l.get("href", "")) for l in root.findall(f"{_ATOM}link") + if l.get("rel") == "next" and l.get("href")), "") + url = nxt if nxt and self.allowed_host(nxt) else "" + return items[(page - 1) * PAGE_SIZE: page * PAGE_SIZE] def _entries(self, root: ET.Element, base: str) -> Iterable[SourceItem]: for e in root.findall(f"{_ATOM}entry"): @@ -334,5 +367,9 @@ def build_providers(catalogs: list[dict] | None, session: requests.Session | Non if parts.scheme not in ("http", "https") or not parts.hostname or parts.username or parts.password: continue key = f"opds-{n}" - providers[key] = OpdsProvider(key, _clip((cat or {}).get("name"), 80) or parts.hostname, url, session) + env = str((cat or {}).get("password_env") or "").strip() + if env and not re.fullmatch(r"[A-Z_][A-Z0-9_]{0,63}", env): + continue # not a plausible variable name: refuse rather than read an arbitrary one + providers[key] = OpdsProvider(key, _clip((cat or {}).get("name"), 80) or parts.hostname, url, session, + username=_clip((cat or {}).get("username"), 120), password_env=env) return providers diff --git a/routes/magazine_sources.py b/routes/magazine_sources.py index 75fce13..5bdfb11 100644 --- a/routes/magazine_sources.py +++ b/routes/magazine_sources.py @@ -15,7 +15,7 @@ import uuid from fastapi import APIRouter, HTTPException, Request from core import magazine_sources as ms -from core.config import _load_settings +from core.config import _load_settings, _save_settings from core.constants import CONFIG_DIR from core.database import background_job_create, background_job_update from routes.magazine import _write_meta @@ -44,6 +44,62 @@ async def magazine_sources_list(): for p in _providers().values()]} +_ENV_NAME = re.compile(r"[A-Z_][A-Z0-9_]{0,63}") +MAX_CATALOGUES = 10 + + +def _clean_catalogues(raw) -> list[dict]: + """Validate a catalogue list from the browser. Anything doubtful is refused, never silently repaired.""" + from urllib.parse import urlsplit + if not isinstance(raw, list) or len(raw) > MAX_CATALOGUES: + raise HTTPException(400, f"Send a list of at most {MAX_CATALOGUES} catalogues") + out, seen = [], set() + for item in raw: + if not isinstance(item, dict): + raise HTTPException(400, "Each catalogue must be an object") + name = re.sub(r"\s+", " ", str(item.get("name") or "")).strip()[:80] + url = str(item.get("url") or "").strip() + parts = urlsplit(url) + if parts.scheme not in ("http", "https") or not parts.hostname: + raise HTTPException(400, f"\u201c{name or url[:40]}\u201d: the address must start with http:// or https://") + if parts.username or parts.password: + raise HTTPException(400, "Put the login in the username field, not in the address") + if len(url) > 500: + raise HTTPException(400, "The address is too long") + username = str(item.get("username") or "").strip()[:120] + env = str(item.get("password_env") or "").strip() + if env and not _ENV_NAME.fullmatch(env): + raise HTTPException(400, "The password variable name may only use capital letters, digits and underscores") + if bool(username) != bool(env): + raise HTTPException(400, "A login needs both a username and the name of the password variable") + if url in seen: + raise HTTPException(400, "That catalogue is already listed") + seen.add(url) + out.append({"name": name or parts.hostname, "url": url, "username": username, "password_env": env}) + return out + + +@router.get("/api/magazine-sources/catalogues") +async def magazine_catalogues_get(): + import os + cats = _load_settings().get("magazine_opds_catalogs") or [] + return {"catalogues": [ + {"name": str(c.get("name") or ""), "url": str(c.get("url") or ""), "username": str(c.get("username") or ""), + "password_env": str(c.get("password_env") or ""), + "password_set": bool(c.get("password_env") and os.environ.get(str(c.get("password_env")))) } + for c in cats if isinstance(c, dict)]} + + +@router.put("/api/magazine-sources/catalogues") +async def magazine_catalogues_put(request: Request): + body = await request.json() + cats = _clean_catalogues(body.get("catalogues") if isinstance(body, dict) else None) + settings = _load_settings() + settings["magazine_opds_catalogs"] = cats + _save_settings(settings) + return {"ok": True, "count": len(cats)} + + @router.get("/api/magazine-sources/{key}/search") async def magazine_sources_search(key: str, q: str = "", page: int = 1): provider = _provider(key) diff --git a/static/js/magazine.js b/static/js/magazine.js index 027fe23..28edfdf 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -182,6 +182,7 @@ async function magLoadLibrary() { state.docs = (data.docs || []).filter((d) => d.kind === 'magazine'); renderLibrary(); magFindLoadSources(); + magCatLoad(); } function renderLibrary() { @@ -475,6 +476,54 @@ async function magFindLoadSources() { } } +// ── Your OPDS catalogues ───────────────────────────────────────────────── + +async function magCatLoad() { + const list = $('mag-cat-list'); + if (!list) return; + try { + const { catalogues } = await api('/api/magazine-sources/catalogues'); + state.catalogues = catalogues || []; + } catch (e) { + const st = $('mag-cat-status'); + if (st) st.textContent = 'Could not load your catalogues: ' + e.message; + return; + } + list.innerHTML = state.catalogues.length ? state.catalogues.map((c, i) => + '
  • ' + esc(c.name) + ' ' + esc(c.url) + '' + + (c.username ? ' · login ' + esc(c.username) + (c.password_set ? '' : ' — password variable ' + esc(c.password_env) + ' is not set on the server') + '' : '') + + '
    • ').join('') + : '
  • None yet. The Internet Archive is always available.
    • '; +} + +async function magCatSave(next, doneText) { + const st = $('mag-cat-status'); + try { + await api('/api/magazine-sources/catalogues', { method: 'PUT', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ catalogues: next.map((c) => ({ name: c.name, url: c.url, username: c.username, password_env: c.password_env })) }) }); + } catch (e) { + if (st) st.textContent = 'Not saved: ' + e.message; + return false; + } + if (st) st.textContent = doneText; + await magCatLoad(); + const sel = $('mag-find-source'); + if (sel) { delete sel.dataset.loaded; await magFindLoadSources(); } + return true; +} + +async function magCatAdd(ev) { + if (ev) ev.preventDefault(); + const entry = { name: $('mag-cat-name').value.trim(), url: $('mag-cat-url').value.trim(), + username: $('mag-cat-user').value.trim(), password_env: $('mag-cat-env').value.trim() }; + if (await magCatSave([...(state.catalogues || []), entry], 'Catalogue added.')) $('mag-cat-form').reset(); +} + +async function magCatRemove(i) { + const next = (state.catalogues || []).filter((_, k) => k !== i); + await magCatSave(next, 'Catalogue removed.'); +} + async function magFindSearch(ev) { if (ev) ev.preventDefault(); const q = ($('mag-find-query')?.value || '').trim(); @@ -5629,6 +5678,8 @@ root.magPageGuide = magPageGuide; root.magArticleFlow = magArticleFlow; root.magTogglePagesTodo = magTogglePagesTodo; root.magFindSearch = magFindSearch; +root.magCatAdd = magCatAdd; +root.magCatRemove = magCatRemove; root.magFindImport = magFindImport; root.magTocPlay = magTocPlay; root.magSeek = magSeek; diff --git a/static/sections/s-magazine.html b/static/sections/s-magazine.html index 2757820..21e1ff8 100644 --- a/static/sections/s-magazine.html +++ b/static/sections/s-magazine.html @@ -58,6 +58,19 @@ +
    + Your catalogues (OPDS) +

    Add your own library (Calibre-Web, Kavita, Komga) or any OPDS address. If it needs a login, enter the username and the name of the environment variable that holds the password — the password itself is never stored here.

    +
      +
      + + + + + +
      +

      +

        diff --git a/static/style.css b/static/style.css index ba3066d..b20240a 100644 --- a/static/style.css +++ b/static/style.css @@ -5617,3 +5617,11 @@ details.mag-report-group summary { cursor: pointer; margin-bottom: 6px; } .mag-page-done { color: var(--subtext); } .mag-page-note { color: var(--subtext); font-size: 12px; } .mag-card.needs-you { box-shadow: 0 0 0 2px var(--accent); } + +.mag-cat { margin: 10px 0; } +.mag-cat > summary { cursor: pointer; font-weight: 600; } +.mag-cat-list { list-style: none; margin: 8px 0; padding: 0; display: grid; gap: 6px; } +.mag-cat-list li { display: flex; gap: 10px; align-items: center; justify-content: space-between; padding: 6px 10px; + border: 1px solid var(--border); border-radius: var(--radius); overflow-wrap: anywhere; } +.mag-cat-form { display: grid; grid-template-columns: repeat(auto-fit, minmax(200px, 1fr)); gap: 10px; align-items: end; } +.mag-cat-form label { display: flex; flex-direction: column; gap: 4px; font-size: 13px; } diff --git a/tests/magazine.spec.js b/tests/magazine.spec.js index 8cdd1b4..d40c458 100644 --- a/tests/magazine.spec.js +++ b/tests/magazine.spec.js @@ -1806,3 +1806,54 @@ test.describe('Page cards say what to do', () => { expect(sizes.swatch).toBeGreaterThanOrEqual(18); }); }); + +test('catalogues can be added and removed, and the source list follows', async ({ page }) => { + await mockBackend(page); + let cats = []; + const puts = []; + await page.route('**/api/magazine-sources/catalogues', route => { + if (route.request().method() === 'PUT') { + cats = route.request().postDataJSON().catalogues.map(c => ({ ...c, password_set: !!c.password_env })); + puts.push(route.request().postDataJSON()); + return route.fulfill({ json: { ok: true, count: cats.length } }); + } + return route.fulfill({ json: { catalogues: cats } }); + }); + await page.route('**/api/magazine-sources', route => route.fulfill({ json: { sources: [ + { key: 'internet-archive', label: 'Internet Archive', kind: 'archive' }, + ...cats.map((c, i) => ({ key: `opds-${i + 1}`, label: c.name, kind: 'opds' }))] } })); + await page.goto('/', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#app-skeleton', { state: 'detached' }); + await navTo(page, 's-magazine'); + await page.locator('#mag-cat summary').click(); + await expect(page.locator('#mag-cat-list')).toContainText('None yet'); + await page.locator('#mag-cat-name').fill('My NAS'); + await page.locator('#mag-cat-url').fill('http://nas.local:8083/opds/'); + await page.locator('#mag-cat-user').fill('me'); + await page.locator('#mag-cat-env').fill('NAS_PW'); + await page.getByRole('button', { name: /Add catalogue/ }).click(); + await expect(page.locator('#mag-cat-status')).toHaveText('Catalogue added.'); + await expect(page.locator('#mag-cat-list')).toContainText('My NAS'); + // The form never carries a password, only the name of the variable that holds it. + expect(JSON.stringify(puts[0])).not.toMatch(/"password"/); + expect(puts[0].catalogues[0]).toEqual({ name: 'My NAS', url: 'http://nas.local:8083/opds/', username: 'me', password_env: 'NAS_PW' }); + await expect(page.locator('#mag-find-source option')).toHaveText(['Internet Archive', 'My NAS']); + await page.getByRole('button', { name: 'Remove My NAS' }).click(); + await expect(page.locator('#mag-cat-status')).toHaveText('Catalogue removed.'); + await expect(page.locator('#mag-find-source option')).toHaveCount(1); +}); + +test('a refused catalogue shows the server reason and keeps the form', async ({ page }) => { + await mockBackend(page); + await page.route('**/api/magazine-sources/catalogues', route => route.request().method() === 'PUT' + ? route.fulfill({ status: 400, json: { detail: 'Put the login in the username field, not in the address' } }) + : route.fulfill({ json: { catalogues: [] } })); + await page.goto('/', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#app-skeleton', { state: 'detached' }); + await navTo(page, 's-magazine'); + await page.locator('#mag-cat summary').click(); + await page.locator('#mag-cat-url').fill('http://me:pw@nas.local/opds'); + await page.getByRole('button', { name: /Add catalogue/ }).click(); + await expect(page.locator('#mag-cat-status')).toContainText('Put the login in the username field'); + await expect(page.locator('#mag-cat-url')).toHaveValue('http://me:pw@nas.local/opds'); +}); diff --git a/tests/test_magazine_sources.py b/tests/test_magazine_sources.py index 5afb389..ba7af39 100644 --- a/tests/test_magazine_sources.py +++ b/tests/test_magazine_sources.py @@ -37,6 +37,7 @@ class FakeSession: def get(self, url, **kw): self.calls.append(url) + self.kwargs = getattr(self, "kwargs", []) + [kw] for prefix, resp in self.routes.items(): if url.startswith(prefix): return resp() if callable(resp) else resp @@ -87,6 +88,116 @@ class OpdsTests(unittest.TestCase): self.provider({}).search(" ") +def feed(entries, next_href=None): + body = '' + for n in entries: + body += (f'Issue {n}') + if next_href: + body += f'' + return FakeResp(body=(body + "").encode()) + + +ROOT = FakeResp(body=b'') + + +class OpdsPagingTests(unittest.TestCase): + def provider(self, routes, **kw): + routes = {**routes, "http://nas.local/opds/": ROOT} + self.session = FakeSession(routes) + return ms.OpdsProvider("opds-1", "NAS", "http://nas.local/opds/", self.session, **kw) + + def test_a_second_results_page_follows_the_catalogues_next_link(self): + # Most specific route first: the fake session matches by prefix. + p = self.provider({"http://nas.local/opds/search?page=2": feed(range(16, 31)), + "http://nas.local/opds/search": feed(range(1, 16), "/opds/search?page=2")}) + self.assertEqual(p.search("x", 1)[0].title, "Issue 1") + page2 = p.search("x", 2) + self.assertEqual([i.title for i in page2], [f"Issue {n}" for n in range(21, 31)]) + + def test_a_next_link_to_another_host_is_not_followed(self): + p = self.provider({"http://nas.local/opds/search": feed([1, 2], "http://evil.example/opds?page=2")}) + self.assertEqual(len(p.search("x", 2)), 0) + self.assertFalse(any("evil.example" in c for c in self.session.calls)) + + def test_a_next_link_loop_ends(self): + p = self.provider({"http://nas.local/opds/search": feed([1], "/opds/search")}) + self.assertEqual(len(p.search("x", 3)), 0) + self.assertLessEqual(len(self.session.calls), 3) + + def test_paging_is_bounded(self): + routes = {f"http://nas.local/opds/search?p={n}": feed([n], f"/opds/search?p={n + 1}") for n in range(1, 50)} + routes["http://nas.local/opds/search"] = feed([0], "/opds/search?p=1") + p = self.provider(routes) + p.search("x", 9) + self.assertLessEqual(len(self.session.calls), 1 + ms.OpdsProvider.MAX_FEED_PAGES) + + +class OpdsLoginTests(unittest.TestCase): + def setUp(self): + import os + self.os = os + os.environ["TEST_OPDS_SECRET"] = "s3cret-value" + self.addCleanup(lambda: os.environ.pop("TEST_OPDS_SECRET", None)) + + def provider(self, routes, **kw): + self.session = FakeSession({**routes, "http://nas.local/opds/": ROOT}) + return ms.OpdsProvider("opds-1", "NAS", "http://nas.local/opds/", self.session, **kw) + + def test_credentials_come_from_the_named_environment_variable_and_go_to_the_catalogue(self): + p = self.provider({"http://nas.local/opds/search": feed([1])}, username="me", password_env="TEST_OPDS_SECRET") + self.assertEqual(len(p.search("x")), 1) + self.assertTrue(all(kw.get("auth") == ("me", "s3cret-value") for kw in self.session.kwargs)) + + def test_an_open_catalogue_sends_no_auth(self): + p = self.provider({"http://nas.local/opds/search": feed([1])}) + p.search("x") + self.assertTrue(all("auth" not in kw for kw in self.session.kwargs)) + + def test_a_missing_password_is_a_clear_error_that_never_shows_a_secret(self): + p = self.provider({}, username="me", password_env="TEST_OPDS_UNSET") + with self.assertRaises(ms.SourceError) as ctx: + p.search("x") + self.assertIn("TEST_OPDS_UNSET", str(ctx.exception)) + self.assertNotIn("s3cret", str(ctx.exception)) + + def test_downloads_are_authenticated_too(self): + import tempfile + from pathlib import Path + p = self.provider({"http://nas.local/dl": FakeResp(body=b"%PDF-1.7 x")}, username="me", password_env="TEST_OPDS_SECRET") + ms.download_pdf(p, "http://nas.local/dl/1.pdf", Path(tempfile.mkdtemp()) / "source.pdf", session=self.session) + self.assertEqual(self.session.kwargs[-1].get("auth"), ("me", "s3cret-value")) + + def test_auth_is_not_sent_when_a_redirect_leaves_the_host(self): + import tempfile + from pathlib import Path + p = self.provider({"http://nas.local/dl": FakeResp(status=302, headers={"location": "http://evil.example/x"})}, + username="me", password_env="TEST_OPDS_SECRET") + with self.assertRaises(ms.SourceError): + ms.download_pdf(p, "http://nas.local/dl/1.pdf", Path(tempfile.mkdtemp()) / "source.pdf", session=self.session) + self.assertFalse(any("evil.example" in c for c in self.session.calls)) + + def test_settings_with_an_implausible_variable_name_are_ignored(self): + provs = ms.build_providers([ + {"name": "ok", "url": "http://nas.local/opds", "username": "me", "password_env": "NAS_PASSWORD"}, + {"name": "bad", "url": "http://nas2.local/opds", "username": "me", "password_env": "x; rm -rf /"}, + {"name": "lower", "url": "http://nas3.local/opds", "username": "me", "password_env": "lowercase"}]) + self.assertEqual(sorted(provs), ["internet-archive", "opds-1"]) + + def test_the_source_list_never_exposes_credentials(self): + from unittest import mock + from fastapi import FastAPI + from fastapi.testclient import TestClient + from routes import magazine_sources as routes + app = FastAPI(); app.include_router(routes.router) + cats = [{"name": "NAS", "url": "http://nas.local/opds", "username": "me", "password_env": "TEST_OPDS_SECRET"}] + with mock.patch.object(routes, "_load_settings", return_value={"magazine_opds_catalogs": cats}): + body = TestClient(app).get("/api/magazine-sources").text + self.assertNotIn("s3cret", body) + self.assertNotIn("TEST_OPDS_SECRET", body) + self.assertNotIn("me\"", body.replace("name\"", "")) + + class ArchiveTests(unittest.TestCase): def test_search_builds_items_and_skips_bad_identifiers(self): docs = [{"identifier": "ct-2026-01", "title": "c't", "creator": ["Heise"], "date": "2026-01-01T00:00:00Z"}, @@ -209,3 +320,61 @@ class RouteTests(unittest.TestCase): with self.mock.patch.object(self.routes, "_load_settings", return_value={}): r = self.client.post("/api/magazine-sources/import", json={"source": "internet-archive"}) self.assertEqual(r.status_code, 400) + + +class CatalogueEndpointTests(unittest.TestCase): + def setUp(self): + from unittest import mock + from fastapi import FastAPI + from fastapi.testclient import TestClient + from routes import magazine_sources as routes + self.store = {"magazine_opds_catalogs": []} + for name, fn in (("_load_settings", lambda: dict(self.store)), ("_save_settings", lambda s: self.store.update(s))): + p = mock.patch.object(routes, name, side_effect=fn) + p.start(); self.addCleanup(p.stop) + app = FastAPI(); app.include_router(routes.router) + self.client = TestClient(app) + + def put(self, cats): + return self.client.put("/api/magazine-sources/catalogues", json={"catalogues": cats}) + + def test_a_valid_list_is_saved_and_read_back(self): + r = self.put([{"name": " My NAS ", "url": "http://nas.local:8083/opds/", "username": "me", "password_env": "NAS_PW"}, + {"name": "", "url": "https://books.example/opds"}]) + self.assertEqual(r.status_code, 200, r.text) + got = self.client.get("/api/magazine-sources/catalogues").json()["catalogues"] + self.assertEqual([c["name"] for c in got], ["My NAS", "books.example"]) + self.assertEqual(got[0]["password_env"], "NAS_PW") + self.assertFalse(got[0]["password_set"]) # the variable is not set in this environment + + def test_credentials_in_the_address_are_refused(self): + self.assertEqual(self.put([{"url": "http://me:pw@nas.local/opds"}]).status_code, 400) + + def test_other_schemes_and_missing_hosts_are_refused(self): + for bad in ("ftp://nas/opds", "javascript:alert(1)", "file:///etc/passwd", "http://", "nas.local/opds", ""): + self.assertEqual(self.put([{"url": bad}]).status_code, 400, bad) + + def test_a_login_needs_both_halves_and_a_plausible_variable_name(self): + self.assertEqual(self.put([{"url": "http://a.local/o", "username": "me"}]).status_code, 400) + self.assertEqual(self.put([{"url": "http://a.local/o", "password_env": "PW"}]).status_code, 400) + self.assertEqual(self.put([{"url": "http://a.local/o", "username": "me", "password_env": "lower; rm"}]).status_code, 400) + + def test_duplicates_too_many_and_wrong_shapes_are_refused(self): + self.assertEqual(self.put([{"url": "http://a.local/o"}, {"url": "http://a.local/o"}]).status_code, 400) + self.assertEqual(self.put([{"url": f"http://h{n}.local/o"} for n in range(11)]).status_code, 400) + self.assertEqual(self.put("nope").status_code, 400) + self.assertEqual(self.put(["nope"]).status_code, 400) + + def test_a_refused_list_changes_nothing(self): + self.put([{"url": "http://ok.local/o"}]) + self.put([{"url": "http://ok.local/o"}, {"url": "ftp://bad"}]) + self.assertEqual([c["url"] for c in self.store["magazine_opds_catalogs"]], ["http://ok.local/o"]) + + def test_the_password_itself_is_never_returned(self): + import os + os.environ["CAT_TEST_PW"] = "topsecret" + self.addCleanup(lambda: os.environ.pop("CAT_TEST_PW", None)) + self.put([{"url": "http://a.local/o", "username": "me", "password_env": "CAT_TEST_PW"}]) + r = self.client.get("/api/magazine-sources/catalogues") + self.assertNotIn("topsecret", r.text) + self.assertTrue(r.json()["catalogues"][0]["password_set"])