From 514a372bc85fd495d331f240e35377a8ebdfb3c4 Mon Sep 17 00:00:00 2001 From: Miroslav Prasil Date: Mon, 30 Apr 2018 10:52:15 +0100 Subject: [PATCH 1/4] Add per-user folder-cipher mapping --- .../up.sql | 32 ++++++ src/api/core/ciphers.rs | 46 ++++---- src/api/core/organizations.rs | 7 +- src/api/identity.rs | 2 +- src/db/models/cipher.rs | 100 ++++++++++++++++-- src/db/models/collection.rs | 12 ++- src/db/models/folder.rs | 45 +++++++- src/db/models/mod.rs | 2 +- src/db/models/organization.rs | 4 +- src/db/models/user.rs | 2 +- src/db/schema.rs | 14 ++- 11 files changed, 217 insertions(+), 49 deletions(-) create mode 100644 migrations/2018-04-27-155151_create_users_ciphers/up.sql diff --git a/migrations/2018-04-27-155151_create_users_ciphers/up.sql b/migrations/2018-04-27-155151_create_users_ciphers/up.sql new file mode 100644 index 00000000..a0788791 --- /dev/null +++ b/migrations/2018-04-27-155151_create_users_ciphers/up.sql @@ -0,0 +1,32 @@ +ALTER TABLE ciphers RENAME TO oldCiphers; + +CREATE TABLE ciphers ( + uuid TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL, + user_uuid TEXT REFERENCES users (uuid), -- Make this optional + organization_uuid TEXT REFERENCES organizations (uuid), -- Add reference to orgs table + -- Remove folder_uuid + type INTEGER NOT NULL, + name TEXT NOT NULL, + notes TEXT, + fields TEXT, + data TEXT NOT NULL, + favorite BOOLEAN NOT NULL +); + +CREATE TABLE folders_ciphers ( + cipher_uuid TEXT NOT NULL REFERENCES ciphers (uuid), + folder_uuid TEXT NOT NULL REFERENCES folders (uuid), + + PRIMARY KEY (cipher_uuid, folder_uuid) +); + +INSERT INTO ciphers (uuid, created_at, updated_at, organization_uuid, type, name, notes, fields, data, favorite) +SELECT uuid, created_at, updated_at, organization_uuid, type, name, notes, fields, data, favorite FROM oldCiphers; + +INSERT INTO folders_ciphers (cipher_uuid, folder_uuid) +SELECT uuid, folder_uuid FROM oldCiphers WHERE folder_uuid IS NOT NULL; + + +DROP TABLE oldCiphers; \ No newline at end of file diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 2a4ceb0e..1a3b183c 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -29,7 +29,7 @@ fn sync(headers: Headers, conn: DbConn) -> JsonResult { let folders_json: Vec = folders.iter().map(|c| c.to_json()).collect(); let ciphers = Cipher::find_by_user(&headers.user.uuid, &conn); - let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &conn)).collect(); + let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &headers.user.uuid, &conn)).collect(); let domains_json = api::core::get_eq_domains(headers).unwrap().into_inner(); @@ -47,7 +47,7 @@ fn sync(headers: Headers, conn: DbConn) -> JsonResult { fn get_ciphers(headers: Headers, conn: DbConn) -> JsonResult { let ciphers = Cipher::find_by_user(&headers.user.uuid, &conn); - let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &conn)).collect(); + let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &headers.user.uuid, &conn)).collect(); Ok(Json(json!({ "Data": ciphers_json, @@ -62,11 +62,11 @@ fn get_cipher(uuid: String, headers: Headers, conn: DbConn) -> JsonResult { None => err!("Cipher doesn't exist") }; - if cipher.user_uuid != headers.user.uuid { + if !cipher.is_accessible_to_user(&headers.user.uuid, &conn) { err!("Cipher is not owned by user") } - Ok(Json(cipher.to_json(&headers.host, &conn))) + Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn))) } #[derive(Deserialize, Debug)] @@ -109,12 +109,12 @@ fn post_ciphers(data: Json, headers: Headers, conn: DbConn) -> JsonR let user_uuid = headers.user.uuid.clone(); let favorite = data.favorite.unwrap_or(false); - let mut cipher = Cipher::new(user_uuid, data.type_, data.name.clone(), favorite); + let mut cipher = Cipher::new(Some(user_uuid), None, data.type_, data.name.clone(), favorite); update_cipher_from_data(&mut cipher, data, &headers, &conn)?; cipher.save(&conn); - Ok(Json(cipher.to_json(&headers.host, &conn))) + Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn))) } fn update_cipher_from_data(cipher: &mut Cipher, data: CipherData, headers: &Headers, conn: &DbConn) -> EmptyResult { @@ -174,7 +174,7 @@ fn update_cipher_from_data(cipher: &mut Cipher, data: CipherData, headers: &Head // Copy the type data and change the names to the correct case copy_values(&type_data, &mut values); - cipher.folder_uuid = data.folderId; + cipher.move_to_folder(data.folderId, &headers.user.uuid, &conn); cipher.name = data.name; cipher.notes = data.notes; cipher.fields = uppercase_fields.map(|f| f.to_string()); @@ -247,11 +247,11 @@ fn post_ciphers_import(data: Json, headers: Headers, conn: DbConn) - let user_uuid = headers.user.uuid.clone(); let favorite = cipher_data.favorite.unwrap_or(false); - let mut cipher = Cipher::new(user_uuid, cipher_data.type_, cipher_data.name.clone(), favorite); + let mut cipher = Cipher::new(Some(user_uuid), None, cipher_data.type_, cipher_data.name.clone(), favorite); if update_cipher_from_data(&mut cipher, cipher_data, &headers, &conn).is_err() { err!("Error creating cipher") } - cipher.folder_uuid = folder_uuid; + //cipher.folder_uuid = folder_uuid; // TODO: This needs to create new folder-cipher mapping cipher.save(&conn); index += 1; @@ -274,8 +274,8 @@ fn put_cipher(uuid: String, data: Json, headers: Headers, conn: DbCo None => err!("Cipher doesn't exist") }; - if cipher.user_uuid != headers.user.uuid { - err!("Cipher is not owned by user") + if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) { + err!("Cipher is not write accessible") } cipher.favorite = data.favorite.unwrap_or(false); @@ -283,7 +283,7 @@ fn put_cipher(uuid: String, data: Json, headers: Headers, conn: DbCo update_cipher_from_data(&mut cipher, data, &headers, &conn)?; cipher.save(&conn); - Ok(Json(cipher.to_json(&headers.host, &conn))) + Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn))) } @@ -294,8 +294,8 @@ fn post_attachment(uuid: String, data: Data, content_type: &ContentType, headers None => err!("Cipher doesn't exist") }; - if cipher.user_uuid != headers.user.uuid { - err!("Cipher is not owned by user") + if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) { + err!("Cipher is not write accessible") } let mut params = content_type.params(); @@ -323,7 +323,7 @@ fn post_attachment(uuid: String, data: Data, content_type: &ContentType, headers attachment.save(&conn); }).expect("Error processing multipart data"); - Ok(Json(cipher.to_json(&headers.host, &conn))) + Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn))) } #[post("/ciphers//attachment//delete")] @@ -347,8 +347,8 @@ fn delete_attachment(uuid: String, attachment_id: String, headers: Headers, conn None => err!("Cipher doesn't exist") }; - if cipher.user_uuid != headers.user.uuid { - err!("Cipher is not owned by user") + if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) { + err!("Cipher cannot be deleted by user") } // Delete attachment @@ -399,7 +399,7 @@ fn move_cipher_selected(data: Json, headers: Headers, conn: DbConn) -> Em if folder.user_uuid != headers.user.uuid { err!("Folder is not owned by user") } - Some(folder_id.to_string()) + Some(folder.uuid) } None => err!("Folder doesn't exist") } @@ -424,12 +424,12 @@ fn move_cipher_selected(data: Json, headers: Headers, conn: DbConn) -> Em None => err!("Cipher doesn't exist") }; - if cipher.user_uuid != headers.user.uuid { - err!("Cipher is not owned by user") + if !cipher.is_accessible_to_user(&headers.user.uuid, &conn) { + err!("Cipher is not accessible by user") } // Move cipher - cipher.folder_uuid = folder_id.clone(); + cipher.move_to_folder(folder_id.clone(), &headers.user.uuid, &conn); cipher.save(&conn); } @@ -464,8 +464,8 @@ fn _delete_cipher_by_uuid(uuid: &str, headers: &Headers, conn: &DbConn) -> Empty None => err!("Cipher doesn't exist"), }; - if cipher.user_uuid != headers.user.uuid { - err!("Cipher is not owned by user") + if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) { + err!("Cipher can't be deleted by user") } _delete_cipher(cipher, conn); diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 1e802172..f719c8e2 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -109,6 +109,7 @@ fn get_user_collections(headers: Headers, conn: DbConn) -> JsonResult { Ok(Json(json!({ "Data": Collection::find_by_user_uuid(&headers.user.uuid, &conn) + .expect("Error loading collections") .iter() .map(|collection| { collection.to_json() @@ -121,9 +122,9 @@ fn get_user_collections(headers: Headers, conn: DbConn) -> JsonResult { fn get_org_collections(org_id: String, headers: Headers, conn: DbConn) -> JsonResult { Ok(Json(json!({ "Data": - Collection::find_by_user_uuid(&headers.user.uuid, &conn) + Collection::find_by_organization_and_user_uuid(&org_id, &headers.user.uuid, &conn) + .unwrap_or(vec![]) .iter() - .filter(|collection| { collection.org_uuid == org_id }) .map(|collection| { collection.to_json() }).collect::(), @@ -230,7 +231,7 @@ struct OrgIdData { #[get("/ciphers/organization-details?")] fn get_org_details(data: OrgIdData, headers: Headers, conn: DbConn) -> JsonResult { let ciphers = Cipher::find_by_org(&data.organizationId, &conn); - let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &conn)).collect(); + let ciphers_json: Vec = ciphers.iter().map(|c| c.to_json(&headers.host, &headers.user.uuid, &conn)).collect(); Ok(Json(json!({ "Data": ciphers_json, diff --git a/src/api/identity.rs b/src/api/identity.rs index 805c334f..5bb7871c 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -97,7 +97,7 @@ fn login(connect_data: Form, device_type: DeviceType, conn: DbConn) }; let user = User::find_by_uuid(&device.user_uuid, &conn).unwrap(); - let orgs = UserOrganization::find_by_user(&user.uuid, &conn); + let orgs = UserOrganization::find_by_user(&user.uuid, &conn).unwrap_or(vec![]); let (access_token, expires_in) = device.refresh_tokens(&user, orgs); device.save(&conn); diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index ee23bd54..aec4e327 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -3,19 +3,19 @@ use serde_json::Value as JsonValue; use uuid::Uuid; -use super::User; +use super::{User, Organization, UserOrganization, FolderCipher}; #[derive(Debug, Identifiable, Queryable, Insertable, Associations)] #[table_name = "ciphers"] #[belongs_to(User, foreign_key = "user_uuid")] +#[belongs_to(Organization, foreign_key = "organization_uuid")] #[primary_key(uuid)] pub struct Cipher { pub uuid: String, pub created_at: NaiveDateTime, pub updated_at: NaiveDateTime, - pub user_uuid: String, - pub folder_uuid: Option, + pub user_uuid: Option, pub organization_uuid: Option, /* @@ -36,7 +36,7 @@ pub struct Cipher { /// Local methods impl Cipher { - pub fn new(user_uuid: String, type_: i32, name: String, favorite: bool) -> Self { + pub fn new(user_uuid: Option, organization_uuid: Option, type_: i32, name: String, favorite: bool) -> Self { let now = Utc::now().naive_utc(); Self { @@ -45,8 +45,7 @@ impl Cipher { updated_at: now, user_uuid, - folder_uuid: None, - organization_uuid: None, + organization_uuid, type_, favorite, @@ -63,11 +62,11 @@ impl Cipher { use diesel; use diesel::prelude::*; use db::DbConn; -use db::schema::ciphers; +use db::schema::*; /// Database methods impl Cipher { - pub fn to_json(&self, host: &str, conn: &DbConn) -> JsonValue { + pub fn to_json(&self, host: &str, user_uuid: &str, conn: &DbConn) -> JsonValue { use serde_json; use util::format_date; use super::Attachment; @@ -94,7 +93,7 @@ impl Cipher { "Id": self.uuid, "Type": self.type_, "RevisionDate": format_date(&self.updated_at), - "FolderId": self.folder_uuid, + "FolderId": self.get_folder_uuid(&user_uuid, &conn), "Favorite": self.favorite, "OrganizationId": self.organization_uuid, "Attachments": attachments_json, @@ -142,6 +141,84 @@ impl Cipher { } } + pub fn move_to_folder(&self, folder_uuid: Option, user_uuid: &str, conn: &DbConn) -> Result<(), &str> { + match self.get_folder_uuid(&user_uuid, &conn) { + None => { + match folder_uuid { + Some(new_folder) => { + let folder_cipher = FolderCipher::new(&new_folder, &self.uuid); + folder_cipher.save(&conn).or(Err("Couldn't save folder setting")) + }, + None => Ok(()) //nothing to do + } + }, + Some(current_folder) => { + match folder_uuid { + Some(new_folder) => { + if current_folder == new_folder { + Ok(()) //nothing to do + } else { + match FolderCipher::find_by_folder_and_cipher(¤t_folder, &self.uuid, &conn) { + Some(current_folder) => { + current_folder.delete(&conn).or(Err("Failed removing old folder mapping")) + }, + None => Ok(()) // Weird, but nothing to do + }; + + FolderCipher::new(&new_folder, &self.uuid) + .save(&conn).or(Err("Couldn't save folder setting")) + } + }, + None => { + match FolderCipher::find_by_folder_and_cipher(¤t_folder, &self.uuid, &conn) { + Some(current_folder) => { + current_folder.delete(&conn).or(Err("Failed removing old folder mapping")) + }, + None => Err("Couldn't move from previous folder") + } + } + } + } + } + } + + pub fn is_write_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool { + match ciphers::table + .filter(ciphers::user_uuid.eq(user_uuid)) + .filter(ciphers::uuid.eq(&self.uuid)) + .first::(&**conn).ok() { + Some(_) => true, // cipher directly owned by user + None =>{ + match self.organization_uuid { + Some(ref org_uuid) => { + match users_organizations::table + .filter(users_organizations::org_uuid.eq(org_uuid)) + .filter(users_organizations::user_uuid.eq(user_uuid)) + .filter(users_organizations::access_all.eq(true)) + .first::(&**conn).ok() { + Some(_) => true, + None => false //TODO R/W access on collection + } + }, + None => false // cipher not in organization and not owned by user + } + } + } + } + + pub fn is_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool { + // TODO also check for read-only access + self.is_write_accessible_to_user(user_uuid, conn) + } + + pub fn get_folder_uuid(&self, user_uuid: &str, conn: &DbConn) -> Option { + folders_ciphers::table.inner_join(folders::table) + .filter(folders::user_uuid.eq(&user_uuid)) + .filter(folders_ciphers::cipher_uuid.eq(&self.uuid)) + .select(folders_ciphers::folder_uuid) + .first::(&**conn).ok() + } + pub fn find_by_uuid(uuid: &str, conn: &DbConn) -> Option { ciphers::table .filter(ciphers::uuid.eq(uuid)) @@ -161,8 +238,9 @@ impl Cipher { } pub fn find_by_folder(folder_uuid: &str, conn: &DbConn) -> Vec { - ciphers::table - .filter(ciphers::folder_uuid.eq(folder_uuid)) + folders_ciphers::table.inner_join(ciphers::table) + .filter(folders_ciphers::folder_uuid.eq(folder_uuid)) + .select(ciphers::all_columns) .load::(&**conn).expect("Error loading ciphers") } } diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 34854e89..08e03772 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -66,11 +66,19 @@ impl Collection { .first::(&**conn).ok() } - pub fn find_by_user_uuid(user_uuid: &str, conn: &DbConn) -> Vec { + pub fn find_by_user_uuid(user_uuid: &str, conn: &DbConn) -> Option> { users_collections::table.inner_join(collections::table) .filter(users_collections::user_uuid.eq(user_uuid)) .select(collections::all_columns) - .load::(&**conn).expect("Error loading user collections") + .load::(&**conn).ok() + } + + pub fn find_by_organization_and_user_uuid(org_uuid: &str, user_uuid: &str, conn: &DbConn) -> Option> { + users_collections::table.inner_join(collections::table) + .filter(users_collections::user_uuid.eq(user_uuid)) + .filter(collections::org_uuid.eq(org_uuid)) + .select(collections::all_columns) + .load::(&**conn).ok() } pub fn find_by_uuid_and_user(uuid: &str, user_uuid: &str, conn: &DbConn) -> Option { diff --git a/src/db/models/folder.rs b/src/db/models/folder.rs index 0dea434c..6319d787 100644 --- a/src/db/models/folder.rs +++ b/src/db/models/folder.rs @@ -3,7 +3,7 @@ use serde_json::Value as JsonValue; use uuid::Uuid; -use super::User; +use super::{User, Cipher}; #[derive(Debug, Identifiable, Queryable, Insertable, Associations)] #[table_name = "folders"] @@ -17,6 +17,16 @@ pub struct Folder { pub name: String, } +#[derive(Debug, Identifiable, Queryable, Insertable, Associations)] +#[table_name = "folders_ciphers"] +#[belongs_to(Cipher, foreign_key = "cipher_uuid")] +#[belongs_to(Folder, foreign_key = "folder_uuid")] +#[primary_key(cipher_uuid, folder_uuid)] +pub struct FolderCipher { + pub cipher_uuid: String, + pub folder_uuid: String, +} + /// Local methods impl Folder { pub fn new(user_uuid: String, name: String) -> Self { @@ -44,10 +54,19 @@ impl Folder { } } +impl FolderCipher { + pub fn new(cipher_uuid: &str, folder_uuid: &str) -> Self { + Self { + cipher_uuid: cipher_uuid.to_string(), + folder_uuid: folder_uuid.to_string(), + } + } +} + use diesel; use diesel::prelude::*; use db::DbConn; -use db::schema::folders; +use db::schema::{folders, folders_ciphers}; /// Database methods impl Folder { @@ -83,3 +102,25 @@ impl Folder { .load::(&**conn).expect("Error loading folders") } } + +impl FolderCipher { + pub fn save(&self, conn: &DbConn) -> QueryResult<()> { + diesel::replace_into(folders_ciphers::table) + .values(&*self) + .execute(&**conn).and(Ok(())) + } + + pub fn delete(self, conn: &DbConn) -> QueryResult<()> { + diesel::delete(folders_ciphers::table + .filter(folders_ciphers::cipher_uuid.eq(self.cipher_uuid)) + .filter(folders_ciphers::folder_uuid.eq(self.folder_uuid)) + ).execute(&**conn).and(Ok(())) + } + + pub fn find_by_folder_and_cipher(folder_uuid: &str, cipher_uuid: &str, conn: &DbConn) -> Option { + folders_ciphers::table + .filter(folders_ciphers::folder_uuid.eq(folder_uuid)) + .filter(folders_ciphers::cipher_uuid.eq(cipher_uuid)) + .first::(&**conn).ok() + } +} diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index d7b4515c..eb7f02c4 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -10,7 +10,7 @@ mod organization; pub use self::attachment::Attachment; pub use self::cipher::Cipher; pub use self::device::Device; -pub use self::folder::Folder; +pub use self::folder::{Folder, FolderCipher}; pub use self::user::User; pub use self::organization::Organization; pub use self::organization::{UserOrganization, UserOrgStatus, UserOrgType}; diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 93893604..3f49280d 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -222,10 +222,10 @@ impl UserOrganization { .first::(&**conn).ok() } - pub fn find_by_user(user_uuid: &str, conn: &DbConn) -> Vec { + pub fn find_by_user(user_uuid: &str, conn: &DbConn) -> Option> { users_organizations::table .filter(users_organizations::user_uuid.eq(user_uuid)) - .load::(&**conn).expect("Error loading user organizations") + .load::(&**conn).ok() } pub fn find_by_org(org_uuid: &str, conn: &DbConn) -> Vec { diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 0287d459..be3d10bd 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -128,7 +128,7 @@ impl User { pub fn to_json(&self, conn: &DbConn) -> JsonValue { use super::UserOrganization; - let orgs = UserOrganization::find_by_user(&self.uuid, conn); + let orgs = UserOrganization::find_by_user(&self.uuid, conn).unwrap_or(vec![]); let orgs_json: Vec = orgs.iter().map(|c| c.to_json(&conn)).collect(); json!({ diff --git a/src/db/schema.rs b/src/db/schema.rs index e80d5b3d..f55c1460 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -12,8 +12,7 @@ table! { uuid -> Text, created_at -> Timestamp, updated_at -> Timestamp, - user_uuid -> Text, - folder_uuid -> Nullable, + user_uuid -> Nullable, organization_uuid -> Nullable, #[sql_name = "type"] type_ -> Integer, @@ -107,8 +106,14 @@ table! { } } +table! { + folders_ciphers (cipher_uuid, folder_uuid) { + cipher_uuid -> Text, + folder_uuid -> Text, + } +} + joinable!(attachments -> ciphers (cipher_uuid)); -joinable!(ciphers -> folders (folder_uuid)); joinable!(ciphers -> users (user_uuid)); joinable!(collections -> organizations (org_uuid)); joinable!(devices -> users (user_uuid)); @@ -117,6 +122,8 @@ joinable!(users_collections -> collections (collection_uuid)); joinable!(users_collections -> users (user_uuid)); joinable!(users_organizations -> organizations (org_uuid)); joinable!(users_organizations -> users (user_uuid)); +joinable!(folders_ciphers -> ciphers (cipher_uuid)); +joinable!(folders_ciphers -> folders (folder_uuid)); allow_tables_to_appear_in_same_query!( attachments, @@ -128,4 +135,5 @@ allow_tables_to_appear_in_same_query!( users, users_collections, users_organizations, + folders_ciphers, ); From c3be1b4298206c084e397b4c0a3e2e840baf6d8e Mon Sep 17 00:00:00 2001 From: Miroslav Prasil Date: Tue, 1 May 2018 16:54:22 +0100 Subject: [PATCH 2/4] Fix FolderCipher creation, handle some errors --- src/api/core/ciphers.rs | 8 ++++++-- src/db/models/cipher.rs | 8 ++++---- src/db/models/folder.rs | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 1a3b183c..a6b95b32 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -174,7 +174,9 @@ fn update_cipher_from_data(cipher: &mut Cipher, data: CipherData, headers: &Head // Copy the type data and change the names to the correct case copy_values(&type_data, &mut values); - cipher.move_to_folder(data.folderId, &headers.user.uuid, &conn); + if cipher.move_to_folder(data.folderId, &headers.user.uuid, &conn).is_err() { + err!("Error saving the folder information") + } cipher.name = data.name; cipher.notes = data.notes; cipher.fields = uppercase_fields.map(|f| f.to_string()); @@ -429,7 +431,9 @@ fn move_cipher_selected(data: Json, headers: Headers, conn: DbConn) -> Em } // Move cipher - cipher.move_to_folder(folder_id.clone(), &headers.user.uuid, &conn); + if cipher.move_to_folder(folder_id.clone(), &headers.user.uuid, &conn).is_err() { + err!("Error saving the folder information") + } cipher.save(&conn); } diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index aec4e327..370996ff 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -163,10 +163,10 @@ impl Cipher { current_folder.delete(&conn).or(Err("Failed removing old folder mapping")) }, None => Ok(()) // Weird, but nothing to do - }; - - FolderCipher::new(&new_folder, &self.uuid) - .save(&conn).or(Err("Couldn't save folder setting")) + }.and_then( + |()| FolderCipher::new(&new_folder, &self.uuid) + .save(&conn).or(Err("Couldn't save folder setting")) + ) } }, None => { diff --git a/src/db/models/folder.rs b/src/db/models/folder.rs index 6319d787..8e0b6720 100644 --- a/src/db/models/folder.rs +++ b/src/db/models/folder.rs @@ -55,10 +55,10 @@ impl Folder { } impl FolderCipher { - pub fn new(cipher_uuid: &str, folder_uuid: &str) -> Self { + pub fn new(folder_uuid: &str, cipher_uuid: &str) -> Self { Self { - cipher_uuid: cipher_uuid.to_string(), folder_uuid: folder_uuid.to_string(), + cipher_uuid: cipher_uuid.to_string(), } } } From a0796acbc73e8e7b9bcd94bab334cc82aa922835 Mon Sep 17 00:00:00 2001 From: Miroslav Prasil Date: Thu, 3 May 2018 17:47:27 +0100 Subject: [PATCH 3/4] Implement suggested improvements --- migrations/2018-04-27-155151_create_users_ciphers/up.sql | 4 ++-- src/api/core/ciphers.rs | 3 +-- src/api/identity.rs | 2 +- src/db/models/organization.rs | 4 ++-- src/db/models/user.rs | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/migrations/2018-04-27-155151_create_users_ciphers/up.sql b/migrations/2018-04-27-155151_create_users_ciphers/up.sql index a0788791..12c096ef 100644 --- a/migrations/2018-04-27-155151_create_users_ciphers/up.sql +++ b/migrations/2018-04-27-155151_create_users_ciphers/up.sql @@ -22,8 +22,8 @@ CREATE TABLE folders_ciphers ( PRIMARY KEY (cipher_uuid, folder_uuid) ); -INSERT INTO ciphers (uuid, created_at, updated_at, organization_uuid, type, name, notes, fields, data, favorite) -SELECT uuid, created_at, updated_at, organization_uuid, type, name, notes, fields, data, favorite FROM oldCiphers; +INSERT INTO ciphers (uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite) +SELECT uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite FROM oldCiphers; INSERT INTO folders_ciphers (cipher_uuid, folder_uuid) SELECT uuid, folder_uuid FROM oldCiphers WHERE folder_uuid IS NOT NULL; diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index a6b95b32..29cf2aed 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -253,8 +253,7 @@ fn post_ciphers_import(data: Json, headers: Headers, conn: DbConn) - if update_cipher_from_data(&mut cipher, cipher_data, &headers, &conn).is_err() { err!("Error creating cipher") } - //cipher.folder_uuid = folder_uuid; // TODO: This needs to create new folder-cipher mapping - + cipher.move_to_folder(folder_uuid, &headers.user.uuid.clone(), &conn).ok(); cipher.save(&conn); index += 1; } diff --git a/src/api/identity.rs b/src/api/identity.rs index 5bb7871c..805c334f 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -97,7 +97,7 @@ fn login(connect_data: Form, device_type: DeviceType, conn: DbConn) }; let user = User::find_by_uuid(&device.user_uuid, &conn).unwrap(); - let orgs = UserOrganization::find_by_user(&user.uuid, &conn).unwrap_or(vec![]); + let orgs = UserOrganization::find_by_user(&user.uuid, &conn); let (access_token, expires_in) = device.refresh_tokens(&user, orgs); device.save(&conn); diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 3f49280d..3bfa3c7b 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -222,10 +222,10 @@ impl UserOrganization { .first::(&**conn).ok() } - pub fn find_by_user(user_uuid: &str, conn: &DbConn) -> Option> { + pub fn find_by_user(user_uuid: &str, conn: &DbConn) -> Vec { users_organizations::table .filter(users_organizations::user_uuid.eq(user_uuid)) - .load::(&**conn).ok() + .load::(&**conn).unwrap_or(vec![]) } pub fn find_by_org(org_uuid: &str, conn: &DbConn) -> Vec { diff --git a/src/db/models/user.rs b/src/db/models/user.rs index be3d10bd..0287d459 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -128,7 +128,7 @@ impl User { pub fn to_json(&self, conn: &DbConn) -> JsonValue { use super::UserOrganization; - let orgs = UserOrganization::find_by_user(&self.uuid, conn).unwrap_or(vec![]); + let orgs = UserOrganization::find_by_user(&self.uuid, conn); let orgs_json: Vec = orgs.iter().map(|c| c.to_json(&conn)).collect(); json!({ From c4360ee6976bdbb5223cab3da09d8d53d6dfefba Mon Sep 17 00:00:00 2001 From: Miroslav Prasil Date: Fri, 4 May 2018 13:42:30 +0100 Subject: [PATCH 4/4] Save extra query when checking write access --- src/db/models/cipher.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index 370996ff..2cbe4e05 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -183,11 +183,8 @@ impl Cipher { } pub fn is_write_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool { - match ciphers::table - .filter(ciphers::user_uuid.eq(user_uuid)) - .filter(ciphers::uuid.eq(&self.uuid)) - .first::(&**conn).ok() { - Some(_) => true, // cipher directly owned by user + match self.user_uuid { + Some(ref self_user_uuid) => self_user_uuid == user_uuid, // cipher directly owned by user None =>{ match self.organization_uuid { Some(ref org_uuid) => {