summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMathijs van Veluw <[email protected]>2024-09-09 11:36:37 +0200
committerGitHub <[email protected]>2024-09-09 11:36:37 +0200
commitdca14285fd1506c28f32108ea82da6e1525a8e99 (patch)
tree0c1e999578492da0c1e5b3283d9f6307e361137b
parent66baa5e7d844a455ffe18a89e9cb8521a013af14 (diff)
downloadvaultwarden-dca14285fd1506c28f32108ea82da6e1525a8e99.tar.gz
vaultwarden-dca14285fd1506c28f32108ea82da6e1525a8e99.zip
Fix sync with new native clients (#4932)
-rw-r--r--src/api/core/accounts.rs2
-rw-r--r--src/api/core/ciphers.rs4
-rw-r--r--src/api/core/organizations.rs2
-rw-r--r--src/db/models/cipher.rs57
4 files changed, 50 insertions, 15 deletions
diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs
index 187b9096..cc9cd6ce 100644
--- a/src/api/core/accounts.rs
+++ b/src/api/core/accounts.rs
@@ -490,7 +490,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
- Cipher::validate_notes(&data.ciphers)?;
+ Cipher::validate_cipher_data(&data.ciphers)?;
let user_uuid = &headers.user.uuid;
diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs
index 6fcde07c..13bf701f 100644
--- a/src/api/core/ciphers.rs
+++ b/src/api/core/ciphers.rs
@@ -233,7 +233,7 @@ pub struct CipherData {
favorite: Option<bool>,
reprompt: Option<i32>,
- password_history: Option<Value>,
+ pub password_history: Option<Value>,
// These are used during key rotation
// 'Attachments' is unused, contains map of {id: filename}
@@ -563,7 +563,7 @@ async fn post_ciphers_import(
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
- Cipher::validate_notes(&data.ciphers)?;
+ Cipher::validate_cipher_data(&data.ciphers)?;
// Read and create the folders
let existing_folders: Vec<String> =
diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs
index e0c3f081..6d9f055a 100644
--- a/src/api/core/organizations.rs
+++ b/src/api/core/organizations.rs
@@ -1596,7 +1596,7 @@ async fn post_org_import(
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
- Cipher::validate_notes(&data.ciphers)?;
+ Cipher::validate_cipher_data(&data.ciphers)?;
let mut collections = Vec::new();
for coll in data.collections {
diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs
index d577971d..1c6e499d 100644
--- a/src/db/models/cipher.rs
+++ b/src/db/models/cipher.rs
@@ -79,19 +79,39 @@ impl Cipher {
}
}
- pub fn validate_notes(cipher_data: &[CipherData]) -> EmptyResult {
+ pub fn validate_cipher_data(cipher_data: &[CipherData]) -> EmptyResult {
let mut validation_errors = serde_json::Map::new();
let max_note_size = CONFIG._max_note_size();
let max_note_size_msg =
format!("The field Notes exceeds the maximum encrypted value length of {} characters.", &max_note_size);
for (index, cipher) in cipher_data.iter().enumerate() {
+ // Validate the note size and if it is exceeded return a warning
if let Some(note) = &cipher.notes {
if note.len() > max_note_size {
validation_errors
.insert(format!("Ciphers[{index}].Notes"), serde_json::to_value([&max_note_size_msg]).unwrap());
}
}
+
+ // Validate the password history if it contains `null` values and if so, return a warning
+ if let Some(Value::Array(password_history)) = &cipher.password_history {
+ for pwh in password_history {
+ if let Value::Object(pwo) = pwh {
+ if pwo.get("password").is_some_and(|p| !p.is_string()) {
+ validation_errors.insert(
+ format!("Ciphers[{index}].Notes"),
+ serde_json::to_value([
+ "The password history contains a `null` value. Only strings are allowed.",
+ ])
+ .unwrap(),
+ );
+ break;
+ }
+ }
+ }
+ }
}
+
if !validation_errors.is_empty() {
let err_json = json!({
"message": "The model state is invalid.",
@@ -153,27 +173,39 @@ impl Cipher {
.as_ref()
.and_then(|s| {
serde_json::from_str::<Vec<LowerCase<Value>>>(s)
- .inspect_err(|e| warn!("Error parsing fields {:?}", e))
+ .inspect_err(|e| warn!("Error parsing fields {e:?} for {}", self.uuid))
.ok()
})
.map(|d| d.into_iter().map(|d| d.data).collect())
.unwrap_or_default();
+
let password_history_json: Vec<_> = self
.password_history
.as_ref()
.and_then(|s| {
serde_json::from_str::<Vec<LowerCase<Value>>>(s)
- .inspect_err(|e| warn!("Error parsing password history {:?}", e))
+ .inspect_err(|e| warn!("Error parsing password history {e:?} for {}", self.uuid))
.ok()
})
- .map(|d| d.into_iter().map(|d| d.data).collect())
+ .map(|d| {
+ // Check every password history item if they are valid and return it.
+ // If a password field has the type `null` skip it, it breaks newer Bitwarden clients
+ d.into_iter()
+ .filter_map(|d| match d.data.get("password") {
+ Some(p) if p.is_string() => Some(d.data),
+ _ => None,
+ })
+ .collect()
+ })
.unwrap_or_default();
// 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 = serde_json::from_str::<LowerCase<Value>>(&self.data)
- .map(|d| d.data)
- .unwrap_or_else(|_| Value::Object(serde_json::Map::new()));
+ let mut type_data_json =
+ serde_json::from_str::<LowerCase<Value>>(&self.data).map(|d| d.data).unwrap_or_else(|_| {
+ warn!("Error parsing data field for {}", self.uuid);
+ Value::Object(serde_json::Map::new())
+ });
// NOTE: This was marked as *Backwards Compatibility 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.
@@ -189,10 +221,13 @@ impl Cipher {
// Fix secure note issues when data is invalid
// This breaks at least the native mobile clients
- if self.atype == 2
- && (self.data.is_empty() || self.data.eq("{}") || self.data.to_ascii_lowercase().eq("{\"type\":null}"))
- {
- type_data_json = json!({"type": 0});
+ if self.atype == 2 {
+ match type_data_json {
+ Value::Object(ref t) if t.get("type").is_some_and(|t| t.is_number()) => {}
+ _ => {
+ type_data_json = json!({"type": 0});
+ }
+ }
}
// Clone the type_data and add some default value.