From 7adc045b806960f0f8f3eb2968672eabed586620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Sun, 9 Dec 2018 17:58:38 +0100 Subject: [PATCH] Updated IP logging to use client_ip, to match old remote behavior. Improved error logging, now it won't show a generic error message in some situations. Removed delete device, which is not needed as it will be overwritten later. Logged more info when an error occurs saving a device. Added orgmanager to JWT claims. --- src/api/identity.rs | 39 ++++++++++++++----------------------- src/auth.rs | 43 +++++++++++++++++++++++++++++------------ src/db/models/device.rs | 2 ++ src/util.rs | 38 ++++++++++++++++++++++-------------- 4 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/api/identity.rs b/src/api/identity.rs index 58628d73..01e37061 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -1,5 +1,3 @@ -use std::net::{IpAddr, Ipv4Addr, SocketAddr}; - use rocket::request::LenientForm; use rocket::Route; @@ -15,6 +13,8 @@ use crate::util::{self, JsonMap}; use crate::api::{ApiResult, EmptyResult, JsonResult}; +use crate::auth::ClientIp; + use crate::CONFIG; pub fn routes() -> Vec { @@ -22,13 +22,13 @@ pub fn routes() -> Vec { } #[post("/connect/token", data = "")] -fn login(data: LenientForm, conn: DbConn, socket: Option) -> JsonResult { +fn login(data: LenientForm, conn: DbConn, ip: ClientIp) -> JsonResult { let data: ConnectData = data.into_inner(); validate_data(&data)?; match data.grant_type { GrantType::refresh_token => _refresh_login(data, conn), - GrantType::password => _password_login(data, conn, socket), + GrantType::password => _password_login(data, conn, ip), } } @@ -56,17 +56,11 @@ fn _refresh_login(data: ConnectData, conn: DbConn) -> JsonResult { "Key": user.key, "PrivateKey": user.private_key, }))), - Err(_) => err!("Failed to add device to user"), + Err(e) => err!("Failed to add device to user", e), } } -fn _password_login(data: ConnectData, conn: DbConn, remote: Option) -> JsonResult { - // Get the ip for error reporting - let ip = match remote { - Some(ip) => ip.ip(), - None => IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - }; - +fn _password_login(data: ConnectData, conn: DbConn, ip: ClientIp) -> JsonResult { // Validate scope let scope = data.scope.as_ref().unwrap(); if scope != "api offline_access" { @@ -79,7 +73,7 @@ fn _password_login(data: ConnectData, conn: DbConn, remote: Option) Some(user) => user, None => err!(format!( "Username or password is incorrect. Try again. IP: {}. Username: {}.", - ip, username + ip.ip, username )), }; @@ -88,7 +82,7 @@ fn _password_login(data: ConnectData, conn: DbConn, remote: Option) if !user.check_valid_password(password) { err!(format!( "Username or password is incorrect. Try again. IP: {}. Username: {}.", - ip, username + ip.ip, username )) } @@ -99,20 +93,15 @@ fn _password_login(data: ConnectData, conn: DbConn, remote: Option) // Find device or create new let mut device = match Device::find_by_uuid(&device_id, &conn) { Some(device) => { - // Check if valid device + // Check if owned device, and recreate if not if device.user_uuid != user.uuid { - match device.delete(&conn) { - Ok(()) => Device::new(device_id, user.uuid.clone(), device_name, device_type), - Err(_) => err!("Tried to delete device not owned by user, but failed"), - } + info!("Device exists but is owned by another user. The old device will be discarded"); + Device::new(device_id, user.uuid.clone(), device_name, device_type) } else { device } } - None => { - // Create new device - Device::new(device_id, user.uuid.clone(), device_name, device_type) - } + None => Device::new(device_id, user.uuid.clone(), device_name, device_type) }; let twofactor_token = twofactor_auth(&user.uuid, &data.clone(), &mut device, &conn)?; @@ -122,8 +111,8 @@ fn _password_login(data: ConnectData, conn: DbConn, remote: Option) let orgs = UserOrganization::find_by_user(&user.uuid, &conn); let (access_token, expires_in) = device.refresh_tokens(&user, orgs); - if device.save(&conn).is_err() { - err!("Failed to add device to user") + if let Err(e) = device.save(&conn) { + err!("Failed to add device to user", e) } let mut result = json!({ diff --git a/src/auth.rs b/src/auth.rs index 5085341c..6f351431 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,7 +1,6 @@ /// /// JWT Handling /// - use crate::util::read_file; use chrono::Duration; @@ -76,6 +75,7 @@ pub struct JWTClaims { pub orgowner: Vec, pub orgadmin: Vec, pub orguser: Vec, + pub orgmanager: Vec, // user security_stamp pub sstamp: String, @@ -90,7 +90,6 @@ pub struct JWTClaims { /// /// Bearer token authentication /// - use rocket::Outcome; use rocket::request::{self, Request, FromRequest}; @@ -139,13 +138,11 @@ impl<'a, 'r> FromRequest<'a, 'r> for Headers { // Get access_token let access_token: &str = match request.headers().get_one("Authorization") { - Some(a) => { - match a.rsplit("Bearer ").next() { - Some(split) => split, - None => err_handler!("No access token provided") - } - } - None => err_handler!("No access token provided") + Some(a) => match a.rsplit("Bearer ").next() { + Some(split) => split, + None => err_handler!("No access token provided"), + }, + None => err_handler!("No access token provided"), }; // Check JWT token is valid and get device and user from it @@ -256,7 +253,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for AdminHeaders { Outcome::Failure(f) => Outcome::Failure(f), Outcome::Success(headers) => { if headers.org_user_type >= UserOrgType::Admin { - Outcome::Success(Self{ + Outcome::Success(Self { host: headers.host, device: headers.device, user: headers.user, @@ -285,7 +282,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for OwnerHeaders { Outcome::Failure(f) => Outcome::Failure(f), Outcome::Success(headers) => { if headers.org_user_type == UserOrgType::Owner { - Outcome::Success(Self{ + Outcome::Success(Self { host: headers.host, device: headers.device, user: headers.user, @@ -296,4 +293,26 @@ impl<'a, 'r> FromRequest<'a, 'r> for OwnerHeaders { } } } -} \ No newline at end of file +} + +/// +/// Client IP address detection +/// +use std::net::IpAddr; + +pub struct ClientIp { + pub ip: IpAddr, +} + +impl<'a, 'r> FromRequest<'a, 'r> for ClientIp { + type Error = (); + + fn from_request(request: &'a Request<'r>) -> request::Outcome { + let ip = match request.client_ip() { + Some(addr) => addr, + None => "0.0.0.0".parse().unwrap(), + }; + + Outcome::Success(ClientIp { ip }) + } +} diff --git a/src/db/models/device.rs b/src/db/models/device.rs index 81c2d0f9..b415d5e7 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -74,6 +74,7 @@ impl Device { let orgowner: Vec<_> = orgs.iter().filter(|o| o.type_ == 0).map(|o| o.org_uuid.clone()).collect(); let orgadmin: Vec<_> = orgs.iter().filter(|o| o.type_ == 1).map(|o| o.org_uuid.clone()).collect(); let orguser: Vec<_> = orgs.iter().filter(|o| o.type_ == 2).map(|o| o.org_uuid.clone()).collect(); + let orgmanager: Vec<_> = orgs.iter().filter(|o| o.type_ == 3).map(|o| o.org_uuid.clone()).collect(); // Create the JWT claims struct, to send to the client @@ -92,6 +93,7 @@ impl Device { orgowner, orgadmin, orguser, + orgmanager, sstamp: user.security_stamp.to_string(), device: self.uuid.to_string(), diff --git a/src/util.rs b/src/util.rs index 33ab85ca..e43dbcac 100644 --- a/src/util.rs +++ b/src/util.rs @@ -2,22 +2,32 @@ /// Macros /// #[macro_export] -macro_rules! err { - ($err:expr, $msg:expr) => {{ - error!("{}", $msg); +macro_rules! _err_object { + ($msg:expr) => {{ err_json!(json!({ - "error": $err, - "error_description": $err, - "ErrorModel": { - "Message": $msg, - "ValidationErrors": null, - "ExceptionMessage": null, - "ExceptionStackTrace": null, - "InnerExceptionMessage": null, - "Object": "error" - }})) + "Message": "", + "error": "", + "error_description": "", + "ValidationErrors": {"": [ $msg ]}, + "ErrorModel": { + "Message": $msg, + "Object": "error" + }, + "Object": "error" + })) }}; - ($msg:expr) => { err!("unknown_error", $msg) } +} + +#[macro_export] +macro_rules! err { + ($msg:expr) => {{ + error!("{}", $msg); + _err_object!($msg) + }}; + ($usr_msg:expr, $log_value:expr) => {{ + error!("{}: {:#?}", $usr_msg, $log_value); + _err_object!($usr_msg) + }} } #[macro_export]