Merge branch 'main' into quenting/rust-1.87

Author: sandhose

crates/cli/src/commands/manage.rs

@@ -149,6 +149,10 @@ enum Subcommand {
     UnlockUser {
         /// User to unlock
         username: String,
+
+        /// Whether to reactivate the user if it had been deactivated
+        #[arg(long)]
+        reactivate: bool,
     },
 
     /// Register a user
@@ -535,8 +539,12 @@ impl Options {
                 Ok(ExitCode::SUCCESS)
             }
 
-            SC::UnlockUser { username } => {
-                let _span = info_span!("cli.manage.lock_user", user.username = username).entered();
+            SC::UnlockUser {
+                username,
+                reactivate,
+            } => {
+                let _span =
+                    info_span!("cli.manage.unlock_user", user.username = username).entered();
                 let config = DatabaseConfig::extract_or_default(figment)
                     .map_err(anyhow::Error::from_boxed)?;
                 let mut conn = database_connection_from_config(&config).await?;
@@ -549,10 +557,14 @@ impl Options {
                     .await?
                     .context("User not found")?;
 
-                warn!(%user.id, "User scheduling user reactivation");
-                repo.queue_job()
-                    .schedule_job(&mut rng, &clock, ReactivateUserJob::new(&user))
-                    .await?;
+                if reactivate {
+                    warn!(%user.id, "Scheduling user reactivation");
+                    repo.queue_job()
+                        .schedule_job(&mut rng, &clock, ReactivateUserJob::new(&user))
+                        .await?;
+                } else {
+                    repo.user().unlock(user).await?;
+                }
 
                 repo.into_inner().commit().await?;
 

crates/handlers/src/admin/v1/mod.rs

@@ -94,6 +94,10 @@ where
             "/users/{id}/deactivate",
             post_with(self::users::deactivate, self::users::deactivate_doc),
         )
+        .api_route(
+            "/users/{id}/reactivate",
+            post_with(self::users::reactivate, self::users::reactivate_doc),
+        )
         .api_route(
             "/users/{id}/lock",
             post_with(self::users::lock, self::users::lock_doc),

crates/handlers/src/admin/v1/users/add.rs

@@ -10,11 +10,8 @@ use aide::{NoApi, OperationIo, transform::TransformOperation};
 use axum::{Json, extract::State, response::IntoResponse};
 use hyper::StatusCode;
 use mas_axum_utils::record_error;
-use mas_matrix::HomeserverConnection;
-use mas_storage::{
-    BoxRng,
-    queue::{ProvisionUserJob, QueueJobRepositoryExt as _},
-};
+use mas_matrix::{HomeserverConnection, ProvisionRequest};
+use mas_storage::BoxRng;
 use schemars::JsonSchema;
 use serde::Deserialize;
 use tracing::warn;
@@ -168,9 +165,13 @@ pub async fn handler(
 
     let user = repo.user().add(&mut rng, &clock, params.username).await?;
 
-    repo.queue_job()
-        .schedule_job(&mut rng, &clock, ProvisionUserJob::new(&user))
-        .await?;
+    homeserver
+        .provision_user(&ProvisionRequest::new(
+            homeserver.mxid(&user.username),
+            &user.sub,
+        ))
+        .await
+        .map_err(RouteError::Homeserver)?;
 
     repo.save().await?;
 
@@ -183,6 +184,7 @@ pub async fn handler(
 #[cfg(test)]
 mod tests {
     use hyper::{Request, StatusCode};
+    use mas_matrix::HomeserverConnection;
     use mas_storage::{RepositoryAccess, user::UserRepository};
     use sqlx::PgPool;
 
@@ -218,6 +220,11 @@ mod tests {
             .unwrap();
 
         assert_eq!(user.username, "alice");
+
+        // Check that the user was created on the homeserver
+        let mxid = state.homeserver_connection.mxid("alice");
+        let result = state.homeserver_connection.query_user(&mxid).await;
+        assert!(result.is_ok());
     }
 
     #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]

crates/handlers/src/admin/v1/users/deactivate.rs

@@ -12,6 +12,8 @@ use mas_storage::{
     BoxRng,
     queue::{DeactivateUserJob, QueueJobRepositoryExt as _},
 };
+use schemars::JsonSchema;
+use serde::Deserialize;
 use tracing::info;
 use ulid::Ulid;
 
@@ -49,18 +51,40 @@ impl IntoResponse for RouteError {
     }
 }
 
-pub fn doc(operation: TransformOperation) -> TransformOperation {
+/// # JSON payload for the `POST /api/admin/v1/users/:id/deactivate` endpoint
+#[derive(Default, Deserialize, JsonSchema)]
+#[serde(rename = "DeactivateUserRequest")]
+pub struct Request {
+    /// 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 {
+    operation
+        .inner_mut()
+        .request_body
+        .as_mut()
+        .unwrap()
+        .as_item_mut()
+        .unwrap()
+        .required = false;
+
     operation
         .id("deactivateUser")
         .summary("Deactivate a user")
-        .description("Calling this endpoint will lock and deactivate the user, preventing them from doing any action.
-This invalidates any existing session, and will ask the homeserver to make them leave all rooms.")
+        .description(
+            "Calling this endpoint will deactivate the user, preventing them from doing any action.
+This invalidates any existing session, and will ask the homeserver to make them leave all rooms.",
+        )
         .tag("user")
         .response_with::<200, Json<SingleResponse<User>>, _>(|t| {
             // In the samples, the third user is the one locked
             let [_alice, _bob, charlie, ..] = User::samples();
             let id = charlie.id();
-            let response = SingleResponse::new(charlie, format!("/api/admin/v1/users/{id}/deactivate"));
+            let response =
+                SingleResponse::new(charlie, format!("/api/admin/v1/users/{id}/deactivate"));
             t.description("User was deactivated").example(response)
         })
         .response_with::<404, RouteError, _>(|t| {
@@ -76,21 +100,25 @@ pub async fn handler(
     }: CallContext,
     NoApi(mut rng): NoApi<BoxRng>,
     id: UlidPathParam,
+    body: Option<Json<Request>>,
 ) -> Result<Json<SingleResponse<User>>, RouteError> {
+    let Json(params) = body.unwrap_or_default();
     let id = *id;
-    let mut user = repo
+    let user = repo
         .user()
         .lookup(id)
         .await?
         .ok_or(RouteError::NotFound(id))?;
 
-    if user.locked_at.is_none() {
-        user = repo.user().lock(&clock, user).await?;
-    }
+    let user = repo.user().deactivate(&clock, user).await?;
 
     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.skip_erase),
+        )
         .await?;
 
     repo.save().await?;
@@ -105,14 +133,13 @@ pub async fn handler(
 mod tests {
     use chrono::Duration;
     use hyper::{Request, StatusCode};
-    use insta::assert_json_snapshot;
+    use insta::{allow_duplicates, assert_json_snapshot};
     use mas_storage::{Clock, RepositoryAccess, user::UserRepository};
-    use sqlx::PgPool;
+    use sqlx::{PgPool, types::Json};
 
     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, skip_erase: Option<bool>) {
         setup();
         let mut state = TestState::from_pool(pool.clone()).await.unwrap();
         let token = state.token_with_scope("urn:mas:admin").await;
@@ -125,19 +152,44 @@ mod tests {
             .unwrap();
         repo.save().await.unwrap();
 
-        let request = Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id))
-            .bearer(&token)
-            .empty();
+        let request =
+            Request::post(format!("/api/admin/v1/users/{}/deactivate", user.id)).bearer(&token);
+        let request = match skip_erase {
+            None => request.empty(),
+            Some(skip_erase) => request.json(serde_json::json!({
+                "skip_erase": skip_erase,
+            })),
+        };
         let response = state.request(request).await;
         response.assert_status(StatusCode::OK);
         let body: serde_json::Value = response.json();
 
-        // The locked_at timestamp should be the same as the current time
+        // The deactivated_at timestamp should be the same as the current time
         assert_eq!(
-            body["data"]["attributes"]["locked_at"],
+            body["data"]["attributes"]["deactivated_at"],
             serde_json::json!(state.clock.now())
         );
 
+        // Deactivating the user should not lock it
+        assert_eq!(
+            body["data"]["attributes"]["locked_at"],
+            serde_json::Value::Null
+        );
+
+        // It should have scheduled a deactivation job for the user
+        // XXX: we don't have a good way to look for the deactivation job
+        let job: Json<serde_json::Value> = sqlx::query_scalar(
+            "SELECT payload FROM queue_jobs WHERE queue_name = 'deactivate-user'",
+        )
+        .fetch_one(&pool)
+        .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!(!skip_erase.unwrap_or(false))
+        );
+
         // Make sure to run the jobs in the queue
         state.run_jobs_in_queue().await;
 
@@ -148,15 +200,15 @@ mod tests {
         response.assert_status(StatusCode::OK);
         let body: serde_json::Value = response.json();
 
-        assert_json_snapshot!(body, @r#"
+        allow_duplicates!(assert_json_snapshot!(body, @r#"
         {
           "data": {
             "type": "user",
             "id": "01FSHN9AG0MZAA6S4AF7CTV32E",
             "attributes": {
               "username": "alice",
               "created_at": "2022-01-16T14:40:00Z",
-              "locked_at": "2022-01-16T14:40:00Z",
+              "locked_at": null,
               "deactivated_at": "2022-01-16T14:40:00Z",
               "admin": false
             },
@@ -168,7 +220,17 @@ mod tests {
             "self": "/api/admin/v1/users/01FSHN9AG0MZAA6S4AF7CTV32E"
           }
         }
-        "#);
+        "#));
+    }
+
+    #[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_skip_erase(pool: PgPool) {
+        test_deactivate_user_helper(pool, Option::Some(true)).await;
     }
 
     #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
@@ -196,10 +258,16 @@ mod tests {
         response.assert_status(StatusCode::OK);
         let body: serde_json::Value = response.json();
 
-        // The locked_at timestamp should be different from the current time
+        // The deactivated_at timestamp should be the same as the current time
+        assert_eq!(
+            body["data"]["attributes"]["deactivated_at"],
+            serde_json::json!(state.clock.now())
+        );
+
+        // The deactivated_at timestamp should be different from the locked_at timestamp
         assert_ne!(
+            body["data"]["attributes"]["deactivated_at"],
             body["data"]["attributes"]["locked_at"],
-            serde_json::json!(state.clock.now())
         );
 
         // Make sure to run the jobs in the queue

crates/handlers/src/admin/v1/users/lock.rs

@@ -72,15 +72,13 @@ pub async fn handler(
     id: UlidPathParam,
 ) -> Result<Json<SingleResponse<User>>, RouteError> {
     let id = *id;
-    let mut user = repo
+    let user = repo
         .user()
         .lookup(id)
         .await?
         .ok_or(RouteError::NotFound(id))?;
 
-    if user.locked_at.is_none() {
-        user = repo.user().lock(&clock, user).await?;
-    }
+    let user = repo.user().lock(&clock, user).await?;
 
     repo.save().await?;
 
@@ -157,6 +155,10 @@ mod tests {
             body["data"]["attributes"]["locked_at"],
             serde_json::json!(state.clock.now())
         );
+        assert_ne!(
+            body["data"]["attributes"]["locked_at"],
+            serde_json::Value::Null
+        );
     }
 
     #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]

crates/handlers/src/admin/v1/users/mod.rs

@@ -10,6 +10,7 @@ mod deactivate;
 mod get;
 mod list;
 mod lock;
+mod reactivate;
 mod set_admin;
 mod set_password;
 mod unlock;
@@ -21,6 +22,7 @@ pub use self::{
     get::{doc as get_doc, handler as get},
     list::{doc as list_doc, handler as list},
     lock::{doc as lock_doc, handler as lock},
+    reactivate::{doc as reactivate_doc, handler as reactivate},
     set_admin::{doc as set_admin_doc, handler as set_admin},
     set_password::{doc as set_password_doc, handler as set_password},
     unlock::{doc as unlock_doc, handler as unlock},