Move password check out of LoginRequest.

src/client.rs

@@ -94,14 +94,20 @@ impl Client {
   /// Runs the client, potentially forever.
   pub fn run(&mut self) {
     info!("Logging in...");
+
+    let credentials = server::Credentials::new(
+      config::USERNAME.to_string(),
+      config::PASSWORD.to_string(),
+    )
+    .unwrap();
+
+    let version = server::Version {
+      major: config::VER_MAJOR,
+      minor: config::VER_MINOR,
+    };
+
     self.send_to_server(server::ServerRequest::LoginRequest(
-      server::LoginRequest::new(
-        config::USERNAME,
-        config::PASSWORD,
-        config::VER_MAJOR,
-        config::VER_MINOR,
-      )
-      .unwrap(),
+      credentials.into_login_request(version),
     ));
 
     self.login_status = LoginStatus::AwaitingResponse;

src/proto/core/value.rs

@@ -15,6 +15,7 @@
 //!
 //! See the `frame` module for the codec applied to byte streams.
 
+use std::error;
 use std::io;
 use std::net;
 
@@ -36,6 +37,8 @@ pub trait Encode<T> {
   fn encode(&mut self, value: T) -> io::Result<()>;
 }
 
+pub trait ComparableError: error::Error + PartialEq {}
+
 // TODO: Add backtrace fields to each enum variant once std::backtrace is
 // stabilized.
 #[derive(PartialEq, Error, Debug)]

src/proto/packet.rs

@@ -82,7 +82,7 @@ impl MutPacket {
   /// Provides the main way to write data into a binary packet.
   pub fn write_value<T>(&mut self, val: &T) -> io::Result<()>
   where
-    T: WriteToPacket,
+    T: WriteToPacket + ?Sized,
   {
     val.write_to_packet(self)
   }

src/proto/server/client.rs

@@ -9,7 +9,7 @@ use tokio::net;
 
 use crate::proto::core::frame::FrameStream;
 use crate::proto::server::{
-  LoginRequest, LoginResponse, ServerRequest, ServerResponse,
+  Credentials, LoginResponse, ServerRequest, ServerResponse,
 };
 
 /// Specifies a protocol version.
@@ -32,8 +32,7 @@ impl Default for Version {
 
 /// Specifies options for a new `Client`.
 pub struct ClientOptions {
-  pub user_name: String,
-  pub password: String,
+  pub credentials: Credentials,
   pub version: Version,
 }
 
@@ -45,9 +44,6 @@ pub struct Client {
 /// An error that arose while logging in to a remote server.
 #[derive(Debug, Error)]
 pub enum ClientLoginError {
-  #[error("invalid credentials: {0}")]
-  InvalidCredentials(String),
-
   #[error("login failed: {0}")]
   LoginFailed(String),
 
@@ -83,7 +79,7 @@ impl Client {
       frame_stream: FrameStream::new(tcp_stream),
     };
 
-    client.handshake(&options).await?;
+    client.handshake(options).await?;
 
     Ok(client)
   }
@@ -92,17 +88,11 @@ impl Client {
   // Called this way because `login` is already taken.
   async fn handshake(
     &mut self,
-    options: &ClientOptions,
+    options: ClientOptions,
   ) -> Result<(), ClientLoginError> {
-    let login_request = LoginRequest::new(
-      &options.user_name,
-      &options.password,
-      options.version.major,
-      options.version.minor,
-    )
-    .map_err(|err| ClientLoginError::InvalidCredentials(err.to_string()))?;
-
-    let request = ServerRequest::LoginRequest(login_request);
+    let request = ServerRequest::LoginRequest(
+      options.credentials.into_login_request(options.version),
+    );
     self.frame_stream.write(&request).await?;
 
     let response = self.frame_stream.read().await?;
@@ -188,7 +178,7 @@ mod tests {
   use tokio::net;
 
   use crate::proto::server::testing::fake_server;
-  use crate::proto::server::*;
+  use crate::proto::server::{Credentials, ServerRequest, UserStatusRequest};
 
   use super::{Client, ClientOptions, Version};
 
@@ -206,9 +196,12 @@ mod tests {
 
     let stream = net::TcpStream::connect(handle.address()).await.unwrap();
 
+    let user_name = "alice".to_string();
+    let password = "sekrit".to_string();
+    let credentials = Credentials::new(user_name, password).unwrap();
+
     let options = ClientOptions {
-      user_name: "alice".to_string(),
-      password: "sekrit".to_string(),
+      credentials,
       version: Version::default(),
     };
     let client = Client::login(stream, options).await.unwrap();

src/proto/server/credentials.rs

@@ -1,10 +1,16 @@
 //! Defines the `Credentials` struct, for use when logging in to a server.
 
+use crypto::digest::Digest;
+use crypto::md5::Md5;
+
+use crate::proto::server::{LoginRequest, Version};
+
 /// Credentials for logging in a client to a server.
 #[derive(Debug, Eq, PartialEq)]
 pub struct Credentials {
   user_name: String,
   password: String,
+  digest: String,
 }
 
 impl Credentials {
@@ -16,9 +22,15 @@ impl Credentials {
       return None;
     }
 
+    let mut hasher = Md5::new();
+    hasher.input_str(&user_name);
+    hasher.input_str(&password);
+    let digest = hasher.result_str();
+
     Some(Credentials {
       user_name,
       password,
+      digest,
     })
   }
 
@@ -31,10 +43,27 @@ impl Credentials {
   pub fn password(&self) -> &str {
     &self.password
   }
+
+  /// Returns md5(user_name + password).
+  pub fn digest(&self) -> &str {
+    &self.digest
+  }
+
+  pub fn into_login_request(self, version: Version) -> LoginRequest {
+    LoginRequest {
+      username: self.user_name,
+      password: self.password,
+      digest: self.digest,
+      major: version.major,
+      minor: version.minor,
+    }
+  }
 }
 
 #[cfg(test)]
 mod tests {
+  use crate::proto::server::{LoginRequest, Version};
+
   use super::Credentials;
 
   #[test]
@@ -52,4 +81,47 @@ mod tests {
     assert_eq!(credentials.user_name(), "alice");
     assert_eq!(credentials.password(), "sekrit");
   }
+
+  #[test]
+  fn digest() {
+    let user_name = "alice".to_string();
+    let password = "sekrit".to_string();
+
+    let credentials = Credentials::new(user_name, password).unwrap();
+
+    // To generate the expected value on the command line;
+    //
+    // ```sh
+    // $ user_name="alice"
+    // $ password="sekrit"
+    // $ echo -e "${user_name}${password}" | md5sum
+    // ```
+    assert_eq!(credentials.digest(), "286da88eb442032bdd3913979af76e8a");
+  }
+
+  #[test]
+  fn into_login_request() {
+    let user_name = "alice".to_string();
+    let password = "sekrit".to_string();
+    let digest = "286da88eb442032bdd3913979af76e8a".to_string();
+
+    let credentials =
+      Credentials::new(user_name.clone(), password.clone()).unwrap();
+
+    let version = Version {
+      major: 13,
+      minor: 37,
+    };
+
+    assert_eq!(
+      credentials.into_login_request(version),
+      LoginRequest {
+        username: user_name,
+        password,
+        digest,
+        major: 13,
+        minor: 37,
+      }
+    );
+  }
 }

src/proto/server/mod.rs

@@ -6,6 +6,7 @@ mod response;
 #[cfg(test)]
 mod testing;
 
+pub use self::client::Version;
 pub use self::credentials::Credentials;
 pub use self::request::*;
 pub use self::response::*;

src/proto/server/request.rs

@@ -1,8 +1,5 @@
 use std::io;
 
-use crypto::digest::Digest;
-use crypto::md5::Md5;
-
 use crate::proto::core::value::{
   ValueDecode, ValueDecodeError, ValueDecoder, ValueEncode, ValueEncodeError,
   ValueEncoder,
@@ -10,16 +7,6 @@ use crate::proto::core::value::{
 use crate::proto::packet::{MutPacket, WriteToPacket};
 use crate::proto::server::constants::*;
 
-/* ------- *
- * Helpers *
- * ------- */
-
-fn md5_str(string: &str) -> String {
-  let mut hasher = Md5::new();
-  hasher.input_str(string);
-  hasher.result_str()
-}
-
 /*================*
  * SERVER REQUEST *
  *================*/
@@ -322,41 +309,12 @@ impl ValueDecode for FileSearchRequest {
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct LoginRequest {
-  username: String,
-  password: String,
-  digest: String,
-  major: u32,
-  minor: u32,
-}
-
-fn userpass_md5(username: &str, password: &str) -> String {
-  let userpass = String::new() + username + password;
-  md5_str(&userpass)
-}
-
-impl LoginRequest {
-  pub fn new(
-    username: &str,
-    password: &str,
-    major: u32,
-    minor: u32,
-  ) -> Result<Self, &'static str> {
-    if password.len() > 0 {
-      Ok(LoginRequest {
-        username: username.to_string(),
-        password: password.to_string(),
-        digest: userpass_md5(username, password),
-        major,
-        minor,
-      })
-    } else {
-      Err("Empty password")
-    }
-  }
-
-  fn has_correct_digest(&self) -> bool {
-    self.digest == userpass_md5(&self.username, &self.password)
-  }
+  // TODO: Rename to `user_name`.
+  pub username: String,
+  pub password: String,
+  pub digest: String,
+  pub major: u32,
+  pub minor: u32,
 }
 
 impl WriteToPacket for LoginRequest {
@@ -636,23 +594,15 @@ mod tests {
     }))
   }
 
-  #[test]
-  #[should_panic]
-  fn new_login_request_with_empty_password() {
-    LoginRequest::new("alice", "", 1337, 42).unwrap();
-  }
-
-  #[test]
-  fn new_login_request_has_correct_digest() {
-    let request = LoginRequest::new("alice", "password1234", 1337, 42).unwrap();
-    assert!(request.has_correct_digest());
-  }
-
   #[test]
   fn roundtrip_login_request() {
-    roundtrip(ServerRequest::LoginRequest(
-      LoginRequest::new("alice", "password1234", 1337, 42).unwrap(),
-    ))
+    roundtrip(ServerRequest::LoginRequest(LoginRequest {
+      username: "alice".to_string(),
+      password: "sekrit".to_string(),
+      digest: "abcdef".to_string(),
+      major: 13,
+      minor: 37,
+    }))
   }
 
   #[test]