diff --git a/crates/cli/src/commands/manage.rs b/crates/cli/src/commands/manage.rs index 97c019175..41b9a11f7 100644 --- a/crates/cli/src/commands/manage.rs +++ b/crates/cli/src/commands/manage.rs @@ -542,7 +542,7 @@ impl Options { warn!(%user.id, "User scheduling user reactivation"); repo.queue_job() - .schedule_job(&mut rng, &clock, ReactivateUserJob::new(&user)) + .schedule_job(&mut rng, &clock, ReactivateUserJob::new(&user, true)) .await?; repo.into_inner().commit().await?; diff --git a/crates/handlers/src/admin/v1/mod.rs b/crates/handlers/src/admin/v1/mod.rs index c1d29aad5..af1951019 100644 --- a/crates/handlers/src/admin/v1/mod.rs +++ b/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), diff --git a/crates/handlers/src/admin/v1/users/deactivate.rs b/crates/handlers/src/admin/v1/users/deactivate.rs index 87c2361a2..7a6bd8e4e 100644 --- a/crates/handlers/src/admin/v1/users/deactivate.rs +++ b/crates/handlers/src/admin/v1/users/deactivate.rs @@ -137,6 +137,7 @@ mod tests { body["data"]["attributes"]["locked_at"], serde_json::json!(state.clock.now()) ); + // TODO: have test coverage on deactivated_at timestamp // Make sure to run the jobs in the queue state.run_jobs_in_queue().await; @@ -201,6 +202,11 @@ mod tests { body["data"]["attributes"]["locked_at"], serde_json::json!(state.clock.now()) ); + assert_ne!( + body["data"]["attributes"]["locked_at"], + serde_json::Value::Null + ); + // TODO: have test coverage on deactivated_at timestamp // Make sure to run the jobs in the queue state.run_jobs_in_queue().await; diff --git a/crates/handlers/src/admin/v1/users/lock.rs b/crates/handlers/src/admin/v1/users/lock.rs index ed99b6a75..9db2a065a 100644 --- a/crates/handlers/src/admin/v1/users/lock.rs +++ b/crates/handlers/src/admin/v1/users/lock.rs @@ -157,6 +157,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")] diff --git a/crates/handlers/src/admin/v1/users/mod.rs b/crates/handlers/src/admin/v1/users/mod.rs index b9c0b5ea6..37484b75b 100644 --- a/crates/handlers/src/admin/v1/users/mod.rs +++ b/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}, diff --git a/crates/handlers/src/admin/v1/users/reactivate.rs b/crates/handlers/src/admin/v1/users/reactivate.rs new file mode 100644 index 000000000..44c5ae88c --- /dev/null +++ b/crates/handlers/src/admin/v1/users/reactivate.rs @@ -0,0 +1,223 @@ +// Copyright 2025 New Vector Ltd. +// +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +// Please see LICENSE files in the repository root for full details. + +use aide::{NoApi, OperationIo, transform::TransformOperation}; +use axum::{Json, response::IntoResponse}; +use hyper::StatusCode; +use mas_axum_utils::record_error; +use mas_storage::{ + BoxRng, + queue::{QueueJobRepositoryExt as _, ReactivateUserJob}, +}; +use tracing::info; +use ulid::Ulid; + +use crate::{ + admin::{ + call_context::CallContext, + model::{Resource, User}, + params::UlidPathParam, + response::{ErrorResponse, SingleResponse}, + }, + impl_from_error_for_route, +}; + +#[derive(Debug, thiserror::Error, OperationIo)] +#[aide(output_with = "Json")] +pub enum RouteError { + #[error(transparent)] + Internal(Box), + + #[error("User ID {0} not found")] + NotFound(Ulid), +} + +impl_from_error_for_route!(mas_storage::RepositoryError); + +impl IntoResponse for RouteError { + fn into_response(self) -> axum::response::Response { + let error = ErrorResponse::from_error(&self); + let sentry_event_id = record_error!(self, Self::Internal(_)); + let status = match self { + Self::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR, + Self::NotFound(_) => StatusCode::NOT_FOUND, + }; + (status, sentry_event_id, Json(error)).into_response() + } +} + +pub fn doc(operation: TransformOperation) -> TransformOperation { + operation + .id("reactivateUser") + .summary("Reactivate a user") + .description("Calling this endpoint will reactivate a deactivated user, both locally and on the Matrix homeserver.") + .tag("user") + .response_with::<200, Json>, _>(|t| { + // In the samples, the third user is the one locked + let [sample, ..] = User::samples(); + let id = sample.id(); + let response = SingleResponse::new(sample, format!("/api/admin/v1/users/{id}/reactivate")); + t.description("User was reactivated").example(response) + }) + .response_with::<404, RouteError, _>(|t| { + let response = ErrorResponse::from_error(&RouteError::NotFound(Ulid::nil())); + t.description("User ID not found").example(response) + }) +} + +#[tracing::instrument(name = "handler.admin.v1.users.reactivate", skip_all)] +pub async fn handler( + CallContext { + mut repo, clock, .. + }: CallContext, + NoApi(mut rng): NoApi, + id: UlidPathParam, +) -> Result>, RouteError> { + let id = *id; + let user = repo + .user() + .lookup(id) + .await? + .ok_or(RouteError::NotFound(id))?; + + info!(%user.id, "Scheduling reactivation of user"); + repo.queue_job() + .schedule_job(&mut rng, &clock, ReactivateUserJob::new(&user, false)) + .await?; + + repo.save().await?; + + Ok(Json(SingleResponse::new( + User::from(user), + format!("/api/admin/v1/users/{id}/reactivate"), + ))) +} + +#[cfg(test)] +mod tests { + use hyper::{Request, StatusCode}; + use mas_matrix::{HomeserverConnection, ProvisionRequest}; + use mas_storage::{Clock, RepositoryAccess, user::UserRepository}; + use sqlx::{PgPool, types::Json}; + + use crate::test_utils::{RequestBuilderExt, ResponseExt, TestState, setup}; + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_reactivate_deactivated_user(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(); + let user = repo.user().lock(&state.clock, user).await.unwrap(); + let user = repo.user().deactivate(&state.clock, user).await.unwrap(); + repo.save().await.unwrap(); + + // Provision and immediately deactivate the user on the homeserver, + // because this endpoint will try to reactivate it + let mxid = state.homeserver_connection.mxid(&user.username); + state + .homeserver_connection + .provision_user(&ProvisionRequest::new(&mxid, &user.sub)) + .await + .unwrap(); + state + .homeserver_connection + .delete_user(&mxid, true) + .await + .unwrap(); + + // The user should be deactivated on the homeserver + let mx_user = state.homeserver_connection.query_user(&mxid).await.unwrap(); + assert!(mx_user.deactivated); + + let request = Request::post(format!("/api/admin/v1/users/{}/reactivate", user.id)) + .bearer(&token) + .empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + let body: serde_json::Value = response.json(); + + // The user should remain locked after being reactivated + assert_eq!( + body["data"]["attributes"]["locked_at"], + serde_json::json!(state.clock.now()) + ); + // TODO: have test coverage on deactivated_at timestamp + + // It should have scheduled a reactivation job for the user + // XXX: we don't have a good way to look for the reactivation job + let job: Json = sqlx::query_scalar( + "SELECT payload FROM queue_jobs WHERE queue_name = 'reactivate-user'", + ) + .fetch_one(&pool) + .await + .expect("Reactivation job to be scheduled"); + assert_eq!(job["user_id"], serde_json::json!(user.id)); + assert_eq!(job["unlock"], serde_json::Value::Bool(false)); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_reactivate_active_user(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/{}/reactivate", user.id)) + .bearer(&token) + .empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + let body: serde_json::Value = response.json(); + + assert_eq!( + body["data"]["attributes"]["locked_at"], + serde_json::Value::Null + ); + // TODO: have test coverage on deactivated_at timestamp + + // It should have scheduled a reactivation job for the user + // XXX: we don't have a good way to look for the reactivation job + let job: Json = sqlx::query_scalar( + "SELECT payload FROM queue_jobs WHERE queue_name = 'reactivate-user'", + ) + .fetch_one(&pool) + .await + .expect("Reactivation job to be scheduled"); + assert_eq!(job["user_id"], serde_json::json!(user.id)); + assert_eq!(job["unlock"], serde_json::Value::Bool(false)); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_reactivate_unknown_user(pool: PgPool) { + setup(); + let mut state = TestState::from_pool(pool).await.unwrap(); + let token = state.token_with_scope("urn:mas:admin").await; + + let request = Request::post("/api/admin/v1/users/01040G2081040G2081040G2081/reactivate") + .bearer(&token) + .empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::NOT_FOUND); + let body: serde_json::Value = response.json(); + assert_eq!( + body["errors"][0]["title"], + "User ID 01040G2081040G2081040G2081 not found" + ); + } +} diff --git a/crates/handlers/src/admin/v1/users/unlock.rs b/crates/handlers/src/admin/v1/users/unlock.rs index 6e0311eec..e74d80aea 100644 --- a/crates/handlers/src/admin/v1/users/unlock.rs +++ b/crates/handlers/src/admin/v1/users/unlock.rs @@ -141,7 +141,7 @@ mod tests { assert_eq!( body["data"]["attributes"]["locked_at"], - serde_json::json!(null) + serde_json::Value::Null ); } @@ -158,6 +158,7 @@ mod tests { .await .unwrap(); let user = repo.user().lock(&state.clock, user).await.unwrap(); + let user = repo.user().deactivate(&state.clock, user).await.unwrap(); repo.save().await.unwrap(); // Provision the user on the homeserver @@ -187,8 +188,10 @@ mod tests { assert_eq!( body["data"]["attributes"]["locked_at"], - serde_json::json!(null) + serde_json::Value::Null ); + // TODO: have test coverage on deactivated_at timestamp + // The user should be reactivated on the homeserver let mx_user = state.homeserver_connection.query_user(&mxid).await.unwrap(); assert!(!mx_user.deactivated); diff --git a/crates/storage-pg/.sqlx/query-98a5491eb5f10997ac1f3718c835903ac99d9bb8ca4d79c908b25a6d1209b9b1.json b/crates/storage-pg/.sqlx/query-98a5491eb5f10997ac1f3718c835903ac99d9bb8ca4d79c908b25a6d1209b9b1.json new file mode 100644 index 000000000..75f013b53 --- /dev/null +++ b/crates/storage-pg/.sqlx/query-98a5491eb5f10997ac1f3718c835903ac99d9bb8ca4d79c908b25a6d1209b9b1.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE users\n SET deactivated_at = NULL\n WHERE user_id = $1\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "98a5491eb5f10997ac1f3718c835903ac99d9bb8ca4d79c908b25a6d1209b9b1" +} diff --git a/crates/storage-pg/src/user/mod.rs b/crates/storage-pg/src/user/mod.rs index 14957ba5f..6abc29d9a 100644 --- a/crates/storage-pg/src/user/mod.rs +++ b/crates/storage-pg/src/user/mod.rs @@ -384,6 +384,39 @@ impl UserRepository for PgUserRepository<'_> { Ok(user) } + #[tracing::instrument( + name = "db.user.reactivate", + skip_all, + fields( + db.query.text, + %user.id, + ), + err, + )] + async fn reactivate(&mut self, mut user: User) -> Result { + if user.deactivated_at.is_none() { + return Ok(user); + } + + let res = sqlx::query!( + r#" + UPDATE users + SET deactivated_at = NULL + WHERE user_id = $1 + "#, + Uuid::from(user.id), + ) + .traced() + .execute(&mut *self.conn) + .await?; + + DatabaseError::ensure_affected_rows(&res, 1)?; + + user.deactivated_at = None; + + Ok(user) + } + #[tracing::instrument( name = "db.user.set_can_request_admin", skip_all, diff --git a/crates/storage/src/queue/tasks.rs b/crates/storage/src/queue/tasks.rs index eb16f6e29..87fb41486 100644 --- a/crates/storage/src/queue/tasks.rs +++ b/crates/storage/src/queue/tasks.rs @@ -257,10 +257,11 @@ impl InsertableJob for DeactivateUserJob { const QUEUE_NAME: &'static str = "deactivate-user"; } -/// A job to reactivate a user +/// A job to reactivate and optionally unlock a user #[derive(Serialize, Deserialize, Debug, Clone)] pub struct ReactivateUserJob { user_id: Ulid, + unlock: bool, } impl ReactivateUserJob { @@ -269,9 +270,13 @@ impl ReactivateUserJob { /// # Parameters /// /// * `user` - The user to reactivate + /// * `unlock` - Whether the user should be unlocked on reactivation #[must_use] - pub fn new(user: &User) -> Self { - Self { user_id: user.id } + pub fn new(user: &User, unlock: bool) -> Self { + Self { + user_id: user.id, + unlock, + } } /// The ID of the user to reactivate @@ -279,6 +284,12 @@ impl ReactivateUserJob { pub fn user_id(&self) -> Ulid { self.user_id } + + /// Whether the user should be unlocked on reactivation + #[must_use] + pub fn unlock(&self) -> bool { + self.unlock + } } impl InsertableJob for ReactivateUserJob { diff --git a/crates/storage/src/user/mod.rs b/crates/storage/src/user/mod.rs index 64b1d6d79..f864157b1 100644 --- a/crates/storage/src/user/mod.rs +++ b/crates/storage/src/user/mod.rs @@ -244,6 +244,19 @@ pub trait UserRepository: Send + Sync { /// Returns [`Self::Error`] if the underlying repository fails async fn deactivate(&mut self, clock: &dyn Clock, user: User) -> Result; + /// Reactivate a [`User`] + /// + /// Returns the reactivated [`User`] + /// + /// # Parameters + /// + /// * `user`: The [`User`] to reactivate + /// + /// # Errors + /// + /// Returns [`Self::Error`] if the underlying repository fails + async fn reactivate(&mut self, user: User) -> Result; + /// Set whether a [`User`] can request admin /// /// Returns the [`User`] with the new `can_request_admin` value @@ -315,6 +328,7 @@ repository_impl!(UserRepository: async fn lock(&mut self, clock: &dyn Clock, user: User) -> Result; async fn unlock(&mut self, user: User) -> Result; async fn deactivate(&mut self, clock: &dyn Clock, user: User) -> Result; + async fn reactivate(&mut self, user: User) -> Result; async fn set_can_request_admin( &mut self, user: User, diff --git a/crates/tasks/src/user.rs b/crates/tasks/src/user.rs index b5f64dd42..290dab28f 100644 --- a/crates/tasks/src/user.rs +++ b/crates/tasks/src/user.rs @@ -137,9 +137,25 @@ impl RunnableJob for ReactivateUserJob { .await .map_err(JobError::retry)?; - // We want to unlock the user from our side only once it has been reactivated on - // the homeserver - let _user = repo.user().unlock(user).await.map_err(JobError::retry)?; + // Now reactivate the user in our database + let user = repo + .user() + .reactivate(user) + .await + .context("Failed to reactivate user") + .map_err(JobError::retry)?; + + if self.unlock() { + // We want to unlock the user from our side only once it has been reactivated on + // the homeserver + let _user = repo + .user() + .unlock(user) + .await + .context("Failed to unlock user") + .map_err(JobError::retry)?; + } + repo.save().await.map_err(JobError::retry)?; Ok(()) diff --git a/docs/api/spec.json b/docs/api/spec.json index 0082ea37c..3673c54bc 100644 --- a/docs/api/spec.json +++ b/docs/api/spec.json @@ -1409,6 +1409,76 @@ } } }, + "/api/admin/v1/users/{id}/reactivate": { + "post": { + "tags": [ + "user" + ], + "summary": "Reactivate a user", + "description": "Calling this endpoint will reactivate a deactivated user, both locally and on the Matrix homeserver.", + "operationId": "reactivateUser", + "parameters": [ + { + "in": "path", + "name": "id", + "required": true, + "schema": { + "title": "The ID of the resource", + "$ref": "#/components/schemas/ULID" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "User was reactivated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SingleResponse_for_User" + }, + "example": { + "data": { + "type": "user", + "id": "030C1G60R30C1G60R30C1G60R3", + "attributes": { + "username": "charlie", + "created_at": "1970-01-01T00:00:00Z", + "locked_at": "1970-01-01T00:00:00Z", + "deactivated_at": null, + "admin": false + }, + "links": { + "self": "/api/admin/v1/users/030C1G60R30C1G60R30C1G60R3" + } + }, + "links": { + "self": "/api/admin/v1/users/030C1G60R30C1G60R30C1G60R3/reactivate" + } + } + } + } + }, + "404": { + "description": "User ID not found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + }, + "example": { + "errors": [ + { + "title": "User ID 00000000000000000000000000 not found" + } + ] + } + } + } + } + } + } + }, "/api/admin/v1/users/{id}/lock": { "post": { "tags": [