Merge pull request #1042 from jjlin/hide-passwords

Add support for hiding passwords in a collection

migrations/mysql/2020-07-01-214531_add_hide_passwords/up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE users_collections
+ADD COLUMN hide_passwords BOOLEAN NOT NULL DEFAULT FALSE;

migrations/postgresql/2020-07-01-214531_add_hide_passwords/up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE users_collections
+ADD COLUMN hide_passwords BOOLEAN NOT NULL DEFAULT FALSE;

migrations/sqlite/2020-07-01-214531_add_hide_passwords/up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE users_collections
+ADD COLUMN hide_passwords BOOLEAN NOT NULL DEFAULT 0; -- FALSE

src/api/core/organizations.rs

@@ -374,7 +374,7 @@ fn get_collection_users(org_id: String, coll_id: String, _headers: AdminHeaders,
         .map(|col_user| {
             UserOrganization::find_by_user_and_org(&col_user.user_uuid, &org_id, &conn)
                 .unwrap()
-                .to_json_read_only(col_user.read_only)
+                .to_json_user_access_restrictions(&col_user)
         })
         .collect();
 
@@ -408,7 +408,9 @@ fn put_collection_users(
             continue;
         }
 
-        CollectionUser::save(&user.user_uuid, &coll_id, d.ReadOnly, &conn)?;
+        CollectionUser::save(&user.user_uuid, &coll_id,
+                             d.ReadOnly, d.HidePasswords,
+                             &conn)?;
     }
 
     Ok(())
@@ -452,6 +454,7 @@ fn get_org_users(org_id: String, _headers: AdminHeaders, conn: DbConn) -> JsonRe
 struct CollectionData {
     Id: String,
     ReadOnly: bool,
+    HidePasswords: bool,
 }
 
 #[derive(Deserialize)]
@@ -523,7 +526,9 @@ fn send_invite(org_id: String, data: JsonUpcase<InviteData>, headers: AdminHeade
                 match Collection::find_by_uuid_and_org(&col.Id, &org_id, &conn) {
                     None => err!("Collection not found in Organization"),
                     Some(collection) => {
-                        CollectionUser::save(&user.uuid, &collection.uuid, col.ReadOnly, &conn)?;
+                        CollectionUser::save(&user.uuid, &collection.uuid,
+                                             col.ReadOnly, col.HidePasswords,
+                                             &conn)?;
                     }
                 }
             }
@@ -778,7 +783,9 @@ fn edit_user(
             match Collection::find_by_uuid_and_org(&col.Id, &org_id, &conn) {
                 None => err!("Collection not found in Organization"),
                 Some(collection) => {
-                    CollectionUser::save(&user_to_edit.user_uuid, &collection.uuid, col.ReadOnly, &conn)?;
+                    CollectionUser::save(&user_to_edit.user_uuid, &collection.uuid,
+                                         col.ReadOnly, col.HidePasswords,
+                                         &conn)?;
                 }
             }
         }

src/db/models/cipher.rs

@@ -82,6 +82,15 @@ impl Cipher {
         let fields_json = self.fields.as_ref().and_then(|s| serde_json::from_str(s).ok()).unwrap_or(Value::Null);
         let password_history_json = self.password_history.as_ref().and_then(|s| serde_json::from_str(s).ok()).unwrap_or(Value::Null);
 
+        let (read_only, hide_passwords) =
+            match self.get_access_restrictions(&user_uuid, &conn) {
+                Some((ro, hp)) => (ro, hp),
+                None => {
+                    error!("Cipher ownership assertion failure");
+                    (true, true)
+                },
+            };
+
         // Get the data or a default empty value to avoid issues with the mobile apps
         let mut data_json: Value = serde_json::from_str(&self.data).unwrap_or_else(|_| json!({
             "Fields":null,
@@ -105,7 +114,15 @@ impl Cipher {
         }
         // TODO: ******* Backwards compat end **********
 
+        // There are three types of cipher response models in upstream
+        // Bitwarden: "cipherMini", "cipher", and "cipherDetails" (in order
+        // of increasing level of detail). bitwarden_rs currently only
+        // supports the "cipherDetails" type, though it seems like the
+        // Bitwarden clients will ignore extra fields.
+        //
+        // Ref: https://github.com/bitwarden/server/blob/master/src/Core/Models/Api/Response/CipherResponseModel.cs
         let mut json_object = json!({
+            "Object": "cipherDetails",
             "Id": self.uuid,
             "Type": self.atype,
             "RevisionDate": format_date(&self.updated_at),
@@ -115,6 +132,8 @@ impl Cipher {
             "OrganizationId": self.organization_uuid,
             "Attachments": attachments_json,
             "OrganizationUseTotp": true,
+
+            // This field is specific to the cipherDetails type.
             "CollectionIds": self.get_collections(user_uuid, &conn),
 
             "Name": self.name,
@@ -123,8 +142,11 @@ impl Cipher {
 
             "Data": data_json,
 
-            "Object": "cipher",
-            "Edit": true,
+            // These values are true by default, but can be false if the
+            // cipher belongs to a collection where the org owner has enabled
+            // the "Read Only" or "Hide Passwords" restrictions for the user.
+            "Edit": !read_only,
+            "ViewPassword": !hide_passwords,
 
             "PasswordHistory": password_history_json,
         });
@@ -241,64 +263,78 @@ impl Cipher {
         }
     }
 
-    pub fn is_write_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool {
-        ciphers::table
+    /// Returns whether this cipher is directly owned by the user.
+    pub fn is_owned_by_user(&self, user_uuid: &str) -> bool {
+        self.user_uuid.is_some() && self.user_uuid.as_ref().unwrap() == user_uuid
+    }
+
+    /// Returns whether this cipher is owned by an org in which the user has full access.
+    pub fn is_in_full_access_org(&self, user_uuid: &str, conn: &DbConn) -> bool {
+        if let Some(ref org_uuid) = self.organization_uuid {
+            if let Some(user_org) = UserOrganization::find_by_user_and_org(&user_uuid, &org_uuid, &conn) {
+                return user_org.has_full_access();
+            }
+        }
+
+        false
+    }
+
+    /// Returns the user's access restrictions to this cipher. A return value
+    /// of None means that this cipher does not belong to the user, and is
+    /// not in any collection the user has access to. Otherwise, the user has
+    /// access to this cipher, and Some(read_only, hide_passwords) represents
+    /// the access restrictions.
+    pub fn get_access_restrictions(&self, user_uuid: &str, conn: &DbConn) -> Option<(bool, bool)> {
+        // Check whether this cipher is directly owned by the user, or is in
+        // a collection that the user has full access to. If so, there are no
+        // access restrictions.
+        if self.is_owned_by_user(&user_uuid) || self.is_in_full_access_org(&user_uuid, &conn) {
+            return Some((false, false));
+        }
+
+        // Check whether this cipher is in any collections accessible to the
+        // user. If so, retrieve the access flags for each collection.
+        let query = ciphers::table
             .filter(ciphers::uuid.eq(&self.uuid))
-            .left_join(
-                users_organizations::table.on(ciphers::organization_uuid
-                    .eq(users_organizations::org_uuid.nullable())
-                    .and(users_organizations::user_uuid.eq(user_uuid))),
-            )
-            .left_join(ciphers_collections::table)
-            .left_join(
-                users_collections::table
-                    .on(ciphers_collections::collection_uuid.eq(users_collections::collection_uuid)),
-            )
-            .filter(ciphers::user_uuid.eq(user_uuid).or(
-                // Cipher owner
-                users_organizations::access_all.eq(true).or(
-                    // access_all in Organization
-                    users_organizations::atype.le(UserOrgType::Admin as i32).or(
-                        // Org admin or owner
-                        users_collections::user_uuid.eq(user_uuid).and(
-                            users_collections::read_only.eq(false), //R/W access to collection
-                        ),
-                    ),
-                ),
-            ))
-            .select(ciphers::all_columns)
-            .first::<Self>(&**conn)
-            .ok()
-            .is_some()
+            .inner_join(ciphers_collections::table.on(
+                ciphers::uuid.eq(ciphers_collections::cipher_uuid)))
+            .inner_join(users_collections::table.on(
+                ciphers_collections::collection_uuid.eq(users_collections::collection_uuid)
+                    .and(users_collections::user_uuid.eq(user_uuid))))
+            .select((users_collections::read_only, users_collections::hide_passwords));
+
+        // There's an edge case where a cipher can be in multiple collections
+        // with inconsistent access flags. For example, a cipher could be in
+        // one collection where the user has read-only access, but also in
+        // another collection where the user has read/write access. To handle
+        // this, we do a boolean OR of all values in each of the `read_only`
+        // and `hide_passwords` columns. This could ideally be done as part
+        // of the query, but Diesel doesn't support a max() or bool_or()
+        // function on booleans and this behavior isn't portable anyway.
+        if let Some(vec) = query.load::<(bool, bool)>(&**conn).ok() {
+            let mut read_only = false;
+            let mut hide_passwords = false;
+            for (ro, hp) in vec.iter() {
+                read_only |= ro;
+                hide_passwords |= hp;
+            }
+
+            Some((read_only, hide_passwords))
+        } else {
+            // This cipher isn't in any collections accessible to the user.
+            None
+        }
+    }
+
+    pub fn is_write_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool {
+        match self.get_access_restrictions(&user_uuid, &conn) {
+            Some((read_only, _hide_passwords)) => !read_only,
+            None => false,
+        }
     }
 
     pub fn is_accessible_to_user(&self, user_uuid: &str, conn: &DbConn) -> bool {
-        ciphers::table
-            .filter(ciphers::uuid.eq(&self.uuid))
-            .left_join(
-                users_organizations::table.on(ciphers::organization_uuid
-                    .eq(users_organizations::org_uuid.nullable())
-                    .and(users_organizations::user_uuid.eq(user_uuid))),
-            )
-            .left_join(ciphers_collections::table)
-            .left_join(
-                users_collections::table
-                    .on(ciphers_collections::collection_uuid.eq(users_collections::collection_uuid)),
-            )
-            .filter(ciphers::user_uuid.eq(user_uuid).or(
-                // Cipher owner
-                users_organizations::access_all.eq(true).or(
-                    // access_all in Organization
-                    users_organizations::atype.le(UserOrgType::Admin as i32).or(
-                        // Org admin or owner
-                        users_collections::user_uuid.eq(user_uuid), // Access to Collection
-                    ),
-                ),
-            ))
-            .select(ciphers::all_columns)
-            .first::<Self>(&**conn)
-            .ok()
-            .is_some()
+        self.get_access_restrictions(&user_uuid, &conn).is_some()
     }
 
     pub fn get_folder_uuid(&self, user_uuid: &str, conn: &DbConn) -> Option<String> {

src/db/models/collection.rs

@@ -199,6 +199,7 @@ pub struct CollectionUser {
     pub user_uuid: String,
     pub collection_uuid: String,
     pub read_only: bool,
+    pub hide_passwords: bool,
 }
 
 /// Database methods
@@ -214,7 +215,7 @@ impl CollectionUser {
     }
 
     #[cfg(feature = "postgresql")]
-    pub fn save(user_uuid: &str, collection_uuid: &str, read_only: bool, conn: &DbConn) -> EmptyResult {
+    pub fn save(user_uuid: &str, collection_uuid: &str, read_only: bool, hide_passwords: bool, conn: &DbConn) -> EmptyResult {
         User::update_uuid_revision(&user_uuid, conn);
 
         diesel::insert_into(users_collections::table)
@@ -222,16 +223,18 @@ impl CollectionUser {
                 users_collections::user_uuid.eq(user_uuid),
                 users_collections::collection_uuid.eq(collection_uuid),
                 users_collections::read_only.eq(read_only),
+                users_collections::hide_passwords.eq(hide_passwords),
             ))
             .on_conflict((users_collections::user_uuid, users_collections::collection_uuid))
             .do_update()
             .set(users_collections::read_only.eq(read_only))
+            .set(users_collections::hide_passwords.eq(hide_passwords))
             .execute(&**conn)
             .map_res("Error adding user to collection")
     }
 
     #[cfg(not(feature = "postgresql"))]
-    pub fn save(user_uuid: &str, collection_uuid: &str, read_only: bool, conn: &DbConn) -> EmptyResult {
+    pub fn save(user_uuid: &str, collection_uuid: &str, read_only: bool, hide_passwords: bool, conn: &DbConn) -> EmptyResult {
         User::update_uuid_revision(&user_uuid, conn);
 
         diesel::replace_into(users_collections::table)
@@ -239,6 +242,7 @@ impl CollectionUser {
                 users_collections::user_uuid.eq(user_uuid),
                 users_collections::collection_uuid.eq(collection_uuid),
                 users_collections::read_only.eq(read_only),
+                users_collections::hide_passwords.eq(hide_passwords),
             ))
             .execute(&**conn)
             .map_res("Error adding user to collection")

src/db/models/organization.rs

@@ -310,10 +310,11 @@ impl UserOrganization {
         })
     }
 
-    pub fn to_json_read_only(&self, read_only: bool) -> Value {
+    pub fn to_json_user_access_restrictions(&self, col_user: &CollectionUser) -> Value {
         json!({
             "Id": self.uuid,
-            "ReadOnly": read_only
+            "ReadOnly": col_user.read_only,
+            "HidePasswords": col_user.hide_passwords,
         })
     }
 
@@ -324,7 +325,11 @@ impl UserOrganization {
             let collections = CollectionUser::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn);
             collections
                 .iter()
-                .map(|c| json!({"Id": c.collection_uuid, "ReadOnly": c.read_only}))
+                .map(|c| json!({
+                    "Id": c.collection_uuid,
+                    "ReadOnly": c.read_only,
+                    "HidePasswords": c.hide_passwords,
+                }))
                 .collect()
         };
 
@@ -388,8 +393,13 @@ impl UserOrganization {
         Ok(())
     }
 
+    pub fn has_status(self, status: UserOrgStatus) -> bool {
+        self.status == status as i32
+    }
+
     pub fn has_full_access(self) -> bool {
-        self.access_all || self.atype >= UserOrgType::Admin
+        (self.access_all || self.atype >= UserOrgType::Admin) &&
+            self.has_status(UserOrgStatus::Confirmed)
     }
 
     pub fn find_by_uuid(uuid: &str, conn: &DbConn) -> Option<Self> {

src/db/schemas/mysql/schema.rs

@@ -141,6 +141,7 @@ table! {
         user_uuid -> Varchar,
         collection_uuid -> Varchar,
         read_only -> Bool,
+        hide_passwords -> Bool,
     }
 }
 

src/db/schemas/postgresql/schema.rs

@@ -141,6 +141,7 @@ table! {
         user_uuid -> Text,
         collection_uuid -> Text,
         read_only -> Bool,
+        hide_passwords -> Bool,
     }
 }
 

src/db/schemas/sqlite/schema.rs

@@ -141,6 +141,7 @@ table! {
         user_uuid -> Text,
         collection_uuid -> Text,
         read_only -> Bool,
+        hide_passwords -> Bool,
     }
 }