diff --git a/CHANGELOG.md b/CHANGELOG.md index 5598d6b..9b15061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ matters, what was wrong before -- the reasoning is the point, not the diff. See [README.md](README.md) for what the thing is, and [docs/](docs/) for how to run it. +## 2026-09-11 — OPML import and export are per person + +Importing an OPML now subscribes you to every feed in it. Feeds already in the catalogue cost +nothing, and unknown ones are added under the OPML's title. Before this, import only added URLs +missing from `config.toml` and subscribed nobody. So importing an export from another account did +nothing at all, and a genuinely new feed had no subscriber and was never scanned. The page said +"Imported 0 feed(s)". `ipx import` had the same gap; it now subscribes the first admin. Both go +through `subscribe_opml`. + +Export now lists only your own subscriptions. It used to write out the whole catalogue to anyone +signed in, including other people's feeds and any private URLs in them. + +--- + ## 2026-09-11 — The log is admin-only `GET /api/logs` now returns `403` to anyone who is not an admin, and the page hides the Log button diff --git a/docs/architecture.md b/docs/architecture.md index 2441f5b..f19c531 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -108,7 +108,8 @@ else a `401`. | `POST /api/feeds/{id}/read-all`, `POST /api/feeds/{id}/download-latest` | | | `POST /api/entries/{feed}/{guid}/flags`, `…/position` | your read, starred, position | | `POST /api/enclosures/{id}/download`, `DELETE /api/enclosures/{id}` | `?force=true` overrides the shared-file warning | -| `POST /api/fetch`, `GET /api/opml`, `POST /api/opml` | | +| `POST /api/fetch` | | +| `GET /api/opml`, `POST /api/opml` | export your subscriptions; subscribe to every feed in an OPML | | `GET /api/settings`, `PATCH /api/settings` | admin-only to write | | `GET /api/users`, `POST /api/users`, `PATCH /api/users/{id}`, `DELETE /api/users/{id}` | admin-only; the only admin cannot be demoted or removed | | `GET /api/events` | SSE, the same broadcast the socket carries | diff --git a/docs/cli.md b/docs/cli.md index b2e3499..f7e85ac 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -15,7 +15,7 @@ processes must never download the same thing. `--local` forces the work to happe | `ipx fetch [FEED] [--force]` | Scan everything, or one feed. `--force` ignores the TTL | | `ipx add [--folder X] [--keywords a,b]` | Subscribe; the id comes from the feed title | | `ipx rm ` | Unsubscribe; downloads and history are kept | -| `ipx import ` / `ipx export ` | Move subscriptions in or out | +| `ipx import ` / `ipx export ` | Move subscriptions in or out. Import subscribes the first admin, as the shared web token does; in the web UI it subscribes whoever is signed in | | `ipx reap [--dry-run]` | Run retention now | | `ipx user ` | Accounts for the web UI | | `ipx daemon [--web ADDR]` | Scheduler, control socket and web UI | diff --git a/src/main.rs b/src/main.rs index 580ae9f..8a40df7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -602,44 +602,91 @@ fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> { } async fn import(ctx: &Ctx, config_path: &std::path::Path, file: &std::path::Path) -> Result<()> { - let mut cfg = (*ctx.cfg()).clone(); let text = std::fs::read_to_string(file) .with_context(|| format!("reading {}", file.display()))?; - let doc = opml::OPML::from_str(&text).map_err(|e| anyhow::anyhow!("parsing OPML: {e}"))?; + // The CLI speaks for the operator, as the shared web token does. + let admin = ctx + .db + .users()? + .into_iter() + .find(|u| u.is_admin) + .ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add --admin"))?; + let (added, had) = subscribe_opml(ctx, config_path, &text, admin.id)?; + println!("subscribed {} to {added} feed(s); {had} already there", admin.name); + Ok(()) +} +/// Subscribes one person to every feed in an OPML document, for the CLI and the web alike. +/// A feed already in the catalogue costs nothing; an unknown one is added under the OPML's +/// title rather than refetching each. Returns (newly subscribed, already subscribed). +/// +/// Before accounts, importing only added unknown URLs to config.toml. Once subscriptions +/// decided what each person sees, that imported nothing at all for a feed someone else +/// already had, and a new one had no subscriber, so it was never scanned. +pub fn subscribe_opml( + ctx: &Ctx, + config_path: &std::path::Path, + xml: &str, + user_id: i64, +) -> Result<(usize, usize)> { + let doc = opml::OPML::from_str(xml) + .map_err(|e| anyhow::anyhow!("that does not parse as OPML: {e}"))?; let mut found = vec![]; collect_outlines(&doc.body.outlines, &mut found); - let mut added = 0; + let known = subscriptions(ctx)?; + let mut cfg = (*ctx.cfg()).clone(); + let mut ids = vec![]; + let mut grew = false; for (title, url) in found { - if cfg.feeds.values().any(|f| f.url == url) { - continue; - } - // Name it from the OPML title rather than refetching every feed. - let id = config::unique_slug(&title, &cfg.feeds); - cfg.feeds.insert( - id.clone(), - config::Feed { - url, - folder: None, - group: None, - media_types: None, - schedule: None, - keywords: vec![], - allow_explicit: false, - auto_download: true, - max_new_per_check: None, - username: None, - password: None, - password_env: None, - }, - ); - println!("added {id}"); - added += 1; + let existing = known + .iter() + .find(|s| s.cfg.url == url) + .map(|s| s.id.clone()) + // The same URL listed twice in one file. + .or_else(|| cfg.feeds.iter().find(|(_, f)| f.url == url).map(|(id, _)| id.clone())); + let id = match existing { + Some(id) => id, + None => { + let id = config::unique_slug(&title, &cfg.feeds); + cfg.feeds.insert( + id.clone(), + config::Feed { + url, + folder: None, + group: None, + media_types: None, + schedule: None, + keywords: vec![], + allow_explicit: false, + auto_download: true, + max_new_per_check: None, + username: None, + password: None, + password_env: None, + }, + ); + grew = true; + id + } + }; + ids.push(id); } - cfg.save(config_path)?; - println!("{added} feed(s) imported"); - Ok(()) + if grew { + cfg.save(config_path)?; + ctx.reload_cfg(config_path)?; + } + + let (mut added, mut had) = (0, 0); + for id in ids { + if ctx.db.subscription(user_id, &id)?.is_some() { + had += 1; + } else { + ctx.db.subscribe(user_id, &id)?; + added += 1; + } + } + Ok((added, had)) } /// OPML nests feeds inside folder outlines, so this walks the whole tree. diff --git a/src/web.rs b/src/web.rs index dfde3ed..04258a2 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1119,8 +1119,14 @@ async fn download_latest( } /// Subscriptions as OPML, so they can move to another podcast app. -async fn export_opml(State(state): State) -> Result { - let cfg = state.ctx.cfg(); +async fn export_opml( + State(state): State, + user: crate::db::User, +) -> Result { + // Yours, not the whole catalogue: other people's feeds, and any private URLs in them, are + // not yours to download. This used to export config.toml to whoever asked. + let mine: std::collections::HashSet = + state.ctx.db.subscriptions_for(user.id)?.into_iter().map(|s| s.feed_id).collect(); let mut doc = opml::OPML { head: Some(opml::Head { title: Some("ipx subscriptions".into()), @@ -1128,15 +1134,19 @@ async fn export_opml(State(state): State) -> Result, + user: crate::db::User, Json(body): Json, ) -> Result, ApiError> { - let doc = opml::OPML::from_str(&body.xml) - .map_err(|e| anyhow::anyhow!("that does not parse as OPML: {e}"))?; - let mut found = vec![]; - crate::collect_outlines(&doc.body.outlines, &mut found); - - let mut cfg = (*state.ctx.cfg()).clone(); - let mut added = 0; - for (title, url) in found { - if cfg.feeds.values().any(|f| f.url == url) { - continue; - } - let id = crate::config::unique_slug(&title, &cfg.feeds); - cfg.feeds.insert( - id, - crate::config::Feed { - url, - folder: None, - group: None, - media_types: None, - schedule: None, - keywords: vec![], - allow_explicit: false, - auto_download: true, - max_new_per_check: None, - username: None, - password: None, - password_env: None, - }, - ); - added += 1; - } - cfg.save(&state.config_path)?; - state.ctx.reload_cfg(&state.config_path)?; - Ok(Json(serde_json::json!({ "added": added }))) + let (added, already) = + crate::subscribe_opml(&state.ctx, &state.config_path, &body.xml, user.id)?; + Ok(Json(serde_json::json!({ "added": added, "already": already }))) } #[derive(Serialize)] diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 1656856..7cf5132 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -412,3 +412,43 @@ test('the only admin cannot be demoted or removed', async ({ page }) => { const me = (await (await page.request.get('/api/users')).json()).find(u => u.name === 'admin'); expect((await page.request.delete(`/api/users/${me.id}`)).status()).toBe(400); }); + +test('importing an OPML subscribes you, and export lists only your feeds', async ({ browser }) => { + // Regression: import only added URLs the catalogue lacked and subscribed nobody, so a feed + // someone else already had imported as nothing at all. + const { execFileSync } = require('child_process'); + const setup = require('./global-setup'); + const env = { + ...process.env, + IPX_CONFIG: `${setup.root}/config/config.toml`, + IPX_DATA_DIR: `${setup.root}/data`, + }; + try { + execFileSync('./target/debug/ipx', ['user', 'add', 'opal'], { input: 'opalpassword', env }); + } catch (e) { + if (!String(e.stderr || e.stdout).includes('already exists')) throw e; + } + const ctx = await browser.newContext(); + const page = await ctx.newPage(); + await page.goto('/login'); + await page.locator('#name').fill('opal'); + await page.locator('#pw').fill('opalpassword'); + await page.locator('button[type=submit]').click(); + await expect(page.locator('#feedlist')).toContainText('No feeds.'); + + // Test Show is already in the catalogue, because the admin reads it. + const xml = 't' + + ''; + expect(await (await page.request.post('/api/opml', { data: { xml } })).json()) + .toEqual({ added: 1, already: 0 }); + expect(await (await page.request.post('/api/opml', { data: { xml } })).json()) + .toEqual({ added: 0, already: 1 }); + await page.reload(); + await expect(page.locator('.feed', { hasText: 'Test Show' })).toBeVisible({ timeout: 20_000 }); + + // The admin also reads Picture Blog; that is not opal's to export. + const out = await (await page.request.get('/api/opml')).text(); + expect(out).toContain('show.xml'); + expect(out).not.toContain('pics.xml'); + await ctx.close(); +}); diff --git a/web/index.html b/web/index.html index 490c8bd..614acb4 100644 --- a/web/index.html +++ b/web/index.html @@ -1397,7 +1397,7 @@ function opmlModal(){ $('#oimp').onclick=async()=>{ try{ const r=await api('/api/opml',{method:'POST',body:JSON.stringify({xml:$('#opmlText').value})}); - closeModal(); toast(`Imported ${r.added} feed(s)`); loadFeeds(true); + closeModal(); toast(`Subscribed to ${r.added} feed(s)`+(r.already?`, ${r.already} you already had`:'')); loadFeeds(true); }catch(e){ toast(e.message,true); } }; }