From 58cd2ba993a474137be1494f5ebdc0f1f76e704e Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 3 Jul 2025 13:22:17 -0400 Subject: [PATCH 1/6] Add "erase" option to REST deactivate request body This allows using the endpoint to deactivate a user without deleting it. TODO: make the request body optional. --- .../handlers/src/admin/v1/users/deactivate.rs | 13 ++++++++++- docs/api/spec.json | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 55194a613..7b9ea2e4e 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -8,6 +8,8 @@ use aide::{NoApi, OperationIo, transform::TransformOperation}; use axum::{Json, response::IntoResponse}; use hyper::StatusCode; use mas_axum_utils::record_error; +use schemars::JsonSchema; +use serde::Deserialize; use mas_storage::{ BoxRng, queue::{DeactivateUserJob, QueueJobRepositoryExt as _}, @@ -49,6 +51,14 @@ impl IntoResponse for RouteError { } } +/// # JSON payload for the `POST /api/admin/v1/users/:id/deactivate` endpoint +#[derive(Deserialize, JsonSchema)] +#[serde(rename = "DeactivateUserRequest")] +pub struct Request { + /// Whether the user should be GDPR-erased from the homeserver. + erase: bool, +} + pub fn doc(operation: TransformOperation) -> TransformOperation { operation .id("deactivateUser") @@ -76,6 +86,7 @@ pub async fn handler( }: CallContext, NoApi(mut rng): NoApi, id: UlidPathParam, + Json(params): Json, ) -> Result>, RouteError> { let id = *id; let mut user = repo @@ -90,7 +101,7 @@ pub async fn handler( info!(%user.id, "Scheduling deactivation of user"); repo.queue_job() - .schedule_job(&mut rng, &clock, DeactivateUserJob::new(&user, true)) + .schedule_job(&mut rng, &clock, DeactivateUserJob::new(&user, params.erase)) .await?; repo.save().await?; diff --git a/docs/api/spec.json b/docs/api/spec.json index 0082ea37c..c0e2ee510 100644 --- a/docs/api/spec.json +++ b/docs/api/spec.json @@ -1359,6 +1359,16 @@ "style": "simple" } ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DeactivateUserRequest" + } + } + }, + "required": true + }, "responses": { "200": { "description": "User was deactivated", @@ -3872,6 +3882,19 @@ } } }, + "DeactivateUserRequest": { + "title": "JSON payload for the `POST /api/admin/v1/users/:id/deactivate` endpoint", + "type": "object", + "required": [ + "erase" + ], + "properties": { + "erase": { + "description": "Whether the user should be GDPR-erased from the homeserver.", + "type": "boolean" + } + } + }, "UserEmailFilter": { "type": "object", "properties": { From c1ddb1d2ac8f56af8564f85dea11db385b5da94a Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 3 Jul 2025 14:02:16 -0400 Subject: [PATCH 2/6] Lint --- crates/handlers/src/admin/v1/users/deactivate.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 7b9ea2e4e..1f3700cfd 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -8,12 +8,12 @@ use aide::{NoApi, OperationIo, transform::TransformOperation}; use axum::{Json, response::IntoResponse}; use hyper::StatusCode; use mas_axum_utils::record_error; -use schemars::JsonSchema; -use serde::Deserialize; use mas_storage::{ BoxRng, queue::{DeactivateUserJob, QueueJobRepositoryExt as _}, }; +use schemars::JsonSchema; +use serde::Deserialize; use tracing::info; use ulid::Ulid; @@ -101,7 +101,11 @@ pub async fn handler( info!(%user.id, "Scheduling deactivation of user"); repo.queue_job() - .schedule_job(&mut rng, &clock, DeactivateUserJob::new(&user, params.erase)) + .schedule_job( + &mut rng, + &clock, + DeactivateUserJob::new(&user, params.erase), + ) .await?; repo.save().await?; From f8b4dcc6c25e5e250754f47b915a216f17324e2b Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Fri, 4 Jul 2025 14:30:42 -0400 Subject: [PATCH 3/6] Require "erase" key in deactivation request body If body is absent, treat "erase" as true. If body is present, require "erase" to be present in the body. --- .../handlers/src/admin/v1/users/deactivate.rs | 67 +++++++++++++++++-- docs/api/spec.json | 2 +- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 1f3700cfd..49c478b29 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -59,6 +59,12 @@ pub struct Request { erase: bool, } +impl Default for Request { + fn default() -> Self { + Self { erase: true } + } +} + pub fn doc(operation: TransformOperation) -> TransformOperation { operation .id("deactivateUser") @@ -86,8 +92,9 @@ pub async fn handler( }: CallContext, NoApi(mut rng): NoApi, id: UlidPathParam, - Json(params): Json, + body: Option>, ) -> Result>, RouteError> { + let Json(params) = body.unwrap_or_default(); let id = *id; let mut user = repo .user() @@ -125,8 +132,7 @@ mod tests { use crate::test_utils::{RequestBuilderExt, ResponseExt, TestState, setup}; - #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_deactivate_user(pool: PgPool) { + async fn test_deactivate_user_helper(pool: PgPool, erase: Option) { setup(); let mut state = TestState::from_pool(pool.clone()).await.unwrap(); let token = state.token_with_scope("urn:mas:admin").await; @@ -140,8 +146,13 @@ mod tests { repo.save().await.unwrap(); let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)) - .bearer(&token) - .empty(); + .bearer(&token); + let request = match erase { + None => request.empty(), + Some(erase) => request.json(serde_json::json!({ + "erase": erase, + })), + }; let response = state.request(request).await; response.assert_status(StatusCode::OK); let body: serde_json::Value = response.json(); @@ -161,6 +172,52 @@ mod tests { .await .expect("Deactivation job to be scheduled"); assert_eq!(job["user_id"], serde_json::json!(user.id)); + assert_eq!(job["hs_erase"], serde_json::json!(erase.unwrap_or(true))); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_deactivate_user(pool: PgPool) { + test_deactivate_user_helper(pool, Option::None).await; + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_deactivate_user_with_explicit_erase(pool: PgPool) { + test_deactivate_user_helper(pool, Option::Some(true)).await; + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_deactivate_user_without_erase(pool: PgPool) { + test_deactivate_user_helper(pool, Option::Some(false)).await; + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_deactivate_user_missing_erase(pool: PgPool) { + setup(); + let mut state = TestState::from_pool(pool.clone()).await.unwrap(); + let token = state.token_with_scope("urn:mas:admin").await; + + let mut repo = state.repository().await.unwrap(); + let user = repo + .user() + .add(&mut state.rng(), &state.clock, "alice".to_owned()) + .await + .unwrap(); + repo.save().await.unwrap(); + + let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)) + .bearer(&token) + .json(serde_json::json!({})); + let response = state.request(request).await; + response.assert_status(StatusCode::UNPROCESSABLE_ENTITY); + + // It should have not scheduled a deactivation job for the user + let count: i64 = sqlx::query_scalar( + "SELECT COUNT(1) FROM queue_jobs WHERE queue_name = 'deactivate-user'", + ) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 0); } #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] diff --git a/docs/api/spec.json b/docs/api/spec.json index c0e2ee510..cb597e144 100644 --- a/docs/api/spec.json +++ b/docs/api/spec.json @@ -1367,7 +1367,7 @@ } } }, - "required": true + "required": false }, "responses": { "200": { From 048a1cb92793176a823fb47235034aac13f763db Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Fri, 4 Jul 2025 14:50:36 -0400 Subject: [PATCH 4/6] Lint --- crates/handlers/src/admin/v1/users/deactivate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 49c478b29..383c6bdfd 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -145,8 +145,8 @@ mod tests { .unwrap(); repo.save().await.unwrap(); - let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)) - .bearer(&token); + let request = + Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)).bearer(&token); let request = match erase { None => request.empty(), Some(erase) => request.json(serde_json::json!({ From 88f5df36d47d07c367fab48f250aa3d2c7309751 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 10 Jul 2025 13:26:58 -0400 Subject: [PATCH 5/6] Force optional request body for JSON schema --- crates/handlers/src/admin/v1/users/deactivate.rs | 11 ++++++++++- docs/api/spec.json | 3 +-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 383c6bdfd..9834ec507 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -65,7 +65,16 @@ impl Default for Request { } } -pub fn doc(operation: TransformOperation) -> TransformOperation { +pub fn doc(mut operation: TransformOperation) -> TransformOperation { + operation + .inner_mut() + .request_body + .as_mut() + .unwrap() + .as_item_mut() + .unwrap() + .required = false; + operation .id("deactivateUser") .summary("Deactivate a user") diff --git a/docs/api/spec.json b/docs/api/spec.json index cb597e144..f2a04ea9b 100644 --- a/docs/api/spec.json +++ b/docs/api/spec.json @@ -1366,8 +1366,7 @@ "$ref": "#/components/schemas/DeactivateUserRequest" } } - }, - "required": false + } }, "responses": { "200": { From 23a87a02d2aba58821f420e9af2bb3adb0746599 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Mon, 14 Jul 2025 01:02:51 -0400 Subject: [PATCH 6/6] Negate erase option and make optional This makes it more intuitive for an empty request body to be equivalent to the option being set to false. --- .../handlers/src/admin/v1/users/deactivate.rs | 66 +++++-------------- docs/api/spec.json | 8 +-- 2 files changed, 18 insertions(+), 56 deletions(-) diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index f268d9b15..076c06914 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -52,17 +52,13 @@ impl IntoResponse for RouteError { } /// # JSON payload for the `POST /api/admin/v1/users/:id/deactivate` endpoint -#[derive(Deserialize, JsonSchema)] +#[derive(Default, Deserialize, JsonSchema)] #[serde(rename = "DeactivateUserRequest")] pub struct Request { - /// Whether the user should be GDPR-erased from the homeserver. - erase: bool, -} - -impl Default for Request { - fn default() -> Self { - Self { erase: true } - } + /// Whether to skip requesting the homeserver to GDPR-erase the user upon + /// deactivation. + #[serde(default)] + skip_erase: bool, } pub fn doc(mut operation: TransformOperation) -> TransformOperation { @@ -120,7 +116,7 @@ pub async fn handler( .schedule_job( &mut rng, &clock, - DeactivateUserJob::new(&user, params.erase), + DeactivateUserJob::new(&user, !params.skip_erase), ) .await?; @@ -142,7 +138,7 @@ mod tests { use crate::test_utils::{RequestBuilderExt, ResponseExt, TestState, setup}; - async fn test_deactivate_user_helper(pool: PgPool, erase: Option) { + async fn test_deactivate_user_helper(pool: PgPool, skip_erase: Option) { setup(); let mut state = TestState::from_pool(pool.clone()).await.unwrap(); let token = state.token_with_scope("urn:mas:admin").await; @@ -157,10 +153,10 @@ mod tests { let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)).bearer(&token); - let request = match erase { + let request = match skip_erase { None => request.empty(), - Some(erase) => request.json(serde_json::json!({ - "erase": erase, + Some(skip_erase) => request.json(serde_json::json!({ + "skip_erase": skip_erase, })), }; let response = state.request(request).await; @@ -182,7 +178,10 @@ mod tests { .await .expect("Deactivation job to be scheduled"); assert_eq!(job["user_id"], serde_json::json!(user.id)); - assert_eq!(job["hs_erase"], serde_json::json!(erase.unwrap_or(true))); + assert_eq!( + job["hs_erase"], + serde_json::json!(!skip_erase.unwrap_or(false)) + ); // Make sure to run the jobs in the queue state.run_jobs_in_queue().await; @@ -223,45 +222,10 @@ mod tests { } #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_deactivate_user_with_explicit_erase(pool: PgPool) { + async fn test_deactivate_user_skip_erase(pool: PgPool) { test_deactivate_user_helper(pool, Option::Some(true)).await; } - #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_deactivate_user_without_erase(pool: PgPool) { - test_deactivate_user_helper(pool, Option::Some(false)).await; - } - - #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] - async fn test_deactivate_user_missing_erase(pool: PgPool) { - setup(); - let mut state = TestState::from_pool(pool.clone()).await.unwrap(); - let token = state.token_with_scope("urn:mas:admin").await; - - let mut repo = state.repository().await.unwrap(); - let user = repo - .user() - .add(&mut state.rng(), &state.clock, "alice".to_owned()) - .await - .unwrap(); - repo.save().await.unwrap(); - - let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)) - .bearer(&token) - .json(serde_json::json!({})); - let response = state.request(request).await; - response.assert_status(StatusCode::UNPROCESSABLE_ENTITY); - - // It should have not scheduled a deactivation job for the user - let count: i64 = sqlx::query_scalar( - "SELECT COUNT(1) FROM queue_jobs WHERE queue_name = 'deactivate-user'", - ) - .fetch_one(&pool) - .await - .unwrap(); - assert_eq!(count, 0); - } - #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_deactivate_locked_user(pool: PgPool) { setup(); diff --git a/docs/api/spec.json b/docs/api/spec.json index f2a04ea9b..5cb1beba1 100644 --- a/docs/api/spec.json +++ b/docs/api/spec.json @@ -3884,12 +3884,10 @@ "DeactivateUserRequest": { "title": "JSON payload for the `POST /api/admin/v1/users/:id/deactivate` endpoint", "type": "object", - "required": [ - "erase" - ], "properties": { - "erase": { - "description": "Whether the user should be GDPR-erased from the homeserver.", + "skip_erase": { + "description": "Whether to skip requesting the homeserver to GDPR-erase the user upon deactivation.", + "default": false, "type": "boolean" } }