From 8c2cf677835fa4c55ee84a38e815df5fff430436 Mon Sep 17 00:00:00 2001 From: Ginger Date: Tue, 5 May 2026 09:09:38 -0400 Subject: [PATCH] refactor: Remove support for guest user registration --- complement/complement.config.toml | 2 - conduwuit-example.toml | 15 --- src/admin/query/users.rs | 14 --- src/admin/user/commands.rs | 24 ++-- src/admin/utils.rs | 2 +- src/api/client/account/mod.rs | 19 +-- src/api/client/account/register.rs | 182 +++++++-------------------- src/api/client/directory.rs | 10 -- src/api/client/membership/join.rs | 41 ++---- src/api/client/membership/knock.rs | 11 +- src/api/client/session.rs | 46 +------ src/api/client/unversioned.rs | 2 +- src/core/config/mod.rs | 15 --- src/service/appservice/mod.rs | 5 +- src/service/emergency/mod.rs | 18 ++- src/service/password_reset/mod.rs | 8 +- src/service/rooms/state_cache/mod.rs | 4 +- src/service/uiaa/mod.rs | 18 ++- src/service/users/mod.rs | 72 ++++++++--- 19 files changed, 152 insertions(+), 356 deletions(-) diff --git a/complement/complement.config.toml b/complement/complement.config.toml index 790f18cbe..dc7e66d6c 100644 --- a/complement/complement.config.toml +++ b/complement/complement.config.toml @@ -7,7 +7,6 @@ [global] address = "0.0.0.0" allow_device_name_federation = true -allow_guest_registration = true allow_public_room_directory_over_federation = true allow_registration = true database_path = "/database" @@ -32,7 +31,6 @@ rocksdb_log_level = "info" rocksdb_max_log_files = 1 rocksdb_recovery_mode = 0 rocksdb_paranoid_file_checks = true -log_guest_registrations = false allow_legacy_media = true startup_netburst = true startup_netburst_keep = -1 diff --git a/conduwuit-example.toml b/conduwuit-example.toml index 557628eaa..a1cad85e0 100644 --- a/conduwuit-example.toml +++ b/conduwuit-example.toml @@ -1271,21 +1271,6 @@ # #brotli_compression = false -# Set to true to allow user type "guest" registrations. Some clients like -# Element attempt to register guest users automatically. -# -#allow_guest_registration = false - -# Set to true to log guest registrations in the admin room. Note that -# these may be noisy or unnecessary if you're a public homeserver. -# -#log_guest_registrations = false - -# Set to true to allow guest registrations/users to auto join any rooms -# specified in `auto_join_rooms`. -# -#allow_guests_auto_join_rooms = false - # Enable the legacy unauthenticated Matrix media repository endpoints. # These endpoints consist of: # - /_matrix/media/*/config diff --git a/src/admin/query/users.rs b/src/admin/query/users.rs index 497a4f528..dd49415f7 100644 --- a/src/admin/query/users.rs +++ b/src/admin/query/users.rs @@ -15,10 +15,6 @@ pub enum UsersCommand { IterUsers2, - PasswordHash { - user_id: OwnedUserId, - }, - ListDevices { user_id: OwnedUserId, }, @@ -235,16 +231,6 @@ async fn count_users(&self) -> Result { .await } -#[admin_command] -async fn password_hash(&self, user_id: OwnedUserId) -> Result { - let timer = tokio::time::Instant::now(); - let result = self.services.users.password_hash(&user_id).await; - let query_time = timer.elapsed(); - - self.write_str(&format!("Query completed in {query_time:?}:\n\n```rs\n{result:#?}\n```")) - .await -} - #[admin_command] async fn list_devices(&self, user_id: OwnedUserId) -> Result { let timer = tokio::time::Instant::now(); diff --git a/src/admin/user/commands.rs b/src/admin/user/commands.rs index ce621dad1..a6e7ca76f 100644 --- a/src/admin/user/commands.rs +++ b/src/admin/user/commands.rs @@ -24,6 +24,7 @@ use ruma::{ tag::{TagEvent, TagEventContent, TagInfo}, }, }; +use service::users::HashedPassword; use crate::{ admin_command, get_room_info, @@ -70,7 +71,7 @@ pub(super) async fn create_user(&self, username: String, password: Option return Err!("Couldn't reset the password for user {user_id}: {e}"), - | Ok(()) => { - write!(self, "Successfully reset the password for user {user_id}: `{new_password}`") - }, - } + .set_password(&user_id, Some(HashedPassword::new(&new_password)?)); + + self.write_str(&format!( + "Successfully reset the password for user {user_id}: `{new_password}`" + )) .await?; if logout { @@ -562,7 +558,6 @@ pub(super) async fn force_join_list_of_local_users( &room_id, Some(String::from(BULK_JOIN_REASON)), &servers, - &None, ) .await { @@ -646,7 +641,6 @@ pub(super) async fn force_join_all_local_users( &room_id, Some(String::from(BULK_JOIN_REASON)), &servers, - &None, ) .await { @@ -685,7 +679,7 @@ pub(super) async fn force_join_room( self.services.globals.user_is_local(&user_id), "Parsed user_id must be a local user" ); - join_room_by_id_helper(self.services, &user_id, &room_id, None, &servers, &None).await?; + join_room_by_id_helper(self.services, &user_id, &room_id, None, &servers).await?; self.write_str(&format!("{user_id} has been joined to {room_id}.")) .await diff --git a/src/admin/utils.rs b/src/admin/utils.rs index ea9696b27..3df7d844a 100644 --- a/src/admin/utils.rs +++ b/src/admin/utils.rs @@ -48,7 +48,7 @@ pub(crate) fn parse_local_user_id(services: &Services, user_id: &str) -> Result< Ok(user_id) } -/// Parses user ID that is an active (not guest or deactivated) local user +/// Parses user ID that is an active (not deactivated) local user pub(crate) async fn parse_active_local_user_id( services: &Services, user_id: &str, diff --git a/src/api/client/account/mod.rs b/src/api/client/account/mod.rs index 29a6b98f8..b1692952a 100644 --- a/src/api/client/account/mod.rs +++ b/src/api/client/account/mod.rs @@ -24,7 +24,7 @@ use ruma::{ power_levels::RoomPowerLevelsEventContent, }, }; -use service::{mailer::messages, uiaa::Identity}; +use service::{mailer::messages, uiaa::Identity, users::HashedPassword}; use super::{DEVICE_ID_LENGTH, TOKEN_LENGTH, join_room_by_id_helper}; use crate::Ruma; @@ -150,8 +150,7 @@ pub(crate) async fn change_password_route( services .users - .set_password(&sender_user, Some(&body.new_password)) - .await?; + .set_password(&sender_user, Some(HashedPassword::new(&body.new_password)?)); if body.logout_devices { // Logout all devices except the current one @@ -239,19 +238,11 @@ pub(crate) async fn request_password_change_token_via_email_route( /// /// Note: Also works for Application Services pub(crate) async fn whoami_route( - State(services): State, + State(_): State, body: Ruma, ) -> Result { - let is_guest = services - .users - .is_deactivated(body.sender_user()) - .await - .map_err(|_| { - err!(Request(Forbidden("Application service has not registered this user."))) - })? && body.appservice_info.is_none(); - - Ok(assign!(whoami::v3::Response::new(body.sender_user().to_owned(), is_guest), { - device_id: body.sender_device.clone(), + Ok(assign!(whoami::v3::Response::new(body.sender_user().to_owned(), false), { + device_id: body.sender_device, })) } diff --git a/src/api/client/account/register.rs b/src/api/client/account/register.rs index 16fb84be8..1e765a558 100644 --- a/src/api/client/account/register.rs +++ b/src/api/client/account/register.rs @@ -10,7 +10,6 @@ use conduwuit::{ use conduwuit_service::Services; use futures::{FutureExt, StreamExt}; use lettre::{Address, message::Mailbox}; -use register::RegistrationKind; use ruma::{ OwnedUserId, UserId, api::client::{ @@ -28,7 +27,7 @@ use ruma::{ push, }; use serde_json::value::RawValue; -use service::mailer::messages; +use service::{mailer::messages, users::HashedPassword}; use super::{DEVICE_ID_LENGTH, TOKEN_LENGTH, join_room_by_id_helper}; use crate::Ruma; @@ -42,16 +41,6 @@ const RANDOM_USER_ID_LENGTH: usize = 10; /// You can use [`GET /// /_matrix/client/v3/register/available`](fn.get_register_available_route. /// html) to check if the user id is valid and available. -/// -/// - Only works if registration is enabled -/// - If type is guest: ignores all parameters except -/// initial_device_display_name -/// - If sender is not appservice: Requires UIAA (but we only use a dummy stage) -/// - If type is not guest and no username is given: Always fails after UIAA -/// check -/// - Creates a new account and populates it with default account data -/// - If `inhibit_login` is false: Creates a device and returns device id and -/// access_token #[allow(clippy::doc_markdown)] #[tracing::instrument(skip_all, fields(%client), name = "register", level = "info")] pub(crate) async fn register_route( @@ -59,7 +48,6 @@ pub(crate) async fn register_route( ClientIp(client): ClientIp, body: Ruma, ) -> Result { - let is_guest = body.kind == RegistrationKind::Guest; let emergency_mode_enabled = services.config.emergency_password.is_some(); // Allow registration if it's enabled in the config file or if this is the first @@ -68,69 +56,19 @@ pub(crate) async fn register_route( services.config.allow_registration || services.firstrun.is_first_run(); if !allow_registration && body.appservice_info.is_none() { - match (body.username.as_ref(), body.initial_device_display_name.as_ref()) { - | (Some(username), Some(device_display_name)) => { - info!( - %is_guest, - user = %username, - device_name = %device_display_name, - "Rejecting registration attempt as registration is disabled" - ); - }, - | (Some(username), _) => { - info!( - %is_guest, - user = %username, - "Rejecting registration attempt as registration is disabled" - ); - }, - | (_, Some(device_display_name)) => { - info!( - %is_guest, - device_name = %device_display_name, - "Rejecting registration attempt as registration is disabled" - ); - }, - | (None, _) => { - info!( - %is_guest, - "Rejecting registration attempt as registration is disabled" - ); - }, - } - - return Err!(Request(Forbidden( - "This server is not accepting registrations at this time." - ))); - } - - if is_guest && !services.config.allow_guest_registration { info!( - "Guest registration disabled, rejecting guest registration attempt, initial device \ - name: \"{}\"", - body.initial_device_display_name.as_deref().unwrap_or("") + ?body.username, + ?body.initial_device_display_name, + "Rejecting registration attempt as registration is disabled" ); - return Err!(Request(GuestAccessForbidden("Guest registration is disabled."))); - } - // forbid guests from registering if there is not a real admin user yet. give - // generic user error. - if is_guest && services.firstrun.is_first_run() { - warn!( - "Guest account attempted to register before a real admin user has been registered, \ - rejecting registration. Guest's initial device name: \"{}\"", - body.initial_device_display_name.as_deref().unwrap_or("") - ); return Err!(Request(Forbidden( "This server is not accepting registrations at this time." ))); } - // Appeservices and guests get to skip auth - let skip_auth = body.appservice_info.is_some() || is_guest; - - let identity = if skip_auth { - // Appservices and guests have no identity + let identity = if body.appservice_info.is_some() { + // Appservices can skip auth None } else { // Perform UIAA to determine the user's identity @@ -157,13 +95,9 @@ pub(crate) async fn register_route( } }); - let user_id = determine_registration_user_id( - &services, - supplied_username, - is_guest, - emergency_mode_enabled, - ) - .await?; + let user_id = + determine_registration_user_id(&services, supplied_username, emergency_mode_enabled) + .await?; if body.body.login_type == Some(LoginType::ApplicationService) { // For appservice logins, make sure that the user ID is in the appservice's @@ -187,7 +121,13 @@ pub(crate) async fn register_route( return Err!(Request(Exclusive("Username is reserved by an appservice."))); } - let password = if is_guest { None } else { body.password.as_deref() }; + let password = if body.appservice_info.is_some() { + None + } else if let Some(password) = body.password.as_deref() { + Some(HashedPassword::new(password)?) + } else { + return Err!(Request(InvalidParam("A password must be provided"))); + }; // Create user services.users.create(&user_id, password).await?; @@ -222,7 +162,9 @@ pub(crate) async fn register_route( // Generate new device id if the user didn't specify one let (token, device) = if !body.inhibit_login { - let device_id = if is_guest { None } else { body.device_id.clone() } + let device_id = body + .device_id + .clone() .unwrap_or_else(|| utils::random_string(DEVICE_ID_LENGTH).into()); // Generate new token for the device @@ -263,8 +205,7 @@ pub(crate) async fn register_route( let device_display_name = body.initial_device_display_name.as_deref().unwrap_or(""); - // log in conduit admin channel if a non-guest user registered - if body.appservice_info.is_none() && !is_guest { + if body.appservice_info.is_none() { if !device_display_name.is_empty() { let notice = format!( "New user \"{user_id}\" registered on this server from IP {client} and device \ @@ -285,65 +226,32 @@ pub(crate) async fn register_route( } } - // log in conduit admin channel if a guest registered - if body.appservice_info.is_none() && is_guest && services.config.log_guest_registrations { - debug_info!("New guest user \"{user_id}\" registered on this server."); + // Make the first user to register an administrator and disable first-run mode. + let was_first_user = services.firstrun.empower_first_user(&user_id).await?; - if !device_display_name.is_empty() { - if services.server.config.admin_room_notices { - services - .admin - .notice(&format!( - "Guest user \"{user_id}\" with device display name \ - \"{device_display_name}\" registered on this server from IP {client}" - )) - .await; - } - } else { - #[allow(clippy::collapsible_else_if)] - if services.server.config.admin_room_notices { - services - .admin - .notice(&format!( - "Guest user \"{user_id}\" with no device display name registered on \ - this server from IP {client}", - )) - .await; - } - } - } - - if !is_guest { - // Make the first user to register an administrator and disable first-run mode. - let was_first_user = services.firstrun.empower_first_user(&user_id).await?; - - // If the registering user was not the first and we're suspending users on - // register, suspend them. - if !was_first_user && services.config.suspend_on_register { - // Note that we can still do auto joins for suspended users + // If the registering user was not the first and we're suspending users on + // register, suspend them. + if !was_first_user && services.config.suspend_on_register { + // Note that we can still do auto joins for suspended users + services + .users + .suspend_account(&user_id, &services.globals.server_user) + .await; + // And send an @room notice to the admin room, to prompt admins to review the + // new user and ideally unsuspend them if deemed appropriate. + if services.server.config.admin_room_notices { services - .users - .suspend_account(&user_id, &services.globals.server_user) - .await; - // And send an @room notice to the admin room, to prompt admins to review the - // new user and ideally unsuspend them if deemed appropriate. - if services.server.config.admin_room_notices { - services - .admin - .send_loud_message(RoomMessageEventContent::text_plain(format!( - "User {user_id} has been suspended as they are not the first user on \ - this server. Please review and unsuspend them if appropriate." - ))) - .await - .ok(); - } + .admin + .send_loud_message(RoomMessageEventContent::text_plain(format!( + "User {user_id} has been suspended as they are not the first user on this \ + server. Please review and unsuspend them if appropriate." + ))) + .await + .ok(); } } - if body.appservice_info.is_none() - && !services.server.config.auto_join_rooms.is_empty() - && (services.config.allow_guests_auto_join_rooms || !is_guest) - { + if body.appservice_info.is_none() && !services.server.config.auto_join_rooms.is_empty() { for room in &services.server.config.auto_join_rooms { let Ok(room_id) = services.rooms.alias.resolve(room).await else { error!( @@ -372,7 +280,6 @@ pub(crate) async fn register_route( &room_id, Some("Automatically joining this room upon registration".to_owned()), &[services.globals.server_name().to_owned(), room_server_name.to_owned()], - &body.appservice_info, ) .boxed() .await @@ -511,12 +418,9 @@ async fn create_registration_uiaa_session( async fn determine_registration_user_id( services: &Services, supplied_username: Option, - is_guest: bool, emergency_mode_enabled: bool, ) -> Result { - if let Some(supplied_username) = supplied_username - && !is_guest - { + if let Some(supplied_username) = supplied_username { // The user gets to pick their username. Do some validation to make sure it's // acceptable. @@ -569,7 +473,7 @@ async fn determine_registration_user_id( Ok(user_id) } else { - // The user is a guest or didn't specify a username. Generate a username for + // The user didn't specify a username. Generate a username for // them. loop { diff --git a/src/api/client/directory.rs b/src/api/client/directory.rs index 33bbd5ea0..f4dff5049 100644 --- a/src/api/client/directory.rs +++ b/src/api/client/directory.rs @@ -122,16 +122,6 @@ pub(crate) async fn set_room_visibility_route( return Err!(Request(UserSuspended("You cannot perform this action while suspended."))); } - if services - .users - .is_deactivated(sender_user) - .await - .unwrap_or(false) - && body.appservice_info.is_none() - { - return Err!(Request(Forbidden("Guests cannot publish to room directories"))); - } - if !user_can_publish_room(&services, sender_user, &body.room_id).await? { return Err!(Request(Forbidden("User is not allowed to publish this room"))); } diff --git a/src/api/client/membership/join.rs b/src/api/client/membership/join.rs index 248d2a897..b708a145a 100644 --- a/src/api/client/membership/join.rs +++ b/src/api/client/membership/join.rs @@ -39,7 +39,6 @@ use ruma::{ }; use service::{ Services, - appservice::RegistrationInfo, rooms::{ state::RoomMutexGuard, state_compressor::{CompressedState, HashSetCompressStateEvent}, @@ -112,16 +111,9 @@ pub(crate) async fn join_room_by_id_route( shuffle(&mut servers); let servers = deprioritize(servers, &services.config.deprioritize_joins_through_servers); - join_room_by_id_helper( - &services, - sender_user, - &body.room_id, - body.reason.clone(), - &servers, - &body.appservice_info, - ) - .boxed() - .await + join_room_by_id_helper(&services, sender_user, &body.room_id, body.reason.clone(), &servers) + .boxed() + .await } /// # `POST /_matrix/client/r0/join/{roomIdOrAlias}` @@ -140,7 +132,6 @@ pub(crate) async fn join_room_by_id_or_alias_route( body: Ruma, ) -> Result { let sender_user = body.sender_user(); - let appservice_info = &body.appservice_info; let body = &body.body; if services.users.is_suspended(sender_user).await? { return Err!(Request(UserSuspended("You cannot perform this action while suspended."))); @@ -235,16 +226,10 @@ pub(crate) async fn join_room_by_id_or_alias_route( }; let servers = deprioritize(servers, &services.config.deprioritize_joins_through_servers); - let join_room_response = join_room_by_id_helper( - &services, - sender_user, - &room_id, - body.reason.clone(), - &servers, - appservice_info, - ) - .boxed() - .await?; + let join_room_response = + join_room_by_id_helper(&services, sender_user, &room_id, body.reason.clone(), &servers) + .boxed() + .await?; Ok(join_room_by_id_or_alias::v3::Response::new(join_room_response.room_id)) } @@ -255,21 +240,9 @@ pub async fn join_room_by_id_helper( room_id: &RoomId, reason: Option, servers: &[OwnedServerName], - appservice_info: &Option, ) -> Result { let state_lock = services.rooms.state.mutex.lock(room_id).await; - let user_is_guest = services - .users - .is_deactivated(sender_user) - .await - .unwrap_or(false) - && appservice_info.is_none(); - - if user_is_guest && !services.rooms.state_accessor.guest_can_join(room_id).await { - return Err!(Request(Forbidden("Guests are not allowed to join this room"))); - } - if services .rooms .state_cache diff --git a/src/api/client/membership/knock.rs b/src/api/client/membership/knock.rs index 0f86213af..b19cc232c 100644 --- a/src/api/client/membership/knock.rs +++ b/src/api/client/membership/knock.rs @@ -238,15 +238,8 @@ async fn knock_room_by_id_helper( // join_room_by_id_helper We need to release the lock here and let // join_room_by_id_helper acquire it again drop(state_lock); - match join_room_by_id_helper( - services, - sender_user, - room_id, - reason.clone(), - servers, - &None, - ) - .await + match join_room_by_id_helper(services, sender_user, room_id, reason.clone(), servers) + .await { | Ok(_) => return Ok(knock_room::v3::Response::new(room_id.to_owned())), | Err(e) => { diff --git a/src/api/client/session.rs b/src/api/client/session.rs index 4fda0b7ce..e292566cc 100644 --- a/src/api/client/session.rs +++ b/src/api/client/session.rs @@ -4,10 +4,9 @@ use axum::extract::State; use axum_client_ip::ClientIp; use conduwuit::{ Err, Result, debug, err, info, - utils::{self, ReadyExt, hash, stream::BroadbandExt}, + utils::{self, ReadyExt, stream::BroadbandExt}, warn, }; -use conduwuit_core::debug_error; use conduwuit_service::Services; use futures::StreamExt; use lettre::Address; @@ -54,37 +53,6 @@ pub(crate) async fn get_login_types_route( ])) } -/// Authenticates the given user by its ID and its password. -/// -/// Returns the user ID if successful, and an error otherwise. -#[tracing::instrument(skip_all, fields(%user_id), name = "password", level = "debug")] -pub(crate) async fn password_login( - services: &Services, - user_id: &UserId, - lowercased_user_id: &UserId, - password: &str, -) -> Result { - let (hash, user_id) = match services.users.password_hash(user_id).await { - | Ok(hash) => (hash, user_id), - | Err(_) => services - .users - .password_hash(lowercased_user_id) - .await - .map(|hash| (hash, lowercased_user_id)) - .map_err(|_| err!(Request(Forbidden("Invalid identifier or password."))))?, - }; - - if hash.is_empty() { - return Err!(Request(UserDeactivated("The user has been deactivated"))); - } - - hash::verify_password(password, &hash) - .inspect_err(|e| debug_error!("{e}")) - .map_err(|_| err!(Request(Forbidden("Invalid identifier or password."))))?; - - Ok(user_id.to_owned()) -} - pub(crate) async fn handle_login( services: &Services, identifier: Option<&UserIdentifier>, @@ -115,15 +83,7 @@ pub(crate) async fn handle_login( UserId::parse_with_server_name(user_id_or_localpart, &services.config.server_name) .map_err(|_| err!(Request(InvalidUsername("User ID is malformed"))))?; - let lowercased_user_id = UserId::parse_with_server_name( - user_id.localpart().to_lowercase(), - &services.config.server_name, - ) - .unwrap(); - - if !services.globals.user_is_local(&user_id) - || !services.globals.user_is_local(&lowercased_user_id) - { + if !services.globals.user_is_local(&user_id) { return Err!(Request(InvalidParam("User ID does not belong to this homeserver"))); } @@ -136,7 +96,7 @@ pub(crate) async fn handle_login( return Err!(Request(Forbidden("This account is not permitted to log in."))); } - password_login(services, &user_id, &lowercased_user_id, password).await + services.users.check_password(&user_id, password).await } /// # `POST /_matrix/client/v3/login` diff --git a/src/api/client/unversioned.rs b/src/api/client/unversioned.rs index a92d3b206..c2f887a96 100644 --- a/src/api/client/unversioned.rs +++ b/src/api/client/unversioned.rs @@ -80,7 +80,7 @@ pub(crate) async fn conduwuit_server_version() -> Result { /// /// conduwuit-specific API to return the amount of users registered on this /// homeserver. Endpoint is disabled if federation is disabled for privacy. This -/// only includes active users (not deactivated, no guests, etc) +/// only includes active users (not deactivated, etc) pub(crate) async fn conduwuit_local_user_count( State(services): State, ) -> Result { diff --git a/src/core/config/mod.rs b/src/core/config/mod.rs index 05fdb001b..eb28d4dda 100644 --- a/src/core/config/mod.rs +++ b/src/core/config/mod.rs @@ -1486,21 +1486,6 @@ pub struct Config { #[serde(default)] pub brotli_compression: bool, - /// Set to true to allow user type "guest" registrations. Some clients like - /// Element attempt to register guest users automatically. - #[serde(default)] - pub allow_guest_registration: bool, - - /// Set to true to log guest registrations in the admin room. Note that - /// these may be noisy or unnecessary if you're a public homeserver. - #[serde(default)] - pub log_guest_registrations: bool, - - /// Set to true to allow guest registrations/users to auto join any rooms - /// specified in `auto_join_rooms`. - #[serde(default)] - pub allow_guests_auto_join_rooms: bool, - /// Enable the legacy unauthenticated Matrix media repository endpoints. /// These endpoints consist of: /// - /_matrix/media/*/config diff --git a/src/service/appservice/mod.rs b/src/service/appservice/mod.rs index c21b6fa05..d492f3188 100644 --- a/src/service/appservice/mod.rs +++ b/src/service/appservice/mod.rs @@ -121,10 +121,7 @@ impl Service { .unwrap_or(false) { // Reactivate the appservice user if it was accidentally deactivated - self.services - .users - .set_password(&appservice_user_id, None) - .await?; + self.services.users.set_password(&appservice_user_id, None); } self.registration_info diff --git a/src/service/emergency/mod.rs b/src/service/emergency/mod.rs index 5617ac821..31fa8604c 100644 --- a/src/service/emergency/mod.rs +++ b/src/service/emergency/mod.rs @@ -9,7 +9,10 @@ use ruma::{ push::Ruleset, }; -use crate::{Dep, account_data, config, globals, users}; +use crate::{ + Dep, account_data, config, globals, + users::{self, HashedPassword}, +}; pub struct Service { services: Services, @@ -51,10 +54,15 @@ impl Service { async fn set_emergency_access(&self) -> Result { let server_user = &self.services.globals.server_user; - self.services - .users - .set_password(server_user, self.services.config.emergency_password.as_deref()) - .await?; + self.services.users.set_password( + server_user, + self.services + .config + .emergency_password + .as_deref() + .map(HashedPassword::new) + .transpose()?, + ); let (ruleset, pwd_set) = match self.services.config.emergency_password { | Some(_) => (Ruleset::server_default(server_user), true), diff --git a/src/service/password_reset/mod.rs b/src/service/password_reset/mod.rs index a4a3f44ef..2de82d988 100644 --- a/src/service/password_reset/mod.rs +++ b/src/service/password_reset/mod.rs @@ -6,7 +6,10 @@ use conduwuit::{Err, Result, utils}; use data::{Data, ResetTokenInfo}; use ruma::OwnedUserId; -use crate::{Dep, globals, users}; +use crate::{ + Dep, globals, + users::{self, HashedPassword}, +}; pub const PASSWORD_RESET_PATH: &str = "/_continuwuity/account/reset_password"; pub const RESET_TOKEN_QUERY_PARAM: &str = "token"; @@ -100,8 +103,7 @@ impl Service { self.db.remove_token(&token); self.services .users - .set_password(&info.user, Some(new_password)) - .await?; + .set_password(&info.user, Some(HashedPassword::new(new_password)?)); } Ok(()) diff --git a/src/service/rooms/state_cache/mod.rs b/src/service/rooms/state_cache/mod.rs index 4620b2509..c54970638 100644 --- a/src/service/rooms/state_cache/mod.rs +++ b/src/service/rooms/state_cache/mod.rs @@ -238,7 +238,7 @@ pub async fn room_joined_count(&self, room_id: &RoomId) -> Result { #[implement(Service)] #[tracing::instrument(skip(self), level = "debug")] /// Returns an iterator of all our local users in the room, even if they're -/// deactivated/guests +/// deactivated pub fn local_users_in_room<'a>( &'a self, room_id: &'a RoomId, @@ -248,7 +248,7 @@ pub fn local_users_in_room<'a>( } /// Returns an iterator of all our local joined users in a room who are -/// active (not deactivated, not guest) +/// active (not deactivated) #[implement(Service)] #[tracing::instrument(skip(self), level = "trace")] pub fn active_local_users_in_room<'a>( diff --git a/src/service/uiaa/mod.rs b/src/service/uiaa/mod.rs index 7ce165dac..7fa0ef334 100644 --- a/src/service/uiaa/mod.rs +++ b/src/service/uiaa/mod.rs @@ -4,7 +4,7 @@ use std::{ sync::Arc, }; -use conduwuit::{Err, Error, Result, error, utils, utils::hash}; +use conduwuit::{Err, Error, Result, error, utils}; use lettre::Address; use ruma::{ UserId, @@ -377,15 +377,13 @@ impl Service { )); }; - // Check if password is correct - let mut password_verified = false; - - // First try local password hash verification - if let Ok(hash) = self.services.users.password_hash(&user_id).await { - password_verified = hash::verify_password(password, &hash).is_ok(); - } - - if password_verified { + if self + .services + .users + .check_password(&user_id, password) + .await + .is_ok() + { identity.try_set_localpart(user_id.localpart().to_owned())?; Ok(AuthType::Password) diff --git a/src/service/users/mod.rs b/src/service/users/mod.rs index cea0693f1..1524e3dda 100644 --- a/src/service/users/mod.rs +++ b/src/service/users/mod.rs @@ -3,7 +3,7 @@ pub(super) mod dehydrated_device; use std::{collections::BTreeMap, mem, net::IpAddr, sync::Arc}; use conduwuit::{ - Err, Error, Result, Server, debug_warn, err, trace, + Err, Error, Result, Server, debug_error, debug_warn, err, trace, utils::{self, ReadyExt, stream::TryIgnore, string::Unquoted}, }; use database::{Deserialized, Ignore, Interfix, Json, Map}; @@ -38,6 +38,19 @@ pub struct UserSuspension { pub suspended_by: String, } +/// A password hash. This is only for use when setting a user's password, +/// if the hash needs to be kept around for a while without keeping the password +/// in memory. +pub struct HashedPassword(String); + +impl HashedPassword { + pub fn new(password: &str) -> Result { + Ok(Self(utils::hash::password(password).map_err(|e| { + err!(Request(InvalidParam("Password does not meet the requirements: {e}"))) + })?)) + } +} + pub struct Service { services: Services, db: Data, @@ -171,16 +184,23 @@ impl Service { /// Create a new user account on this homeserver. #[inline] - pub async fn create(&self, user_id: &UserId, password: Option<&str>) -> Result<()> { + pub async fn create(&self, user_id: &UserId, password: Option) -> Result<()> { if !self.services.globals.user_is_local(user_id) && password.is_some() { return Err!("Cannot create a nonlocal user with a set password"); } - self.set_password(user_id, password).await?; + self.set_password(user_id, password); Ok(()) } + // /// Create a new account for a local human or bot user. + // pub async fn create_local_account( + // &self, + // username: String, + // password: + // ) + /// Deactivate account pub async fn deactivate_account(&self, user_id: &UserId) -> Result<()> { // Remove all associated devices @@ -192,7 +212,7 @@ impl Service { // result in an empty string, so the user will not be able to log in again. // Systems like changing the password without logging in should check if the // account is deactivated. - self.set_password(user_id, None).await?; + self.set_password(user_id, None); // TODO: Unhook 3PID Ok(()) @@ -338,25 +358,37 @@ impl Service { .ready_filter_map(|(u, p): (OwnedUserId, &[u8])| (!p.is_empty()).then_some(u)) } - /// Returns the password hash for the given user. - pub async fn password_hash(&self, user_id: &UserId) -> Result { - self.db.userid_password.get(user_id).await.deserialized() + /// Set a user's password. + pub fn set_password(&self, user_id: &UserId, password: Option) { + if let Some(hash) = password { + self.db.userid_password.insert(user_id, hash.0); + } else { + self.db.userid_password.insert(user_id, b""); + } } - /// Hash and set the user's password to the Argon2 hash - pub async fn set_password(&self, user_id: &UserId, password: Option<&str>) -> Result<()> { - password - .map(utils::hash::password) - .transpose() - .map_err(|e| { - err!(Request(InvalidParam("Password does not meet the requirements: {e}"))) - })? - .map_or_else( - || self.db.userid_password.insert(user_id, b""), - |hash| self.db.userid_password.insert(user_id, hash), - ); + /// Check a user's password. + pub async fn check_password(&self, user_id: &UserId, password: &str) -> Result { + let (hash, user_id): (String, OwnedUserId) = + if let Ok(hash) = self.db.userid_password.get(user_id).await.deserialized() { + (hash, user_id.to_owned()) + } else { + // We also check the lowercased version of the user ID to handle legacy user IDs + // better + let lowercase_user_id = UserId::parse(user_id.as_str().to_lowercase()).unwrap(); - Ok(()) + if let Ok(hash) = self.db.userid_password.get(user_id).await.deserialized() { + (hash, lowercase_user_id) + } else { + return Err!(Request(UserDeactivated("This user is deactivated."))); + } + }; + + utils::hash::verify_password(password, &hash) + .inspect_err(|e| debug_error!("{e}")) + .map_err(|_| err!(Request(Forbidden("Invalid identifier or password."))))?; + + Ok(user_id) } /// Returns the displayname of a user on this homeserver.