From 49a2de14aeea2b37b6f21f3ad4853f8037fbeaa5 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Mon, 7 Sep 2026 01:43:10 +0000 Subject: [PATCH] Approve access requests from the dashboard with an emailed temporary password Each open access request on /dashboard/users gains an Approve form with a suggested username. Approving creates a user-role account with a 20-character random temporary password, emails it to the requester with the sign-in link, and marks the request done; if the email fails the account is deleted so the admin can retry. Approval refuses when mail is not configured. Migration 0008 adds users.must_change_password. A middleware on the signed-in routers sends flagged users to /account?change=1 until they set a new password; login honours the flag regardless of `next`, and the CLI's `users passwd` clears it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01QVPagF6jfDv78CC5Jv2wp4 --- README.md | 21 +- migrations/0008_password_reset.sql | 1 + src/mail.rs | 16 ++ src/web/dashboard/users.rs | 350 ++++++++++++++++++++++++- src/web/mod.rs | 5 +- src/web/session.rs | 171 +++++++++++- src/web/templates/account.html | 2 +- src/web/templates/dashboard/users.html | 4 +- src/web/users.rs | 102 ++++++- 9 files changed, 631 insertions(+), 41 deletions(-) create mode 100644 migrations/0008_password_reset.sql diff --git a/README.md b/README.md index 6bf5175..14a40e0 100644 --- a/README.md +++ b/README.md @@ -206,14 +206,17 @@ article chapters, rate articles, and use every `/dashboard/*` page, including settings and jobs. Personalization is shared across accounts for now. Visitors can request an account at `/request-access`; admins review open -requests on `/dashboard/users`, create accounts with `daily-epub users add -` on the server, and then mark each request done. Accounts are -deliberately managed on the host, not in the browser. Usernames -are case-insensitive and passwords must be 12–1024 characters. Bootstrap with -`daily-epub users add --admin`; use `users passwd`, `role`, +requests on `/dashboard/users`. Approving a request creates a `user` account, +emails a random temporary password to the requester, and marks the request +done; the new user must choose a new password on first sign-in. Approval is +available only when `[mail]` is active, so an account is never created with a +password that cannot be delivered. Admins can instead create an account with +`daily-epub users add ` on the server and mark the request done. +Usernames are case-insensitive and passwords must be 12–1024 characters. +Bootstrap with `daily-epub users add --admin`; use `users passwd`, `role`, `disable`/`enable`, `list`, and `logout` for later administration. Password changes and disabling a user revoke that user's sessions. `/dashboard/users` -is a read-only view of roles, status, login times, and open sessions. +also shows roles, status, login times, and open sessions. When `[mail]` is active and `mail.notify_to` is set, each stored request queues a plain-text SMTP notification with a direct dashboard review link; SMTP runs in the background and never delays the visitor's response. Mail settings are @@ -268,8 +271,8 @@ articles do not yet have embeddings to compare. | `GET /issues/{date}/read` | User or admin | Open the Standard edition in BookOrbit's web reader when the integration is enabled. | | `GET /robots.txt`, `/static/{file}` | Public | Crawler policy and embedded CSS, JavaScript, and favicon. | | `GET/POST /login`, `POST /logout` | Public/session | Sign in and out; login POSTs share a per-client-IP throttle budget with access requests. | -| `GET/POST /request-access` | Public | Request a reader account; POSTs share the login throttle, while GET stays unlimited. Requests are reviewed by an admin and fulfilled with the CLI. | -| `GET /account`, `POST /account/password`, `/account/logout-all` | User or admin | Change the current password or revoke sessions. | +| `GET/POST /request-access` | Public | Request a reader account; POSTs share the login throttle, while GET stays unlimited. Requests are reviewed by an admin. | +| `GET /account`, `POST /account/password`, `/account/logout-all` | User or admin | Change the current password or revoke sessions; temporary-password users must change it before opening protected pages. | | `POST /rate` | Admin | Append an attributed dashboard rating event. | | `GET /dashboard` | Admin | Run, budget, rating, job, and config overview. | | `GET /dashboard/runs[/{id}]`, `/articles[/{id}]`, `/ratings`, `/stats` | Admin | Pipeline history, article explanations, rating contributions/history, historical URL imports, and evaluation stats. | @@ -277,7 +280,7 @@ articles do not yet have embeddings to compare. | `GET/POST /dashboard/profile`, `POST /dashboard/profile/restore` | Admin | Edit `profile.md`, inspect prompts/adjustments, and restore a version. | | `GET/POST /dashboard/settings`, `POST /dashboard/settings/providers`, `GET /dashboard/settings/history` | Admin | Edit validated configuration and inspect its audit log. | | `GET /dashboard/jobs`, `GET /dashboard/jobs/{id}`, `POST /dashboard/jobs/{name}` | Admin | Start fixed systemd jobs and inspect status and logs. | -| `GET /dashboard/users`, `POST /dashboard/users/requests/{id}/done` | Admin | Review access requests and view users/open sessions; account edits use the CLI. | +| `GET /dashboard/users`, `POST /dashboard/users/requests/{id}/approve`, `POST /dashboard/users/requests/{id}/done` | Admin | Approve and email access requests, mark requests handled another way, and view users/open sessions; other account edits use the CLI. | | `GET /files/epub/{name}`, `/files/xtc/{name}` | Public if Basic auth is unset; otherwise session or Basic auth | Published downloads. Keeping them public when Basic auth is absent preserves existing OPDS acquisition links. | | `GET /opds`, `/opds/`, `/opds/daily.xml` | Existing optional Basic auth | OPDS acquisition feed. | | `GET /r/...`, `/healthz`, `/issues.json` | Existing policy | HMAC rating links, health, and issue reports. | diff --git a/migrations/0008_password_reset.sql b/migrations/0008_password_reset.sql new file mode 100644 index 0000000..a066f7d --- /dev/null +++ b/migrations/0008_password_reset.sql @@ -0,0 +1 @@ +ALTER TABLE users ADD COLUMN must_change_password INTEGER NOT NULL DEFAULT 0; diff --git a/src/mail.rs b/src/mail.rs index b918025..5cb159e 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -26,6 +26,8 @@ enum Transport { Smtp(AsyncSmtpTransport), #[cfg(test)] Recording(Arc>>), + #[cfg(test)] + Failing, } /// A reusable SMTP sender built from the startup configuration. @@ -78,6 +80,10 @@ impl Mailer { .push(message); return Ok(()); } + #[cfg(test)] + if let Transport::Failing = &self.transport { + anyhow::bail!("test mail delivery failed"); + } let to = parse_mailbox(&message.to, "message recipient")?; let email = lettre::Message::builder() @@ -96,6 +102,8 @@ impl Mailer { } #[cfg(test)] Transport::Recording(_) => unreachable!("recording transport returned above"), + #[cfg(test)] + Transport::Failing => unreachable!("failing transport returned above"), } Ok(()) } @@ -111,6 +119,14 @@ impl Mailer { messages, ) } + + #[cfg(test)] + pub(crate) fn failing() -> Self { + Self { + from: "daily@example.com".parse().expect("valid test mailbox"), + transport: Transport::Failing, + } + } } fn parse_mailbox(value: &str, field: &str) -> anyhow::Result { diff --git a/src/web/dashboard/users.rs b/src/web/dashboard/users.rs index acd626a..d3baf2d 100644 --- a/src/web/dashboard/users.rs +++ b/src/web/dashboard/users.rs @@ -1,11 +1,12 @@ -//! Dashboard: the read-only Users page (`/dashboard/users`, plan §6.1). +//! Dashboard: user accounts and access requests (`/dashboard/users`, plan §6.1). use askama::Template; use axum::Router; -use axum::extract::{Extension, Path, State}; +use axum::extract::{Extension, Form, Path, State}; use axum::response::{IntoResponse, Redirect, Response}; use axum::routing::{get, post}; use axum_login::tower_sessions::Session; +use serde::Deserialize; use sqlx::Row as _; use crate::server::AppState; @@ -28,6 +29,7 @@ struct UserLine { struct AccessRequestLine { id: i64, email: String, + suggested_username: String, reason: String, requested: String, } @@ -38,12 +40,24 @@ struct UsersTemplate { page: Page, requests: Vec, users: Vec, + mail_configured: bool, +} + +#[derive(Debug, Deserialize)] +struct ApproveForm { + username: String, +} + +#[derive(Debug)] +struct OpenRequest { + email: String, } /// Routes contributed by this page group (merged by `dashboard::router`). pub fn routes() -> Router { Router::new() .route("/dashboard/users", get(index)) + .route("/dashboard/users/requests/{id}/approve", post(approve)) .route("/dashboard/users/requests/{id}/done", post(mark_done)) } @@ -62,11 +76,18 @@ async fn index( .await .map_err(db_err)? .into_iter() - .map(|row| AccessRequestLine { - id: row.get("id"), - email: row.get("email"), - reason: row.get::, _>("reason").unwrap_or_default(), - requested: fmt_stored_time(Some(row.get::("requested_at").as_str()), &config), + .map(|row| { + let email: String = row.get("email"); + AccessRequestLine { + id: row.get("id"), + suggested_username: suggested_username(&email), + email, + reason: row.get::, _>("reason").unwrap_or_default(), + requested: fmt_stored_time( + Some(row.get::("requested_at").as_str()), + &config, + ), + } }) .collect(); let users = crate::web::users::list(&state.db) @@ -92,9 +113,76 @@ async fn index( page, requests, users, + mail_configured: state.mailer.is_some(), })) } +async fn approve( + State(state): State, + auth: AuthSession, + Extension(session): Extension, + Path(id): Path, + Form(form): Form, +) -> Result { + let viewer = auth.user().await.ok_or_else(|| WebError::Unauthenticated { + next: "/dashboard/users".into(), + })?; + let request = open_request(&state, id).await?; + let Some(mailer) = state.mailer.as_ref() else { + set_flash( + &session, + "error", + "Email is not configured; create the account with the CLI instead".into(), + ) + .await?; + return Ok(Redirect::to("/dashboard/users").into_response()); + }; + + let username = form.username.trim(); + let (user, temporary_password) = + match crate::web::users::add_with_temporary_password(&state.db, username).await { + Ok(created) => created, + Err(error) => { + set_flash(&session, "error", error.to_string()).await?; + return Ok(Redirect::to("/dashboard/users").into_response()); + } + }; + let sign_in_url = format!( + "{}/login", + state.config().server.public_url.trim_end_matches('/') + ); + let message = crate::mail::Message { + to: request.email.clone(), + subject: "Your Daily EPUB account".into(), + body: format!( + "Username: {username}\nTemporary password: {temporary_password}\nSign in: {sign_in_url}\n\nImportant: you will be asked to choose a new password when you sign in.\n" + ), + }; + if let Err(error) = mailer.send(message).await { + sqlx::query("DELETE FROM users WHERE id = ?") + .bind(user.id) + .execute(state.db.pool()) + .await + .map_err(db_err)?; + set_flash( + &session, + "error", + format!("Could not email {}: {error}", request.email), + ) + .await?; + return Ok(Redirect::to("/dashboard/users").into_response()); + } + + finish_request(&state, id, viewer.id).await?; + set_flash( + &session, + "success", + format!("Created {username} and emailed {}.", request.email), + ) + .await?; + Ok(Redirect::to("/dashboard/users").into_response()) +} + async fn mark_done( State(state): State, auth: AuthSession, @@ -104,12 +192,30 @@ async fn mark_done( let viewer = auth.user().await.ok_or_else(|| WebError::Unauthenticated { next: "/dashboard/users".into(), })?; + finish_request(&state, id, viewer.id).await?; + set_flash(&session, "success", "Access request marked done.".into()).await?; + Ok(Redirect::to("/dashboard/users").into_response()) +} + +async fn open_request(state: &AppState, id: i64) -> Result { + sqlx::query("SELECT email FROM account_requests WHERE id = ? AND status = 'open'") + .bind(id) + .fetch_optional(state.db.pool()) + .await + .map_err(db_err)? + .map(|row| OpenRequest { + email: row.get("email"), + }) + .ok_or(WebError::NotFound) +} + +async fn finish_request(state: &AppState, id: i64, handled_by: i64) -> Result<(), WebError> { let result = sqlx::query( "UPDATE account_requests SET status = 'done', handled_at = ?, handled_by = ? WHERE id = ? AND status = 'open'", ) .bind(crate::db::fmt_ts(jiff::Timestamp::now())) - .bind(viewer.id) + .bind(handled_by) .bind(id) .execute(state.db.pool()) .await @@ -117,30 +223,94 @@ async fn mark_done( if result.rows_affected() == 0 { return Err(WebError::NotFound); } + Ok(()) +} + +async fn set_flash(session: &Session, kind: &str, text: String) -> Result<(), WebError> { session .insert( "flash", Flash { - kind: "success".into(), - text: "Access request marked done.".into(), + kind: kind.into(), + text, }, ) .await - .map_err(|error| WebError::Internal(error.into()))?; - Ok(Redirect::to("/dashboard/users").into_response()) + .map_err(|error| WebError::Internal(error.into())) +} + +fn suggested_username(email: &str) -> String { + email + .split_once('@') + .map_or(email, |(local, _)| local) + .bytes() + .map(|byte| byte.to_ascii_lowercase()) + .filter(|byte| { + byte.is_ascii_lowercase() || byte.is_ascii_digit() || matches!(byte, b'_' | b'-') + }) + .map(char::from) + .collect() } #[cfg(test)] mod tests { use axum::body::Body; use axum::http::{Method, Request, StatusCode, header}; - use sqlx::Row as _; use tower::ServiceExt; + use super::*; + use crate::config::Config; + use crate::server::{AppState, router}; use crate::web::dashboard::tests::{ app_with_users, assert_admin_only, get, login_cookie, response_text, seed, }; + async fn insert_request(db: &crate::db::Db, email: &str) -> i64 { + sqlx::query_scalar( + "INSERT INTO account_requests (email, requested_at) + VALUES (?, '2026-09-05T12:00:00Z') RETURNING id", + ) + .bind(email) + .fetch_one(db.pool()) + .await + .unwrap() + } + + async fn app_with_mailer(db: &crate::db::Db, mailer: crate::mail::Mailer) -> axum::Router { + crate::web::users::add(db, "reader", "correct horse battery", false) + .await + .unwrap(); + crate::web::users::add(db, "admin", "correct horse battery", true) + .await + .unwrap(); + let mut config = Config::default(); + config.server.public_url = "https://daily.example/".into(); + let mut state = AppState::new(db.clone(), config, None); + state.mailer = Some(mailer); + router(state) + } + + async fn post_approve( + app: &axum::Router, + request_id: i64, + cookie: &str, + username: &str, + ) -> axum::response::Response { + app.clone() + .oneshot( + Request::builder() + .method(Method::POST) + .uri(format!("/dashboard/users/requests/{request_id}/approve")) + .header(header::COOKIE, cookie) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header("sec-fetch-site", "same-origin") + .body(Body::from(format!("username={username}"))) + .unwrap(), + ) + .await + .unwrap() + } + #[tokio::test] async fn users_page_is_admin_only_and_lists_accounts_and_sessions() { let seed = seed().await; @@ -158,6 +328,9 @@ mod tests { assert!(body.contains("1 open access request"), "{body}"); assert!(body.contains("reader@example.com"), "{body}"); assert!(body.contains("Daily commute"), "{body}"); + assert!(body.contains("value=\"reader\""), "{body}"); + assert!(body.contains(">Approve"), "{body}"); + assert!(body.contains("Email is not configured"), "{body}"); assert!(body.contains("Mark done"), "{body}"); assert!(body.contains("reader"), "{body}"); assert!(body.contains("admin"), "{body}"); @@ -213,4 +386,155 @@ mod tests { assert!(body.contains("0 open access requests"), "{body}"); assert!(!body.contains("done@example.com"), "{body}"); } + + #[test] + fn username_suggestions_normalize_the_email_local_part() { + assert_eq!( + suggested_username("Some.Name+news@example.com"), + "somenamenews" + ); + assert_eq!(suggested_username("R_E-A_D_E_R@example.com"), "r_e-a_d_e_r"); + assert_eq!(suggested_username("...@example.com"), ""); + } + + #[tokio::test] + async fn approval_creates_a_flagged_user_emails_the_password_and_finishes_request() { + let seed = seed().await; + let request_id = insert_request(&seed.db, "new-reader@example.com").await; + let (mailer, messages) = crate::mail::Mailer::recording(); + let app = app_with_mailer(&seed.db, mailer).await; + let admin = login_cookie(&app, "admin", "correct horse battery").await; + + let response = post_approve(&app, request_id, &admin, "new_reader").await; + assert_eq!(response.status(), StatusCode::SEE_OTHER); + assert_eq!( + response.headers().get(header::LOCATION).unwrap(), + "/dashboard/users" + ); + + let user = crate::web::users::find_by_username(&seed.db, "new_reader") + .await + .unwrap() + .unwrap(); + assert_eq!(user.role, crate::web::users::Role::User); + assert!(user.must_change_password); + let request = + sqlx::query("SELECT status, handled_at, handled_by FROM account_requests WHERE id = ?") + .bind(request_id) + .fetch_one(seed.db.pool()) + .await + .unwrap(); + assert_eq!(request.get::("status"), "done"); + assert!(request.get::, _>("handled_at").is_some()); + assert!(request.get::, _>("handled_by").is_some()); + + let message = { + let messages = messages + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + assert_eq!(messages.len(), 1); + messages[0].clone() + }; + assert_eq!(message.to, "new-reader@example.com"); + assert_eq!(message.subject, "Your Daily EPUB account"); + assert!(message.body.contains("Username: new_reader")); + assert!( + message + .body + .contains("Sign in: https://daily.example/login") + ); + assert!(message.body.contains("choose a new password")); + let password = message + .body + .lines() + .find_map(|line| line.strip_prefix("Temporary password: ")) + .unwrap(); + assert_eq!(password.len(), 20); + assert!(crate::web::users::verify_password( + &user.password_hash, + password + )); + + let page = get(&app, "/dashboard/users", Some(&admin)).await; + let body = response_text(page).await; + assert!( + body.contains("Created new_reader and emailed new-reader@example.com."), + "{body}" + ); + } + + #[tokio::test] + async fn approval_without_mail_refuses_without_changing_request() { + let seed = seed().await; + let request_id = insert_request(&seed.db, "no-mail@example.com").await; + let app = app_with_users(&seed.db).await; + let admin = login_cookie(&app, "admin", "correct horse battery").await; + + let response = post_approve(&app, request_id, &admin, "no_mail").await; + assert_eq!(response.status(), StatusCode::SEE_OTHER); + assert!( + crate::web::users::find_by_username(&seed.db, "no_mail") + .await + .unwrap() + .is_none() + ); + let status: String = sqlx::query_scalar("SELECT status FROM account_requests WHERE id = ?") + .bind(request_id) + .fetch_one(seed.db.pool()) + .await + .unwrap(); + assert_eq!(status, "open"); + let page = get(&app, "/dashboard/users", Some(&admin)).await; + let body = response_text(page).await; + assert!( + body.contains("Email is not configured; create the account with the CLI instead"), + "{body}" + ); + } + + #[tokio::test] + async fn failed_approval_email_deletes_the_user_and_leaves_request_open() { + let seed = seed().await; + let request_id = insert_request(&seed.db, "failure@example.com").await; + let app = app_with_mailer(&seed.db, crate::mail::Mailer::failing()).await; + let admin = login_cookie(&app, "admin", "correct horse battery").await; + + let response = post_approve(&app, request_id, &admin, "delivery_failure").await; + assert_eq!(response.status(), StatusCode::SEE_OTHER); + assert!( + crate::web::users::find_by_username(&seed.db, "delivery_failure") + .await + .unwrap() + .is_none() + ); + let status: String = sqlx::query_scalar("SELECT status FROM account_requests WHERE id = ?") + .bind(request_id) + .fetch_one(seed.db.pool()) + .await + .unwrap(); + assert_eq!(status, "open"); + let page = get(&app, "/dashboard/users", Some(&admin)).await; + let body = response_text(page).await; + assert!( + body.contains("Could not email failure@example.com"), + "{body}" + ); + } + + #[tokio::test] + async fn approval_is_admin_only() { + let seed = seed().await; + let request_id = insert_request(&seed.db, "forbidden@example.com").await; + let app = app_with_users(&seed.db).await; + let reader = login_cookie(&app, "reader", "correct horse battery").await; + + let response = post_approve(&app, request_id, &reader, "forbidden").await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert!( + crate::web::users::find_by_username(&seed.db, "forbidden") + .await + .unwrap() + .is_none() + ); + } } diff --git a/src/web/mod.rs b/src/web/mod.rs index e720645..2e66682 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -676,7 +676,8 @@ pub fn router(config: &crate::config::Config) -> axum::Router axum::Router axum::Router Response { + let path = request.uri().path(); + let allowed = matches!( + (request.method(), path), + (&axum::http::Method::GET, "/account") + | (&axum::http::Method::POST, "/account/password") + | (&axum::http::Method::POST, "/logout") + | (&axum::http::Method::POST, "/account/logout-all") + ); + if !allowed + && auth + .user() + .await + .is_some_and(|user| user.must_change_password) + { + return axum::response::Redirect::to("/account?change=1").into_response(); + } + next.run(request).await +} + #[derive(Debug, Default, Deserialize)] pub struct LoginQuery { #[serde(default)] @@ -283,6 +304,14 @@ struct LoginTemplate { struct AccountTemplate { page: Page, error: String, + change_required: bool, +} + +/// Query parameters accepted by the account page. +#[derive(Debug, Default, Deserialize)] +pub struct AccountQuery { + #[serde(default)] + change: Option, } /// The `` for both renders of the sign-in page. @@ -313,6 +342,11 @@ pub async fn login( .map_err(|error| WebError::Internal(error.into()))? { Some(user) => { + let destination = if user.must_change_password { + "/account?change=1" + } else { + &destination + }; auth.login(&user) .await .map_err(|error| WebError::Internal(error.into()))?; @@ -322,7 +356,7 @@ pub async fn login( .execute(state.db.pool()) .await .map_err(crate::db::DbError::from)?; - Ok(axum::response::Redirect::to(&destination).into_response()) + Ok(axum::response::Redirect::to(destination).into_response()) } None => Ok(( StatusCode::UNAUTHORIZED, @@ -343,13 +377,18 @@ pub async fn logout(auth: AuthSession) -> Result { Ok(axum::response::Redirect::to("/").into_response()) } -pub async fn account(auth: AuthSession) -> Result { +pub async fn account( + auth: AuthSession, + Query(query): Query, +) -> Result { let user = auth.user().await.ok_or_else(|| WebError::Unauthenticated { next: "/account".into(), })?; + let change_required = query.change.as_deref() == Some("1") || user.must_change_password; Ok(Html(AccountTemplate { page: Page::new("Account", Some(user.into()), "account"), error: String::new(), + change_required, }) .into_response()) } @@ -387,6 +426,7 @@ pub async fn change_password( return Ok(( StatusCode::BAD_REQUEST, Html(AccountTemplate { + change_required: user.must_change_password, page: Page::new("Account", Some(user.into()), "account"), error, }), @@ -397,7 +437,7 @@ pub async fn change_password( let password_hash = tokio::task::spawn_blocking(move || users::hash_password(&password)) .await .map_err(|error| WebError::Internal(error.into()))?; - sqlx::query("UPDATE users SET password_hash = ? WHERE id = ?") + sqlx::query("UPDATE users SET password_hash = ?, must_change_password = 0 WHERE id = ?") .bind(&password_hash) .bind(user.id) .execute(state.db.pool()) @@ -409,6 +449,7 @@ pub async fn change_password( .map_err(crate::db::DbError::from)?; let mut updated = user; updated.password_hash = password_hash; + updated.must_change_password = false; auth.login(&updated) .await .map_err(|error| WebError::Internal(error.into()))?; @@ -457,9 +498,12 @@ fn request_origin(headers: &axum::http::HeaderMap) -> Option { mod tests { use std::collections::HashMap; + use axum::body::{Body, to_bytes}; + use axum::http::{Method, Request, StatusCode, header}; use axum_login::tower_sessions::SessionStore; use serde_json::json; use time::Duration; + use tower::ServiceExt as _; use super::*; @@ -546,4 +590,125 @@ mod tests { assert_eq!(valid_next(Some("//evil.example/")), "/"); assert_eq!(valid_next(Some("https://evil.example/")), "/"); } + + #[tokio::test] + async fn temporary_password_forces_change_before_full_issue_access() { + let seed = crate::web::dashboard::tests::seed().await; + let (user, temporary_password) = + users::add_with_temporary_password(&seed.db, "temporary_reader") + .await + .unwrap(); + let article_uri = format!("/issues/{}/articles/1", seed.date); + let app = crate::server::router(AppState::new( + seed.db.clone(), + crate::config::Config::default(), + None, + )); + + let login = app + .clone() + .oneshot( + Request::builder() + .method(Method::POST) + .uri("/login") + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header("sec-fetch-site", "same-origin") + .header("x-forwarded-for", "192.0.2.90") + .body(Body::from(format!( + "username=temporary_reader&password={temporary_password}&next={article_uri}" + ))) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(login.status(), StatusCode::SEE_OTHER); + assert_eq!( + login.headers().get(header::LOCATION).unwrap(), + "/account?change=1" + ); + let cookie = login + .headers() + .get(header::SET_COOKIE) + .unwrap() + .to_str() + .unwrap() + .split(';') + .next() + .unwrap() + .to_string(); + + let blocked = app + .clone() + .oneshot( + Request::builder() + .uri(&article_uri) + .header(header::COOKIE, &cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(blocked.status(), StatusCode::SEE_OTHER); + assert_eq!( + blocked.headers().get(header::LOCATION).unwrap(), + "/account?change=1" + ); + + let account = app + .clone() + .oneshot( + Request::builder() + .uri("/account?change=1") + .header(header::COOKIE, &cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let account_body = String::from_utf8( + to_bytes(account.into_body(), 1024 * 1024) + .await + .unwrap() + .to_vec(), + ) + .unwrap(); + assert!(account_body.contains("Choose a new password to continue.")); + + let changed = app + .clone() + .oneshot( + Request::builder() + .method(Method::POST) + .uri("/account/password") + .header(header::COOKIE, &cookie) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .header("sec-fetch-site", "same-origin") + .body(Body::from(format!( + "current_password={temporary_password}&new_password=a+final+reader+password&confirm_password=a+final+reader+password" + ))) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(changed.status(), StatusCode::SEE_OTHER); + assert!( + !users::find_by_id(&seed.db, user.id) + .await + .unwrap() + .unwrap() + .must_change_password + ); + + let article = app + .oneshot( + Request::builder() + .uri(article_uri) + .header(header::COOKIE, cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(article.status(), StatusCode::OK); + } } diff --git a/src/web/templates/account.html b/src/web/templates/account.html index a3ecec4..8b0ab56 100644 --- a/src/web/templates/account.html +++ b/src/web/templates/account.html @@ -1 +1 @@ -{% extends "layout.html" %}{% block ears %}Reader account{% endblock %}{% block content %}

Reader account

Account

{% if !error.is_empty() %}

{{ error }}

{% endif %}
{% endblock %} +{% extends "layout.html" %}{% block ears %}Reader account{% endblock %}{% block content %}

Reader account

Account

{% if change_required %}

Choose a new password to continue. Your temporary password goes in the current-password field.

{% endif %}{% if !error.is_empty() %}

{{ error }}

{% endif %}
{% endblock %} diff --git a/src/web/templates/dashboard/users.html b/src/web/templates/dashboard/users.html index 420ea4e..70d24bd 100644 --- a/src/web/templates/dashboard/users.html +++ b/src/web/templates/dashboard/users.html @@ -3,7 +3,7 @@

Users

{{ requests.len() }} open access request{% if requests.len() != 1 %}s{% endif %}; every account on this server, with its role and open sessions.

-

Create accounts with daily-epub users add <username> on the server, then mark the request done. Change, disable, enable, or sign out users with the daily-epub users CLI.

+

Approve creates a user account with a temporary password and emails it; the user must choose a new password on first sign-in. Or create accounts with daily-epub users add and mark the request done. Change, disable, enable, or sign out users with the daily-epub users CLI.

Access requests

{% if requests.is_empty() %}

No open requests.

{% else %}
@@ -11,7 +11,7 @@ - +{% endfor %}
EmailReason or commentRequestedAction
{{ request.email }} {% if request.reason.is_empty() %}—{% else %}{{ request.reason }}{% endif %} {{ request.requested }}
{% endif %}
{% if users.len() > 1 %}{% endif %}
diff --git a/src/web/users.rs b/src/web/users.rs index 62c7ced..2c4c49c 100644 --- a/src/web/users.rs +++ b/src/web/users.rs @@ -2,12 +2,16 @@ use std::fmt; use std::str::FromStr; use jiff::Timestamp; +use rand::RngExt as _; use sqlx::Row; use crate::db::{Db, DbError, fmt_ts, parse_ts}; pub const MIN_PASSWORD_LEN: usize = 12; pub const MAX_PASSWORD_LEN: usize = 1024; +const TEMPORARY_PASSWORD_LEN: usize = 20; +const TEMPORARY_PASSWORD_ALPHABET: &[u8] = + b"ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz23456789"; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Role { @@ -49,6 +53,8 @@ pub struct User { pub password_hash: String, pub role: Role, pub disabled: bool, + /// Whether protected routes must redirect this user to change their password. + pub must_change_password: bool, pub created_at: Timestamp, pub last_login_at: Option, } @@ -61,6 +67,7 @@ impl fmt::Debug for User { .field("password_hash", &"[REDACTED]") .field("role", &self.role) .field("disabled", &self.disabled) + .field("must_change_password", &self.must_change_password) .field("created_at", &self.created_at) .field("last_login_at", &self.last_login_at) .finish() @@ -105,7 +112,8 @@ pub fn verify_password(hash: &str, plain: &str) -> bool { pub async fn find_by_username(db: &Db, username: &str) -> Result, DbError> { let row = sqlx::query( - "SELECT id, username, password_hash, role, disabled, created_at, last_login_at + "SELECT id, username, password_hash, role, disabled, must_change_password, + created_at, last_login_at FROM users WHERE username = ? COLLATE NOCASE", ) .bind(username) @@ -116,7 +124,8 @@ pub async fn find_by_username(db: &Db, username: &str) -> Result, D pub async fn find_by_id(db: &Db, id: i64) -> Result, DbError> { let row = sqlx::query( - "SELECT id, username, password_hash, role, disabled, created_at, last_login_at + "SELECT id, username, password_hash, role, disabled, must_change_password, + created_at, last_login_at FROM users WHERE id = ?", ) .bind(id) @@ -126,6 +135,26 @@ pub async fn find_by_id(db: &Db, id: i64) -> Result, DbError> { } pub async fn add(db: &Db, username: &str, password: &str, admin: bool) -> anyhow::Result { + add_user(db, username, password, admin, false).await +} + +/// Create a reader account with a random password that must be changed at first sign-in. +pub async fn add_with_temporary_password( + db: &Db, + username: &str, +) -> anyhow::Result<(User, String)> { + let password = temporary_password(); + let user = add_user(db, username, &password, false, true).await?; + Ok((user, password)) +} + +async fn add_user( + db: &Db, + username: &str, + password: &str, + admin: bool, + must_change_password: bool, +) -> anyhow::Result { validate_username(username)?; validate_password(password)?; if find_by_username(db, username).await?.is_some() { @@ -135,12 +164,14 @@ pub async fn add(db: &Db, username: &str, password: &str, admin: bool) -> anyhow let created_at = Timestamp::now(); let role = if admin { Role::Admin } else { Role::User }; let id: i64 = sqlx::query_scalar( - "INSERT INTO users (username, password_hash, role, created_at) - VALUES (?, ?, ?, ?) RETURNING id", + "INSERT INTO users + (username, password_hash, role, must_change_password, created_at) + VALUES (?, ?, ?, ?, ?) RETURNING id", ) .bind(username) .bind(&hash) .bind(role.as_str()) + .bind(must_change_password) .bind(fmt_ts(created_at)) .fetch_one(db.pool()) .await?; @@ -150,6 +181,7 @@ pub async fn add(db: &Db, username: &str, password: &str, admin: bool) -> anyhow password_hash: hash, role, disabled: false, + must_change_password, created_at, last_login_at: None, }) @@ -158,12 +190,14 @@ pub async fn add(db: &Db, username: &str, password: &str, admin: bool) -> anyhow pub async fn passwd(db: &Db, username: &str, password: &str) -> anyhow::Result { validate_password(password)?; let hash = hash_password(password); - let result = - sqlx::query("UPDATE users SET password_hash = ? WHERE username = ? COLLATE NOCASE") - .bind(hash) - .bind(username) - .execute(db.pool()) - .await?; + let result = sqlx::query( + "UPDATE users SET password_hash = ?, must_change_password = 0 + WHERE username = ? COLLATE NOCASE", + ) + .bind(hash) + .bind(username) + .execute(db.pool()) + .await?; require_one(username, result.rows_affected())?; logout(db, username).await } @@ -204,8 +238,9 @@ pub async fn logout(db: &Db, username: &str) -> anyhow::Result { pub async fn list(db: &Db) -> anyhow::Result> { let rows = sqlx::query( - "SELECT u.id, u.username, u.password_hash, u.role, u.disabled, u.created_at, - u.last_login_at, COUNT(s.id) AS open_sessions + "SELECT u.id, u.username, u.password_hash, u.role, u.disabled, + u.must_change_password, u.created_at, u.last_login_at, + COUNT(s.id) AS open_sessions FROM users u LEFT JOIN sessions s ON s.user_id = u.id AND s.expiry > unixepoch() GROUP BY u.id ORDER BY u.username COLLATE NOCASE", ) @@ -221,6 +256,16 @@ pub async fn list(db: &Db) -> anyhow::Result> { .collect() } +fn temporary_password() -> String { + let mut rng = rand::rng(); + (0..TEMPORARY_PASSWORD_LEN) + .map(|_| { + let index = rng.random_range(..TEMPORARY_PASSWORD_ALPHABET.len()); + char::from(TEMPORARY_PASSWORD_ALPHABET[index]) + }) + .collect() +} + fn require_one(username: &str, count: u64) -> anyhow::Result<()> { if count == 0 { anyhow::bail!("user {username:?} was not found"); @@ -240,6 +285,7 @@ fn user_from_row(row: &sqlx::sqlite::SqliteRow) -> Result { password_hash: row.get("password_hash"), role, disabled: row.get("disabled"), + must_change_password: row.get("must_change_password"), created_at: parse_ts("users.created_at", &row.get::("created_at"))?, last_login_at: row .get::, _>("last_login_at") @@ -267,6 +313,7 @@ mod tests { password_hash: hash.clone(), role: Role::User, disabled: false, + must_change_password: false, created_at: Timestamp::now(), last_login_at: None, } @@ -275,6 +322,36 @@ mod tests { ); } + #[tokio::test] + async fn temporary_password_is_valid_and_passwd_clears_the_change_flag() { + let dir = tempfile::tempdir().unwrap(); + let db = Db::open_and_migrate(&dir.path().join("db.sqlite")) + .await + .unwrap(); + let (user, password) = add_with_temporary_password(&db, "reader").await.unwrap(); + + assert_eq!(password.len(), TEMPORARY_PASSWORD_LEN); + assert!( + password + .bytes() + .all(|byte| TEMPORARY_PASSWORD_ALPHABET.contains(&byte)) + ); + validate_password(&password).unwrap(); + assert!(verify_password(&user.password_hash, &password)); + assert!(user.must_change_password); + + passwd(&db, "reader", "a final operator password") + .await + .unwrap(); + assert!( + !find_by_username(&db, "reader") + .await + .unwrap() + .unwrap() + .must_change_password + ); + } + #[tokio::test] async fn user_operations_validate_and_are_case_insensitive() { let dir = tempfile::tempdir().unwrap(); @@ -286,6 +363,7 @@ mod tests { .await .unwrap(); assert_eq!(user.role, Role::Admin); + assert!(!user.must_change_password); assert!( add(&db, "reader", "another valid password", false) .await