From 7fe721247c6eae87963a6638bc2d4164f426d6ac Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Tue, 6 Sep 2022 20:59:41 -0700 Subject: [PATCH] Allow mods/admins to log in when server is full. As a side effect, this moves the initial game server sync message into the login code, since that's the first place we can check for admin permissions and we want to avoid sending large messages to users who are not authenticated (especially if the player cap has been reached; previously, the player cap check limited the damage that could be done by unauthenticated players). Some fallout from this is that we don't synchronize the Player component anymore, which had some minor effects on voxygen. This update also breaks Torvus, since Client::new now expects the username and password to be provided from the getgo--an accompanying MR will be submitted to fix it. --- CHANGELOG.md | 1 + client/examples/chat-cli/main.rs | 15 +++--- client/src/bin/bot/main.rs | 47 +++++++++++------ client/src/bin/swarm/main.rs | 18 ++++--- client/src/lib.rs | 68 ++++++++++++++---------- common/net/src/msg/server.rs | 2 +- common/net/src/synced_components.rs | 9 +--- server/src/lib.rs | 79 +++------------------------- server/src/sys/entity_sync.rs | 5 +- server/src/sys/msg/register.rs | 12 ++++- voxygen/src/hud/mod.rs | 6 +-- voxygen/src/menu/main/client_init.rs | 12 ++--- 12 files changed, 119 insertions(+), 155 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0d7a52f0b..3ef623f1a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Fixed +- Moderators and admins are no longer blocked from logging in when there are too many players. ## [0.13.0] - 2022-07-23 diff --git a/client/examples/chat-cli/main.rs b/client/examples/chat-cli/main.rs index ee79e3bc3f..87f9113870 100644 --- a/client/examples/chat-cli/main.rs +++ b/client/examples/chat-cli/main.rs @@ -51,19 +51,20 @@ fn main() { // Create a client. let mut client = runtime - .block_on(Client::new(addr, runtime2, &mut None)) + .block_on(Client::new( + addr, + runtime2, + &mut None, + &username, + &password, + |provider| provider == "https://auth.veloren.net", + )) .expect("Failed to create client instance"); println!("Server info: {:?}", client.server_info()); println!("Players online: {:?}", client.players().collect::>()); - runtime - .block_on(client.register(username, password, |provider| { - provider == "https://auth.veloren.net" - })) - .unwrap(); - let (tx, rx) = mpsc::channel(); thread::spawn(move || { loop { diff --git a/client/src/bin/bot/main.rs b/client/src/bin/bot/main.rs index d4529199f2..fa34344ffd 100644 --- a/client/src/bin/bot/main.rs +++ b/client/src/bin/bot/main.rs @@ -13,6 +13,7 @@ mod settings; mod tui; use common::comp::body::humanoid::Body; +use common_net::msg::ServerInfo; use settings::Settings; use tui::Cmd; @@ -46,31 +47,47 @@ pub fn main() { pub struct BotClient { settings: Settings, runtime: Arc, - menu_client: Client, + server_info: ServerInfo, bot_clients: HashMap, clock: Clock, } -pub fn make_client(runtime: &Arc, server: &str) -> Client { +pub fn make_client( + runtime: &Arc, + server: &str, + server_info: &mut Option, + username: &str, + password: &str, +) -> Option { let runtime_clone = Arc::clone(runtime); let addr = ConnectionArgs::Tcp { prefer_ipv6: false, hostname: server.to_owned(), }; runtime - .block_on(Client::new(addr, runtime_clone, &mut None)) - .expect("Failed to connect to server") + .block_on(Client::new( + addr, + runtime_clone, + server_info, + username, + password, + |_| true, + )) + .ok() } impl BotClient { pub fn new(settings: Settings) -> BotClient { let runtime = Arc::new(Runtime::new().unwrap()); - let menu_client: Client = make_client(&runtime, &settings.server); + let mut server_info = None; + // Don't care if we connect, just trying to grab the server info. + let _ = make_client(&runtime, &settings.server, &mut server_info, "", ""); + let server_info = server_info.expect("Failed to connect to server."); let clock = Clock::new(Duration::from_secs_f64(1.0 / 60.0)); BotClient { settings, runtime, - menu_client, + server_info, bot_clients: HashMap::new(), clock, } @@ -106,7 +123,7 @@ impl BotClient { None => vec![prefix.to_string()], }; info!("usernames: {:?}", usernames); - if let Some(auth_addr) = self.menu_client.server_info().auth_provider.as_ref() { + if let Some(auth_addr) = self.server_info.auth_provider.as_ref() { let (scheme, authority) = auth_addr.split_once("://").expect("invalid auth url"); let scheme = scheme .parse::() @@ -156,20 +173,16 @@ impl BotClient { for cred in creds.iter() { let runtime = Arc::clone(&self.runtime); - let server = self.settings.server.clone(); + let server = &self.settings.server; + // TODO: log the clients in in parallel instead of in series let client = self .bot_clients .entry(cred.username.clone()) - .or_insert_with(|| make_client(&runtime, &server)); + .or_insert_with(|| { + make_client(&runtime, server, &mut None, &cred.username, &cred.password) + .expect("Failed to connect to server") + }); - // TODO: log the clients in in parallel instead of in series - if let Err(e) = runtime.block_on(client.register( - cred.username.clone(), - cred.password.clone(), - |_| true, - )) { - warn!("error logging in {:?}: {:?}", cred.username, e); - } let body = BotClient::create_default_body(); client.create_character( cred.username.clone(), diff --git a/client/src/bin/swarm/main.rs b/client/src/bin/swarm/main.rs index c776f8186f..646b9c4834 100644 --- a/client/src/bin/swarm/main.rs +++ b/client/src/bin/swarm/main.rs @@ -11,6 +11,7 @@ use std::{ use structopt::StructOpt; use tokio::runtime::Runtime; use vek::*; +use common_state::State; use veloren_client::{addr::ConnectionArgs, Client}; #[derive(Clone, Copy, StructOpt)] @@ -108,17 +109,20 @@ fn run_client( hostname: "localhost".into(), }; let runtime_clone = Arc::clone(&runtime); + // NOTE: use a no-auth server let mut client = runtime - .block_on(Client::new(addr, runtime_clone, &mut None)) + .block_on(Client::new( + addr, + runtime_clone, + &mut None, + common_state::State::pools(common::resources::GameMode::Client), + &username, + "", + |_| false, + )) .expect("Failed to connect to the server"); client.set_view_distance(opt.vd); - // Login - // NOTE: use a no-auth server - runtime - .block_on(client.register(username.clone(), String::new(), |_| false)) - .expect("Failed to log in"); - let mut clock = common::clock::Clock::new(Duration::from_secs_f32(1.0 / 30.0)); let mut tick = |client: &mut Client| -> Result<(), veloren_client::Error> { diff --git a/client/src/lib.rs b/client/src/lib.rs index f9aed44a8d..02afdda5f5 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -290,6 +290,9 @@ impl Client { // TODO: refactor to avoid needing to use this out parameter mismatched_server_info: &mut Option, pools: common_state::Pools, + username: &str, + password: &str, + auth_trusted: impl FnMut(&str) -> bool, ) -> Result { let network = Network::new(Pid::new(), &runtime); @@ -332,15 +335,24 @@ impl Client { common::util::GIT_HASH.to_string(), common::util::GIT_DATE.to_string(), ); - - // Pass the server info back to the caller to ensure they can access it even - // if this function errors. - mem::swap(mismatched_server_info, &mut Some(server_info.clone())); } + // Pass the server info back to the caller to ensure they can access it even + // if this function errors. + mem::swap(mismatched_server_info, &mut Some(server_info.clone())); debug!("Auth Server: {:?}", server_info.auth_provider); ping_stream.send(PingMsg::Ping)?; + // Register client + Self::register( + username, + password, + auth_trusted, + &server_info, + &mut register_stream, + ) + .await?; + // Wait for initial sync let mut ping_interval = tokio::time::interval(Duration::from_secs(1)); let (terrain_tx_, terrain_rx) = mpsc::bounded(TOTAL_PENDING_CHUNKS_LIMIT); @@ -673,7 +685,7 @@ impl Client { let map_bounds = Vec2::new(sea_level, max_height); debug!("Done preparing image..."); - Ok(( + ( state, lod_base, lod_alt, @@ -685,16 +697,15 @@ impl Client { component_recipe_book, max_group_size, client_timeout, - )) + ) }, - ServerInit::TooManyPlayers => Err(Error::TooManyPlayers), - }?; + }; ping_stream.send(PingMsg::Ping)?; debug!("Initial sync done"); Ok(Self { - registered: false, + registered: true, presence: None, runtime, server_info, @@ -762,14 +773,15 @@ impl Client { } /// Request a state transition to `ClientState::Registered`. - pub async fn register( - &mut self, - username: String, - password: String, + async fn register( + username: &str, + password: &str, mut auth_trusted: impl FnMut(&str) -> bool, + server_info: &ServerInfo, + register_stream: &mut Stream, ) -> Result<(), Error> { // Authentication - let token_or_username = match &self.server_info.auth_provider { + let token_or_username = match &server_info.auth_provider { Some(addr) => { // Query whether this is a trusted auth server if auth_trusted(addr) { @@ -789,26 +801,29 @@ impl Client { }; Ok(authc::AuthClient::new(scheme, authority)? - .sign_in(&username, &password) + .sign_in(username, password) .await? .serialize()) } else { Err(Error::AuthServerNotTrusted) } }, - None => Ok(username), + None => Ok(username.to_owned()), }?; - self.send_msg_err(ClientRegister { token_or_username })?; + debug!("Registering client..."); - match self.register_stream.recv::().await? { + register_stream.send(ClientRegister { token_or_username })?; + + match register_stream.recv::().await? { Err(RegisterError::AuthError(err)) => Err(Error::AuthErr(err)), Err(RegisterError::InvalidCharacter) => Err(Error::InvalidCharacter), Err(RegisterError::NotOnWhitelist) => Err(Error::NotOnWhitelist), Err(RegisterError::Kicked(err)) => Err(Error::Kicked(err)), Err(RegisterError::Banned(reason)) => Err(Error::Banned(reason)), + Err(RegisterError::TooManyPlayers) => Err(Error::TooManyPlayers), Ok(()) => { - self.registered = true; + debug!("Client registered successfully."); Ok(()) }, } @@ -2933,6 +2948,9 @@ mod tests { let runtime = Arc::new(Runtime::new().unwrap()); let runtime2 = Arc::clone(&runtime); + let username = "Foo"; + let password = "Bar"; + let auth_server = "auth.veloren.net"; let veloren_client: Result = runtime.block_on(Client::new( ConnectionArgs::Tcp { hostname: "127.0.0.1:9000".to_owned(), @@ -2940,18 +2958,12 @@ mod tests { }, runtime2, &mut None, + username, + password, + |suggestion: &str| suggestion == auth_server, )); let _ = veloren_client.map(|mut client| { - //register - let username: String = "Foo".to_string(); - let password: String = "Bar".to_string(); - let auth_server: String = "auth.veloren.net".to_string(); - let _result: Result<(), Error> = - runtime.block_on(client.register(username, password, |suggestion: &str| { - suggestion == auth_server - })); - //clock let mut clock = Clock::new(Duration::from_secs_f64(SPT)); diff --git a/common/net/src/msg/server.rs b/common/net/src/msg/server.rs index eed4d42830..ffa2a5ab9a 100644 --- a/common/net/src/msg/server.rs +++ b/common/net/src/msg/server.rs @@ -56,7 +56,6 @@ pub struct ServerInfo { #[derive(Debug, Clone, Serialize, Deserialize)] #[allow(clippy::large_enum_variant)] pub enum ServerInit { - TooManyPlayers, GameSync { entity_package: sync::EntityPackage, time_of_day: TimeOfDay, @@ -274,6 +273,7 @@ pub enum RegisterError { Kicked(String), InvalidCharacter, NotOnWhitelist, + TooManyPlayers, //TODO: InvalidAlias, } diff --git a/common/net/src/synced_components.rs b/common/net/src/synced_components.rs index 2675eec32b..1dfdf57da7 100644 --- a/common/net/src/synced_components.rs +++ b/common/net/src/synced_components.rs @@ -42,9 +42,6 @@ macro_rules! synced_components { shockwave: Shockwave, beam_segment: BeamSegment, alignment: Alignment, - // TODO: evaluate if this is used on the client, - // and if so what it is used for - player: Player, // TODO: change this to `SyncFrom::ClientEntity` and sync the bare minimum // from other entities (e.g. just keys needed to show appearance // based on their loadout). Also, it looks like this actually has @@ -210,10 +207,6 @@ impl NetSync for Alignment { const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity; } -impl NetSync for Player { - const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity; -} - impl NetSync for Inventory { const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity; } @@ -222,7 +215,7 @@ impl NetSync for SkillSet { const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity; } -// These are synced only from the client's own entity. +// These are synced only from the client's own entity. impl NetSync for Combo { const SYNC_FROM: SyncFrom = SyncFrom::ClientEntity; diff --git a/server/src/lib.rs b/server/src/lib.rs index 0114383e04..8295802119 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -65,7 +65,7 @@ use crate::{ presence::{Presence, RegionSubscription, RepositionOnChunkLoad}, rtsim::RtSim, state_ext::StateExt, - sys::sentinel::{DeletedEntities, TrackedStorages}, + sys::sentinel::DeletedEntities, }; #[cfg(not(feature = "worldgen"))] use common::grid::Grid; @@ -76,7 +76,6 @@ use common::{ cmd::ServerChatCommand, comp, event::{EventBus, ServerEvent}, - recipe::{default_component_recipe_book, default_recipe_book}, resources::{BattleMode, Time, TimeOfDay}, rtsim::RtSimEntity, slowjob::SlowJobPool, @@ -85,9 +84,7 @@ use common::{ }; use common_ecs::{dispatch, run_now}; use common_net::{ - msg::{ - ClientType, DisconnectReason, ServerGeneral, ServerInfo, ServerInit, ServerMsg, WorldMapMsg, - }, + msg::{ClientType, DisconnectReason, ServerGeneral, ServerInfo, ServerMsg}, sync::WorldSyncExt, }; use common_state::{BuildAreas, State}; @@ -198,7 +195,6 @@ pub struct Server { state: State, world: Arc, index: IndexOwned, - map: WorldMapMsg, connection_handler: ConnectionHandler, @@ -402,6 +398,8 @@ impl Server { pois: Vec::new(), }; + state.ecs_mut().insert(map); + #[cfg(feature = "worldgen")] let spawn_point = SpawnPoint({ let index = index.as_index_ref(); @@ -576,8 +574,6 @@ impl Server { state, world, index, - map, - connection_handler, runtime, @@ -644,9 +640,6 @@ impl Server { /// Get a reference to the server's world. pub fn world(&self) -> &World { &self.world } - /// Get a reference to the server's world map. - pub fn map(&self) -> &WorldMapMsg { &self.map } - /// Execute a single server tick, handle input and update the game state by /// the given duration. pub fn tick(&mut self, _input: Input, dt: Duration) -> Result, Error> { @@ -1051,19 +1044,7 @@ impl Server { .map(|mut t| t.maintain()); } - fn initialize_client( - &mut self, - client: connection_handler::IncomingClient, - ) -> Result, Error> { - if self.settings().max_players <= self.state.ecs().read_storage::().join().count() { - trace!( - ?client.participant, - "to many players, wont allow participant to connect" - ); - client.send(ServerInit::TooManyPlayers)?; - return Ok(None); - } - + fn initialize_client(&mut self, client: connection_handler::IncomingClient) -> Entity { let entity = self .state .ecs_mut() @@ -1075,43 +1056,7 @@ impl Server { .read_resource::() .clients_connected .inc(); - // Send client all the tracked components currently attached to its entity as - // well as synced resources (currently only `TimeOfDay`) - debug!("Starting initial sync with client."); - self.state - .ecs() - .read_storage::() - .get(entity) - .expect( - "We just created this entity with a Client component using build(), and we have \ - &mut access to the ecs so it can't have been deleted yet.", - ) - .send(ServerInit::GameSync { - // Send client their entity - entity_package: TrackedStorages::fetch(self.state.ecs()) - .create_entity_package(entity, None, None, None) - .expect( - "We just created this entity as marked() (using create_entity_synced) so \ - it definitely has a uid", - ), - time_of_day: *self.state.ecs().read_resource(), - max_group_size: self.settings().max_player_group_size, - client_timeout: self.settings().client_timeout, - world_map: self.map.clone(), - recipe_book: default_recipe_book().cloned(), - component_recipe_book: default_component_recipe_book().cloned(), - material_stats: (&*self - .state - .ecs() - .read_resource::()) - .clone(), - ability_map: (&*self - .state - .ecs() - .read_resource::()) - .clone(), - })?; - Ok(Some(entity)) + entity } /// Disconnects all clients if requested by either an admin command or @@ -1177,16 +1122,8 @@ impl Server { } while let Ok(incoming) = self.connection_handler.client_receiver.try_recv() { - match self.initialize_client(incoming) { - Ok(None) => (), - Ok(Some(entity)) => { - frontend_events.push(Event::ClientConnected { entity }); - debug!("Done initial sync with client."); - }, - Err(e) => { - debug!(?e, "failed initializing a new client"); - }, - } + let entity = self.initialize_client(incoming); + frontend_events.push(Event::ClientConnected { entity }); } } diff --git a/server/src/sys/entity_sync.rs b/server/src/sys/entity_sync.rs index f090ab8c03..22f1b19109 100644 --- a/server/src/sys/entity_sync.rs +++ b/server/src/sys/entity_sync.rs @@ -6,7 +6,7 @@ use crate::{ }; use common::{ calendar::Calendar, - comp::{Collider, ForceUpdate, InventoryUpdate, Last, Ori, Pos, Vel}, + comp::{Collider, ForceUpdate, InventoryUpdate, Last, Ori, Player, Pos, Vel}, event::EventBus, outcome::Outcome, region::{Event as RegionEvent, RegionMap}, @@ -40,6 +40,7 @@ impl<'a> System<'a> for Sys { ReadStorage<'a, Vel>, ReadStorage<'a, Ori>, ReadStorage<'a, RegionSubscription>, + ReadStorage<'a, Player>, ReadStorage<'a, Presence>, ReadStorage<'a, Client>, WriteStorage<'a, Last>, @@ -70,6 +71,7 @@ impl<'a> System<'a> for Sys { velocities, orientations, subscriptions, + players, presences, clients, mut last_pos, @@ -88,7 +90,6 @@ impl<'a> System<'a> for Sys { let uids = &tracked_storages.uid; let colliders = &tracked_storages.collider; let inventories = &tracked_storages.inventory; - let players = &tracked_storages.player; let is_rider = &tracked_storages.is_rider; // To send entity updates diff --git a/server/src/sys/msg/register.rs b/server/src/sys/msg/register.rs index c9ea7e7a46..b93b3bb23f 100644 --- a/server/src/sys/msg/register.rs +++ b/server/src/sys/msg/register.rs @@ -2,17 +2,20 @@ use crate::{ client::Client, login_provider::{LoginProvider, PendingLogin}, metrics::PlayerMetrics, + sys::sentinel::TrackedStorages, EditableSettings, Settings, }; use common::{ - comp::{Admin, Player, Stats}, + comp::{self, Admin, Player, Stats}, event::{EventBus, ServerEvent}, + recipe::{default_component_recipe_book, default_recipe_book}, + resources::TimeOfDay, uid::{Uid, UidAllocator}, }; use common_ecs::{Job, Origin, Phase, System}; use common_net::msg::{ CharacterInfo, ClientRegister, DisconnectReason, PlayerInfo, PlayerListUpdate, RegisterError, - ServerGeneral, + ServerGeneral, ServerInit, WorldMapMsg, }; use hashbrown::{hash_map, HashMap}; use plugin_api::Health; @@ -41,6 +44,11 @@ pub struct ReadData<'a> { player_metrics: ReadExpect<'a, PlayerMetrics>, settings: ReadExpect<'a, Settings>, editable_settings: ReadExpect<'a, EditableSettings>, + time_of_day: Read<'a, TimeOfDay>, + material_stats: ReadExpect<'a, comp::item::MaterialStatManifest>, + ability_map: ReadExpect<'a, comp::item::tool::AbilityMap>, + map: ReadExpect<'a, WorldMapMsg>, + trackers: TrackedStorages<'a>, _healths: ReadStorage<'a, Health>, // used by plugin feature _plugin_mgr: ReadPlugin<'a>, // used by plugin feature _uid_allocator: Read<'a, UidAllocator>, // used by plugin feature diff --git a/voxygen/src/hud/mod.rs b/voxygen/src/hud/mod.rs index 3dc32455d1..68e0123725 100644 --- a/voxygen/src/hud/mod.rs +++ b/voxygen/src/hud/mod.rs @@ -1272,7 +1272,6 @@ impl Hud { let bodies = ecs.read_storage::(); let items = ecs.read_storage::(); let inventories = ecs.read_storage::(); - let players = ecs.read_storage::(); let msm = ecs.read_resource::(); let entities = ecs.entities(); let me = client.entity(); @@ -1971,7 +1970,6 @@ impl Hud { &mut hp_floater_lists, &uids, &inventories, - players.maybe(), poises.maybe(), (alignments.maybe(), is_mount.maybe()), ) @@ -1995,7 +1993,6 @@ impl Hud { hpfl, uid, inventory, - player, poise, (alignment, is_mount), )| { @@ -2006,7 +2003,8 @@ impl Hud { // TODO: once the site2 rework lands and merchants have dedicated stalls or // buildings, they no longer need to be emphasized via the higher overhead // text radius relative to other NPCs - let is_merchant = stats.name == "Merchant" && player.is_none(); + let is_merchant = + stats.name == "Merchant" && client.player_list().get(uid).is_none(); let dist_sqr = pos.distance_squared(player_pos); // Determine whether to display nametag and healthbar based on whether the // entity has been damaged, is targeted/selected, or is in your group diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 20d7893bd2..2d9bbad53c 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -79,17 +79,13 @@ impl ClientInit { Arc::clone(&runtime2), &mut mismatched_server_info, pools.clone(), + &username, + &password, + trust_fn, ) .await { - Ok(mut client) => { - if let Err(e) = client.register(username, password, trust_fn).await { - last_err = Some(Error::ClientError { - error: e, - mismatched_server_info: None, - }); - break 'tries; - } + Ok(client) => { let _ = tx.send(Msg::Done(Ok(client))); drop(pools); tokio::task::block_in_place(move || drop(runtime2));