Improve login query parsing

src/api/identity.rs

@@ -1,8 +1,7 @@
-use std::collections::HashMap;
 use std::net::{IpAddr, Ipv4Addr, SocketAddr};
 
-use rocket::request::{self, Form, FormItems, FromForm, FromRequest, Request};
+use rocket::request::LenientForm;
-use rocket::{Outcome, Route};
+use rocket::Route;
 
 use rocket_contrib::json::Json;
 use serde_json::Value;
@@ -14,7 +13,7 @@ use db::DbConn;
 
 use util::{self, JsonMap};
 
-use api::{ApiResult, JsonResult};
+use api::{ApiResult, EmptyResult, JsonResult};
 
 use CONFIG;
 
@@ -23,21 +22,22 @@ pub fn routes() -> Vec<Route> {
 }
 
 #[post("/connect/token", data = "<data>")]
-fn login(data: Form<ConnectData>, device_type: DeviceType, conn: DbConn, socket: Option<SocketAddr>) -> JsonResult {
+fn login(data: LenientForm<ConnectData>, conn: DbConn, socket: Option<SocketAddr>) -> JsonResult {
     let data: ConnectData = data.into_inner();
+    validate_data(&data)?;
 
     match data.grant_type {
-        GrantType::RefreshToken => _refresh_login(data, device_type, conn),
+        GrantType::refresh_token => _refresh_login(data, conn),
-        GrantType::Password => _password_login(data, device_type, conn, socket),
+        GrantType::password => _password_login(data, conn, socket),
     }
 }
 
-fn _refresh_login(data: ConnectData, _device_type: DeviceType, conn: DbConn) -> JsonResult {
+fn _refresh_login(data: ConnectData, conn: DbConn) -> JsonResult {
     // Extract token
-    let token = data.get("refresh_token");
+    let token = data.refresh_token.unwrap();
 
     // Get device by refresh token
-    let mut device = match Device::find_by_refresh_token(token, &conn) {
+    let mut device = match Device::find_by_refresh_token(&token, &conn) {
         Some(device) => device,
         None => err!("Invalid refresh token"),
     };
@@ -56,11 +56,11 @@ fn _refresh_login(data: ConnectData, _device_type: DeviceType, conn: DbConn) ->
             "Key": user.key,
             "PrivateKey": user.private_key,
         }))),
-            Err(_) => err!("Failed to add device to user")
+        Err(_) => err!("Failed to add device to user"),
     }
 }
 
-fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, remote: Option<SocketAddr>) -> JsonResult {
+fn _password_login(data: ConnectData, conn: DbConn, remote: Option<SocketAddr>) -> JsonResult {
     // Get the ip for error reporting
     let ip = match remote {
         Some(ip) => ip.ip(),
@@ -68,13 +68,13 @@ fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, rem
     };
 
     // Validate scope
-    let scope = data.get("scope");
+    let scope = data.scope.as_ref().unwrap();
     if scope != "api offline_access" {
         err!("Scope not supported")
     }
 
     // Get the user
-    let username = data.get("username");
+    let username = data.username.as_ref().unwrap();
     let user = match User::find_by_mail(username, &conn) {
         Some(user) => user,
         None => err!(format!(
@@ -84,7 +84,7 @@ fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, rem
     };
 
     // Check password
-    let password = data.get("password");
+    let password = data.password.as_ref().unwrap();
     if !user.check_valid_password(password) {
         err!(format!(
             "Username or password is incorrect. Try again. IP: {}. Username: {}.",
@@ -92,17 +92,9 @@ fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, rem
         ))
     }
 
-    // Let's only use the header and ignore the 'devicetype' parameter
+    let device_type: i32 = util::try_parse_string(data.device_type.as_ref()).expect("Invalid type");
-    let device_type_num = device_type.0;
+    let device_id = data.device_identifier.clone().expect("Missing device id");
-
+    let device_name = data.device_name.clone().expect("Missing device name");
-    let (device_id, device_name) = if data.is_device {
-        (
-            data.get("deviceidentifier").clone(),
-            data.get("devicename").clone(),
-        )
-    } else {
-        (format!("web-{}", user.uuid), String::from("web"))
-    };
 
     // Find device or create new
     let mut device = match Device::find_by_uuid(&device_id, &conn) {
@@ -110,8 +102,8 @@ fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, rem
             // Check if valid device
             if device.user_uuid != user.uuid {
                 match device.delete(&conn) {
-                    Ok(()) => Device::new(device_id, user.uuid.clone(), device_name, device_type_num),
+                    Ok(()) => Device::new(device_id, user.uuid.clone(), device_name, device_type),
-                    Err(_) => err!("Tried to delete device not owned by user, but failed")
+                    Err(_) => err!("Tried to delete device not owned by user, but failed"),
                 }
             } else {
                 device
@@ -119,11 +111,11 @@ fn _password_login(data: ConnectData, device_type: DeviceType, conn: DbConn, rem
         }
         None => {
             // Create new device
-            Device::new(device_id, user.uuid.clone(), device_name, device_type_num)
+            Device::new(device_id, user.uuid.clone(), device_name, device_type)
         }
     };
 
-    let twofactor_token = twofactor_auth(&user.uuid, &data, &mut device, &conn)?;
+    let twofactor_token = twofactor_auth(&user.uuid, &data.clone(), &mut device, &conn)?;
 
     // Common
     let user = User::find_by_uuid(&device.user_uuid, &conn).unwrap();
@@ -168,13 +160,10 @@ fn twofactor_auth(
         return Ok(None);
     }
 
-    let provider = match util::try_parse_string(data.get_opt("twoFactorProvider")) {
+    let provider = data.two_factor_provider.unwrap_or(providers[0]); // If we aren't given a two factor provider, asume the first one
-        Some(provider) => provider,
-        None => providers[0], // If we aren't given a two factor provider, asume the first one
-    };
 
-    let twofactor_code = match data.get_opt("twoFactorToken") {
+    let twofactor_code = match data.two_factor_token {
-        Some(code) => code,
+        Some(ref code) => code,
         None => err_json!(_json_err_twofactor(&providers, user_uuid, conn)?),
     };
 
@@ -182,8 +171,8 @@ fn twofactor_auth(
 
     match TwoFactorType::from_i32(provider) {
         Some(TwoFactorType::Remember) => {
-            match &device.twofactor_remember {
+            match device.twofactor_remember {
-                Some(remember) if remember == twofactor_code => return Ok(None), // No twofactor token needed here
+                Some(ref remember) if remember == twofactor_code => return Ok(None), // No twofactor token needed here
                 _ => err_json!(_json_err_twofactor(&providers, user_uuid, conn)?),
             }
         }
@@ -207,13 +196,13 @@ fn twofactor_auth(
         Some(TwoFactorType::U2f) => {
             use api::core::two_factor;
 
-            two_factor::validate_u2f_login(user_uuid, twofactor_code, conn)?;
+            two_factor::validate_u2f_login(user_uuid, &twofactor_code, conn)?;
         }
 
         _ => err!("Invalid two factor provider"),
     }
 
-    if util::try_parse_string_or(data.get_opt("twoFactorRemember"), 0) == 1 {
+    if data.two_factor_remember.unwrap_or(0) == 1 {
         Ok(Some(device.refresh_twofactor_remember()))
     } else {
         device.delete_twofactor_remember();
@@ -271,91 +260,63 @@ fn _json_err_twofactor(providers: &[i32], user_uuid: &str, conn: &DbConn) -> Api
     Ok(result)
 }
 
-#[derive(Clone, Copy)]
+#[derive(FromForm, Debug, Clone)]
-struct DeviceType(i32);
+#[allow(non_snake_case)]
-
-impl<'a, 'r> FromRequest<'a, 'r> for DeviceType {
-    type Error = &'static str;
-
-    fn from_request(request: &'a Request<'r>) -> request::Outcome<Self, Self::Error> {
-        let headers = request.headers();
-        let type_opt = headers.get_one("Device-Type");
-        let type_num = util::try_parse_string_or(type_opt, 0);
-
-        Outcome::Success(DeviceType(type_num))
-    }
-}
-
-#[derive(Debug)]
 struct ConnectData {
     grant_type: GrantType,
-    is_device: bool,
+
-    data: HashMap<String, String>,
+    // Needed for grant_type="refresh_token"
+    refresh_token: Option<String>,
+
+    // Needed for grant_type="password"
+    client_id: Option<String>, // web, cli, desktop, browser, mobile
+    password: Option<String>,
+    scope: Option<String>,
+    username: Option<String>,
+
+    #[form(field = "deviceIdentifier")]
+    device_identifier: Option<String>,
+    #[form(field = "deviceName")]
+    device_name: Option<String>,
+    #[form(field = "deviceType")]
+    device_type: Option<String>,
+
+    // Needed for two-factor auth
+    #[form(field = "twoFactorProvider")]
+    two_factor_provider: Option<i32>,
+    #[form(field = "twoFactorToken")]
+    two_factor_token: Option<String>,
+    #[form(field = "twoFactorRemember")]
+    two_factor_remember: Option<i32>,
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(FromFormValue, Debug, Clone, Copy)]
+#[allow(non_camel_case_types)]
 enum GrantType {
-    RefreshToken,
+    refresh_token,
-    Password,
+    password,
 }
 
-impl ConnectData {
+fn validate_data(data: &ConnectData) -> EmptyResult {
-    fn get(&self, key: &str) -> &String {
+    match data.grant_type {
-        &self.data[&key.to_lowercase()]
+        GrantType::refresh_token => {
+            _check_is_some(&data.refresh_token, "refresh_token cannot be blank")
+        }
+        GrantType::password => {
+            _check_is_some(&data.client_id, "client_id cannot be blank")?;
+            _check_is_some(&data.password, "password cannot be blank")?;
+            _check_is_some(&data.scope, "scope cannot be blank")?;
+            _check_is_some(&data.username, "username cannot be blank")?;
+            _check_is_some(&data.device_identifier, "device_identifier cannot be blank")?;
+            _check_is_some(&data.device_name, "device_name cannot be blank")?;
+            _check_is_some(&data.device_type, "device_type cannot be blank")
         }
-
-    fn get_opt(&self, key: &str) -> Option<&String> {
-        self.data.get(&key.to_lowercase())
     }
 }
 
-const VALUES_REFRESH: [&str; 1] = ["refresh_token"];
+fn _check_is_some<T>(value: &Option<T>, msg: &str) -> EmptyResult {
-const VALUES_PASSWORD: [&str; 5] = ["client_id", "grant_type", "password", "scope", "username"];
+    if value.is_none() {
-const VALUES_DEVICE: [&str; 3] = ["deviceidentifier", "devicename", "devicetype"];
+        err!(msg)
-
-impl<'f> FromForm<'f> for ConnectData {
-    type Error = String;
-
-    fn from_form(items: &mut FormItems<'f>, _strict: bool) -> Result<Self, Self::Error> {
-        let mut data = HashMap::new();
-
-        // Insert data into map
-        for item in items {
-            let (key, value) = item.key_value_decoded();
-            data.insert(key.to_lowercase(), value);
     }
-
+    Ok(())
-        // Validate needed values
-        let (grant_type, is_device) = match data.get("grant_type").map(String::as_ref) {
-            Some("refresh_token") => {
-                check_values(&data, &VALUES_REFRESH)?;
-                (GrantType::RefreshToken, false) // Device doesn't matter here
-            }
-            Some("password") => {
-                check_values(&data, &VALUES_PASSWORD)?;
-
-                let is_device = match data["client_id"].as_ref() {
-                    "browser" | "mobile" => check_values(&data, &VALUES_DEVICE)?,
-                    _ => false,
-                };
-                (GrantType::Password, is_device)
-            }
-            _ => return Err("Grant type not supported".to_string()),
-        };
-
-        Ok(ConnectData {
-            grant_type,
-            is_device,
-            data,
-        })
-    }
-}
-
-fn check_values(map: &HashMap<String, String>, values: &[&str]) -> Result<bool, String> {
-    for value in values {
-        if !map.contains_key(*value) {
-            return Err(format!("{} cannot be blank", value));
-        }
-    }
-    Ok(true)
 }