From 25c16cf03a24607d0dee956b740147a74cfbc9f0 Mon Sep 17 00:00:00 2001 From: qaz741wsd856 Date: Mon, 1 Dec 2025 05:11:50 +0000 Subject: [PATCH] refactor: unify cipher data definition --- src/handlers/accounts.rs | 44 ++++++++++++---------- src/handlers/ciphers.rs | 30 +++------------ src/handlers/import.rs | 8 +--- src/models/cipher.rs | 79 +++++++++++++++++++++++++--------------- src/models/import.rs | 36 +----------------- src/models/user.rs | 35 +----------------- 6 files changed, 84 insertions(+), 148 deletions(-) diff --git a/src/handlers/accounts.rs b/src/handlers/accounts.rs index 9a7599c..d362a08 100644 --- a/src/handlers/accounts.rs +++ b/src/handlers/accounts.rs @@ -293,15 +293,29 @@ pub async fn post_rotatekey( } // Validate data integrity using D1 batch operations - // Step 1: Count check - ensure request has exactly the same number of items - // Step 2: EXCEPT check - ensure request has exactly the same IDs - let request_cipher_ids: Vec = payload + // Step 1: Ensure all personal ciphers have id (required for key rotation) + // Step 2: Count check - ensure request has exactly the same number of items as DB + // Step 3: EXCEPT check - ensure request has exactly the same IDs as DB + let personal_ciphers: Vec<_> = payload .account_data .ciphers .iter() .filter(|c| c.organization_id.is_none()) - .map(|c| c.id.clone()) .collect(); + + let request_cipher_ids: Vec = personal_ciphers + .iter() + .filter_map(|c| c.id.clone()) + .collect(); + + // All personal ciphers must have an id for key rotation + if personal_ciphers.len() != request_cipher_ids.len() { + log::error!("All ciphers must have an id for key rotation: {:?} != {:?}", personal_ciphers.len(), request_cipher_ids.len()); + return Err(AppError::BadRequest( + "All ciphers must have an id for key rotation".to_string(), + )); + } + // Filter out null folder IDs (Bitwarden client bug: https://github.com/bitwarden/clients/issues/8453) let request_folder_ids: Vec = payload .account_data @@ -400,23 +414,15 @@ pub async fn post_rotatekey( // Update all ciphers with new encrypted data (batch operation) // Only update personal ciphers (organization_id is None) let mut cipher_statements: Vec = - Vec::with_capacity(payload.account_data.ciphers.len()); - for cipher in &payload.account_data.ciphers { - // Skip organization ciphers - if cipher.organization_id.is_some() { - continue; - } + Vec::with_capacity(personal_ciphers.len()); + for cipher in personal_ciphers { + // id is guaranteed to exist (validated above) + let cipher_id = cipher.id.as_ref().unwrap(); let cipher_data = CipherData { name: cipher.name.clone(), notes: cipher.notes.clone(), - login: cipher.login.clone(), - card: cipher.card.clone(), - identity: cipher.identity.clone(), - secure_note: cipher.secure_note.clone(), - fields: cipher.fields.clone(), - password_history: cipher.password_history.clone(), - reprompt: cipher.reprompt, + type_fields: cipher.type_fields.clone(), }; let data = serde_json::to_string(&cipher_data).map_err(|_| AppError::Internal)?; @@ -426,9 +432,9 @@ pub async fn post_rotatekey( "UPDATE ciphers SET data = ?1, folder_id = ?2, favorite = ?3, updated_at = ?4 WHERE id = ?5 AND user_id = ?6", data, cipher.folder_id, - cipher.favorite, + cipher.favorite.unwrap_or(false), now, - cipher.id, + cipher_id, user_id ) .map_err(|_| AppError::Database)?; diff --git a/src/handlers/ciphers.rs b/src/handlers/ciphers.rs index ee33a81..7239813 100644 --- a/src/handlers/ciphers.rs +++ b/src/handlers/ciphers.rs @@ -26,13 +26,7 @@ pub async fn create_cipher( let cipher_data = CipherData { name: cipher_data_req.name, notes: cipher_data_req.notes, - login: cipher_data_req.login, - card: cipher_data_req.card, - identity: cipher_data_req.identity, - secure_note: cipher_data_req.secure_note, - fields: cipher_data_req.fields, - password_history: cipher_data_req.password_history, - reprompt: cipher_data_req.reprompt, + type_fields: cipher_data_req.type_fields, }; let data_value = serde_json::to_value(&cipher_data).map_err(|_| AppError::Internal)?; @@ -43,7 +37,7 @@ pub async fn create_cipher( organization_id: cipher_data_req.organization_id.clone(), r#type: cipher_data_req.r#type, data: data_value, - favorite: cipher_data_req.favorite, + favorite: cipher_data_req.favorite.unwrap_or(false), folder_id: cipher_data_req.folder_id.clone(), deleted_at: None, created_at: now.clone(), @@ -111,13 +105,7 @@ pub async fn update_cipher( let cipher_data = CipherData { name: cipher_data_req.name, notes: cipher_data_req.notes, - login: cipher_data_req.login, - card: cipher_data_req.card, - identity: cipher_data_req.identity, - secure_note: cipher_data_req.secure_note, - fields: cipher_data_req.fields, - password_history: cipher_data_req.password_history, - reprompt: cipher_data_req.reprompt, + type_fields: cipher_data_req.type_fields, }; let data_value = serde_json::to_value(&cipher_data).map_err(|_| AppError::Internal)?; @@ -128,7 +116,7 @@ pub async fn update_cipher( organization_id: cipher_data_req.organization_id.clone(), r#type: cipher_data_req.r#type, data: data_value, - favorite: cipher_data_req.favorite, + favorite: cipher_data_req.favorite.unwrap_or(false), folder_id: cipher_data_req.folder_id.clone(), deleted_at: None, created_at: existing_cipher.created_at, @@ -403,13 +391,7 @@ pub async fn create_cipher_simple( let cipher_data = CipherData { name: payload.name, notes: payload.notes, - login: payload.login, - card: payload.card, - identity: payload.identity, - secure_note: payload.secure_note, - fields: payload.fields, - password_history: payload.password_history, - reprompt: payload.reprompt, + type_fields: payload.type_fields, }; let data_value = serde_json::to_value(&cipher_data).map_err(|_| AppError::Internal)?; @@ -420,7 +402,7 @@ pub async fn create_cipher_simple( organization_id: payload.organization_id.clone(), r#type: payload.r#type, data: data_value, - favorite: payload.favorite, + favorite: payload.favorite.unwrap_or(false), folder_id: payload.folder_id.clone(), deleted_at: None, created_at: now.clone(), diff --git a/src/handlers/import.rs b/src/handlers/import.rs index 77016da..f21c100 100644 --- a/src/handlers/import.rs +++ b/src/handlers/import.rs @@ -129,13 +129,7 @@ pub async fn import_data( let cipher_data = CipherData { name: import_cipher.name, notes: import_cipher.notes, - login: import_cipher.login, - card: import_cipher.card, - identity: import_cipher.identity, - secure_note: import_cipher.secure_note, - fields: import_cipher.fields, - password_history: import_cipher.password_history, - reprompt: import_cipher.reprompt, + type_fields: import_cipher.type_fields, }; let data_value = serde_json::to_value(&cipher_data).map_err(|_| AppError::Internal)?; diff --git a/src/models/cipher.rs b/src/models/cipher.rs index c5a97e7..b7317d8 100644 --- a/src/models/cipher.rs +++ b/src/models/cipher.rs @@ -1,13 +1,20 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use serde_json::{json, Map, Value}; -// This struct represents the data stored in the `data` column of the `ciphers` table. -#[derive(Debug, Serialize, Deserialize, Clone)] +// Cipher types: +// Login = 1, +// SecureNote = 2, +// Card = 3, +// Identity = 4, +// SshKey = 5 + +/// Common cipher type-specific fields shared across multiple cipher structures. +/// These represent the encrypted content fields that vary based on cipher type. +/// Used with `#[serde(flatten)]` to embed these fields into other structs. +#[derive(Debug, Serialize, Deserialize, Clone, Default)] #[serde(rename_all = "camelCase")] -pub struct CipherData { - pub name: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub notes: Option, +pub struct CipherTypeFields { + // Only one of these should exist, depending on cipher type #[serde(skip_serializing_if = "Option::is_none")] pub login: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -17,6 +24,9 @@ pub struct CipherData { #[serde(skip_serializing_if = "Option::is_none")] pub secure_note: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub ssh_key: Option, + // Common fields + #[serde(skip_serializing_if = "Option::is_none")] pub fields: Option, #[serde(skip_serializing_if = "Option::is_none")] pub password_history: Option, @@ -24,6 +34,17 @@ pub struct CipherData { pub reprompt: Option, } +/// This struct represents the data stored in the `data` column of the `ciphers` table. +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct CipherData { + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub notes: Option, + #[serde(flatten)] + pub type_fields: CipherTypeFields, +} + // Custom deserialization function for booleans fn deserialize_bool_from_int<'de, D>(deserializer: D) -> Result where @@ -209,12 +230,14 @@ impl Serialize for Cipher { let mut secure_note = Value::Null; let mut card = Value::Null; let mut identity = Value::Null; + let mut ssh_key = Value::Null; match self.r#type { 1 => login = data_clone.get("login").cloned().unwrap_or(Value::Null), 2 => secure_note = data_clone.get("secureNote").cloned().unwrap_or(Value::Null), 3 => card = data_clone.get("card").cloned().unwrap_or(Value::Null), 4 => identity = data_clone.get("identity").cloned().unwrap_or(Value::Null), + 5 => ssh_key = data_clone.get("sshKey").cloned().unwrap_or(Value::Null), _ => {} } @@ -222,6 +245,7 @@ impl Serialize for Cipher { response_map.insert("secureNote".to_string(), secure_note); response_map.insert("card".to_string(), card); response_map.insert("identity".to_string(), identity); + response_map.insert("sshKey".to_string(), ssh_key); } else { response_map.insert("name".to_string(), Value::Null); response_map.insert("notes".to_string(), Value::Null); @@ -232,6 +256,7 @@ impl Serialize for Cipher { response_map.insert("secureNote".to_string(), Value::Null); response_map.insert("card".to_string(), Value::Null); response_map.insert("identity".to_string(), Value::Null); + response_map.insert("sshKey".to_string(), Value::Null); } Value::Object(response_map).serialize(serializer) @@ -246,44 +271,38 @@ fn default_true() -> bool { true } -// Represents the "Cipher" object within the incoming request payload. -#[derive(Debug, Serialize, Deserialize)] +/// Represents the "Cipher" object within incoming request payloads. +/// Used for create, update, import, and key rotation scenarios. +/// Aligned with vaultwarden's CipherData structure. +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct CipherRequestData { - #[serde(rename = "type")] - pub r#type: i32, + // Id is optional as it is included only in bulk share / key rotation + #[serde(skip_serializing_if = "Option::is_none")] + pub id: Option, + // Folder id is not included in import (determined by folder_relationships) #[serde(skip_serializing_if = "Option::is_none")] pub folder_id: Option, + #[serde(alias = "organizationID")] #[serde(skip_serializing_if = "Option::is_none")] pub organization_id: Option, + #[serde(rename = "type")] + pub r#type: i32, pub name: String, #[serde(skip_serializing_if = "Option::is_none")] pub notes: Option, #[serde(default)] - pub favorite: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub login: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub card: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub identity: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub secure_note: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub fields: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub password_history: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub reprompt: Option, + pub favorite: Option, + #[serde(flatten)] + pub type_fields: CipherTypeFields, + // The revision datetime (in ISO 8601 format) of the client's local copy + // Used to prevent updating a cipher when client doesn't have the latest version #[serde(skip_serializing_if = "Option::is_none")] pub last_known_revision_date: Option, - /// Cipher key field used for cipher key rotation scenarios - #[serde(skip_serializing_if = "Option::is_none")] - pub key: Option, } -// Represents the full request payload for creating a cipher. -// Supports both camelCase and PascalCase for compatibility with different clients. +/// Represents the full request payload for creating a cipher with collections. +/// Supports both camelCase and PascalCase for compatibility with different clients. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CreateCipherRequest { diff --git a/src/models/import.rs b/src/models/import.rs index bd62eee..4e5e918 100644 --- a/src/models/import.rs +++ b/src/models/import.rs @@ -1,38 +1,6 @@ use serde::Deserialize; -use serde_json::Value; -/// Cipher data structure for import requests. -/// Aligned with vaultwarden's CipherData used in /ciphers/import endpoint. -/// Note: The `key` field is omitted as this service's compatibility version doesn't support per-cipher encryption keys. -#[derive(Deserialize, Debug)] -#[serde(rename_all = "camelCase")] -pub struct ImportCipher { - /// Optional cipher ID (only used in bulk share scenarios, not import) - #[allow(dead_code)] - pub id: Option, - /// Folder ID is not included in import - determined by folder_relationships instead - #[allow(dead_code)] - pub folder_id: Option, - #[serde(alias = "organizationID")] - pub organization_id: Option, - #[serde(rename = "type")] - pub r#type: i32, - pub name: String, - pub notes: Option, - pub fields: Option, - // Only one of these should exist, depending on type - pub login: Option, - pub secure_note: Option, - pub card: Option, - pub identity: Option, - #[serde(default)] - pub favorite: Option, - pub reprompt: Option, - pub password_history: Option, - /// The revision datetime (in ISO 8601 format) of the client's local copy - #[allow(dead_code)] - pub last_known_revision_date: Option, -} +use crate::models::cipher::CipherRequestData; /// Folder data structure for import requests. /// Aligned with vaultwarden's FolderData. @@ -59,7 +27,7 @@ pub struct FolderRelationship { #[derive(Deserialize, Debug)] #[serde(rename_all = "camelCase")] pub struct ImportRequest { - pub ciphers: Vec, + pub ciphers: Vec, pub folders: Vec, #[serde(default)] pub folder_relationships: Vec, diff --git a/src/models/user.rs b/src/models/user.rs index cb9f3a2..7e39b22 100644 --- a/src/models/user.rs +++ b/src/models/user.rs @@ -183,43 +183,10 @@ pub struct RotateAccountKeys { #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct RotateAccountData { - pub ciphers: Vec, + pub ciphers: Vec, pub folders: Vec, } -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct RotateCipherData { - pub id: String, - #[serde(rename = "type")] - pub r#type: i32, - #[serde(skip_serializing_if = "Option::is_none")] - pub folder_id: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub organization_id: Option, - pub name: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub notes: Option, - #[serde(default)] - pub favorite: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub login: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub card: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub identity: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub secure_note: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub fields: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub password_history: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub reprompt: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub key: Option, -} - #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct RotateFolderData {