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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TAC7sLVqfKmY6rsTLXzNgk
This commit is contained in:
2026-09-12 13:37:09 +00:00
parent 1352f0d54d
commit 2ba83c3aed
8 changed files with 143 additions and 54 deletions

View File

@@ -10,11 +10,16 @@ The long form, with what was wrong before and how it was found, is in
## [Unreleased] ## [Unreleased]
### Added
- Settings → Users and `ipx user list` show when each account was added and when it last signed
in, to the hour.
### Changed ### Changed
- Directory and Popular list the feeds inside an OPML one by one, and no longer the OPML itself, - 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. 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. ever read it, and an existing database drops the columns on its next start.
### Fixed ### Fixed

View File

@@ -50,7 +50,7 @@ entries feed_id, guid, title, link, published, description, first_seen,
image, duration, episode, season PK (feed_id, guid) image, duration, episode, season PK (feed_id, guid)
enclosures id, feed_id, guid, url UNIQUE, mime, length, path, state, enclosures id, feed_id, guid, url UNIQUE, mime, length, path, state,
bytes_done, downloaded_at, last_error 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 sessions token, user_id, seen
subscriptions user_id, feed_id, keywords, auto_download, allow_explicit, subscriptions user_id, feed_id, keywords, auto_download, allow_explicit,
max_new_per_check PK (user_id, feed_id) 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. older database.
Schema changes: add the table or column to `SCHEMA`. `CREATE TABLE IF NOT EXISTS` leaves a table 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 that already exists alone, so a new column on one also goes in `migrate()`'s `wanted` list, and a
`migrate()`, checked with `PRAGMA table_info` the way its `retired` list is for drops. None is retired one in its `retired` list; both are checked with `PRAGMA table_info`. Columns from before
needed today: every column added so far predates 0.3.0, the oldest version an upgrade may start 0.3.0, the oldest version an upgrade may start from, need no entry. `Db::memory()` runs the same
from. `Db::memory()` runs the same path as `Db::open`, so a migration cannot pass the tests while path as `Db::open`, so a migration cannot pass the tests while missing in production.
missing in production.
## Control socket ## Control socket

View File

@@ -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::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 `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. `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 ## 2026-09-12 — Cutting what had outlived its reason

137
src/db.rs
View File

@@ -72,7 +72,10 @@ CREATE TABLE IF NOT EXISTS users (
id INTEGER PRIMARY KEY, id INTEGER PRIMARY KEY,
name TEXT NOT NULL UNIQUE COLLATE NOCASE, name TEXT NOT NULL UNIQUE COLLATE NOCASE,
pass_hash TEXT, 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 -- 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 name: String,
pub pass_hash: Option<String>, pub pass_hash: Option<String>,
pub is_admin: bool, pub is_admin: bool,
/// When the account was made and when it last signed in, for whoever maintains the server.
pub created: Option<i64>,
pub last_login: Option<i64>,
}
/// 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<User> {
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. /// A feed derived from an OPML subscription rather than written into the config.
@@ -135,11 +155,17 @@ pub struct Managed {
pub group_id: String, pub group_id: String,
} }
/// Drops the columns later versions retired. CREATE TABLE IF NOT EXISTS leaves a table that /// Adds the columns later versions introduced and drops the ones they retired. CREATE TABLE IF
/// already exists alone, so an installed database needs this done explicitly. A new column on an /// NOT EXISTS leaves a table that already exists alone, so an installed database needs both done
/// existing table would need an ALTER TABLE ADD COLUMN here too; none does yet, since every one /// explicitly. Columns from before 0.3.0, the oldest version an upgrade may start from, need no
/// added so far predates 0.3.0, the oldest version an upgrade may start from. /// entry.
fn migrate(conn: &Connection) -> Result<()> { 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)] = &[ let retired: &[(&str, &str)] = &[
// Read state from before accounts, long since moved to entry_state. Two bugs came from // Read state from before accounts, long since moved to entry_state. Two bugs came from
// queries still reading these after they stopped meaning anything. // queries still reading these after they stopped meaning anything.
@@ -147,16 +173,24 @@ fn migrate(conn: &Connection) -> Result<()> {
("entries", "flagged"), ("entries", "flagged"),
("entries", "position"), ("entries", "position"),
// Written by every insert and read by nothing. // Written by every insert and read by nothing.
("users", "created"),
("subscriptions", "created"), ("subscriptions", "created"),
("sessions", "created"), ("sessions", "created"),
]; ];
for (table, column) in retired { let has = |table: &str, column: &str| -> Result<bool> {
let names: Vec<String> = conn let names: Vec<String> = conn
.prepare(&format!("PRAGMA table_info({table})"))? .prepare(&format!("PRAGMA table_info({table})"))?
.query_map([], |r| r.get(1))? .query_map([], |r| r.get(1))?
.collect::<rusqlite::Result<_>>()?; .collect::<rusqlite::Result<_>>()?;
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"); tracing::info!(table, column, "dropping column");
conn.execute_batch(&format!("ALTER TABLE {table} DROP COLUMN {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<i64> { pub fn create_user(&self, name: &str, pass_hash: Option<&str>, admin: bool) -> Result<i64> {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
conn.execute( conn.execute(
"INSERT INTO users (name, pass_hash, is_admin) VALUES (?1, ?2, ?3)", "INSERT INTO users (name, pass_hash, is_admin, created) VALUES (?1, ?2, ?3, ?4)",
params![name, pass_hash, admin as i64], params![name, pass_hash, admin as i64, now()],
)?; )?;
Ok(conn.last_insert_rowid()) Ok(conn.last_insert_rowid())
} }
pub fn user_by_name(&self, name: &str) -> Result<Option<User>> { pub fn user_by_name(&self, name: &str) -> Result<Option<User>> {
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<Option<User>> { pub fn user_by_id(&self, id: i64) -> Result<Option<User>> {
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<P: rusqlite::ToSql>(&self, sql: &str, key: P) -> Result<Option<User>> { fn one_user<P: rusqlite::ToSql>(&self, sql: &str, key: P) -> Result<Option<User>> {
@@ -1008,12 +1042,7 @@ impl Db {
let mut stmt = conn.prepare(sql)?; let mut stmt = conn.prepare(sql)?;
let mut rows = stmt.query(params![key])?; let mut rows = stmt.query(params![key])?;
Ok(match rows.next()? { Ok(match rows.next()? {
Some(r) => Some(User { Some(r) => Some(user_row(r)?),
id: r.get(0)?,
name: r.get(1)?,
pass_hash: r.get(2)?,
is_admin: r.get::<_, i64>(3)? != 0,
}),
None => None, None => None,
}) })
} }
@@ -1021,16 +1050,9 @@ impl Db {
pub fn users(&self) -> Result<Vec<User>> { pub fn users(&self) -> Result<Vec<User>> {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
let mut stmt = 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 let out = stmt
.query_map([], |r| { .query_map([], user_row)?
Ok(User {
id: r.get(0)?,
name: r.get(1)?,
pass_hash: r.get(2)?,
is_admin: r.get::<_, i64>(3)? != 0,
})
})?
.collect::<rusqlite::Result<Vec<_>>>()?; .collect::<rusqlite::Result<Vec<_>>>()?;
Ok(out) Ok(out)
} }
@@ -1047,6 +1069,17 @@ impl Db {
Ok(()) 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. /// Sessions go with the user: a deleted account must not leave a usable cookie behind.
pub fn delete_user(&self, id: i64) -> Result<()> { pub fn delete_user(&self, id: i64) -> Result<()> {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
@@ -1070,18 +1103,13 @@ impl Db {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
let cutoff = now() - max_idle_secs; let cutoff = now() - max_idle_secs;
let mut stmt = conn.prepare( 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 FROM sessions s JOIN users u ON u.id = s.user_id
WHERE s.token = ?1 AND s.seen >= ?2", WHERE s.token = ?1 AND s.seen >= ?2",
)?; )?;
let mut rows = stmt.query(params![token, cutoff])?; let mut rows = stmt.query(params![token, cutoff])?;
let found = match rows.next()? { let found = match rows.next()? {
Some(r) => Some(User { Some(r) => Some(user_row(r)?),
id: r.get(0)?,
name: r.get(1)?,
pass_hash: r.get(2)?,
is_admin: r.get::<_, i64>(3)? != 0,
}),
None => None, None => None,
}; };
drop(rows); drop(rows);
@@ -1516,21 +1544,26 @@ mod tests {
// The same order as open(): the schema leaves the old tables alone, migrate() fixes them. // The same order as open(): the schema leaves the old tables alone, migrate() fixes them.
conn.execute_batch(SCHEMA).unwrap(); conn.execute_batch(SCHEMA).unwrap();
migrate(&conn).unwrap(); migrate(&conn).unwrap();
for (table, gone) in [ let cols = |table: &str| -> Vec<String> {
("entries", &["read", "flagged", "position"][..]), conn.prepare(&format!("PRAGMA table_info({table})"))
("users", &["created"][..]),
("subscriptions", &["created"][..]),
("sessions", &["created"][..]),
] {
let cols: Vec<String> = conn
.prepare(&format!("PRAGMA table_info({table})"))
.unwrap() .unwrap()
.query_map([], |r| r.get(1)) .query_map([], |r| r.get(1))
.unwrap() .unwrap()
.collect::<rusqlite::Result<_>>() .collect::<rusqlite::Result<_>>()
.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:?}"); 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. // And the rows come through it.
let kept: i64 = conn let kept: i64 = conn
.query_row("SELECT count(*) FROM subscriptions JOIN sessions USING (user_id)", [], |r| r.get(0)) .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); 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] #[test]
fn the_first_admin_starts_with_the_catalogue_and_only_once() { 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 // Cutting this along with the dead read columns left the browser suite's admin with an

View File

@@ -273,11 +273,16 @@ fn user_cmd(ctx: &Arc<Ctx>, cmd: UserCmd) -> Result<()> {
println!("no accounts yet: ipx user add <name>"); println!("no accounts yet: ipx user add <name>");
} }
for u in users { 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!( println!(
"{:<20} {:<8} {}", "{:<20} {:<6} {:<11} added {added} {seen}",
u.name, u.name,
if u.is_admin { "admin" } else { "" }, 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(()) Ok(())

View File

@@ -133,6 +133,11 @@ async fn auth(State(state): State<WebState>, mut req: Request, next: Next) -> Re
None 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. // 2. A session cookie from signing in here.
@@ -157,6 +162,10 @@ async fn auth(State(state): State<WebState>, mut req: Request, next: Next) -> Re
if supplied.is_some_and(|t| constant_time_eq(&t, &token)) { if supplied.is_some_and(|t| constant_time_eq(&t, &token)) {
user = admin_user(&state); user = admin_user(&state);
if from_query.is_some() { 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!( set_cookie = Some(format!(
"{COOKIE}={token}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000" "{COOKIE}={token}; Path=/; HttpOnly; SameSite=Lax; Max-Age=31536000"
)); ));
@@ -253,6 +262,7 @@ async fn login(
let user = user.expect("verified above"); let user = user.expect("verified above");
let token = crate::auth::new_session_token(); let token = crate::auth::new_session_token();
state.ctx.db.create_session(user.id, &token)?; state.ctx.db.create_session(user.id, &token)?;
state.ctx.db.signed_in(user.id)?;
tracing::info!(user = %user.name, "signed in"); tracing::info!(user = %user.name, "signed in");
let days = state.ctx.cfg().web.session_days.max(1); let days = state.ctx.cfg().web.session_days.max(1);
@@ -317,6 +327,7 @@ async fn list_users(
.map(|u| { .map(|u| {
serde_json::json!({ serde_json::json!({
"id": u.id, "name": u.name, "admin": u.is_admin, "password": u.pass_hash.is_some(), "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(); .collect();
@@ -748,7 +759,14 @@ mod tests {
#[test] #[test]
fn only_the_last_admin_is_protected() { 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, false)], 1));
assert!(!last_admin(&[u(1, true), u(2, true)], 1), "another admin remains"); 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"); assert!(!last_admin(&[u(1, true), u(2, false)], 2), "not an admin at all");

View File

@@ -433,6 +433,9 @@ test('an admin adds someone, makes them an admin, and removes them', async ({ pa
await page.locator('#uadd').click(); await page.locator('#uadd').click();
const row = userRow(page, 'pat'); const row = userRow(page, 'pat');
await expect(row).toBeVisible(); 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 expect(row.locator('[data-a="admin"]')).not.toBeChecked();
await row.locator('[data-a="admin"]').check(); await row.locator('[data-a="admin"]').check();

View File

@@ -1708,7 +1708,9 @@ async function usersModal(){
const users = await api('/api/users') || []; const users = await api('/api/users') || [];
openModal(`<h3>Users</h3> openModal(`<h3>Users</h3>
${users.map(u=>`<div class="inline" data-id="${u.id}" style="margin-bottom:8px"> ${users.map(u=>`<div class="inline" data-id="${u.id}" style="margin-bottom:8px">
<b style="flex:1;overflow-wrap:anywhere">${esc(u.name)}</b> <div style="flex:1;min-width:0"><b style="overflow-wrap:anywhere">${esc(u.name)}</b>
<small style="display:block;color:var(--faint)">${u.created?`Added ${dateOf(u.created)}`:'Added before this was kept'} · ${
u.last_login?`signed in ${ago(u.last_login)}`:'never signed in'}</small></div>
${u.password?'':'<span class="tag" title="No password: signs in through the proxy">Proxy</span>'} ${u.password?'':'<span class="tag" title="No password: signs in through the proxy">Proxy</span>'}
<label class="check" style="margin:0"><input type="checkbox" data-a="admin" ${u.admin?'checked':''}> Admin</label> <label class="check" style="margin:0"><input type="checkbox" data-a="admin" ${u.admin?'checked':''}> Admin</label>
<button class="btn ico danger" data-a="rm" title="Remove ${esc(u.name)}" aria-label="Remove ${esc(u.name)}">${ICON.trash}</button></div>`).join('')} <button class="btn ico danger" data-a="rm" title="Remove ${esc(u.name)}" aria-label="Remove ${esc(u.name)}">${ICON.trash}</button></div>`).join('')}