diff --git a/migrations/mysql/2020-08-02-025025_add_favorites_table/down.sql b/migrations/mysql/2020-08-02-025025_add_favorites_table/down.sql new file mode 100644 index 0000000..25eb713 --- /dev/null +++ b/migrations/mysql/2020-08-02-025025_add_favorites_table/down.sql @@ -0,0 +1,13 @@ +ALTER TABLE ciphers +ADD COLUMN favorite BOOLEAN NOT NULL DEFAULT FALSE; + +-- Transfer favorite status for user-owned ciphers. +UPDATE ciphers +SET favorite = TRUE +WHERE EXISTS ( + SELECT * FROM favorites + WHERE favorites.user_uuid = ciphers.user_uuid + AND favorites.cipher_uuid = ciphers.uuid +); + +DROP TABLE favorites; diff --git a/migrations/mysql/2020-08-02-025025_add_favorites_table/up.sql b/migrations/mysql/2020-08-02-025025_add_favorites_table/up.sql new file mode 100644 index 0000000..ebff7e3 --- /dev/null +++ b/migrations/mysql/2020-08-02-025025_add_favorites_table/up.sql @@ -0,0 +1,16 @@ +CREATE TABLE favorites ( + user_uuid CHAR(36) NOT NULL REFERENCES users(uuid), + cipher_uuid CHAR(36) NOT NULL REFERENCES ciphers(uuid), + + PRIMARY KEY (user_uuid, cipher_uuid) +); + +-- Transfer favorite status for user-owned ciphers. +INSERT INTO favorites(user_uuid, cipher_uuid) +SELECT user_uuid, uuid +FROM ciphers +WHERE favorite = TRUE + AND user_uuid IS NOT NULL; + +ALTER TABLE ciphers +DROP COLUMN favorite; diff --git a/migrations/postgresql/2020-08-02-025025_add_favorites_table/down.sql b/migrations/postgresql/2020-08-02-025025_add_favorites_table/down.sql new file mode 100644 index 0000000..25eb713 --- /dev/null +++ b/migrations/postgresql/2020-08-02-025025_add_favorites_table/down.sql @@ -0,0 +1,13 @@ +ALTER TABLE ciphers +ADD COLUMN favorite BOOLEAN NOT NULL DEFAULT FALSE; + +-- Transfer favorite status for user-owned ciphers. +UPDATE ciphers +SET favorite = TRUE +WHERE EXISTS ( + SELECT * FROM favorites + WHERE favorites.user_uuid = ciphers.user_uuid + AND favorites.cipher_uuid = ciphers.uuid +); + +DROP TABLE favorites; diff --git a/migrations/postgresql/2020-08-02-025025_add_favorites_table/up.sql b/migrations/postgresql/2020-08-02-025025_add_favorites_table/up.sql new file mode 100644 index 0000000..9a8787f --- /dev/null +++ b/migrations/postgresql/2020-08-02-025025_add_favorites_table/up.sql @@ -0,0 +1,16 @@ +CREATE TABLE favorites ( + user_uuid VARCHAR(40) NOT NULL REFERENCES users(uuid), + cipher_uuid VARCHAR(40) NOT NULL REFERENCES ciphers(uuid), + + PRIMARY KEY (user_uuid, cipher_uuid) +); + +-- Transfer favorite status for user-owned ciphers. +INSERT INTO favorites(user_uuid, cipher_uuid) +SELECT user_uuid, uuid +FROM ciphers +WHERE favorite = TRUE + AND user_uuid IS NOT NULL; + +ALTER TABLE ciphers +DROP COLUMN favorite; diff --git a/migrations/sqlite/2020-08-02-025025_add_favorites_table/down.sql b/migrations/sqlite/2020-08-02-025025_add_favorites_table/down.sql new file mode 100644 index 0000000..6920b5f --- /dev/null +++ b/migrations/sqlite/2020-08-02-025025_add_favorites_table/down.sql @@ -0,0 +1,13 @@ +ALTER TABLE ciphers +ADD COLUMN favorite BOOLEAN NOT NULL DEFAULT 0; -- FALSE + +-- Transfer favorite status for user-owned ciphers. +UPDATE ciphers +SET favorite = 1 +WHERE EXISTS ( + SELECT * FROM favorites + WHERE favorites.user_uuid = ciphers.user_uuid + AND favorites.cipher_uuid = ciphers.uuid +); + +DROP TABLE favorites; diff --git a/migrations/sqlite/2020-08-02-025025_add_favorites_table/up.sql b/migrations/sqlite/2020-08-02-025025_add_favorites_table/up.sql new file mode 100644 index 0000000..4854114 --- /dev/null +++ b/migrations/sqlite/2020-08-02-025025_add_favorites_table/up.sql @@ -0,0 +1,71 @@ +CREATE TABLE favorites ( + user_uuid TEXT NOT NULL REFERENCES users(uuid), + cipher_uuid TEXT NOT NULL REFERENCES ciphers(uuid), + + PRIMARY KEY (user_uuid, cipher_uuid) +); + +-- Transfer favorite status for user-owned ciphers. +INSERT INTO favorites(user_uuid, cipher_uuid) +SELECT user_uuid, uuid +FROM ciphers +WHERE favorite = 1 + AND user_uuid IS NOT NULL; + +-- Drop the `favorite` column from the `ciphers` table, using the 12-step +-- procedure from . +-- Note that some steps aren't applicable and are omitted. + +-- 1. If foreign key constraints are enabled, disable them using PRAGMA foreign_keys=OFF. +-- +-- Diesel runs each migration in its own transaction. `PRAGMA foreign_keys` +-- is a no-op within a transaction, so this step must be done outside of this +-- file, before starting the Diesel migrations. + +-- 2. Start a transaction. +-- +-- Diesel already runs each migration in its own transaction. + +-- 4. Use CREATE TABLE to construct a new table "new_X" that is in the +-- desired revised format of table X. Make sure that the name "new_X" does +-- not collide with any existing table name, of course. + +CREATE TABLE new_ciphers( + uuid TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL, + user_uuid TEXT REFERENCES users(uuid), + organization_uuid TEXT REFERENCES organizations(uuid), + atype INTEGER NOT NULL, + name TEXT NOT NULL, + notes TEXT, + fields TEXT, + data TEXT NOT NULL, + password_history TEXT, + deleted_at DATETIME +); + +-- 5. Transfer content from X into new_X using a statement like: +-- INSERT INTO new_X SELECT ... FROM X. + +INSERT INTO new_ciphers(uuid, created_at, updated_at, user_uuid, organization_uuid, atype, + name, notes, fields, data, password_history, deleted_at) +SELECT uuid, created_at, updated_at, user_uuid, organization_uuid, atype, + name, notes, fields, data, password_history, deleted_at +FROM ciphers; + +-- 6. Drop the old table X: DROP TABLE X. + +DROP TABLE ciphers; + +-- 7. Change the name of new_X to X using: ALTER TABLE new_X RENAME TO X. + +ALTER TABLE new_ciphers RENAME TO ciphers; + +-- 11. Commit the transaction started in step 2. + +-- 12. If foreign keys constraints were originally enabled, reenable them now. +-- +-- `PRAGMA foreign_keys` is scoped to a database connection, and Diesel +-- migrations are run in a separate database connection that is closed once +-- the migrations finish. diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 7ea2dbe..53760ae 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -303,7 +303,6 @@ pub fn update_cipher_from_data( type_data["PasswordHistory"] = data.PasswordHistory.clone().unwrap_or(Value::Null); // TODO: ******* Backwards compat end ********** - cipher.favorite = data.Favorite.unwrap_or(false); cipher.name = data.Name; cipher.notes = data.Notes; cipher.fields = data.Fields.map(|f| f.to_string()); @@ -312,6 +311,7 @@ pub fn update_cipher_from_data( cipher.save(&conn)?; cipher.move_to_folder(data.FolderId, &headers.user.uuid, &conn)?; + cipher.set_favorite(data.Favorite, &headers.user.uuid, &conn)?; if ut != UpdateType::None { nt.send_cipher_update(ut, &cipher, &cipher.update_users_revision(&conn)); @@ -410,6 +410,11 @@ fn put_cipher(uuid: String, data: JsonUpcase, headers: Headers, conn None => err!("Cipher doesn't exist"), }; + // TODO: Check if only the folder ID or favorite status is being changed. + // These are per-user properties that technically aren't part of the + // cipher itself, so the user shouldn't need write access to change these. + // Interestingly, upstream Bitwarden doesn't properly handle this either. + if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) { err!("Cipher is not write accessible") } diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index 3bf0ea6..5328d9d 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -32,7 +32,6 @@ pub struct Cipher { pub data: String, - pub favorite: bool, pub password_history: Option, pub deleted_at: Option, } @@ -51,7 +50,6 @@ impl Cipher { organization_uuid: None, atype, - favorite: false, name, notes: None, @@ -128,7 +126,7 @@ impl Cipher { "RevisionDate": format_date(&self.updated_at), "DeletedDate": self.deleted_at.map_or(Value::Null, |d| Value::String(format_date(&d))), "FolderId": self.get_folder_uuid(&user_uuid, &conn), - "Favorite": self.favorite, + "Favorite": self.is_favorite(&user_uuid, &conn), "OrganizationId": self.organization_uuid, "Attachments": attachments_json, "OrganizationUseTotp": true, @@ -337,6 +335,50 @@ impl Cipher { self.get_access_restrictions(&user_uuid, &conn).is_some() } + // Returns whether this cipher is a favorite of the specified user. + pub fn is_favorite(&self, user_uuid: &str, conn: &DbConn) -> bool { + let query = favorites::table + .filter(favorites::user_uuid.eq(user_uuid)) + .filter(favorites::cipher_uuid.eq(&self.uuid)) + .count(); + + query.first::(&**conn).ok().unwrap_or(0) != 0 + } + + // Updates whether this cipher is a favorite of the specified user. + pub fn set_favorite(&self, favorite: Option, user_uuid: &str, conn: &DbConn) -> EmptyResult { + if favorite.is_none() { + // No change requested. + return Ok(()); + } + + let (old, new) = (self.is_favorite(user_uuid, &conn), favorite.unwrap()); + match (old, new) { + (false, true) => { + User::update_uuid_revision(user_uuid, &conn); + diesel::insert_into(favorites::table) + .values(( + favorites::user_uuid.eq(user_uuid), + favorites::cipher_uuid.eq(&self.uuid), + )) + .execute(&**conn) + .map_res("Error adding favorite") + } + (true, false) => { + User::update_uuid_revision(user_uuid, &conn); + diesel::delete( + favorites::table + .filter(favorites::user_uuid.eq(user_uuid)) + .filter(favorites::cipher_uuid.eq(&self.uuid)) + ) + .execute(&**conn) + .map_res("Error removing favorite") + } + // Otherwise, the favorite status is already what it should be. + _ => Ok(()) + } + } + pub fn get_folder_uuid(&self, user_uuid: &str, conn: &DbConn) -> Option { folders_ciphers::table .inner_join(folders::table) diff --git a/src/db/schemas/mysql/schema.rs b/src/db/schemas/mysql/schema.rs index 4b8fbc7..a353fa5 100644 --- a/src/db/schemas/mysql/schema.rs +++ b/src/db/schemas/mysql/schema.rs @@ -1,7 +1,7 @@ table! { attachments (id) { - id -> Varchar, - cipher_uuid -> Varchar, + id -> Text, + cipher_uuid -> Text, file_name -> Text, file_size -> Integer, akey -> Nullable, @@ -10,17 +10,16 @@ table! { table! { ciphers (uuid) { - uuid -> Varchar, + uuid -> Text, created_at -> Datetime, updated_at -> Datetime, - user_uuid -> Nullable, - organization_uuid -> Nullable, + user_uuid -> Nullable, + organization_uuid -> Nullable, atype -> Integer, name -> Text, notes -> Nullable, fields -> Nullable, data -> Text, - favorite -> Bool, password_history -> Nullable, deleted_at -> Nullable, } @@ -28,25 +27,25 @@ table! { table! { ciphers_collections (cipher_uuid, collection_uuid) { - cipher_uuid -> Varchar, - collection_uuid -> Varchar, + cipher_uuid -> Text, + collection_uuid -> Text, } } table! { collections (uuid) { - uuid -> Varchar, - org_uuid -> Varchar, + uuid -> Text, + org_uuid -> Text, name -> Text, } } table! { devices (uuid) { - uuid -> Varchar, + uuid -> Text, created_at -> Datetime, updated_at -> Datetime, - user_uuid -> Varchar, + user_uuid -> Text, name -> Text, atype -> Integer, push_token -> Nullable, @@ -55,33 +54,40 @@ table! { } } +table! { + favorites (user_uuid, cipher_uuid) { + user_uuid -> Text, + cipher_uuid -> Text, + } +} + table! { folders (uuid) { - uuid -> Varchar, + uuid -> Text, created_at -> Datetime, updated_at -> Datetime, - user_uuid -> Varchar, + user_uuid -> Text, name -> Text, } } table! { folders_ciphers (cipher_uuid, folder_uuid) { - cipher_uuid -> Varchar, - folder_uuid -> Varchar, + cipher_uuid -> Text, + folder_uuid -> Text, } } table! { invitations (email) { - email -> Varchar, + email -> Text, } } table! { org_policies (uuid) { - uuid -> Varchar, - org_uuid -> Varchar, + uuid -> Text, + org_uuid -> Text, atype -> Integer, enabled -> Bool, data -> Text, @@ -90,7 +96,7 @@ table! { table! { organizations (uuid) { - uuid -> Varchar, + uuid -> Text, name -> Text, billing_email -> Text, } @@ -98,8 +104,8 @@ table! { table! { twofactor (uuid) { - uuid -> Varchar, - user_uuid -> Varchar, + uuid -> Text, + user_uuid -> Text, atype -> Integer, enabled -> Bool, data -> Text, @@ -109,18 +115,18 @@ table! { table! { users (uuid) { - uuid -> Varchar, + uuid -> Text, created_at -> Datetime, updated_at -> Datetime, verified_at -> Nullable, last_verifying_at -> Nullable, login_verify_count -> Integer, - email -> Varchar, - email_new -> Nullable, - email_new_token -> Nullable, + email -> Text, + email_new -> Nullable, + email_new_token -> Nullable, name -> Text, - password_hash -> Blob, - salt -> Blob, + password_hash -> Binary, + salt -> Binary, password_iterations -> Integer, password_hint -> Nullable, akey -> Text, @@ -138,8 +144,8 @@ table! { table! { users_collections (user_uuid, collection_uuid) { - user_uuid -> Varchar, - collection_uuid -> Varchar, + user_uuid -> Text, + collection_uuid -> Text, read_only -> Bool, hide_passwords -> Bool, } @@ -147,9 +153,9 @@ table! { table! { users_organizations (uuid) { - uuid -> Varchar, - user_uuid -> Varchar, - org_uuid -> Varchar, + uuid -> Text, + user_uuid -> Text, + org_uuid -> Text, access_all -> Bool, akey -> Text, status -> Integer, diff --git a/src/db/schemas/postgresql/schema.rs b/src/db/schemas/postgresql/schema.rs index ec13d26..fcacb3e 100644 --- a/src/db/schemas/postgresql/schema.rs +++ b/src/db/schemas/postgresql/schema.rs @@ -20,7 +20,6 @@ table! { notes -> Nullable, fields -> Nullable, data -> Text, - favorite -> Bool, password_history -> Nullable, deleted_at -> Nullable, } @@ -55,6 +54,13 @@ table! { } } +table! { + favorites (user_uuid, cipher_uuid) { + user_uuid -> Text, + cipher_uuid -> Text, + } +} + table! { folders (uuid) { uuid -> Text, diff --git a/src/db/schemas/sqlite/schema.rs b/src/db/schemas/sqlite/schema.rs index ec13d26..fcacb3e 100644 --- a/src/db/schemas/sqlite/schema.rs +++ b/src/db/schemas/sqlite/schema.rs @@ -20,7 +20,6 @@ table! { notes -> Nullable, fields -> Nullable, data -> Text, - favorite -> Bool, password_history -> Nullable, deleted_at -> Nullable, } @@ -55,6 +54,13 @@ table! { } } +table! { + favorites (user_uuid, cipher_uuid) { + user_uuid -> Text, + cipher_uuid -> Text, + } +} + table! { folders (uuid) { uuid -> Text, diff --git a/src/main.rs b/src/main.rs index 2ce16a7..b9c8bcf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -313,12 +313,23 @@ mod migrations { // Disable Foreign Key Checks during migration use diesel::RunQueryDsl; + + // FIXME: Per https://www.postgresql.org/docs/12/sql-set-constraints.html, + // "SET CONSTRAINTS sets the behavior of constraint checking within the + // current transaction", so this setting probably won't take effect for + // any of the migrations since it's being run outside of a transaction. + // Migrations that need to disable foreign key checks should run this + // from within the migration script itself. #[cfg(feature = "postgres")] diesel::sql_query("SET CONSTRAINTS ALL DEFERRED").execute(&connection).expect("Failed to disable Foreign Key Checks during migrations"); + + // Scoped to a connection/session. #[cfg(feature = "mysql")] diesel::sql_query("SET FOREIGN_KEY_CHECKS = 0").execute(&connection).expect("Failed to disable Foreign Key Checks during migrations"); + + // Scoped to a connection. #[cfg(feature = "sqlite")] - diesel::sql_query("PRAGMA defer_foreign_keys = ON").execute(&connection).expect("Failed to disable Foreign Key Checks during migrations"); + diesel::sql_query("PRAGMA foreign_keys = OFF").execute(&connection).expect("Failed to disable Foreign Key Checks during migrations"); embedded_migrations::run_with_output(&connection, &mut stdout()).expect("Can't run migrations"); }