From ea57dc3bc9253b8db8e0815bac344f2cf981894b Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:03:07 +0000 Subject: [PATCH 01/11] Use `matches` macro --- src/db/models/organization.rs | 20 ++++---------------- src/main.rs | 9 +++------ 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 2e93426d..50d46064 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -90,17 +90,11 @@ impl PartialOrd for UserOrgType { } fn gt(&self, other: &i32) -> bool { - match self.partial_cmp(other) { - Some(Ordering::Less) | Some(Ordering::Equal) => false, - _ => true, - } + !matches!(self.partial_cmp(other), Some(Ordering::Less) | Some(Ordering::Equal)) } fn ge(&self, other: &i32) -> bool { - match self.partial_cmp(other) { - Some(Ordering::Less) => false, - _ => true, - } + !matches!(self.partial_cmp(other), Some(Ordering::Less)) } } @@ -119,17 +113,11 @@ impl PartialOrd for i32 { } fn lt(&self, other: &UserOrgType) -> bool { - match self.partial_cmp(other) { - Some(Ordering::Less) | None => true, - _ => false, - } + matches!(self.partial_cmp(other), Some(Ordering::Less) | None) } fn le(&self, other: &UserOrgType) -> bool { - match self.partial_cmp(other) { - Some(Ordering::Less) | Some(Ordering::Equal) | None => true, - _ => false, - } + matches!(self.partial_cmp(other), Some(Ordering::Less) | Some(Ordering::Equal) | None) } } diff --git a/src/main.rs b/src/main.rs index 15b18b3c..dd397d17 100644 --- a/src/main.rs +++ b/src/main.rs @@ -48,10 +48,7 @@ fn main() { let level = LF::from_str(&CONFIG.log_level()).expect("Valid log level"); init_logging(level).ok(); - let extra_debug = match level { - LF::Trace | LF::Debug => true, - _ => false, - }; + let extra_debug = matches!(level, LF::Trace | LF::Debug); check_data_folder(); check_rsa_keys(); @@ -64,10 +61,10 @@ fn main() { const HELP: &str = "\ A Bitwarden API server written in Rust - + USAGE: bitwarden_rs - + FLAGS: -h, --help Prints help information -v, --version Prints the app version From a8138be69b0c051da9f97827a9f5427c98dd3051 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:03:31 +0000 Subject: [PATCH 02/11] Use `if let` more --- src/api/core/accounts.rs | 6 ++---- src/api/icons.rs | 12 +++++++----- src/api/notifications.rs | 14 ++++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 9131575a..2d747fee 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -139,10 +139,8 @@ fn register(data: JsonUpcase, conn: DbConn) -> EmptyResult { } user.last_verifying_at = Some(user.created_at); - } else { - if let Err(e) = mail::send_welcome(&user.email) { - error!("Error sending welcome email: {:#?}", e); - } + } else if let Err(e) = mail::send_welcome(&user.email) { + error!("Error sending welcome email: {:#?}", e); } } diff --git a/src/api/icons.rs b/src/api/icons.rs index 5abcf375..f2ef9f21 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -352,10 +352,12 @@ fn get_favicons_node(node: &std::rc::Rc, icons: &mut Ve } } - if has_rel && href.is_some() { - if let Ok(full_href) = url.join(&href.unwrap()).map(|h| h.into_string()) { - let priority = get_icon_priority(&full_href, sizes); - icons.push(Icon::new(priority, full_href)); + if has_rel { + if let Some(inner_href) = href { + if let Ok(full_href) = url.join(&inner_href).map(|h| h.into_string()) { + let priority = get_icon_priority(&full_href, sizes); + icons.push(Icon::new(priority, full_href)); + } } } } @@ -472,7 +474,7 @@ fn get_icon_url(domain: &str) -> Result { let dom = html5ever::parse_document(markup5ever_rcdom::RcDom::default(), Default::default()) .from_utf8() .read_from(&mut limited_reader)?; - + get_favicons_node(&dom.document, &mut iconlist, &url); } else { // Add the default favicon.ico to the list with just the given domain diff --git a/src/api/notifications.rs b/src/api/notifications.rs index 3a6423c1..3c61b053 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -166,8 +166,8 @@ impl WSHandler { if let Some(params) = path.split('?').nth(1) { let params_iter = params.split('&').take(1); for val in params_iter { - if val.starts_with(ACCESS_TOKEN_KEY) { - return Some(val[ACCESS_TOKEN_KEY.len()..].into()); + if let Some(stripped) = val.strip_prefix(ACCESS_TOKEN_KEY) { + return Some(stripped.into()); } } }; @@ -410,10 +410,12 @@ pub fn start_notification_server() -> WebSocketUsers { if CONFIG.websocket_enabled() { thread::spawn(move || { - let mut settings = ws::Settings::default(); - settings.max_connections = 500; - settings.queue_size = 2; - settings.panic_on_internal = false; + let settings = ws::Settings { + max_connections: 500, + queue_size: 2, + panic_on_internal: false, + ..Default::default() + }; ws::Builder::new() .with_settings(settings) From 9f1240d8d95eeb7fc17e746355cf6ace1577a4ee Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:03:46 +0000 Subject: [PATCH 03/11] Only construct JSON object if it's useful --- src/db/models/cipher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index da3eabab..365865f8 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -104,7 +104,7 @@ impl Cipher { // Get the type_data or a default to an empty json object '{}'. // If not passing an empty object, mobile clients will crash. - let mut type_data_json: Value = serde_json::from_str(&self.data).unwrap_or(json!({})); + let mut type_data_json: Value = serde_json::from_str(&self.data).unwrap_or_else(|_| json!({})); // NOTE: This was marked as *Backwards Compatibilty Code*, but as of January 2021 this is still being used by upstream // Set the first element of the Uris array as Uri, this is needed several (mobile) clients. From 6b1daeba0533516c6d50fd2cebcc1e8060fd6df2 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:19:57 +0000 Subject: [PATCH 04/11] Implement `From` over `Into` https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into --- src/api/core/two_factor/u2f.rs | 10 +++++----- src/auth.rs | 30 +++++++++++++++--------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/api/core/two_factor/u2f.rs b/src/api/core/two_factor/u2f.rs index 7934ab63..f841240b 100644 --- a/src/api/core/two_factor/u2f.rs +++ b/src/api/core/two_factor/u2f.rs @@ -131,12 +131,12 @@ struct RegisterResponseCopy { pub error_code: Option, } -impl Into for RegisterResponseCopy { - fn into(self) -> RegisterResponse { +impl From for RegisterResponse { + fn from(r: RegisterResponseCopy) -> RegisterResponse { RegisterResponse { - registration_data: self.registration_data, - version: self.version, - client_data: self.client_data, + registration_data: r.registration_data, + version: r.version, + client_data: r.client_data, } } } diff --git a/src/auth.rs b/src/auth.rs index 4041cc6b..d1398bbf 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -450,12 +450,12 @@ impl<'a, 'r> FromRequest<'a, 'r> for AdminHeaders { } } -impl Into for AdminHeaders { - fn into(self) -> Headers { +impl From for Headers { + fn from(h: AdminHeaders) -> Headers { Headers { - host: self.host, - device: self.device, - user: self.user, + host: h.host, + device: h.device, + user: h.user, } } } @@ -529,12 +529,12 @@ impl<'a, 'r> FromRequest<'a, 'r> for ManagerHeaders { } } -impl Into for ManagerHeaders { - fn into(self) -> Headers { +impl From for Headers { + fn from(h: ManagerHeaders) -> Headers { Headers { - host: self.host, - device: self.device, - user: self.user, + host: h.host, + device: h.device, + user: h.user, } } } @@ -571,12 +571,12 @@ impl<'a, 'r> FromRequest<'a, 'r> for ManagerHeadersLoose { } } -impl Into for ManagerHeadersLoose { - fn into(self) -> Headers { +impl From for Headers { + fn from(h: ManagerHeadersLoose) -> Headers { Headers { - host: self.host, - device: self.device, - user: self.user, + host: h.host, + device: h.device, + user: h.user, } } } From 49af9cf4f5f8264384c7fa9063299f44e7536068 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:26:32 +0000 Subject: [PATCH 05/11] Correctly camelCase acronyms https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms --- src/api/notifications.rs | 20 ++++++++++---------- src/auth.rs | 36 ++++++++++++++++++------------------ src/db/models/device.rs | 6 +++--- src/error.rs | 8 ++++---- src/main.rs | 2 +- src/util.rs | 16 ++++++++-------- 6 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/api/notifications.rs b/src/api/notifications.rs index 3c61b053..ff3c0054 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -120,7 +120,7 @@ fn convert_option>(option: Option) -> Value { } // Server WebSocket handler -pub struct WSHandler { +pub struct WsHandler { out: Sender, user_uuid: Option, users: WebSocketUsers, @@ -140,7 +140,7 @@ const PING: Token = Token(1); const ACCESS_TOKEN_KEY: &str = "access_token="; -impl WSHandler { +impl WsHandler { fn err(&self, msg: &'static str) -> ws::Result<()> { self.out.close(ws::CloseCode::Invalid)?; @@ -176,7 +176,7 @@ impl WSHandler { } } -impl Handler for WSHandler { +impl Handler for WsHandler { fn on_open(&mut self, hs: Handshake) -> ws::Result<()> { // Path == "/notifications/hub?id===&access_token=" // @@ -240,13 +240,13 @@ impl Handler for WSHandler { } } -struct WSFactory { +struct WsFactory { pub users: WebSocketUsers, } -impl WSFactory { +impl WsFactory { pub fn init() -> Self { - WSFactory { + WsFactory { users: WebSocketUsers { map: Arc::new(CHashMap::new()), }, @@ -254,11 +254,11 @@ impl WSFactory { } } -impl Factory for WSFactory { - type Handler = WSHandler; +impl Factory for WsFactory { + type Handler = WsHandler; fn connection_made(&mut self, out: Sender) -> Self::Handler { - WSHandler { + WsHandler { out, user_uuid: None, users: self.users.clone(), @@ -405,7 +405,7 @@ use rocket::State; pub type Notify<'a> = State<'a, WebSocketUsers>; pub fn start_notification_server() -> WebSocketUsers { - let factory = WSFactory::init(); + let factory = WsFactory::init(); let users = factory.users.clone(); if CONFIG.websocket_enabled() { diff --git a/src/auth.rs b/src/auth.rs index d1398bbf..59d1370d 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -58,28 +58,28 @@ fn decode_jwt(token: &str, issuer: String) -> Result Result { +pub fn decode_login(token: &str) -> Result { decode_jwt(token, JWT_LOGIN_ISSUER.to_string()) } -pub fn decode_invite(token: &str) -> Result { +pub fn decode_invite(token: &str) -> Result { decode_jwt(token, JWT_INVITE_ISSUER.to_string()) } -pub fn decode_delete(token: &str) -> Result { +pub fn decode_delete(token: &str) -> Result { decode_jwt(token, JWT_DELETE_ISSUER.to_string()) } -pub fn decode_verify_email(token: &str) -> Result { +pub fn decode_verify_email(token: &str) -> Result { decode_jwt(token, JWT_VERIFYEMAIL_ISSUER.to_string()) } -pub fn decode_admin(token: &str) -> Result { +pub fn decode_admin(token: &str) -> Result { decode_jwt(token, JWT_ADMIN_ISSUER.to_string()) } #[derive(Debug, Serialize, Deserialize)] -pub struct LoginJWTClaims { +pub struct LoginJwtClaims { // Not before pub nbf: i64, // Expiration time @@ -110,7 +110,7 @@ pub struct LoginJWTClaims { } #[derive(Debug, Serialize, Deserialize)] -pub struct InviteJWTClaims { +pub struct InviteJwtClaims { // Not before pub nbf: i64, // Expiration time @@ -132,9 +132,9 @@ pub fn generate_invite_claims( org_id: Option, user_org_id: Option, invited_by_email: Option, -) -> InviteJWTClaims { +) -> InviteJwtClaims { let time_now = Utc::now().naive_utc(); - InviteJWTClaims { + InviteJwtClaims { nbf: time_now.timestamp(), exp: (time_now + Duration::days(5)).timestamp(), iss: JWT_INVITE_ISSUER.to_string(), @@ -147,7 +147,7 @@ pub fn generate_invite_claims( } #[derive(Debug, Serialize, Deserialize)] -pub struct DeleteJWTClaims { +pub struct DeleteJwtClaims { // Not before pub nbf: i64, // Expiration time @@ -158,9 +158,9 @@ pub struct DeleteJWTClaims { pub sub: String, } -pub fn generate_delete_claims(uuid: String) -> DeleteJWTClaims { +pub fn generate_delete_claims(uuid: String) -> DeleteJwtClaims { let time_now = Utc::now().naive_utc(); - DeleteJWTClaims { + DeleteJwtClaims { nbf: time_now.timestamp(), exp: (time_now + Duration::days(5)).timestamp(), iss: JWT_DELETE_ISSUER.to_string(), @@ -169,7 +169,7 @@ pub fn generate_delete_claims(uuid: String) -> DeleteJWTClaims { } #[derive(Debug, Serialize, Deserialize)] -pub struct VerifyEmailJWTClaims { +pub struct VerifyEmailJwtClaims { // Not before pub nbf: i64, // Expiration time @@ -180,9 +180,9 @@ pub struct VerifyEmailJWTClaims { pub sub: String, } -pub fn generate_verify_email_claims(uuid: String) -> DeleteJWTClaims { +pub fn generate_verify_email_claims(uuid: String) -> DeleteJwtClaims { let time_now = Utc::now().naive_utc(); - DeleteJWTClaims { + DeleteJwtClaims { nbf: time_now.timestamp(), exp: (time_now + Duration::days(5)).timestamp(), iss: JWT_VERIFYEMAIL_ISSUER.to_string(), @@ -191,7 +191,7 @@ pub fn generate_verify_email_claims(uuid: String) -> DeleteJWTClaims { } #[derive(Debug, Serialize, Deserialize)] -pub struct AdminJWTClaims { +pub struct AdminJwtClaims { // Not before pub nbf: i64, // Expiration time @@ -202,9 +202,9 @@ pub struct AdminJWTClaims { pub sub: String, } -pub fn generate_admin_claims() -> AdminJWTClaims { +pub fn generate_admin_claims() -> AdminJwtClaims { let time_now = Utc::now().naive_utc(); - AdminJWTClaims { + AdminJwtClaims { nbf: time_now.timestamp(), exp: (time_now + Duration::minutes(20)).timestamp(), iss: JWT_ADMIN_ISSUER.to_string(), diff --git a/src/db/models/device.rs b/src/db/models/device.rs index b3413ada..77837fca 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -80,8 +80,8 @@ impl Device { let orgmanager: Vec<_> = orgs.iter().filter(|o| o.atype == 3).map(|o| o.org_uuid.clone()).collect(); // Create the JWT claims struct, to send to the client - use crate::auth::{encode_jwt, LoginJWTClaims, DEFAULT_VALIDITY, JWT_LOGIN_ISSUER}; - let claims = LoginJWTClaims { + use crate::auth::{encode_jwt, LoginJwtClaims, DEFAULT_VALIDITY, JWT_LOGIN_ISSUER}; + let claims = LoginJwtClaims { nbf: time_now.timestamp(), exp: (time_now + *DEFAULT_VALIDITY).timestamp(), iss: JWT_LOGIN_ISSUER.to_string(), @@ -117,7 +117,7 @@ impl Device { pub fn save(&mut self, conn: &DbConn) -> EmptyResult { self.updated_at = Utc::now().naive_utc(); - db_run! { conn: + db_run! { conn: sqlite, mysql { crate::util::retry( || diesel::replace_into(devices::table).values(DeviceDb::to_db(self)).execute(conn), diff --git a/src/error.rs b/src/error.rs index fa90f1d3..a42b1809 100644 --- a/src/error.rs +++ b/src/error.rs @@ -38,11 +38,11 @@ use diesel::ConnectionError as DieselConErr; use diesel_migrations::RunMigrationsError as DieselMigErr; use diesel::r2d2::PoolError as R2d2Err; use handlebars::RenderError as HbErr; -use jsonwebtoken::errors::Error as JWTErr; +use jsonwebtoken::errors::Error as JwtErr; use regex::Error as RegexErr; use reqwest::Error as ReqErr; use serde_json::{Error as SerdeErr, Value}; -use std::io::Error as IOErr; +use std::io::Error as IoErr; use std::time::SystemTimeError as TimeErr; use u2f::u2ferror::U2fError as U2fErr; @@ -72,10 +72,10 @@ make_error! { R2d2Error(R2d2Err): _has_source, _api_error, U2fError(U2fErr): _has_source, _api_error, SerdeError(SerdeErr): _has_source, _api_error, - JWTError(JWTErr): _has_source, _api_error, + JWtError(JwtErr): _has_source, _api_error, TemplError(HbErr): _has_source, _api_error, //WsError(ws::Error): _has_source, _api_error, - IOError(IOErr): _has_source, _api_error, + IoError(IoErr): _has_source, _api_error, TimeError(TimeErr): _has_source, _api_error, ReqError(ReqErr): _has_source, _api_error, RegexError(RegexErr): _has_source, _api_error, diff --git a/src/main.rs b/src/main.rs index dd397d17..50975c66 100644 --- a/src/main.rs +++ b/src/main.rs @@ -326,7 +326,7 @@ fn launch_rocket(extra_debug: bool) { .manage(pool) .manage(api::start_notification_server()) .attach(util::AppHeaders()) - .attach(util::CORS()) + .attach(util::Cors()) .attach(util::BetterLogging(extra_debug)) .launch(); diff --git a/src/util.rs b/src/util.rs index de663583..06984c78 100644 --- a/src/util.rs +++ b/src/util.rs @@ -38,9 +38,9 @@ impl Fairing for AppHeaders { } } -pub struct CORS(); +pub struct Cors(); -impl CORS { +impl Cors { fn get_header(headers: &HeaderMap, name: &str) -> String { match headers.get_one(name) { Some(h) => h.to_string(), @@ -51,7 +51,7 @@ impl CORS { // Check a request's `Origin` header against the list of allowed origins. // If a match exists, return it. Otherwise, return None. fn get_allowed_origin(headers: &HeaderMap) -> Option { - let origin = CORS::get_header(headers, "Origin"); + let origin = Cors::get_header(headers, "Origin"); let domain_origin = CONFIG.domain_origin(); let safari_extension_origin = "file://"; if origin == domain_origin || origin == safari_extension_origin { @@ -62,10 +62,10 @@ impl CORS { } } -impl Fairing for CORS { +impl Fairing for Cors { fn info(&self) -> Info { Info { - name: "CORS", + name: "Cors", kind: Kind::Response, } } @@ -73,14 +73,14 @@ impl Fairing for CORS { fn on_response(&self, request: &Request, response: &mut Response) { let req_headers = request.headers(); - if let Some(origin) = CORS::get_allowed_origin(req_headers) { + if let Some(origin) = Cors::get_allowed_origin(req_headers) { response.set_header(Header::new("Access-Control-Allow-Origin", origin)); } // Preflight request if request.method() == Method::Options { - let req_allow_headers = CORS::get_header(req_headers, "Access-Control-Request-Headers"); - let req_allow_method = CORS::get_header(req_headers, "Access-Control-Request-Method"); + let req_allow_headers = Cors::get_header(req_headers, "Access-Control-Request-Headers"); + let req_allow_method = Cors::get_header(req_headers, "Access-Control-Request-Method"); response.set_header(Header::new("Access-Control-Allow-Methods", req_allow_method)); response.set_header(Header::new("Access-Control-Allow-Headers", req_allow_headers)); From 47c2625d38f902c200e083da4ea58fc40bfe3775 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 14:36:50 +0000 Subject: [PATCH 06/11] Prevent `clippy` complaining at method It's not incorrectly wrapped. We care about the return type being `Option`. --- src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/error.rs b/src/error.rs index a42b1809..a0b28a4b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -152,6 +152,7 @@ impl MapResult for Option { } } +#[allow(clippy::unnecessary_wraps)] const fn _has_source(e: T) -> Option { Some(e) } From 3e5971b9dbfa0eabe69b682d848009741b435758 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 15:07:26 +0000 Subject: [PATCH 07/11] Remove unnecessary result return types --- src/api/admin.rs | 14 +++++----- src/api/core/accounts.rs | 11 ++++---- src/api/core/ciphers.rs | 14 +++++----- src/api/core/folders.rs | 6 ++-- src/api/core/mod.rs | 21 +++++++------- src/api/core/organizations.rs | 50 +++++++++++++++++----------------- src/api/core/two_factor/mod.rs | 6 ++-- src/api/notifications.rs | 8 +++--- 8 files changed, 66 insertions(+), 64 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index 39fbc691..756f61c5 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -13,7 +13,7 @@ use rocket::{ use rocket_contrib::json::Json; use crate::{ - api::{ApiResult, EmptyResult, JsonResult, NumberOrString}, + api::{ApiResult, EmptyResult, NumberOrString}, auth::{decode_admin, encode_jwt, generate_admin_claims, ClientIp}, config::ConfigBuilder, db::{backup_database, models::*, DbConn, DbConnType}, @@ -291,17 +291,17 @@ fn test_smtp(data: Json, _token: AdminToken) -> EmptyResult { } #[get("/logout")] -fn logout(mut cookies: Cookies, referer: Referer) -> Result { +fn logout(mut cookies: Cookies, referer: Referer) -> Redirect { cookies.remove(Cookie::named(COOKIE_NAME)); - Ok(Redirect::to(admin_url(referer))) + Redirect::to(admin_url(referer)) } #[get("/users")] -fn get_users_json(_token: AdminToken, conn: DbConn) -> JsonResult { +fn get_users_json(_token: AdminToken, conn: DbConn) -> Json { let users = User::get_all(&conn); let users_json: Vec = users.iter().map(|u| u.to_json(&conn)).collect(); - Ok(Json(Value::Array(users_json))) + Json(Value::Array(users_json)) } #[get("/users/overview")] @@ -548,9 +548,9 @@ fn diagnostics(_token: AdminToken, _conn: DbConn) -> ApiResult> { } #[get("/diagnostics/config")] -fn get_diagnostics_config(_token: AdminToken) -> JsonResult { +fn get_diagnostics_config(_token: AdminToken) -> Json { let support_json = CONFIG.get_support_json(); - Ok(Json(support_json)) + Json(support_json) } #[post("/config", data = "")] diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 2d747fee..6e45a947 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -1,5 +1,6 @@ use chrono::Utc; use rocket_contrib::json::Json; +use serde_json::Value; use crate::{ api::{EmptyResult, JsonResult, JsonUpcase, Notify, NumberOrString, PasswordData, UpdateType}, @@ -148,8 +149,8 @@ fn register(data: JsonUpcase, conn: DbConn) -> EmptyResult { } #[get("/accounts/profile")] -fn profile(headers: Headers, conn: DbConn) -> JsonResult { - Ok(Json(headers.user.to_json(&conn))) +fn profile(headers: Headers, conn: DbConn) -> Json { + Json(headers.user.to_json(&conn)) } #[derive(Deserialize, Debug)] @@ -610,7 +611,7 @@ struct PreloginData { } #[post("/accounts/prelogin", data = "")] -fn prelogin(data: JsonUpcase, conn: DbConn) -> JsonResult { +fn prelogin(data: JsonUpcase, conn: DbConn) -> Json { let data: PreloginData = data.into_inner().data; let (kdf_type, kdf_iter) = match User::find_by_mail(&data.Email, &conn) { @@ -618,10 +619,10 @@ fn prelogin(data: JsonUpcase, conn: DbConn) -> JsonResult { None => (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT), }; - Ok(Json(json!({ + Json(json!({ "Kdf": kdf_type, "KdfIterations": kdf_iter - }))) + })) } #[derive(Deserialize)] #[allow(non_snake_case)] diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index c9abb3d4..7b0de205 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -84,7 +84,7 @@ struct SyncData { } #[get("/sync?")] -fn sync(data: Form, headers: Headers, conn: DbConn) -> JsonResult { +fn sync(data: Form, headers: Headers, conn: DbConn) -> Json { let user_json = headers.user.to_json(&conn); let folders = Folder::find_by_user(&headers.user.uuid, &conn); @@ -113,10 +113,10 @@ fn sync(data: Form, headers: Headers, conn: DbConn) -> JsonResult { let domains_json = if data.exclude_domains { Value::Null } else { - api::core::_get_eq_domains(headers, true).unwrap().into_inner() + api::core::_get_eq_domains(headers, true).into_inner() }; - Ok(Json(json!({ + Json(json!({ "Profile": user_json, "Folders": folders_json, "Collections": collections_json, @@ -125,11 +125,11 @@ fn sync(data: Form, headers: Headers, conn: DbConn) -> JsonResult { "Domains": domains_json, "Sends": sends_json, "Object": "sync" - }))) + })) } #[get("/ciphers")] -fn get_ciphers(headers: Headers, conn: DbConn) -> JsonResult { +fn get_ciphers(headers: Headers, conn: DbConn) -> Json { let ciphers = Cipher::find_by_user_visible(&headers.user.uuid, &conn); let ciphers_json: Vec = ciphers @@ -137,11 +137,11 @@ fn get_ciphers(headers: Headers, conn: DbConn) -> JsonResult { .map(|c| c.to_json(&headers.host, &headers.user.uuid, &conn)) .collect(); - Ok(Json(json!({ + Json(json!({ "Data": ciphers_json, "Object": "list", "ContinuationToken": null - }))) + })) } #[get("/ciphers/")] diff --git a/src/api/core/folders.rs b/src/api/core/folders.rs index 63bf40e2..2779fe61 100644 --- a/src/api/core/folders.rs +++ b/src/api/core/folders.rs @@ -20,16 +20,16 @@ pub fn routes() -> Vec { } #[get("/folders")] -fn get_folders(headers: Headers, conn: DbConn) -> JsonResult { +fn get_folders(headers: Headers, conn: DbConn) -> Json { let folders = Folder::find_by_user(&headers.user.uuid, &conn); let folders_json: Vec = folders.iter().map(Folder::to_json).collect(); - Ok(Json(json!({ + Json(json!({ "Data": folders_json, "Object": "list", "ContinuationToken": null, - }))) + })) } #[get("/folders/")] diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index f0b449c4..36e83f0e 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -34,17 +34,18 @@ pub fn routes() -> Vec { // use rocket::Route; use rocket_contrib::json::Json; +use rocket::response::Response; use serde_json::Value; use crate::{ - api::{EmptyResult, JsonResult, JsonUpcase}, + api::{JsonResult, JsonUpcase}, auth::Headers, db::DbConn, error::Error, }; #[put("/devices/identifier//clear-token")] -fn clear_device_token(uuid: String) -> EmptyResult { +fn clear_device_token<'a>(uuid: String) -> Response<'a> { // This endpoint doesn't have auth header let _ = uuid; @@ -53,11 +54,11 @@ fn clear_device_token(uuid: String) -> EmptyResult { // This only clears push token // https://github.com/bitwarden/core/blob/master/src/Api/Controllers/DevicesController.cs#L109 // https://github.com/bitwarden/core/blob/master/src/Core/Services/Implementations/DeviceService.cs#L37 - Ok(()) + Response::new() } #[put("/devices/identifier//token", data = "")] -fn put_device_token(uuid: String, data: JsonUpcase, headers: Headers) -> JsonResult { +fn put_device_token(uuid: String, data: JsonUpcase, headers: Headers) -> Json { let _data: Value = data.into_inner().data; // Data has a single string value "PushToken" let _ = uuid; @@ -65,13 +66,13 @@ fn put_device_token(uuid: String, data: JsonUpcase, headers: Headers) -> // TODO: This should save the push token, but we don't have push functionality - Ok(Json(json!({ + Json(json!({ "Id": headers.device.uuid, "Name": headers.device.name, "Type": headers.device.atype, "Identifier": headers.device.uuid, "CreationDate": crate::util::format_date(&headers.device.created_at), - }))) + })) } #[derive(Serialize, Deserialize, Debug)] @@ -85,11 +86,11 @@ struct GlobalDomain { const GLOBAL_DOMAINS: &str = include_str!("../../static/global_domains.json"); #[get("/settings/domains")] -fn get_eq_domains(headers: Headers) -> JsonResult { +fn get_eq_domains(headers: Headers) -> Json { _get_eq_domains(headers, false) } -fn _get_eq_domains(headers: Headers, no_excluded: bool) -> JsonResult { +fn _get_eq_domains(headers: Headers, no_excluded: bool) -> Json { let user = headers.user; use serde_json::from_str; @@ -106,11 +107,11 @@ fn _get_eq_domains(headers: Headers, no_excluded: bool) -> JsonResult { globals.retain(|g| !g.Excluded); } - Ok(Json(json!({ + Json(json!({ "EquivalentDomains": equivalent_domains, "GlobalEquivalentDomains": globals, "Object": "domains", - }))) + })) } #[derive(Deserialize, Debug)] diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 34a0acdb..5698c187 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -192,8 +192,8 @@ fn post_organization( // GET /api/collections?writeOnly=false #[get("/collections")] -fn get_user_collections(headers: Headers, conn: DbConn) -> JsonResult { - Ok(Json(json!({ +fn get_user_collections(headers: Headers, conn: DbConn) -> Json { + Json(json!({ "Data": Collection::find_by_user_uuid(&headers.user.uuid, &conn) .iter() @@ -201,12 +201,12 @@ fn get_user_collections(headers: Headers, conn: DbConn) -> JsonResult { .collect::(), "Object": "list", "ContinuationToken": null, - }))) + })) } #[get("/organizations//collections")] -fn get_org_collections(org_id: String, _headers: AdminHeaders, conn: DbConn) -> JsonResult { - Ok(Json(json!({ +fn get_org_collections(org_id: String, _headers: AdminHeaders, conn: DbConn) -> Json { + Json(json!({ "Data": Collection::find_by_organization(&org_id, &conn) .iter() @@ -214,7 +214,7 @@ fn get_org_collections(org_id: String, _headers: AdminHeaders, conn: DbConn) -> .collect::(), "Object": "list", "ContinuationToken": null, - }))) + })) } #[post("/organizations//collections", data = "")] @@ -441,30 +441,30 @@ struct OrgIdData { } #[get("/ciphers/organization-details?")] -fn get_org_details(data: Form, headers: Headers, conn: DbConn) -> JsonResult { +fn get_org_details(data: Form, headers: Headers, conn: DbConn) -> Json { let ciphers = Cipher::find_by_org(&data.organization_id, &conn); let ciphers_json: Vec = ciphers .iter() .map(|c| c.to_json(&headers.host, &headers.user.uuid, &conn)) .collect(); - Ok(Json(json!({ + Json(json!({ "Data": ciphers_json, "Object": "list", "ContinuationToken": null, - }))) + })) } #[get("/organizations//users")] -fn get_org_users(org_id: String, _headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult { +fn get_org_users(org_id: String, _headers: ManagerHeadersLoose, conn: DbConn) -> Json { let users = UserOrganization::find_by_org(&org_id, &conn); let users_json: Vec = users.iter().map(|c| c.to_json_user_details(&conn)).collect(); - Ok(Json(json!({ + Json(json!({ "Data": users_json, "Object": "list", "ContinuationToken": null, - }))) + })) } #[derive(Deserialize)] @@ -930,15 +930,15 @@ fn post_org_import( } #[get("/organizations//policies")] -fn list_policies(org_id: String, _headers: AdminHeaders, conn: DbConn) -> JsonResult { +fn list_policies(org_id: String, _headers: AdminHeaders, conn: DbConn) -> Json { let policies = OrgPolicy::find_by_org(&org_id, &conn); let policies_json: Vec = policies.iter().map(OrgPolicy::to_json).collect(); - Ok(Json(json!({ + Json(json!({ "Data": policies_json, "Object": "list", "ContinuationToken": null - }))) + })) } #[get("/organizations//policies/token?")] @@ -1017,8 +1017,8 @@ fn get_organization_tax(org_id: String, _headers: Headers, _conn: DbConn) -> Emp } #[get("/plans")] -fn get_plans(_headers: Headers, _conn: DbConn) -> JsonResult { - Ok(Json(json!({ +fn get_plans(_headers: Headers, _conn: DbConn) -> Json { + Json(json!({ "Object": "list", "Data": [ { @@ -1065,17 +1065,17 @@ fn get_plans(_headers: Headers, _conn: DbConn) -> JsonResult { } ], "ContinuationToken": null - }))) + })) } #[get("/plans/sales-tax-rates")] -fn get_plans_tax_rates(_headers: Headers, _conn: DbConn) -> JsonResult { +fn get_plans_tax_rates(_headers: Headers, _conn: DbConn) -> Json { // Prevent a 404 error, which also causes Javascript errors. - Ok(Json(json!({ + Json(json!({ "Object": "list", "Data": [], "ContinuationToken": null - }))) + })) } #[derive(Deserialize, Debug)] @@ -1128,7 +1128,7 @@ fn import(org_id: String, data: JsonUpcase, headers: Headers, con // If user is not part of the organization, but it exists } else if UserOrganization::find_by_email_and_org(&user_data.Email, &org_id, &conn).is_none() { if let Some (user) = User::find_by_mail(&user_data.Email, &conn) { - + let user_org_status = if CONFIG.mail_enabled() { UserOrgStatus::Invited as i32 } else { @@ -1157,18 +1157,18 @@ fn import(org_id: String, data: JsonUpcase, headers: Headers, con Some(headers.user.email.clone()), )?; } - } + } } } // If this flag is enabled, any user that isn't provided in the Users list will be removed (by default they will be kept unless they have Deleted == true) if data.OverwriteExisting { - for user_org in UserOrganization::find_by_org_and_type(&org_id, UserOrgType::User as i32, &conn) { + for user_org in UserOrganization::find_by_org_and_type(&org_id, UserOrgType::User as i32, &conn) { if let Some (user_email) = User::find_by_uuid(&user_org.user_uuid, &conn).map(|u| u.email) { if !data.Users.iter().any(|u| u.Email == user_email) { user_org.delete(&conn)?; } - } + } } } diff --git a/src/api/core/two_factor/mod.rs b/src/api/core/two_factor/mod.rs index 77e2cf2e..a3dfd319 100644 --- a/src/api/core/two_factor/mod.rs +++ b/src/api/core/two_factor/mod.rs @@ -38,15 +38,15 @@ pub fn routes() -> Vec { } #[get("/two-factor")] -fn get_twofactor(headers: Headers, conn: DbConn) -> JsonResult { +fn get_twofactor(headers: Headers, conn: DbConn) -> Json { let twofactors = TwoFactor::find_by_user(&headers.user.uuid, &conn); let twofactors_json: Vec = twofactors.iter().map(TwoFactor::to_json_provider).collect(); - Ok(Json(json!({ + Json(json!({ "Data": twofactors_json, "Object": "list", "ContinuationToken": null, - }))) + })) } #[post("/two-factor/get-recover", data = "")] diff --git a/src/api/notifications.rs b/src/api/notifications.rs index ff3c0054..8876937d 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -5,7 +5,7 @@ use rocket_contrib::json::Json; use serde_json::Value as JsonValue; use crate::{ - api::{EmptyResult, JsonResult}, + api::EmptyResult, auth::Headers, db::DbConn, Error, CONFIG, @@ -31,7 +31,7 @@ fn websockets_err() -> EmptyResult { } #[post("/hub/negotiate")] -fn negotiate(_headers: Headers, _conn: DbConn) -> JsonResult { +fn negotiate(_headers: Headers, _conn: DbConn) -> Json { use crate::crypto; use data_encoding::BASE64URL; @@ -47,10 +47,10 @@ fn negotiate(_headers: Headers, _conn: DbConn) -> JsonResult { // Rocket SSE support: https://github.com/SergioBenitez/Rocket/issues/33 // {"transport":"ServerSentEvents", "transferFormats":["Text"]}, // {"transport":"LongPolling", "transferFormats":["Text","Binary"]} - Ok(Json(json!({ + Json(json!({ "connectionId": conn_id, "availableTransports": available_transports - }))) + })) } // From 828a060698dba968e98064af90ffd8022b4d490e Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 15:10:00 +0000 Subject: [PATCH 08/11] Run clippy on CI --- .github/workflows/build.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d7ed5618..e2084f9b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -82,10 +82,11 @@ jobs: with: profile: minimal target: ${{ matrix.target-triple }} + components: clippy # End Uses the rust-toolchain file to determine version - # Run cargo tests (In release mode to speed up cargo build afterwards) + # Run cargo tests (In release mode to speed up future builds) - name: '`cargo test --release --features ${{ matrix.features }} --target ${{ matrix.target-triple }}`' uses: actions-rs/cargo@v1 with: @@ -94,6 +95,15 @@ jobs: # End Run cargo tests + # Run cargo clippy (In release mode to speed up future builds) + - name: '`cargo clippy --release --features ${{ matrix.features }} --target ${{ matrix.target-triple }}`' + uses: actions-rs/cargo@v1 + with: + command: clippy + args: --release --features ${{ matrix.features }} --target ${{ matrix.target-triple }} + # End Run cargo clippy + + # Build the binary - name: '`cargo build --release --features ${{ matrix.features }} --target ${{ matrix.target-triple }}`' uses: actions-rs/cargo@v1 From da55d5ec700dc430a8b75917ac9d4834b1f0ace8 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sat, 27 Mar 2021 15:21:00 +0000 Subject: [PATCH 09/11] Also run actions CI on pull request `push` only counts for pushes to branches on the repo, not forks --- .github/workflows/build.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e2084f9b..81151f17 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,6 +10,15 @@ on: - "docker/**" - "hooks/**" - "tools/**" + pull_request: + # Ignore when there are only changes done too one of these paths + paths-ignore: + - "**.md" + - "**.txt" + - "azure-pipelines.yml" + - "docker/**" + - "hooks/**" + - "tools/**" jobs: build: From 0bf0125e8271e7c8449da50d0572de5147043ca7 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sun, 28 Mar 2021 10:49:29 +0100 Subject: [PATCH 10/11] Reverse negation on ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel García --- src/db/models/organization.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 50d46064..1eeb04d2 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -90,11 +90,11 @@ impl PartialOrd for UserOrgType { } fn gt(&self, other: &i32) -> bool { - !matches!(self.partial_cmp(other), Some(Ordering::Less) | Some(Ordering::Equal)) + matches!(self.partial_cmp(other), Some(Ordering::Greater)) } fn ge(&self, other: &i32) -> bool { - !matches!(self.partial_cmp(other), Some(Ordering::Less)) + matches!(self.partial_cmp(other), Some(Ordering::Greater) | Some(Ordering::Equal)) } } From 81fa33ebb5089e6d9a610622dfdfd4551658f081 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Sun, 28 Mar 2021 10:59:49 +0100 Subject: [PATCH 11/11] Remove unnecessary reference --- src/api/admin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index e1ced1d7..d484407a 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -564,7 +564,7 @@ fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> ApiResu "running_within_docker": running_within_docker, "has_http_access": has_http_access, "ip_header_exists": &ip_header.0.is_some(), - "ip_header_match": ip_header_name == &CONFIG.ip_header(), + "ip_header_match": ip_header_name == CONFIG.ip_header(), "ip_header_name": ip_header_name, "ip_header_config": &CONFIG.ip_header(), "uses_proxy": uses_proxy,