refactor login

refactor login so it's self contained in a single function
will be useful in adding other password based login method such as ldap

plume-cli/src/users.rs

@@ -109,15 +109,7 @@ fn new<'a>(args: &ArgMatches<'a>, conn: &Connection) {
             rpassword::read_password().expect("Couldn't read your password.")
         });
 
-    NewUser::new_local(
-        conn,
-        username,
-        display_name,
-        admin,
-        &bio,
-        email,
-        User::hash_pass(&password).expect("Couldn't hash password"),
-    )
+    NewUser::new_local(conn, username, display_name, admin, &bio, email, &password)
         .expect("Couldn't save new user");
 }
 

plume-models/src/users.rs

@@ -333,17 +333,35 @@ impl User {
         })
     }
 
-    pub fn hash_pass(pass: &str) -> Result<String> {
+    fn hash_pass(pass: &str) -> Result<String> {
         bcrypt::hash(pass, 10).map_err(Error::from)
     }
 
-    pub fn auth(&self, pass: &str) -> bool {
+    fn auth(&self, pass: &str) -> bool {
         self.hashed_password
             .clone()
             .map(|hashed| bcrypt::verify(pass, hashed.as_ref()).unwrap_or(false))
             .unwrap_or(false)
     }
 
+    pub fn connect(rocket: &PlumeRocket, name: &str, password: &str) -> Result<Self> {
+        let user = User::find_by_email(&*rocket.conn, &name)
+            .or_else(|_| User::find_by_fqn(&rocket, &name));
+        match user {
+            Ok(user) => {
+                if user.auth(password) {
+                    Ok(user)
+                } else {
+                    Err(Error::NotFound)
+                }
+            }
+            Err(_) => {
+                User::get(&rocket.conn, 1)?.auth(password);
+                Err(Error::NotFound)
+            }
+        }
+    }
+
     pub fn reset_password(&self, conn: &Connection, pass: &str) -> Result<()> {
         diesel::update(self)
             .set(users::hashed_password.eq(User::hash_pass(pass)?))
@@ -923,7 +941,7 @@ impl NewUser {
         is_admin: bool,
         summary: &str,
         email: String,
-        password: String,
+        password: &str,
     ) -> Result<User> {
         let (pub_key, priv_key) = gen_keypair();
         User::insert(
@@ -935,7 +953,7 @@ impl NewUser {
                 summary: summary.to_owned(),
                 summary_html: SafeString::new(&utils::md_to_html(&summary, None, false, None).0),
                 email: Some(email),
-                hashed_password: Some(password),
+                hashed_password: Some(User::hash_pass(password)?),
                 instance_id: Instance::get_local()?.id,
                 ap_url: String::new(),
                 public_key: String::from_utf8(pub_key).or(Err(Error::Signature))?,
@@ -964,7 +982,7 @@ pub(crate) mod tests {
             true,
             "Hello there, I'm the admin",
             "admin@example.com".to_owned(),
-            "invalid_admin_password".to_owned(),
+            "invalid_admin_password",
         )
         .unwrap();
         let user = NewUser::new_local(
@@ -974,7 +992,7 @@ pub(crate) mod tests {
             false,
             "Hello there, I'm no one",
             "user@example.com".to_owned(),
-            "invalid_user_password".to_owned(),
+            "invalid_user_password",
         )
         .unwrap();
         let other = NewUser::new_local(
@@ -984,7 +1002,7 @@ pub(crate) mod tests {
             false,
             "Hello there, I'm someone else",
             "other@example.com".to_owned(),
-            "invalid_other_password".to_owned(),
+            "invalid_other_password",
         )
         .unwrap();
         vec![admin, user, other]
@@ -1003,7 +1021,7 @@ pub(crate) mod tests {
                 false,
                 "Hello I'm a test",
                 "test@example.com".to_owned(),
-                User::hash_pass("test_password").unwrap(),
+                "test_password",
             )
             .unwrap();
 
@@ -1109,7 +1127,7 @@ pub(crate) mod tests {
                 false,
                 "Hello I'm a test",
                 "test@example.com".to_owned(),
-                User::hash_pass("test_password").unwrap(),
+                "test_password",
             )
             .unwrap();
 

src/api/mod.rs

@@ -62,8 +62,7 @@ pub fn oauth(
     let conn = &*rockets.conn;
     let app = App::find_by_client_id(conn, &query.client_id)?;
     if app.client_secret == query.client_secret {
-        if let Ok(user) = User::find_by_fqn(&rockets, &query.username) {
-            if user.auth(&query.password) {
+        if let Ok(user) = User::connect(&rockets, &query.username, &query.password) {
             let token = ApiToken::insert(
                 conn,
                 NewApiToken {
@@ -81,15 +80,6 @@ pub fn oauth(
                 "error": "Invalid credentials"
             })))
         }
-        } else {
-            // Making fake password verification to avoid different
-            // response times that would make it possible to know
-            // if a username is registered or not.
-            User::get(conn, 1)?.auth(&query.password);
-            Ok(Json(json!({
-                "error": "Invalid credentials"
-            })))
-        }
     } else {
         Ok(Json(json!({
             "error": "Invalid client_secret"

src/routes/session.rs

@@ -33,53 +33,29 @@ pub fn new(m: Option<String>, rockets: PlumeRocket) -> Ructe {
     ))
 }
 
-#[derive(Default, FromForm, Validate)]
+#[derive(Default, FromForm)]
 pub struct LoginForm {
-    #[validate(length(min = "1", message = "We need an email, or a username to identify you"))]
     pub email_or_name: String,
-    #[validate(length(min = "1", message = "Your password can't be empty"))]
     pub password: String,
 }
 
 #[post("/login", data = "<form>")]
 pub fn create(
     form: LenientForm<LoginForm>,
-    mut cookies: Cookies,
     rockets: PlumeRocket,
+    mut cookies: Cookies,
 ) -> RespondOrRedirect {
     let conn = &*rockets.conn;
-    let user = User::find_by_email(&*conn, &form.email_or_name)
-        .or_else(|_| User::find_by_fqn(&rockets, &form.email_or_name));
-    let mut errors = match form.validate() {
-        Ok(_) => ValidationErrors::new(),
-        Err(e) => e,
-    };
 
-    let user_id = if let Ok(user) = user {
-        if !user.auth(&form.password) {
-            let mut err = ValidationError::new("invalid_login");
-            err.message = Some(Cow::from("Invalid username, or password"));
-            errors.add("email_or_name", err);
-            String::new()
-        } else {
+    let user_id = if let Ok(user) = User::connect(&rockets, &form.email_or_name, &form.password) {
         user.id.to_string()
-        }
     } else {
-        // Fake password verification, only to avoid different login times
-        // that could be used to see if an email adress is registered or not
-        User::get(&*conn, 1)
-            .map(|u| u.auth(&form.password))
-            .expect("No user is registered");
-
+        let mut errors = ValidationErrors::new();
         let mut err = ValidationError::new("invalid_login");
         err.message = Some(Cow::from("Invalid username, or password"));
         errors.add("email_or_name", err);
-        String::new()
-    };
-
-    if !errors.is_empty() {
         return render!(session::login(&rockets.to_context(), None, &*form, errors)).into();
-    }
+    };
 
     cookies.add_private(
         Cookie::build(AUTH_COOKIE, user_id)
@@ -111,7 +87,7 @@ pub fn create(
             &(conn, &rockets.intl.catalog, None, None),
             None,
             &*form,
-            errors
+            ValidationErrors::new()
         ))
         .into()
     }

src/routes/user.rs

@@ -520,7 +520,7 @@ pub fn create(
                 false,
                 "",
                 form.email.to_string(),
-                User::hash_pass(&form.password).map_err(to_validation)?,
+                &form.password,
             )
             .map_err(to_validation)?;
             Ok(Flash::success(