diff --git a/client/src/error.rs b/client/src/error.rs index 6003bcb046..b81e806286 100644 --- a/client/src/error.rs +++ b/client/src/error.rs @@ -8,8 +8,8 @@ pub enum Error { ServerTimeout, ServerShutdown, TooManyPlayers, - InvalidAuth, AlreadyLoggedIn, + AuthErr(String), AuthClientError(AuthClientError), AuthServerNotTrusted, //TODO: InvalidAlias, diff --git a/client/src/lib.rs b/client/src/lib.rs index fc85c38ff6..71f69f5444 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -18,7 +18,7 @@ use common::{ event::{EventBus, SfxEvent, SfxEventItem}, msg::{ validate_chat_msg, ChatMsgValidationError, ClientMsg, ClientState, PlayerListUpdate, - RequestStateError, ServerError, ServerInfo, ServerMsg, MAX_BYTES_CHAT_MSG, + RegisterError, RequestStateError, ServerInfo, ServerMsg, MAX_BYTES_CHAT_MSG, }, net::PostBox, state::State, @@ -87,13 +87,13 @@ impl Client { let mut postbox = PostBox::to(addr)?; // Wait for initial sync - let (state, entity, server_info, world_map) = match postbox.next_message() { - Some(ServerMsg::InitialSync { + let (state, entity, server_info, world_map) = match postbox.next_message()? { + ServerMsg::InitialSync { entity_package, server_info, time_of_day, world_map: (map_size, world_map), - }) => { + } => { // TODO: Display that versions don't match in Voxygen if server_info.git_hash != common::util::GIT_HASH.to_string() { log::warn!( @@ -132,9 +132,7 @@ impl Client { (state, entity, server_info, (world_map, map_size)) }, - Some(ServerMsg::Error(ServerError::TooManyPlayers)) => { - return Err(Error::TooManyPlayers); - }, + ServerMsg::TooManyPlayers => return Err(Error::TooManyPlayers), _ => return Err(Error::ServerWentMad), }; @@ -203,11 +201,15 @@ impl Client { self.client_state = ClientState::Pending; loop { - match self.postbox.next_message() { - Some(ServerMsg::StateAnswer(Err((RequestStateError::Denied, _)))) => { - break Err(Error::InvalidAuth); + match self.postbox.next_message()? { + ServerMsg::StateAnswer(Err((RequestStateError::RegisterDenied(err), state))) => { + self.client_state = state; + break Err(match err { + RegisterError::AlreadyLoggedIn => Error::AlreadyLoggedIn, + RegisterError::AuthError(err) => Error::AuthErr(err), + }); }, - Some(ServerMsg::StateAnswer(Ok(ClientState::Registered))) => break Ok(()), + ServerMsg::StateAnswer(Ok(ClientState::Registered)) => break Ok(()), _ => {}, } } @@ -572,12 +574,8 @@ impl Client { if new_msgs.len() > 0 { for msg in new_msgs { match msg { - ServerMsg::Error(e) => match e { - ServerError::TooManyPlayers => return Err(Error::ServerWentMad), - ServerError::InvalidAuth => return Err(Error::InvalidAuth), - ServerError::AlreadyLoggedIn => return Err(Error::AlreadyLoggedIn), - ServerError::AuthError(_) => unreachable!(), - //TODO: ServerError::InvalidAlias => return Err(Error::InvalidAlias), + ServerMsg::TooManyPlayers => { + return Err(Error::ServerWentMad); }, ServerMsg::Shutdown => return Err(Error::ServerShutdown), ServerMsg::InitialSync { .. } => return Err(Error::ServerWentMad), @@ -720,10 +718,6 @@ impl Client { self.client_state = state; }, ServerMsg::StateAnswer(Err((error, state))) => { - if error == RequestStateError::Denied { - warn!("Connection denied!"); - return Err(Error::InvalidAuth); - } warn!( "StateAnswer: {:?}. Server thinks client is in state {:?}.", error, state diff --git a/common/src/msg/mod.rs b/common/src/msg/mod.rs index 1baee67e15..1c04fece94 100644 --- a/common/src/msg/mod.rs +++ b/common/src/msg/mod.rs @@ -6,7 +6,7 @@ pub mod server; pub use self::{ client::ClientMsg, ecs_packet::EcsCompPacket, - server::{PlayerListUpdate, RequestStateError, ServerError, ServerInfo, ServerMsg}, + server::{PlayerListUpdate, RegisterError, RequestStateError, ServerInfo, ServerMsg}, }; #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] diff --git a/common/src/msg/server.rs b/common/src/msg/server.rs index 8159dfb874..c517eec665 100644 --- a/common/src/msg/server.rs +++ b/common/src/msg/server.rs @@ -8,14 +8,6 @@ use authc::AuthClientError; use hashbrown::HashMap; use vek::*; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub enum RequestStateError { - Denied, - Already, - Impossible, - WrongMessage, -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ServerInfo { pub name: String, @@ -79,21 +71,28 @@ pub enum ServerMsg { chunk: Result, ()>, }, TerrainBlockUpdates(HashMap, Block>), - Error(ServerError), Disconnect, Shutdown, + TooManyPlayers, } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub enum ServerError { - TooManyPlayers, - InvalidAuth, +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub enum RequestStateError { + RegisterDenied(RegisterError), + Denied, + Already, + Impossible, + WrongMessage, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub enum RegisterError { AlreadyLoggedIn, AuthError(String), //TODO: InvalidAlias, } -impl From for ServerError { +impl From for RegisterError { fn from(err: AuthClientError) -> Self { Self::AuthError(err.to_string()) } } diff --git a/common/src/net/post2.rs b/common/src/net/post2.rs index bae88d25f2..ee9e7303e5 100644 --- a/common/src/net/post2.rs +++ b/common/src/net/post2.rs @@ -119,16 +119,16 @@ impl PostBox { pub fn send_message(&mut self, msg: S) { let _ = self.send_tx.send(msg); } - pub fn next_message(&mut self) -> Option { - if self.error.is_some() { - return None; + pub fn next_message(&mut self) -> Result { + if let Some(e) = self.error.clone() { + return Err(e); } - match self.recv_rx.recv().ok()? { - Ok(msg) => Some(msg), + match self.recv_rx.recv().map_err(|_| Error::ChannelFailure)? { + Ok(msg) => Ok(msg), Err(e) => { - self.error = Some(e); - None + self.error = Some(e.clone()); + Err(e) }, } } diff --git a/server/src/auth_provider.rs b/server/src/auth_provider.rs index 6634e5bf51..4ec2abb306 100644 --- a/server/src/auth_provider.rs +++ b/server/src/auth_provider.rs @@ -1,5 +1,5 @@ use authc::{AuthClient, AuthToken, Uuid}; -use common::msg::ServerError; +use common::msg::RegisterError; use hashbrown::HashMap; use std::str::FromStr; @@ -42,28 +42,26 @@ impl AuthProvider { } } - pub fn query(&mut self, username_or_token: String) -> Result { + pub fn query(&mut self, username_or_token: String) -> Result<(), RegisterError> { // Based on whether auth server is provided or not we expect an username or // token match &self.auth_server { // Token from auth server expected Some(srv) => { log::info!("Validating '{}' token.", &username_or_token); - if let Ok(token) = AuthToken::from_str(&username_or_token) { - match srv.validate(token) { - Ok(uuid) => { - if self.accounts.contains_key(&uuid) { - return Err(ServerError::AlreadyLoggedIn); - } - let username = srv.uuid_to_username(uuid.clone())?; - self.accounts.insert(uuid, username); - Ok(true) - }, - Err(e) => Err(ServerError::from(e)), - } - } else { - Ok(false) + // Parse token + let token = AuthToken::from_str(&username_or_token) + .map_err(|e| RegisterError::AuthError(e.to_string()))?; + // Validate token + let uuid = srv.validate(token)?; + // Check if already logged in + if self.accounts.contains_key(&uuid) { + return Err(RegisterError::AlreadyLoggedIn); } + // Log in + let username = srv.uuid_to_username(uuid.clone())?; + self.accounts.insert(uuid, username); + Ok(()) }, // Username is expected None => { @@ -73,9 +71,9 @@ impl AuthProvider { if !self.accounts.contains_key(&uuid) { log::info!("New User '{}'", username); self.accounts.insert(uuid, username); - Ok(true) + Ok(()) } else { - Err(ServerError::AlreadyLoggedIn) + Err(RegisterError::AlreadyLoggedIn) } }, } diff --git a/server/src/lib.rs b/server/src/lib.rs index 47fc395176..c3cc142a63 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -27,7 +27,7 @@ use common::{ assets, comp, effect::Effect, event::{EventBus, ServerEvent}, - msg::{ClientMsg, ClientState, ServerError, ServerInfo, ServerMsg}, + msg::{ClientMsg, ClientState, ServerInfo, ServerMsg}, net::PostOffice, state::{State, TimeOfDay}, sync::{Uid, WorldSyncExt}, @@ -505,7 +505,7 @@ impl Server { <= self.state.ecs().read_storage::().join().count() { // Note: in this case the client is dropped - client.notify(ServerMsg::Error(ServerError::TooManyPlayers)); + client.notify(ServerMsg::TooManyPlayers); } else { let entity = self .state diff --git a/server/src/sys/message.rs b/server/src/sys/message.rs index 425e10cc42..b7d0fa182d 100644 --- a/server/src/sys/message.rs +++ b/server/src/sys/message.rs @@ -127,10 +127,12 @@ impl<'a> System<'a> for Sys { player, token_or_username, } if player.is_valid() => { - if !accounts.query(token_or_username.clone()).unwrap_or(false) { - // TO-DO: Set a less generic error here. - client.error_state(RequestStateError::Denied); - break; + match accounts.query(token_or_username.clone()) { + Err(err) => { + client.error_state(RequestStateError::RegisterDenied(err)); + break; + }, + Ok(()) => {}, } match client.client_state { ClientState::Connected => { diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index f4fb75a122..0d44cb8c16 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -81,7 +81,7 @@ impl PlayState for MainMenuState { "Server not found".into() }, InitError::ClientError(err) => match err { - client::Error::InvalidAuth => "Invalid credentials".into(), + client::Error::AuthErr(e) => format!("Auth error on server: {}", e), client::Error::TooManyPlayers => "Server is full".into(), client::Error::AuthServerNotTrusted => { "Auth server not trusted".into()