fix(admin): Complete MSC4323 suspend and lock endpoints

changelog.d/+55af97cf8.feature.md

@@ -0,0 +1 @@
+Fixed MSC4323 suspend/lock admin endpoints: the lock endpoint now locks instead of suspending users, and error codes match the ratified spec. Contributed by @eleboucher.

src/api/client/admin/lock.rs

@@ -1,6 +1,5 @@
 use axum::extract::State;
 use conduwuit::{Err, Result};
-use futures::future::{join, join3};
 use ruma::api::client::admin::{is_user_locked, lock_user};
 
 use crate::Ruma;
@@ -12,17 +11,32 @@ pub(crate) async fn get_lock_status(
 	State(services): State<crate::State>,
 	body: Ruma<is_user_locked::v1::Request>,
 ) -> Result<is_user_locked::v1::Response> {
-	let (admin, status) = join(
-		services.users.is_admin(body.identity.expect_sender_user()?),
-		services.users.status(&body.user_id),
-	)
-	.await;
+	let sender_user = body.identity.expect_sender_user()?;
+	let sender_is_admin = services.users.is_admin(sender_user).await;
 
-	if !admin {
+	if !sender_is_admin {
 		return Err!(Request(Forbidden("Only server administrators can use this endpoint")));
 	}
 
-	status.ensure_active()?;
+	if body.user_id != *sender_user {
+		let target_is_admin = services.users.is_admin(&body.user_id).await;
+
+		if target_is_admin {
+			return Err!(Request(Forbidden(
+				"You cannot view the lock status of another server administrator"
+			)));
+		}
+	}
+
+	if !services.globals.user_is_local(&body.user_id) {
+		return Err!(Request(InvalidParam("User does not belong to the local server")));
+	}
+
+	let status = services.users.status(&body.user_id).await;
+
+	if !status.is_active() {
+		return Err!(Request(NotFound("This account does not exist")));
+	}
 
 	Ok(is_user_locked::v1::Response::new(
 		services.users.is_locked(&body.user_id).await?,
@@ -37,28 +51,31 @@ pub(crate) async fn put_lock_status(
 	body: Ruma<lock_user::v1::Request>,
 ) -> Result<lock_user::v1::Response> {
 	let sender_user = body.identity.expect_sender_user()?;
+	let sender_is_admin = services.users.is_admin(sender_user).await;
 
-	let (sender_admin, status, target_admin) = join3(
-		services.users.is_admin(sender_user),
-		services.users.status(&body.user_id),
-		services.users.is_admin(&body.user_id),
-	)
-	.await;
-
-	if !sender_admin {
+	if !sender_is_admin {
 		return Err!(Request(Forbidden("Only server administrators can use this endpoint")));
 	}
 
-	status.ensure_active()?;
-
 	if body.user_id == *sender_user {
 		return Err!(Request(Forbidden("You cannot lock yourself")));
 	}
 
-	if target_admin {
+	let target_is_admin = services.users.is_admin(&body.user_id).await;
+	if target_is_admin {
 		return Err!(Request(Forbidden("You cannot lock another server administrator")));
 	}
 
+	if !services.globals.user_is_local(&body.user_id) {
+		return Err!(Request(InvalidParam("User does not belong to the local server")));
+	}
+
+	let status = services.users.status(&body.user_id).await;
+
+	if !status.is_active() {
+		return Err!(Request(NotFound("This account does not exist")));
+	}
+
 	if services.users.is_locked(&body.user_id).await? == body.locked {
 		// No change
 		return Ok(lock_user::v1::Response::new(body.locked));
@@ -67,16 +84,16 @@ pub(crate) async fn put_lock_status(
 	let action = if body.locked {
 		services
 			.users
-			.suspend_account(&body.user_id, sender_user)
+			.lock_account(&body.user_id, sender_user)
 			.await;
 		"locked"
 	} else {
-		services.users.unsuspend_account(&body.user_id).await;
+		services.users.unlock_account(&body.user_id).await;
 		"unlocked"
 	};
 
 	if services.config.admin_room_notices {
-		// Notify the admin room that an account has been un/suspended
+		// Notify the admin room that an account has been un/locked
 		services
 			.admin
 			.send_text(&format!("{} has been {} by {}.", body.user_id, action, sender_user))

src/api/client/admin/suspend.rs

@@ -1,6 +1,5 @@
 use axum::extract::State;
 use conduwuit::{Err, Result};
-use futures::future::{join, join3};
 use ruma::api::client::admin::{is_user_suspended, suspend_user};
 
 use crate::Ruma;
@@ -12,17 +11,32 @@ pub(crate) async fn get_suspended_status(
 	State(services): State<crate::State>,
 	body: Ruma<is_user_suspended::v1::Request>,
 ) -> Result<is_user_suspended::v1::Response> {
-	let (admin, status) = join(
-		services.users.is_admin(body.identity.expect_sender_user()?),
-		services.users.status(&body.user_id),
-	)
-	.await;
+	let sender_user = body.identity.expect_sender_user()?;
+	let sender_is_admin = services.users.is_admin(sender_user).await;
 
-	if !admin {
+	if !sender_is_admin {
 		return Err!(Request(Forbidden("Only server administrators can use this endpoint")));
 	}
 
-	status.ensure_active()?;
+	if body.user_id != *sender_user {
+		let target_is_admin = services.users.is_admin(&body.user_id).await;
+
+		if target_is_admin {
+			return Err!(Request(Forbidden(
+				"You cannot view the suspension status of another server administrator"
+			)));
+		}
+	}
+
+	if !services.globals.user_is_local(&body.user_id) {
+		return Err!(Request(InvalidParam("User does not belong to the local server")));
+	}
+
+	let status = services.users.status(&body.user_id).await;
+
+	if !status.is_active() {
+		return Err!(Request(NotFound("This account does not exist")));
+	}
 
 	Ok(is_user_suspended::v1::Response::new(
 		services.users.is_suspended(&body.user_id).await?,
@@ -37,28 +51,31 @@ pub(crate) async fn put_suspended_status(
 	body: Ruma<suspend_user::v1::Request>,
 ) -> Result<suspend_user::v1::Response> {
 	let sender_user = body.identity.expect_sender_user()?;
+	let sender_is_admin = services.users.is_admin(sender_user).await;
 
-	let (sender_admin, status, target_admin) = join3(
-		services.users.is_admin(sender_user),
-		services.users.status(&body.user_id),
-		services.users.is_admin(&body.user_id),
-	)
-	.await;
-
-	if !sender_admin {
+	if !sender_is_admin {
 		return Err!(Request(Forbidden("Only server administrators can use this endpoint")));
 	}
 
-	status.ensure_active()?;
-
 	if body.user_id == *sender_user {
 		return Err!(Request(Forbidden("You cannot suspend yourself")));
 	}
 
-	if target_admin {
+	let target_is_admin = services.users.is_admin(&body.user_id).await;
+	if target_is_admin {
 		return Err!(Request(Forbidden("You cannot suspend another server administrator")));
 	}
 
+	if !services.globals.user_is_local(&body.user_id) {
+		return Err!(Request(InvalidParam("User does not belong to the local server")));
+	}
+
+	let status = services.users.status(&body.user_id).await;
+
+	if !status.is_active() {
+		return Err!(Request(NotFound("This account does not exist")));
+	}
+
 	if services.users.is_suspended(&body.user_id).await? == body.suspended {
 		// No change
 		return Ok(suspend_user::v1::Response::new(body.suspended));