From 69eca21fb5a9d5596c7922386e0e0541e6db8e49 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 16 May 2026 23:52:53 +0200 Subject: [PATCH] feat: password change endpoint with full session rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the pre-1.0 password-change story flagged by the audit. Browser users and bot owners both go through PATCH /api/v1/auth/me/password with the current + new password in the body. Implementation in `api::auth::change_password`: - CurrentUser-gated: 401 if unauthenticated. - Verifies current_password against the stored argon2 hash. Wrong current → 401 unauthenticated, matching the login contract. - new_password runs through the same `validate_password` used at registration (≥8 chars). Weak → 400 invalid_input. - On success, wraps the swap in a single transaction: - UPDATE users.password_hash with a fresh argon2 hash. - DELETE every session for this user (signs out other devices — any cookie stolen before the change is dead now). - INSERT a new session and mint a fresh cookie so the caller stays logged in. - 204 + Set-Cookie on success. Bot tokens (api_tokens) are intentionally left alone. They're explicit opt-in credentials that the user can already audit and revoke individually via DELETE /auth/tokens/{id}; rotating them on every password change would surprise CI scripts. Repo refactor: `repo::session::create` accepts `impl PgExecutor<'_>` (same pattern feat/uploads used for chapters), and a new `session::delete_all_for_user` covers the "sign out everywhere" write. The existing `delete_by_token_hash` (used by logout) is unchanged. Coverage in tests/api_auth.rs (4 cases): - change_password_rotates_sessions_and_swaps_credentials — happy path asserts the new cookie differs from the original, that both the original cookie AND a second-device cookie become invalid, that the new cookie keeps working, that login with the old password fails (401) and login with the new password succeeds. - change_password_rejects_wrong_current_with_401 — wrong current password returns 401 unauthenticated. - change_password_rejects_weak_new_password — new_password "short" returns 400 invalid_input. - change_password_requires_authentication — no cookie returns 401. README updated with the new endpoint in the auth table. Lockstep version bump to 0.10.0. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 1 + backend/Cargo.toml | 2 +- backend/src/api/auth.rs | 52 ++++++++++++- backend/src/repo/session.rs | 22 +++++- backend/tests/api_auth.rs | 147 ++++++++++++++++++++++++++++++++++++ backend/tests/common/mod.rs | 23 ++++++ frontend/package.json | 2 +- 7 files changed, 242 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0d53d5b..b725f05 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ Everything is namespaced under `/api/v1/`. `/api/*` outside the version prefix i | POST | `/api/v1/auth/login` | Same shape; rotates to a fresh session. | | POST | `/api/v1/auth/logout` | Invalidates the session, clears the cookie. | | GET | `/api/v1/auth/me` | Current user, or 401 if anonymous. | +| PATCH | `/api/v1/auth/me/password` | `{current_password,new_password}` → 401 on wrong current; on success deletes **all** of the user's sessions and mints a fresh one for the caller. Bot tokens stay (use DELETE /tokens/{id} to revoke). | | POST | `/api/v1/auth/tokens` | `{name}` → bot bearer token (returned once). | | DELETE | `/api/v1/auth/tokens/{id}` | Revoke a bot token (owner-only). | | POST | `/api/v1/mangas` | Multipart: `metadata` JSON + optional `cover` image. | diff --git a/backend/Cargo.toml b/backend/Cargo.toml index d9c1773..6cd513f 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.9.4" +version = "0.10.0" edition = "2021" [lib] diff --git a/backend/src/api/auth.rs b/backend/src/api/auth.rs index 7c997b0..bdf531c 100644 --- a/backend/src/api/auth.rs +++ b/backend/src/api/auth.rs @@ -7,7 +7,7 @@ use axum::extract::{Path, State}; use axum::http::StatusCode; use axum::response::IntoResponse; -use axum::routing::{delete, get, post}; +use axum::routing::{delete, get, patch, post}; use axum::{Json, Router}; use axum_extra::extract::cookie::{Cookie, CookieJar, SameSite}; use chrono::{Duration, Utc}; @@ -29,6 +29,7 @@ pub fn routes() -> Router { .route("/auth/login", post(login)) .route("/auth/logout", post(logout)) .route("/auth/me", get(me)) + .route("/auth/me/password", patch(change_password)) .route("/auth/tokens", post(create_token)) .route("/auth/tokens/:id", delete(delete_token)) } @@ -49,6 +50,12 @@ pub struct CreateTokenInput { pub name: String, } +#[derive(Debug, Deserialize)] +pub struct ChangePassword { + pub current_password: String, + pub new_password: String, +} + #[derive(Debug, Serialize)] pub struct CreatedTokenResponse { #[serde(flatten)] @@ -111,6 +118,49 @@ async fn me(CurrentUser(user): CurrentUser) -> AppResult> { Ok(Json(AuthResponse { user })) } +/// `PATCH /api/v1/auth/me/password` — change the current user's password. +/// +/// Verifies `current_password` against the stored argon2 hash (401 on +/// mismatch, matching the login contract). Validates `new_password` +/// against the same min-length rule used at registration. On success, +/// inside a single transaction: +/// - UPDATE users.password_hash with the new argon2 hash +/// - DELETE all existing sessions for this user (signs out other +/// devices; the stolen-cookie attack surface is closed) +/// - INSERT a fresh session and return it as a new cookie so the +/// caller stays logged in +/// +/// Bot tokens (`api_tokens`) are left alone: the user opted into them +/// explicitly and can revoke individually via DELETE /auth/tokens/{id}. +async fn change_password( + State(state): State, + CurrentUser(user): CurrentUser, + jar: CookieJar, + Json(input): Json, +) -> AppResult { + if !verify_password(&input.current_password, &user.password_hash) { + return Err(AppError::Unauthenticated); + } + validate_password(&input.new_password)?; + + let new_hash = hash_password(&input.new_password)?; + + let mut tx = state.db.begin().await?; + sqlx::query("UPDATE users SET password_hash = $1 WHERE id = $2") + .bind(&new_hash) + .bind(user.id) + .execute(&mut *tx) + .await?; + repo::session::delete_all_for_user(&mut *tx, user.id).await?; + let (raw, hash) = generate_token(); + let expires_at = Utc::now() + Duration::days(state.auth.session_ttl_days); + repo::session::create(&mut *tx, user.id, &hash, expires_at).await?; + tx.commit().await?; + + let jar = jar.add(build_session_cookie(raw, &state.auth)); + Ok((StatusCode::NO_CONTENT, jar)) +} + async fn create_token( State(state): State, CurrentUser(user): CurrentUser, diff --git a/backend/src/repo/session.rs b/backend/src/repo/session.rs index 0ff9ef1..9555cec 100644 --- a/backend/src/repo/session.rs +++ b/backend/src/repo/session.rs @@ -2,14 +2,17 @@ //! the raw value never hits the DB. use chrono::{DateTime, Utc}; -use sqlx::PgPool; +use sqlx::{PgExecutor, PgPool}; use uuid::Uuid; use crate::domain::Session; use crate::error::AppResult; -pub async fn create( - pool: &PgPool, +/// Accepts any `PgExecutor` so callers can pass `&PgPool` for simple +/// inserts or `&mut *tx` to run inside a transaction (e.g., password +/// change rotates the user's sessions atomically with the hash UPDATE). +pub async fn create<'e, E: PgExecutor<'e>>( + executor: E, user_id: Uuid, token_hash: &[u8], expires_at: DateTime, @@ -24,11 +27,22 @@ pub async fn create( .bind(user_id) .bind(token_hash) .bind(expires_at) - .fetch_one(pool) + .fetch_one(executor) .await?; Ok(row) } +pub async fn delete_all_for_user<'e, E: PgExecutor<'e>>( + executor: E, + user_id: Uuid, +) -> AppResult<()> { + sqlx::query("DELETE FROM sessions WHERE user_id = $1") + .bind(user_id) + .execute(executor) + .await?; + Ok(()) +} + /// Returns the session iff `token_hash` matches and it hasn't expired. pub async fn find_active(pool: &PgPool, token_hash: &[u8]) -> AppResult> { let row = sqlx::query_as::<_, Session>( diff --git a/backend/tests/api_auth.rs b/backend/tests/api_auth.rs index 689b067..cac4b76 100644 --- a/backend/tests/api_auth.rs +++ b/backend/tests/api_auth.rs @@ -233,6 +233,153 @@ async fn logout_clears_session(pool: PgPool) { assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); } +#[sqlx::test(migrations = "./migrations")] +async fn change_password_rotates_sessions_and_swaps_credentials(pool: PgPool) { + let h = common::harness(pool); + let (username, cookie) = common::register_user(&h.app).await; + + // Log in a second time to seed a "second device" session that + // should also be invalidated by the password change. + let second_resp = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": username, "password": "hunter2hunter2" }), + )) + .await + .unwrap(); + let second_cookie = common::extract_session_cookie(&second_resp).unwrap(); + assert_ne!(cookie, second_cookie); + + // Change the password. + let resp = h + .app + .clone() + .oneshot(common::patch_json_with_cookie( + "/api/v1/auth/me/password", + json!({ + "current_password": "hunter2hunter2", + "new_password": "freshpassfreshpass" + }), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NO_CONTENT); + let rotated_cookie = + common::extract_session_cookie(&resp).expect("password change must mint a new cookie"); + assert_ne!(cookie, rotated_cookie, "session must rotate"); + + // Both original cookies are dead (other devices signed out). + for stale in [&cookie, &second_cookie] { + let resp = h + .app + .clone() + .oneshot(common::get_with_cookie("/api/v1/auth/me", stale)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "stale cookie {stale} should be invalid" + ); + } + + // The rotated cookie is live. + let resp = h + .app + .clone() + .oneshot(common::get_with_cookie("/api/v1/auth/me", &rotated_cookie)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + // Old password no longer logs in. + let resp = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": username, "password": "hunter2hunter2" }), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + + // New password does. + let resp = h + .app + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": username, "password": "freshpassfreshpass" }), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); +} + +#[sqlx::test(migrations = "./migrations")] +async fn change_password_rejects_wrong_current_with_401(pool: PgPool) { + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + let resp = h + .app + .oneshot(common::patch_json_with_cookie( + "/api/v1/auth/me/password", + json!({ + "current_password": "definitelyNotIt", + "new_password": "freshpassfreshpass" + }), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + let body = common::body_json(resp).await; + assert_eq!(body["error"]["code"], "unauthenticated"); +} + +#[sqlx::test(migrations = "./migrations")] +async fn change_password_rejects_weak_new_password(pool: PgPool) { + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + let resp = h + .app + .oneshot(common::patch_json_with_cookie( + "/api/v1/auth/me/password", + json!({ + "current_password": "hunter2hunter2", + "new_password": "short" + }), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = common::body_json(resp).await; + assert_eq!(body["error"]["code"], "invalid_input"); +} + +#[sqlx::test(migrations = "./migrations")] +async fn change_password_requires_authentication(pool: PgPool) { + let h = common::harness(pool); + let resp = h + .app + .oneshot(common::patch_json( + "/api/v1/auth/me/password", + json!({ + "current_password": "hunter2hunter2", + "new_password": "freshpassfreshpass" + }), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); +} + #[sqlx::test(migrations = "./migrations")] async fn me_rejects_expired_session(pool: PgPool) { use chrono::{Duration, Utc}; diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index c617c9a..ee4735d 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -169,6 +169,29 @@ pub fn post_json_with_bearer( .unwrap() } +pub fn patch_json(uri: &str, body: serde_json::Value) -> Request { + Request::builder() + .method("PATCH") + .uri(uri) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(body.to_string())) + .unwrap() +} + +pub fn patch_json_with_cookie( + uri: &str, + body: serde_json::Value, + cookie: &str, +) -> Request { + Request::builder() + .method("PATCH") + .uri(uri) + .header(header::CONTENT_TYPE, "application/json") + .header(header::COOKIE, cookie) + .body(Body::from(body.to_string())) + .unwrap() +} + pub fn delete_with_cookie(uri: &str, cookie: &str) -> Request { Request::builder() .method("DELETE") diff --git a/frontend/package.json b/frontend/package.json index 77b4848..9702efb 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.9.4", + "version": "0.10.0", "private": true, "type": "module", "scripts": {