diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index bd99e512..c46128cb 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -41,6 +41,11 @@ impl EncryptedItem { } } + pub fn matches_exact(&self, attributes: &impl AsAttributes, key: Option<&Key>) -> bool { + let attributes = attributes.as_attributes(); + self.hashed_attributes.len() == attributes.len() && self.matches(&attributes, key) + } + fn try_decrypt_inner(&self, key: Option<&Key>) -> Result { match key { Some(key) => self.try_decrypt_encrypted(key), diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs index f40adf6d..cd73465a 100644 --- a/client/src/file/api/mod.rs +++ b/client/src/file/api/mod.rs @@ -235,6 +235,23 @@ impl Keyring { Ok(()) } + pub(crate) fn remove_items_exact( + &mut self, + attributes: &impl AsAttributes, + key: Option<&Key>, + ) -> Result<(), Error> { + for item in &self.items { + if item.matches_exact(attributes, key) && !item.is_valid(key) { + return Err(Error::MacError); + } + } + + self.items + .retain(|item| !item.matches_exact(attributes, key)); + + Ok(()) + } + fn as_bytes(&self) -> Result, Error> { let mut blob = FILE_HEADER.to_vec(); diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs index 3263d35d..0c521038 100644 --- a/client/src/file/mod.rs +++ b/client/src/file/mod.rs @@ -93,6 +93,18 @@ impl Item { Self::Locked(locked) => locked.inner.matches(attributes, key), } } + + /// Check if this item has exactly the given attributes. + pub fn matches_attributes_exact( + &self, + attributes: &impl AsAttributes, + key: Option<&Key>, + ) -> bool { + match self { + Self::Unlocked(unlocked) => unlocked.matches_exact(attributes), + Self::Locked(locked) => locked.inner.matches_exact(attributes, key), + } + } } #[derive(Debug)] diff --git a/client/src/file/unlocked_item.rs b/client/src/file/unlocked_item.rs index 9cb3f50e..52e8f5b5 100644 --- a/client/src/file/unlocked_item.rs +++ b/client/src/file/unlocked_item.rs @@ -62,6 +62,11 @@ impl UnlockedItem { &self.attributes } + /// Check whether the attribute maps match. + pub fn matches_exact(&self, attributes: &impl AsAttributes) -> bool { + self.attributes == attributes.as_attributes() + } + /// Retrieve the item attributes as a typed schema. /// /// # Example diff --git a/client/src/file/unlocked_keyring.rs b/client/src/file/unlocked_keyring.rs index 7daace77..c309068d 100644 --- a/client/src/file/unlocked_keyring.rs +++ b/client/src/file/unlocked_keyring.rs @@ -465,10 +465,10 @@ impl UnlockedKeyring { let item = { let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; + let item = UnlockedItem::new(label, attributes, secret); if replace { - keyring.remove_items(attributes, key.as_deref())?; + keyring.remove_items_exact(item.attributes(), key.as_deref())?; } - let item = UnlockedItem::new(label, attributes, secret); let encrypted_item = item.encrypt(key.as_deref())?; keyring.items.push(encrypted_item); item @@ -540,10 +540,10 @@ impl UnlockedKeyring { let _span = tracing::debug_span!("bulk_create", items_to_create = items.len()); for (label, attributes, secret, replace) in items { + let item = UnlockedItem::new(label, &attributes, secret); if replace { - keyring.remove_items(&attributes, key.as_deref())?; + keyring.remove_items_exact(item.attributes(), key.as_deref())?; } - let item = UnlockedItem::new(label, &attributes, secret); let encrypted_item = item.encrypt(key.as_deref())?; keyring.items.push(encrypted_item); } diff --git a/client/tests/file_unlocked_keyring.rs b/client/tests/file_unlocked_keyring.rs index 3bc4e2fb..91b2f951 100644 --- a/client/tests/file_unlocked_keyring.rs +++ b/client/tests/file_unlocked_keyring.rs @@ -549,6 +549,75 @@ async fn item_replacement_behavior() -> Result<(), Error> { Ok(()) } +#[tokio::test] +async fn item_replacement_matches_attributes() -> Result<(), Error> { + let temp_dir = tempdir().unwrap(); + let keyring_path = temp_dir.path().join("replace_attributes_test.keyring"); + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; + + keyring + .create_item( + "Alice", + &[("app", "test"), ("user", "alice")], + "alice-secret", + false, + ) + .await?; + keyring + .create_item( + "Bob", + &[("app", "test"), ("user", "bob")], + "bob-secret", + false, + ) + .await?; + + // A coarser attribute set must not replace either item. + keyring + .create_item("Coarse", &[("app", "test")], "coarse-secret", true) + .await?; + + let items = keyring.search_items(&[("app", "test")]).await?; + assert_eq!(items.len(), 3); + assert_eq!( + keyring + .search_items(&[("app", "test"), ("user", "alice")]) + .await? + .len(), + 1 + ); + assert_eq!( + keyring + .search_items(&[("app", "test"), ("user", "bob")]) + .await? + .len(), + 1 + ); + + // Replacing the coarse item must not add a duplicate. + keyring + .create_item( + "Updated Coarse", + &[("app", "test")], + "updated-coarse-secret", + true, + ) + .await?; + + let items = keyring.search_items(&[("app", "test")]).await?; + assert_eq!(items.len(), 3); + assert_eq!( + items + .iter() + .filter(|item| item.attributes().get("user").is_none()) + .map(|item| item.label()) + .collect::>(), + vec!["Updated Coarse"] + ); + + Ok(()) +} + #[tokio::test] async fn empty_keyring_operations() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); @@ -815,6 +884,28 @@ async fn bulk_create_items() -> Result<(), Error> { // Verify the item was replaced - should still have 3 items total let all_items_after = keyring.search_items(&[("app", "bulk-app")]).await?; assert_eq!(all_items_after.len(), 3); + + // A coarse replacement must preserve the three specific items. + keyring + .create_items(vec![( + "Coarse Item".to_string(), + HashMap::from([("app".to_string(), "bulk-app".to_string())]), + Secret::text("coarse-secret"), + true, + )]) + .await?; + assert_eq!(keyring.search_items(&[("app", "bulk-app")]).await?.len(), 4); + + // Replacing the coarse item removes only its predecessor. + keyring + .create_items(vec![( + "Updated Coarse Item".to_string(), + HashMap::from([("app".to_string(), "bulk-app".to_string())]), + Secret::text("updated-coarse-secret"), + true, + )]) + .await?; + assert_eq!(keyring.search_items(&[("app", "bulk-app")]).await?.len(), 4); Ok(()) } diff --git a/server/src/collection/mod.rs b/server/src/collection/mod.rs index 08704f20..460083a3 100644 --- a/server/src/collection/mod.rs +++ b/server/src/collection/mod.rs @@ -308,7 +308,7 @@ impl Collection { // Remove any existing items with the same attributes if replace { let existing_items = self - .search_items_with_key(&attributes, key.as_deref()) + .search_items_exact_with_key(&attributes, key.as_deref()) .await?; if !existing_items.is_empty() { let mut items = self.items.lock().await; @@ -531,6 +531,28 @@ impl Collection { Ok(matching_items) } + async fn search_items_exact_with_key( + &self, + attributes: &HashMap, + key: Option<&oo7::Key>, + ) -> Result, ServiceError> { + let mut matching_items = Vec::new(); + let items = self.items.lock().await; + + for item_wrapper in items.iter() { + let inner = item_wrapper.inner.lock().await; + let Some(file_item) = inner.as_ref() else { + continue; + }; + + if file_item.matches_attributes_exact(attributes, key) { + matching_items.push(item_wrapper.clone()); + } + } + + Ok(matching_items) + } + pub async fn item_from_path(&self, path: &ObjectPath<'_>) -> Option { let items = self.items.lock().await; diff --git a/server/src/collection/tests.rs b/server/src/collection/tests.rs index 1e23609b..a2dba90f 100644 --- a/server/src/collection/tests.rs +++ b/server/src/collection/tests.rs @@ -226,6 +226,70 @@ async fn create_item_with_replace() -> Result<(), Box> { Ok(()) } +#[tokio::test] +async fn create_item_with_replace_matches_attributes() -> Result<(), Box> { + let setup = TestServiceSetup::plain_session(true).await?; + + setup + .create_item( + "Alice", + &[("application", "myapp"), ("username", "alice")], + "alice-password", + false, + ) + .await?; + setup + .create_item( + "Bob", + &[("application", "myapp"), ("username", "bob")], + "bob-password", + false, + ) + .await?; + + // A coarser attribute set must not replace either item. + setup + .create_item( + "Coarse", + &[("application", "myapp")], + "coarse-password", + true, + ) + .await?; + + let items = setup.collections[0].items().await?; + assert_eq!(items.len(), 3); + assert_eq!( + setup.collections[0] + .search_items(&[("application", "myapp"), ("username", "alice")]) + .await? + .len(), + 1 + ); + assert_eq!( + setup.collections[0] + .search_items(&[("application", "myapp"), ("username", "bob")]) + .await? + .len(), + 1 + ); + + // Replacing the coarse item must not add a duplicate. + setup + .create_item( + "Updated Coarse", + &[("application", "myapp")], + "updated-coarse-password", + true, + ) + .await?; + + let items = setup.collections[0].items().await?; + assert_eq!(items.len(), 3); + + Ok(()) +} + #[tokio::test] async fn label_property() -> Result<(), Box> { let setup = TestServiceSetup::plain_session(true).await?;