Skip to content

Commit

Permalink
partial(errors): get some core-crypto test cases passing again
Browse files Browse the repository at this point in the history
At this point the next failing case appears to be related to logic
errors, not type errors. Which is unfortunate, as none of the logic
was supposed to change.
  • Loading branch information
coriolinus committed Dec 6, 2024
1 parent 5adb7f2 commit f966cbb
Show file tree
Hide file tree
Showing 28 changed files with 416 additions and 145 deletions.
10 changes: 5 additions & 5 deletions crypto/src/e2e_identity/enabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Client {

#[cfg(test)]
mod tests {
use crate::{e2e_identity::error::Error, prelude::MlsCredentialType, test_utils::*};
use crate::{e2e_identity::error::Error, mls, prelude::MlsCredentialType, test_utils::*, RecursiveError};
use openmls_traits::types::SignatureScheme;
use wasm_bindgen_test::*;

Expand Down Expand Up @@ -77,8 +77,8 @@ mod tests {
Box::pin(async move {
assert!(matches!(
cc.context.e2ei_is_enabled(case.signature_scheme()).await.unwrap_err(),
Error::CryptoError(boxed)
if matches!(*boxed, CryptoError::MlsNotInitialized)
Error::Recursive(RecursiveError::MlsClient { source, .. })
if matches!(*source, mls::client::Error::MlsNotInitialized)
));
})
})
Expand All @@ -97,8 +97,8 @@ mod tests {
};
assert!(matches!(
cc.context.e2ei_is_enabled(other_sc).await.unwrap_err(),
Error::CryptoError(boxed)
if matches!(*boxed, CryptoError::CredentialNotFound(_))
Error::Recursive(RecursiveError::MlsClient { source, .. })
if matches!(*source, mls::client::Error::CredentialNotFound(_))
));
})
})
Expand Down
9 changes: 9 additions & 0 deletions crypto/src/error/recursive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ pub enum RecursiveError {
#[source]
source: Box<crate::mls::credential::Error>,
},
/// Wrap a [crate::test_utils::TestError] for recursion.
#[cfg(test)]
#[error(transparent)]
Test(#[from] Box<crate::test_utils::TestError>),
}

impl RecursiveError {
Expand Down Expand Up @@ -122,4 +126,9 @@ impl RecursiveError {
source: Box::new(into_source.into()),
}
}

#[cfg(test)]
pub(crate) fn test<E: Into<crate::test_utils::TestError>>() -> impl FnOnce(E) -> Self {
move |into_source| Self::Test(Box::new(into_source.into()))
}
}
3 changes: 2 additions & 1 deletion crypto/src/group_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ mod tests {
_identity: Option<Self::IdentityType>,
_keystore: &impl FetchFromDatabase,
) -> crate::Result<Option<Self>> {
let id = std::str::from_utf8(id)?;
// it's not worth adding a variant to the Error type here to handle test dummy values
let id = std::str::from_utf8(id).expect("dummy value ids are strings");
Ok(Some(id.into()))
}

Expand Down
20 changes: 15 additions & 5 deletions crypto/src/mls/buffer_external_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
async fn should_buffer_and_reapply_messages_after_external_commit_merged(case: TestCase) {
use crate::mls;
use crate::{mls, RecursiveError};

run_test_with_client_ids(
case.clone(),
Expand Down Expand Up @@ -133,13 +133,15 @@ mod tests {
let decrypt = bob_central.context.decrypt_message(&id, m).await;
assert!(matches!(
decrypt.unwrap_err(),
mls::conversation::Error::UnmergedPendingGroup
mls::conversation::Error::Recursive(RecursiveError::Mls{source, ..})
if matches!(*source, mls::Error::UnmergedPendingGroup)
));
}
let decrypt = bob_central.context.decrypt_message(&id, app_msg).await;
assert!(matches!(
decrypt.unwrap_err(),
mls::conversation::Error::UnmergedPendingGroup
mls::conversation::Error::Recursive(RecursiveError::Mls{source, ..})
if matches!(*source, mls::Error::UnmergedPendingGroup)
));

// Bob should have buffered the messages
Expand Down Expand Up @@ -192,6 +194,8 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
async fn should_not_reapply_buffered_messages_when_external_commit_contains_remove(case: TestCase) {
use crate::mls;

run_test_with_client_ids(
case.clone(),
["alice", "bob"],
Expand All @@ -214,9 +218,15 @@ mod tests {

// Since Alice missed Bob's commit she should buffer this message
let decrypt = alice_central.context.decrypt_message(&id, msg1).await;
assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
assert!(matches!(
decrypt.unwrap_err(),
mls::conversation::Error::BufferedFutureMessage
));
let decrypt = alice_central.context.decrypt_message(&id, msg2).await;
assert!(matches!(decrypt.unwrap_err(), CryptoError::BufferedFutureMessage));
assert!(matches!(
decrypt.unwrap_err(),
mls::conversation::Error::BufferedFutureMessage
));
assert_eq!(alice_central.context.count_entities().await.pending_messages, 2);

let gi = bob_central.get_group_info(&id).await;
Expand Down
10 changes: 5 additions & 5 deletions crypto/src/mls/client/identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl Client {

#[cfg(test)]
mod tests {
use crate::test_utils::*;
use crate::{mls, test_utils::*};
use openmls::prelude::SignaturePublicKey;
use rand::Rng;
use wasm_bindgen_test::*;
Expand Down Expand Up @@ -240,9 +240,6 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
async fn pushing_duplicates_should_fail(case: TestCase) {
use crate::mls::client::error::Error;
use crate::CryptoError;

run_test_with_client_ids(case.clone(), ["alice"], move |[mut central]| {
Box::pin(async move {
let cert = central.get_intermediate_ca().cloned();
Expand All @@ -256,7 +253,10 @@ mod tests {
cb,
)
.await;
assert!(matches!(push.unwrap_err(), Error::CryptoError(boxed) if matches!(*boxed, CryptoError::CredentialBundleConflict)));
assert!(matches!(
push.unwrap_err(),
mls::client::Error::CredentialBundleConflict
));
})
})
.await
Expand Down
1 change: 1 addition & 0 deletions crypto/src/mls/client/key_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use crate::{KeystoreError, MlsError};
/// Default number of KeyPackages a client generates the first time it's created
#[cfg(not(test))]
pub const INITIAL_KEYING_MATERIAL_COUNT: usize = 100;
/// Default number of KeyPackages a client generates the first time it's created
#[cfg(test)]
pub const INITIAL_KEYING_MATERIAL_COUNT: usize = 10;

Expand Down
3 changes: 3 additions & 0 deletions crypto/src/mls/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,9 @@ impl Client {

#[cfg(test)]
impl Client {
// test functions are not held to the same documentation standard as proper functions
#![allow(missing_docs)]

pub async fn random_generate(
case: &crate::test_utils::TestCase,
backend: &MlsCryptoProvider,
Expand Down
2 changes: 1 addition & 1 deletion crypto/src/mls/conversation/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ mod tests {

use crate::test_utils::*;

use super::*;
use super::{Error, *};

wasm_bindgen_test_configure!(run_in_browser);

Expand Down
2 changes: 2 additions & 0 deletions crypto/src/mls/conversation/db_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub struct EntitiesCount {
}

impl MlsCentral {
/// Count the entities
pub async fn count_entities(&self) -> EntitiesCount {
let keystore = self.mls_backend.keystore();
let credential = keystore.count::<MlsCredential>().await.unwrap();
Expand Down Expand Up @@ -55,6 +56,7 @@ impl MlsCentral {
}

impl CentralContext {
/// Count the entities
pub async fn count_entities(&self) -> EntitiesCount {
let keystore = self.keystore().await.unwrap();
let credential = keystore.count::<MlsCredential>().await.unwrap();
Expand Down
40 changes: 36 additions & 4 deletions crypto/src/mls/conversation/decrypt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,8 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
async fn cannot_decrypt_proposal_no_callback(case: TestCase) {
use crate::mls;

run_test_with_client_ids(
case.clone(),
["alice", "bob", "alice2"],
Expand Down Expand Up @@ -1076,7 +1078,14 @@ mod tests {
.await
.unwrap_err();

assert!(matches!(error, Error::CallbacksNotSet));
assert!(matches!(
error,
mls::conversation::Error::Recursive(RecursiveError::Mls {
source,
..
})
if matches!(*source, mls::Error::CallbacksNotSet)
));

bob_central.context.set_callbacks(None).await.unwrap();
let error = bob_central
Expand All @@ -1085,7 +1094,14 @@ mod tests {
.await
.unwrap_err();

assert!(matches!(error, Error::CallbacksNotSet));
assert!(matches!(
error,
mls::conversation::Error::Recursive(RecursiveError::Mls {
source,
..
})
if matches!(*source, mls::Error::CallbacksNotSet)
));
})
},
)
Expand All @@ -1095,6 +1111,8 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
async fn cannot_decrypt_proposal_validation(case: TestCase) {
use crate::mls;

run_test_with_client_ids(
case.clone(),
["alice", "bob", "alice2"],
Expand Down Expand Up @@ -1130,7 +1148,14 @@ mod tests {
.await
.unwrap_err();

assert!(matches!(error, CryptoError::UnauthorizedExternalAddProposal));
assert!(matches!(
error,
mls::conversation::Error::Recursive(RecursiveError::Mls {
source,
..
})
if matches!(*source, mls::Error::UnauthorizedExternalAddProposal)
));

bob_central.context.set_callbacks(None).await.unwrap();
let error = bob_central
Expand All @@ -1139,7 +1164,14 @@ mod tests {
.await
.unwrap_err();

assert!(matches!(error, CryptoError::CallbacksNotSet));
assert!(matches!(
error,
mls::conversation::Error::Recursive(RecursiveError::Mls {
source,
..
})
if matches!(*source, mls::Error::CallbacksNotSet)
));
})
},
)
Expand Down
23 changes: 15 additions & 8 deletions crypto/src/mls/conversation/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ impl MlsCentral {

#[cfg(test)]
mod tests {

use super::super::error::Error;
use crate::{prelude::MlsError, test_utils::*};
use crate::{mls, test_utils::*, LeafError, MlsErrorKind};
use openmls::prelude::ExportSecretError;

use wasm_bindgen_test::*;
Expand Down Expand Up @@ -156,10 +154,10 @@ mod tests {

let result = alice_central.context.export_secret_key(&id, usize::MAX).await;
let error = result.unwrap_err();
let error = error.downcast_mls().unwrap().0;
assert!(matches!(
// let error = error.downcast_mls().unwrap().0;
assert!(innermost_source_matches!(
error,
MlsError::MlsExportSecretError(ExportSecretError::KeyLengthTooLong)
MlsErrorKind::MlsExportSecretError(ExportSecretError::KeyLengthTooLong)
));
})
})
Expand All @@ -180,7 +178,12 @@ mod tests {

let unknown_id = b"not_found".to_vec();
let error = alice_central.context.get_client_ids(&unknown_id).await.unwrap_err();
assert!(matches!(error, Error::ConversationNotFound(c) if c == unknown_id));
// assert!(matches!(error, Error::ConversationNotFound(c) if c == unknown_id));
assert!(matches!(
error,
mls::conversation::Error::Leaf(LeafError::ConversationNotFound(c))
if c == unknown_id
))
})
})
.await
Expand Down Expand Up @@ -225,7 +228,11 @@ mod tests {

let unknown_id = b"not_found".to_vec();
let error = alice_central.context.get_client_ids(&unknown_id).await.unwrap_err();
assert!(matches!(error, Error::ConversationNotFound(c) if c == unknown_id));
assert!(matches!(
error,
mls::conversation::Error::Leaf(LeafError::ConversationNotFound(c))
if c == unknown_id
))
})
})
.await
Expand Down
3 changes: 3 additions & 0 deletions crypto/src/mls/conversation/group_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ impl MlsGroupInfoBundle {

#[cfg(test)]
impl MlsGroupInfoBundle {
// test functions are not held to the same standard
#![allow(missing_docs)]

pub fn get_group_info(self) -> openmls::prelude::group_info::VerifiableGroupInfo {
match self.get_payload().extract() {
openmls::prelude::MlsMessageInBody::GroupInfo(vgi) => vgi,
Expand Down
24 changes: 16 additions & 8 deletions crypto/src/mls/conversation/leaf_node_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ mod tests {
use openmls::prelude::Lifetime;
use wasm_bindgen_test::*;

use crate::{test_utils::*, CryptoError, MlsError};
use crate::{mls, test_utils::*, MlsError, MlsErrorKind};

use openmls::prelude::{AddMembersError, KeyPackageVerifyError, ProposeAddMemberError};

wasm_bindgen_test_configure!(run_in_browser);
Expand Down Expand Up @@ -47,9 +48,14 @@ mod tests {
let proposal_creation = alice_central.context.new_add_proposal(&id, invalid_kp).await;
assert!(matches!(
proposal_creation.unwrap_err(),
CryptoError::MlsError(MlsError::ProposeAddMemberError(
ProposeAddMemberError::KeyPackageVerifyError(KeyPackageVerifyError::InvalidLeafNode(_))
))
mls::Error::Mls(MlsError {
source: MlsErrorKind::ProposeAddMemberError(
ProposeAddMemberError::KeyPackageVerifyError(
KeyPackageVerifyError::InvalidLeafNode(_)
)
),
..
})
));
assert!(alice_central.pending_proposals(&id).await.is_empty());

Expand All @@ -73,12 +79,14 @@ mod tests {
.await;

let error = commit_creation.unwrap_err();
let error = error.downcast_mls().unwrap().0;
assert!(matches!(
error,
MlsError::MlsAddMembersError(AddMembersError::KeyPackageVerifyError(
KeyPackageVerifyError::InvalidLeafNode(_)
))
mls::conversation::Error::Mls(MlsError {
source: MlsErrorKind::MlsAddMembersError(AddMembersError::KeyPackageVerifyError(
KeyPackageVerifyError::InvalidLeafNode(_)
)),
..
})
));
assert!(alice_central.pending_proposals(&id).await.is_empty());
assert!(alice_central.pending_commit(&id).await.is_none());
Expand Down
4 changes: 3 additions & 1 deletion crypto/src/mls/conversation/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,11 +469,13 @@ mod tests {
#[apply(all_cred_cipher)]
#[wasm_bindgen_test]
pub async fn should_fail_when_conversation_not_found(case: TestCase) {
use crate::LeafError;

run_test_with_client_ids(case.clone(), ["alice"], move |[alice_central]| {
Box::pin(async move {
let id = conversation_id();
let clear = alice_central.context.clear_pending_commit(&id).await;
assert!(matches!(clear.unwrap_err(), Error::ConversationNotFound(conv_id) if conv_id == id))
assert!(matches!(clear.unwrap_err(), Error::Leaf(LeafError::ConversationNotFound(conv_id)) if conv_id == id))
})
})
.await
Expand Down
Loading

0 comments on commit f966cbb

Please sign in to comment.