From 2ba83c3aed521141819917774c8aa266e4e9c173 Mon Sep 17 00:00:00 2001 From: rays Date: Sat, 12 Sep 2026 13:37:09 +0000 Subject: [PATCH] Keep when each account was added and when it last signed in users.created comes back, beside a new last_login, for whoever maintains the server. A password sign-in, the token link and a request through the proxy all count, recorded to the hour so the proxy's per-request vouching is not a write each time. Settings -> Users and ipx user list show both. The three user queries now share one row mapping. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TAC7sLVqfKmY6rsTLXzNgk --- CHANGELOG.md | 7 ++- docs/architecture.md | 11 ++-- docs/history.md | 6 ++ src/db.rs | 137 +++++++++++++++++++++++++++++-------------- src/main.rs | 9 ++- src/web.rs | 20 ++++++- tests/ui/app.spec.js | 3 + web/index.html | 4 +- 8 files changed, 143 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd65b3e..a090fd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,16 @@ The long form, with what was wrong before and how it was found, is in ## [Unreleased] +### Added + +- Settings → Users and `ipx user list` show when each account was added and when it last signed + in, to the hour. + ### Changed - Directory and Popular list the feeds inside an OPML one by one, and no longer the OPML itself, so you can subscribe to just the shows you want. -- The database no longer records when accounts, subscriptions and sign-ins were created. Nothing +- The database no longer records when subscriptions and sign-in sessions were created. Nothing ever read it, and an existing database drops the columns on its next start. ### Fixed diff --git a/docs/architecture.md b/docs/architecture.md index 885cbd2..48d8966 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -50,7 +50,7 @@ entries feed_id, guid, title, link, published, description, first_seen, image, duration, episode, season PK (feed_id, guid) enclosures id, feed_id, guid, url UNIQUE, mime, length, path, state, bytes_done, downloaded_at, last_error -users id, name, pass_hash, is_admin +users id, name, pass_hash, is_admin, created, last_login sessions token, user_id, seen subscriptions user_id, feed_id, keywords, auto_download, allow_explicit, max_new_per_check PK (user_id, feed_id) @@ -63,11 +63,10 @@ before accounts; two bugs came from queries still reading them, and `migrate()` older database. Schema changes: add the table or column to `SCHEMA`. `CREATE TABLE IF NOT EXISTS` leaves a table -that already exists alone, so a new column on one also needs an `ALTER TABLE ADD COLUMN` in -`migrate()`, checked with `PRAGMA table_info` the way its `retired` list is for drops. None is -needed today: every column added so far predates 0.3.0, the oldest version an upgrade may start -from. `Db::memory()` runs the same path as `Db::open`, so a migration cannot pass the tests while -missing in production. +that already exists alone, so a new column on one also goes in `migrate()`'s `wanted` list, and a +retired one in its `retired` list; both are checked with `PRAGMA table_info`. Columns from before +0.3.0, the oldest version an upgrade may start from, need no entry. `Db::memory()` runs the same +path as `Db::open`, so a migration cannot pass the tests while missing in production. ## Control socket diff --git a/docs/history.md b/docs/history.md index 7d8e070..07e80d9 100644 --- a/docs/history.md +++ b/docs/history.md @@ -21,6 +21,12 @@ column drops earlier the same day, and one stray `entry_state` row. The code had - `Db::subscribed_feed_ids` had no callers, though its doc said the scanner walked it. `Db::subscriber_count` had one caller asking whether it was above zero, which `subscriber_counts().contains_key` answers. `Managed.orphaned` was selected and never read. +- `users.created` came back the same afternoon, with `last_login` beside it. Nothing read it, but + when an account was made and when it last signed in is what you want to know when tidying + accounts, and it cannot be recovered later. Both existing accounts got their creation times back + from the backup taken before the drop, and a last sign-in from their newest session in it. + `last_login` is kept to the hour, because the proxy vouches for every request and that would + otherwise be a write each time. ## 2026-09-12 — Cutting what had outlived its reason diff --git a/src/db.rs b/src/db.rs index a0cf7d0..06d33fe 100644 --- a/src/db.rs +++ b/src/db.rs @@ -72,7 +72,10 @@ CREATE TABLE IF NOT EXISTS users ( id INTEGER PRIMARY KEY, name TEXT NOT NULL UNIQUE COLLATE NOCASE, pass_hash TEXT, - is_admin INTEGER NOT NULL DEFAULT 0 + is_admin INTEGER NOT NULL DEFAULT 0, + -- For whoever maintains the server. NULL where it is not known. + created INTEGER, + last_login INTEGER ); -- What one person wants from a feed. The feed, its items and its files are shared; this @@ -124,6 +127,23 @@ pub struct User { pub name: String, pub pass_hash: Option, pub is_admin: bool, + /// When the account was made and when it last signed in, for whoever maintains the server. + pub created: Option, + pub last_login: Option, +} + +/// The columns `user_row` reads, in its order. +const USER_COLS: &str = "id, name, pass_hash, is_admin, created, last_login"; + +fn user_row(r: &rusqlite::Row<'_>) -> rusqlite::Result { + Ok(User { + id: r.get(0)?, + name: r.get(1)?, + pass_hash: r.get(2)?, + is_admin: r.get::<_, i64>(3)? != 0, + created: r.get(4)?, + last_login: r.get(5)?, + }) } /// A feed derived from an OPML subscription rather than written into the config. @@ -135,11 +155,17 @@ pub struct Managed { pub group_id: String, } -/// Drops the columns later versions retired. CREATE TABLE IF NOT EXISTS leaves a table that -/// already exists alone, so an installed database needs this done explicitly. A new column on an -/// existing table would need an ALTER TABLE ADD COLUMN here too; none does yet, since every one -/// added so far predates 0.3.0, the oldest version an upgrade may start from. +/// Adds the columns later versions introduced and drops the ones they retired. CREATE TABLE IF +/// NOT EXISTS leaves a table that already exists alone, so an installed database needs both done +/// explicitly. Columns from before 0.3.0, the oldest version an upgrade may start from, need no +/// entry. fn migrate(conn: &Connection) -> Result<()> { + let wanted: &[(&str, &str, &str)] = &[ + // For whoever maintains the server. An audit dropped `created` as unread on 2026-09-12, + // and it came back the same day with `last_login` beside it. + ("users", "created", "INTEGER"), + ("users", "last_login", "INTEGER"), + ]; let retired: &[(&str, &str)] = &[ // Read state from before accounts, long since moved to entry_state. Two bugs came from // queries still reading these after they stopped meaning anything. @@ -147,16 +173,24 @@ fn migrate(conn: &Connection) -> Result<()> { ("entries", "flagged"), ("entries", "position"), // Written by every insert and read by nothing. - ("users", "created"), ("subscriptions", "created"), ("sessions", "created"), ]; - for (table, column) in retired { + let has = |table: &str, column: &str| -> Result { let names: Vec = conn .prepare(&format!("PRAGMA table_info({table})"))? .query_map([], |r| r.get(1))? .collect::>()?; - if names.iter().any(|c| c == column) { + Ok(names.iter().any(|c| c == column)) + }; + for (table, column, ty) in wanted { + if !has(table, column)? { + tracing::info!(table, column, "adding column"); + conn.execute_batch(&format!("ALTER TABLE {table} ADD COLUMN {column} {ty}"))?; + } + } + for (table, column) in retired { + if has(table, column)? { tracing::info!(table, column, "dropping column"); conn.execute_batch(&format!("ALTER TABLE {table} DROP COLUMN {column}"))?; } @@ -989,18 +1023,18 @@ impl Db { pub fn create_user(&self, name: &str, pass_hash: Option<&str>, admin: bool) -> Result { let conn = self.conn.lock().unwrap(); conn.execute( - "INSERT INTO users (name, pass_hash, is_admin) VALUES (?1, ?2, ?3)", - params![name, pass_hash, admin as i64], + "INSERT INTO users (name, pass_hash, is_admin, created) VALUES (?1, ?2, ?3, ?4)", + params![name, pass_hash, admin as i64, now()], )?; Ok(conn.last_insert_rowid()) } pub fn user_by_name(&self, name: &str) -> Result> { - self.one_user("SELECT id, name, pass_hash, is_admin FROM users WHERE name = ?1", name) + self.one_user(&format!("SELECT {USER_COLS} FROM users WHERE name = ?1"), name) } pub fn user_by_id(&self, id: i64) -> Result> { - self.one_user("SELECT id, name, pass_hash, is_admin FROM users WHERE id = ?1", id) + self.one_user(&format!("SELECT {USER_COLS} FROM users WHERE id = ?1"), id) } fn one_user(&self, sql: &str, key: P) -> Result> { @@ -1008,12 +1042,7 @@ impl Db { let mut stmt = conn.prepare(sql)?; let mut rows = stmt.query(params![key])?; Ok(match rows.next()? { - Some(r) => Some(User { - id: r.get(0)?, - name: r.get(1)?, - pass_hash: r.get(2)?, - is_admin: r.get::<_, i64>(3)? != 0, - }), + Some(r) => Some(user_row(r)?), None => None, }) } @@ -1021,16 +1050,9 @@ impl Db { pub fn users(&self) -> Result> { let conn = self.conn.lock().unwrap(); let mut stmt = - conn.prepare("SELECT id, name, pass_hash, is_admin FROM users ORDER BY name")?; + conn.prepare(&format!("SELECT {USER_COLS} FROM users ORDER BY name"))?; let out = stmt - .query_map([], |r| { - Ok(User { - id: r.get(0)?, - name: r.get(1)?, - pass_hash: r.get(2)?, - is_admin: r.get::<_, i64>(3)? != 0, - }) - })? + .query_map([], user_row)? .collect::>>()?; Ok(out) } @@ -1047,6 +1069,17 @@ impl Db { Ok(()) } + /// Records a sign-in, to the hour: the proxy vouches for every request, and writing each one + /// would buy nothing. + pub fn signed_in(&self, id: i64) -> Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE users SET last_login = ?2 WHERE id = ?1 AND coalesce(last_login, 0) <= ?2 - 3600", + params![id, now()], + )?; + Ok(()) + } + /// Sessions go with the user: a deleted account must not leave a usable cookie behind. pub fn delete_user(&self, id: i64) -> Result<()> { let conn = self.conn.lock().unwrap(); @@ -1070,18 +1103,13 @@ impl Db { let conn = self.conn.lock().unwrap(); let cutoff = now() - max_idle_secs; let mut stmt = conn.prepare( - "SELECT u.id, u.name, u.pass_hash, u.is_admin + "SELECT u.id, u.name, u.pass_hash, u.is_admin, u.created, u.last_login FROM sessions s JOIN users u ON u.id = s.user_id WHERE s.token = ?1 AND s.seen >= ?2", )?; let mut rows = stmt.query(params![token, cutoff])?; let found = match rows.next()? { - Some(r) => Some(User { - id: r.get(0)?, - name: r.get(1)?, - pass_hash: r.get(2)?, - is_admin: r.get::<_, i64>(3)? != 0, - }), + Some(r) => Some(user_row(r)?), None => None, }; drop(rows); @@ -1516,21 +1544,26 @@ mod tests { // The same order as open(): the schema leaves the old tables alone, migrate() fixes them. conn.execute_batch(SCHEMA).unwrap(); migrate(&conn).unwrap(); - for (table, gone) in [ - ("entries", &["read", "flagged", "position"][..]), - ("users", &["created"][..]), - ("subscriptions", &["created"][..]), - ("sessions", &["created"][..]), - ] { - let cols: Vec = conn - .prepare(&format!("PRAGMA table_info({table})")) + let cols = |table: &str| -> Vec { + conn.prepare(&format!("PRAGMA table_info({table})")) .unwrap() .query_map([], |r| r.get(1)) .unwrap() .collect::>() - .unwrap(); + .unwrap() + }; + for (table, gone) in [ + ("entries", &["read", "flagged", "position"][..]), + ("subscriptions", &["created"][..]), + ("sessions", &["created"][..]), + ] { + let cols = cols(table); assert!(!cols.iter().any(|c| gone.contains(&c.as_str())), "{table}: {cols:?}"); } + // users.created is not retired: it keeps what it held, and last_login joins it. + let users = cols("users"); + assert!(users.iter().any(|c| c == "last_login"), "{users:?}"); + assert_eq!(conn.query_row("SELECT created FROM users", [], |r| r.get::<_, i64>(0)).unwrap(), 0); // And the rows come through it. let kept: i64 = conn .query_row("SELECT count(*) FROM subscriptions JOIN sessions USING (user_id)", [], |r| r.get(0)) @@ -1538,6 +1571,24 @@ mod tests { assert_eq!(kept, 1); } + #[test] + fn an_account_knows_when_it_was_made_and_last_signed_in() { + let db = Db::memory().unwrap(); + let id = db.create_user("ray", None, true).unwrap(); + let get = || db.user_by_id(id).unwrap().unwrap(); + assert!(get().created.is_some_and(|t| t > 0)); + assert_eq!(get().last_login, None, "made, but never signed in"); + db.signed_in(id).unwrap(); + let first = get().last_login.unwrap(); + // Within the hour, the proxy vouching again writes nothing; after it, it does. + db.exec_for_test(&format!("UPDATE users SET last_login = {} WHERE id = {id}", first - 60)).unwrap(); + db.signed_in(id).unwrap(); + assert_eq!(get().last_login, Some(first - 60)); + db.exec_for_test(&format!("UPDATE users SET last_login = {} WHERE id = {id}", first - 7200)).unwrap(); + db.signed_in(id).unwrap(); + assert!(get().last_login.unwrap() >= first); + } + #[test] fn the_first_admin_starts_with_the_catalogue_and_only_once() { // Cutting this along with the dead read columns left the browser suite's admin with an diff --git a/src/main.rs b/src/main.rs index 7f9ece5..5713b95 100644 --- a/src/main.rs +++ b/src/main.rs @@ -273,11 +273,16 @@ fn user_cmd(ctx: &Arc, cmd: UserCmd) -> Result<()> { println!("no accounts yet: ipx user add "); } for u in users { + let added = u + .created + .and_then(|t| chrono::DateTime::from_timestamp(t, 0)) + .map_or("?".into(), |d| d.format("%Y-%m-%d").to_string()); + let seen = u.last_login.map_or("never signed in".into(), |t| format!("signed in {}", ago(Some(t)))); println!( - "{:<20} {:<8} {}", + "{:<20} {:<6} {:<11} added {added} {seen}", u.name, if u.is_admin { "admin" } else { "" }, - if u.pass_hash.is_some() { "password" } else { "proxy only" } + if u.pass_hash.is_some() { "password" } else { "proxy only" }, ); } Ok(()) diff --git a/src/web.rs b/src/web.rs index 2d13c3c..99f2102 100644 --- a/src/web.rs +++ b/src/web.rs @@ -133,6 +133,11 @@ async fn auth(State(state): State, mut req: Request, next: Next) -> Re None } }; + // Every request comes vouched for; signed_in keeps one an hour. Failing to note the time + // must not turn anyone away, so its error goes unanswered. + if let Some(u) = &user { + let _ = state.ctx.db.signed_in(u.id); + } } // 2. A session cookie from signing in here. @@ -157,6 +162,10 @@ async fn auth(State(state): State, mut req: Request, next: Next) -> Re if supplied.is_some_and(|t| constant_time_eq(&t, &token)) { user = admin_user(&state); if from_query.is_some() { + // The token link is a sign-in; the cookie it leaves behind is not one each time. + if let Some(u) = &user { + let _ = state.ctx.db.signed_in(u.id); + } set_cookie = Some(format!( "{COOKIE}={token}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000" )); @@ -253,6 +262,7 @@ async fn login( let user = user.expect("verified above"); let token = crate::auth::new_session_token(); state.ctx.db.create_session(user.id, &token)?; + state.ctx.db.signed_in(user.id)?; tracing::info!(user = %user.name, "signed in"); let days = state.ctx.cfg().web.session_days.max(1); @@ -317,6 +327,7 @@ async fn list_users( .map(|u| { serde_json::json!({ "id": u.id, "name": u.name, "admin": u.is_admin, "password": u.pass_hash.is_some(), + "created": u.created, "last_login": u.last_login, }) }) .collect(); @@ -748,7 +759,14 @@ mod tests { #[test] fn only_the_last_admin_is_protected() { - let u = |id, is_admin| crate::db::User { id, name: format!("u{id}"), pass_hash: None, is_admin }; + let u = |id, is_admin| crate::db::User { + id, + name: format!("u{id}"), + pass_hash: None, + is_admin, + created: None, + last_login: None, + }; assert!(last_admin(&[u(1, true), u(2, false)], 1)); assert!(!last_admin(&[u(1, true), u(2, true)], 1), "another admin remains"); assert!(!last_admin(&[u(1, true), u(2, false)], 2), "not an admin at all"); diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index b49027c..60dac53 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -433,6 +433,9 @@ test('an admin adds someone, makes them an admin, and removes them', async ({ pa await page.locator('#uadd').click(); const row = userRow(page, 'pat'); await expect(row).toBeVisible(); + // When each account was added and last signed in; the admin signed in with the token link. + await expect(row).toContainText(/Added .* never signed in/); + await expect(userRow(page, 'admin')).toContainText(/signed in \d+m ago/); await expect(row.locator('[data-a="admin"]')).not.toBeChecked(); await row.locator('[data-a="admin"]').check(); diff --git a/web/index.html b/web/index.html index c70969e..dc2157d 100644 --- a/web/index.html +++ b/web/index.html @@ -1708,7 +1708,9 @@ async function usersModal(){ const users = await api('/api/users') || []; openModal(`

Users

${users.map(u=>`
- ${esc(u.name)} +
${esc(u.name)} + ${u.created?`Added ${dateOf(u.created)}`:'Added before this was kept'} · ${ + u.last_login?`signed in ${ago(u.last_login)}`:'never signed in'}
${u.password?'':'Proxy'}
`).join('')}