Skip to content

Commit 86b0c3d

Browse files
committed
chore: fix a test which was failing due to changed semantics
This test assumed that e2ei rotation implicitly cleared out all existing keypackages and also generated an equal number of new ones. That behavior cannot remain with the new keypackage API, so we needed to adjust this test to test the actual behavior.
1 parent dba8ed1 commit 86b0c3d

File tree

2 files changed

+17
-65
lines changed

2 files changed

+17
-65
lines changed

crypto/src/test_utils/context.rs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use super::{
1616
};
1717
use crate::{
1818
CertificateBundle, Ciphersuite, CoreCrypto, CredentialRef, CredentialType, MlsConversationDecryptMessage,
19-
RecursiveError, WireIdentity,
19+
WireIdentity,
2020
e2e_identity::{
2121
device_status::DeviceStatus,
2222
id::{QualifiedE2eiClientId, WireQualifiedClientId},
@@ -30,7 +30,6 @@ pub const TEAM: &'static str = "world";
3030

3131
pub struct RotateAllResult<'a> {
3232
pub(crate) commits: Vec<OperationGuard<'a, Commit>>,
33-
pub(crate) new_key_packages: Vec<KeyPackage>,
3433
}
3534

3635
impl SessionContext {
@@ -210,12 +209,11 @@ impl SessionContext {
210209
client.add_credential_producing_arc(credential).await.unwrap()
211210
}
212211

213-
pub(crate) async fn create_key_packages_and_update_credential_in_all_conversations<'a>(
212+
pub(crate) async fn update_credential_in_all_conversations<'a>(
214213
&self,
215214
all_conversations: Vec<TestConversation<'a>>,
216215
cb: &Credential,
217216
cipher_suite: Ciphersuite,
218-
key_package_count: usize,
219217
) -> Result<RotateAllResult<'a>> {
220218
assert_eq!(cipher_suite, cb.ciphersuite);
221219

@@ -225,20 +223,7 @@ impl SessionContext {
225223
commits.push(commit_guard);
226224
}
227225

228-
let credential_ref = &CredentialRef::from_credential(cb);
229-
let mut new_key_packages = Vec::with_capacity(key_package_count);
230-
for _ in 0..key_package_count {
231-
new_key_packages.push(
232-
self.session
233-
.generate_keypackage(credential_ref, None)
234-
.await
235-
.map_err(RecursiveError::mls_client("generating keypackage"))?,
236-
);
237-
}
238-
Ok(RotateAllResult {
239-
commits,
240-
new_key_packages,
241-
})
226+
Ok(RotateAllResult { commits })
242227
}
243228

244229
pub async fn get_e2ei_client_id(&self) -> wire_e2e_identity::prelude::E2eiClientId {

crypto/src/transaction_context/e2e_identity/rotate.rs

Lines changed: 14 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ mod tests {
203203
let [alice, bob, charlie] = case.sessions_with_pki_env().await;
204204
Box::pin(async move {
205205
const N: usize = 50;
206-
const NB_KEY_PACKAGE: usize = 50;
207206

208207
let mut conversations = vec![];
209208

@@ -270,18 +269,14 @@ mod tests {
270269
.unwrap();
271270

272271
let result = alice
273-
.create_key_packages_and_update_credential_in_all_conversations(
274-
conversations,
275-
&cb,
276-
*enrollment.ciphersuite(),
277-
NB_KEY_PACKAGE,
278-
)
272+
.update_credential_in_all_conversations(conversations, &cb, *enrollment.ciphersuite())
279273
.await
280274
.unwrap();
281275

282276
let after_rotate = alice.transaction.count_entities().await;
283-
// verify we have indeed created the right amount of new X509 KeyPackages
284-
assert_eq!(after_rotate.key_package - before_rotate.key_package, NB_KEY_PACKAGE);
277+
278+
// rotation neither creates nor deletes keypackages
279+
assert_eq!(after_rotate.key_package, before_rotate.key_package);
285280

286281
// and a new Credential has been persisted in the keystore
287282
assert_eq!(after_rotate.credential - before_rotate.credential, 1);
@@ -294,26 +289,7 @@ mod tests {
294289
.await;
295290
}
296291

297-
// Verify that all the new KeyPackages contain the new identity
298-
let new_credentials = result
299-
.new_key_packages
300-
.iter()
301-
.map(|kp| kp.leaf_node().to_credential_with_key());
302-
for c in new_credentials {
303-
assert_eq!(c.credential.credential_type(), openmls::prelude::CredentialType::X509);
304-
let identity = c.extract_identity(case.ciphersuite(), None).unwrap();
305-
assert_eq!(
306-
identity.x509_identity.as_ref().unwrap().display_name,
307-
e2ei_utils::NEW_DISPLAY_NAME
308-
);
309-
assert_eq!(
310-
identity.x509_identity.as_ref().unwrap().handle,
311-
format!("wireapp://%40{}@world.com", e2ei_utils::NEW_HANDLE)
312-
);
313-
}
314-
315292
// Alice has to delete her old KeyPackages
316-
317293
// But first let's verify the previous credential material is present
318294
assert!(
319295
alice
@@ -326,15 +302,10 @@ mod tests {
326302
.is_some()
327303
);
328304

329-
// we also have generated the right amount of private encryption keys
305+
// rotation neither creates nor deletes private keys or key packages
330306
let before_delete = alice.transaction.count_entities().await;
331-
assert_eq!(
332-
before_delete.hpke_private_key - before_rotate.hpke_private_key,
333-
NB_KEY_PACKAGE
334-
);
335-
336-
// 1 has been created per new KeyPackage created in the rotation
337-
assert_eq!(before_delete.key_package - before_rotate.key_package, NB_KEY_PACKAGE);
307+
assert_eq!(before_delete.hpke_private_key, before_rotate.hpke_private_key);
308+
assert_eq!(before_delete.key_package, before_rotate.key_package);
338309

339310
// Checks are done, now let's delete the old credential.
340311
// This should have the consequence to purge the all the stale keypackages as well.
@@ -344,34 +315,30 @@ mod tests {
344315
.await
345316
.unwrap();
346317

347-
// Alice should just have the number of X509 KeyPackages she requested
318+
// No keypackages were automatically created.
348319
let nb_x509_kp = alice
349320
.count_key_package(case.ciphersuite(), Some(CredentialType::X509))
350321
.await;
351-
assert_eq!(nb_x509_kp, NB_KEY_PACKAGE);
352-
// in both cases, Alice should not anymore have any Basic KeyPackage
322+
assert_eq!(nb_x509_kp, 0);
323+
// Because we removed her old credential, Alice should not anymore have any Basic KeyPackages
353324
let nb_basic_kp = alice
354325
.count_key_package(case.ciphersuite(), Some(CredentialType::Basic))
355326
.await;
356327
assert_eq!(nb_basic_kp, 0);
357328

358-
// and since all of Alice's unclaimed KeyPackages have been purged, so should be her old Credential
359-
360329
// Also the old Credential has been removed from the keystore
361330
let after_delete = alice.transaction.count_entities().await;
362331
assert_eq!(after_delete.credential, 1);
363332
assert!(alice.find_credential_from_keystore(&old_credential).await.is_none());
364333

365334
// and all her Private HPKE keys...
366-
assert_eq!(after_delete.hpke_private_key, NB_KEY_PACKAGE);
335+
assert_eq!(after_delete.hpke_private_key, 0);
367336

368337
// ...and encryption keypairs
369-
assert_eq!(
370-
after_rotate.encryption_keypair - after_delete.encryption_keypair,
371-
INITIAL_KEYING_MATERIAL_COUNT
372-
);
338+
assert_eq!(after_delete.encryption_keypair, 0);
373339

374-
// Now charlie tries to add Alice to a conversation with her new KeyPackages
340+
// Now charlie tries to add Alice to a conversation
341+
// (the create_conversation helper implicitly generates keypackages as needed)
375342
let conversation = case
376343
.create_conversation([&charlie])
377344
.await

0 commit comments

Comments
 (0)