diff --git a/database/src/fs.rs b/database/src/fs.rs index 95e5134..2c5c6ef 100644 --- a/database/src/fs.rs +++ b/database/src/fs.rs @@ -888,6 +888,13 @@ mod tests { db.check_consistency().expect("inconsistent database"); } + #[test] + fn same_email_3() { + let (_tmp_dir, mut db, log_path) = open_db(); + test::test_same_email_3(&mut db, &log_path); + db.check_consistency().expect("inconsistent database"); + } + #[test] fn no_selfsig() { let (_tmp_dir, mut db, log_path) = open_db(); diff --git a/database/src/lib.rs b/database/src/lib.rs index 1225dad..1625c54 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -103,11 +103,11 @@ impl Query { } } -#[derive(Debug,PartialEq,Eq)] +#[derive(Debug,PartialEq,Eq,PartialOrd,Ord)] pub enum EmailAddressStatus { - Revoked, - NotPublished, Published, + NotPublished, + Revoked, } pub enum ImportResult { @@ -262,7 +262,9 @@ pub trait Database: Sync + Send { } }) .collect(); - email_status.sort_by(|(e1,_),(e2,_)| e1.cmp(e2)); + email_status.sort(); + // EmailAddressStatus is ordered published, unpublished, revoked. if there are multiple for + // the same address, we keep the first. email_status.dedup_by(|(e1, _), (e2, _)| e1 == e2); // Abort if no changes were made @@ -413,7 +415,9 @@ pub trait Database: Sync + Send { } }) .collect(); - email_status.sort_by(|(e1,_),(e2,_)| e1.cmp(e2)); + email_status.sort(); + // EmailAddressStatus is ordered published, unpublished, revoked. if there are multiple for + // the same address, we keep the first. email_status.dedup_by(|(e1, _), (e2, _)| e1 == e2); Ok(TpkStatus { is_revoked, email_status, unparsed_uids }) diff --git a/database/src/test.rs b/database/src/test.rs index 2e57ef2..17d0bcd 100644 --- a/database/src/test.rs +++ b/database/src/test.rs @@ -870,7 +870,9 @@ pub fn test_same_email_1(db: &mut impl Database, log_path: &Path) { } // If a key has multiple user ids with the same email address, make -// sure things still work. +// sure things still work. We do this twice (see above), to +// make sure the order isn't relevant when revoking one user id +// but leaving the other. pub fn test_same_email_2(db: &mut impl Database, log_path: &Path) { use std::{thread, time}; @@ -935,11 +937,80 @@ pub fn test_same_email_2(db: &mut impl Database, log_path: &Path) { }, tpk_status); // fetch by both user ids. We should still get both user ids. - // TODO should this still deliver uid2.clone()? assert_eq!(get_userids(&db.by_email(&email).unwrap()[..]), vec![ uid1.clone() ]); +} + +// If a key has multiple user ids with the same email address, make +// sure things still work. We do this twice (see above), to +// make sure the order isn't relevant when revoking one user id +// but leaving the other. +pub fn test_same_email_3(db: &mut impl Database, log_path: &Path) { + use std::{thread, time}; + + let str_uid1 = "A "; + let str_uid2 = "B "; + let tpk = CertBuilder::new() + .add_userid(str_uid1) + .add_userid(str_uid2) + .generate() + .unwrap() + .0; + let uid1 = UserID::from(str_uid1); + let uid2 = UserID::from(str_uid2); + let email = Email::from_str(str_uid1).unwrap(); + let fpr = Fingerprint::try_from(tpk.fingerprint()).unwrap(); + + // upload key + let tpk_status = db.merge(tpk.clone()).unwrap().into_tpk_status(); + check_log_entry(log_path, &fpr); + + // verify uid1 + assert_eq!(TpkStatus { + is_revoked: false, + email_status: vec!( + (email.clone(), EmailAddressStatus::NotPublished), + ), + unparsed_uids: 0, + }, tpk_status); + db.set_email_published(&fpr, &tpk_status.email_status[0].0).unwrap(); + + // fetch by both user ids. assert_eq!(get_userids(&db.by_email(&email).unwrap()[..]), - vec![ uid1.clone() ]); + vec![ uid1.clone(), uid2.clone() ]); + + thread::sleep(time::Duration::from_secs(2)); + + // revoke one uid + let sig = { + let uid = tpk.userids() + .with_policy(&*POLICY, None) + .find(|b| *b.userid() == uid1).unwrap(); + assert_eq!(RevocationStatus::NotAsFarAsWeKnow, uid.revoked()); + + let mut keypair = tpk.primary_key().bundle().key().clone() + .mark_parts_secret().unwrap() + .into_keypair().unwrap(); + UserIDRevocationBuilder::new() + .set_reason_for_revocation(ReasonForRevocation::UIDRetired, b"It was the maid :/").unwrap() + .build(&mut keypair, &tpk, uid.userid(), None) + .unwrap() + }; + assert_eq!(sig.typ(), SignatureType::CertificationRevocation); + let tpk = tpk.merge_packets(vec![sig.into()]).unwrap(); + let tpk_status = db.merge(tpk).unwrap().into_tpk_status(); + check_log_entry(log_path, &fpr); + assert_eq!(TpkStatus { + is_revoked: false, + email_status: vec!( + (email.clone(), EmailAddressStatus::Published), + ), + unparsed_uids: 0, + }, tpk_status); + + // fetch by both user ids. We should still get both user ids. + assert_eq!(get_userids(&db.by_email(&email).unwrap()[..]), + vec![ uid2.clone() ]); } pub fn test_bad_uids(db: &mut impl Database, log_path: &Path) {