New admin API endpoint to reactivate users & align what lock/unlock/deactivate does in the admin API (#4768)

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
@@ -527,8 +531,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)?;
                 let mut conn = database_connection_from_config(&config).await?;
                 let txn = conn.begin().await?;
@@ -540,10 +548,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/deactivate.rs

@@ -53,14 +53,17 @@ pub fn doc(operation: TransformOperation) -> TransformOperation {
     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| {
@@ -78,15 +81,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().deactivate(&clock, user).await?;
 
     info!(%user.id, "Scheduling deactivation of user");
     repo.queue_job()
@@ -132,12 +133,18 @@ mod tests {
         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
+        );
+
         // Make sure to run the jobs in the queue
         state.run_jobs_in_queue().await;
 
@@ -156,7 +163,7 @@ mod tests {
             "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
             },
@@ -196,10 +203,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},