Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions client/src/file/api/encrypted_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<UnlockedItem, Error> {
match key {
Some(key) => self.try_decrypt_encrypted(key),
Expand Down
17 changes: 17 additions & 0 deletions client/src/file/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<u8>, Error> {
let mut blob = FILE_HEADER.to_vec();

Expand Down
12 changes: 12 additions & 0 deletions client/src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
5 changes: 5 additions & 0 deletions client/src/file/unlocked_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions client/src/file/unlocked_keyring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
bilelmoussaoui marked this conversation as resolved.
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
Expand Down Expand Up @@ -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 {
Comment thread
bilelmoussaoui marked this conversation as resolved.
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);
}
Expand Down
91 changes: 91 additions & 0 deletions client/tests/file_unlocked_keyring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>>(),
vec!["Updated Coarse"]
);

Ok(())
}

#[tokio::test]
async fn empty_keyring_operations() -> Result<(), Error> {
let temp_dir = tempdir().unwrap();
Expand Down Expand Up @@ -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(())
}

Expand Down
24 changes: 23 additions & 1 deletion server/src/collection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -531,6 +531,28 @@ impl Collection {
Ok(matching_items)
}

async fn search_items_exact_with_key(
&self,
attributes: &HashMap<String, String>,
key: Option<&oo7::Key>,
) -> Result<Vec<item::Item>, 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<item::Item> {
let items = self.items.lock().await;

Expand Down
64 changes: 64 additions & 0 deletions server/src/collection/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,70 @@ async fn create_item_with_replace() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}

#[tokio::test]
async fn create_item_with_replace_matches_attributes() -> Result<(), Box<dyn std::error::Error>> {
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<dyn std::error::Error>> {
let setup = TestServiceSetup::plain_session(true).await?;
Expand Down
Loading