Align user ID designators in sq pki link retract.

- User IDs have to be explicitly given, or `--all` has to be used to
    select them all (this was previously the default).

  - This aligns the retract subcommand with the other link and vouch
    management commands.

  - Fixes #442.

src/cli/pki/link.rs

@@ -116,6 +116,7 @@ and any associated user IDs.  This effectively invalidates all links.",
             command: &[
                 "sq", "pki", "link", "retract",
                 "--cert=EB28F26E2739A4870ECC47726F0073F60FD0CBF0",
+                "--all",
             ],
         }),
     ],
@@ -538,8 +539,8 @@ to force the signature to be re-created anyway.",
 
     #[command(flatten)]
     pub userids: UserIDDesignators<
-        userid_designator::AnyUserIDEmailArgs,
+        userid_designator::AllAnyUserIDEmailArgs,
-        userid_designator::OptionalValueNoLinting>,
+        userid_designator::AllMatchesNonSelfSignedNoLinting>,
 }
 
 const RETRACT_EXAMPLES: Actions = Actions {
@@ -573,6 +574,7 @@ and any associated user IDs.  This effectively invalidates all links.",
             command: &[
                 "sq", "pki", "link", "retract",
                 "--cert=EB28F26E2739A4870ECC47726F0073F60FD0CBF0",
+                "--all",
             ],
         }),
     ],

src/cli/types/userid_designator.rs

@@ -85,6 +85,11 @@ pub type AllExistingAndAddXUserIDEmailArgs
     = <AllUserIDsArg
        as std::ops::BitOr<ExistingAndAddXUserIDEmailArgs>>::Output;
 
+/// Enables --all, --userid, and --email (but not --name, --userid-or-add,
+/// --email-or-add, or --name-or-add).
+pub type AllAnyUserIDEmailArgs
+    = <AllUserIDsArg as std::ops::BitOr<AnyUserIDEmailArgs>>::Output;
+
 /// Enables --userid, --email, --name, --userid-or-add,
 /// --email-or-add, and --name-or-add (but not --all).
 pub type ExistingAndAddXUserIDEmailNameArgs
@@ -107,12 +112,18 @@ pub type OptionalValue = typenum::U2;
 /// the `--allow-non-canonical-userid` flag.
 pub type NoLinting = typenum::U4;
 
+/// Makes --all match non-self signed user IDs.
+pub type AllMatchesNonSelfSigned = typenum::U8;
+
 pub type OneValueNoLinting
     = <OneValue as std::ops::BitOr<NoLinting>>::Output;
 
 pub type OptionalValueNoLinting
     = <OptionalValue as std::ops::BitOr<NoLinting>>::Output;
 
+pub type AllMatchesNonSelfSignedNoLinting
+    = <AllMatchesNonSelfSigned as std::ops::BitOr<NoLinting>>::Output;
+
 /// A user ID designator.
 #[derive(Debug, Clone)]
 pub enum UserIDDesignator {
@@ -286,6 +297,9 @@ pub struct UserIDDesignators<Arguments, Options=typenum::U0>
     /// Use all self-signed user IDs.
     pub all: Option<bool>,
 
+    /// Whether --all should match non-self signed user IDs.
+    all_matches_non_self_signed: bool,
+
     /// Whether --allow-non-canonical-userids was passed.
     pub allow_non_canonical_userids: bool,
 
@@ -332,6 +346,11 @@ impl<Arguments, Options> UserIDDesignators<Arguments, Options> {
         self.all
     }
 
+    /// Returns whether --all should match non-self signed user IDs.
+    pub fn all_matches_non_self_signed(&self) -> bool {
+        self.all_matches_non_self_signed
+    }
+
     /// Returns whether the allow-non-canonical-userids flag was set.
     pub fn allow_non_canonical_userids(&self) -> bool {
         self.allow_non_canonical_userids
@@ -622,6 +641,8 @@ where
 
         let options = Options::to_usize();
         let no_linting = (options & NoLinting::to_usize()) > 0;
+        let all_matches_non_self_signed =
+            (options & AllMatchesNonSelfSigned::to_usize()) > 0;
 
         // Can't provide both ExistingUserIDArg and AnyUserIDArg.
         assert!(! (userid_arg && any_userid_arg));
@@ -639,6 +660,7 @@ where
         } else {
             None
         };
+        self.all_matches_non_self_signed = all_matches_non_self_signed;
 
         if let Some(Some(userids))
             = matches.try_get_many::<String>("userid")
@@ -745,6 +767,7 @@ where
             designators: Vec::new(),
             arguments: std::marker::PhantomData,
             all: None,
+            all_matches_non_self_signed: false,
             allow_non_canonical_userids: false,
         };
 
@@ -790,6 +813,7 @@ impl ResolvedUserID {
 
     /// Return implicitly resolved user IDs for all user IDs
     /// associated with a certificate.
+    #[allow(dead_code)]
     pub fn implicit_for_cert(cert: &Cert) -> Vec<Self> {
         cert.userids()
             .map(|ua| Self::implicit(ua.userid().clone()))

src/commands/key/generate.rs

@@ -355,6 +355,7 @@ pub fn generate(
                               with:"))
             .sq().arg("pki").arg("link").arg("retract")
             .arg_value("--cert", cert.fingerprint())
+            .arg("--all")
             .done();
     }
 
@@ -365,6 +366,7 @@ pub fn generate(
                               with:"))
             .sq().arg("pki").arg("link").arg("retract")
             .arg_value("--cert", cert.fingerprint())
+            .arg("--all")
             .done();
     }
 

src/commands/pki/link.rs

@@ -17,7 +17,6 @@ use crate::parse_notations;
 use crate::cli::pki::link;
 use crate::cli::types::Expiration;
 use crate::cli::types::TrustAmount;
-use crate::cli::types::userid_designator::ResolvedUserID;
 
 pub fn link(sq: Sq, c: link::Command) -> Result<()> {
     use link::Subcommands::*;
@@ -122,17 +121,7 @@ pub fn retract(sq: Sq, c: link::RetractCommand)
         = sq.resolve_cert(&c.cert, sequoia_wot::FULLY_TRUSTED)?;
 
     let vc = cert.with_policy(NULL_POLICY, Some(sq.time))?;
-    let mut userids = c.userids.resolve(&vc)?;
+    let userids = c.userids.resolve(&vc)?;
-
-    let user_supplied_userids = if userids.is_empty() {
-        // Nothing was specified.  Retract links for all user IDs
-        // (self signed or not).
-        userids = ResolvedUserID::implicit_for_cert(&cert);
-
-        false
-    } else {
-        true
-    };
 
     let notations = parse_notations(c.notation)?;
 
@@ -142,7 +131,7 @@ pub fn retract(sq: Sq, c: link::RetractCommand)
         &trust_root,
         &cert,
         &userids[..],
-        user_supplied_userids,
+        true, // User supplied user IDs.
         &[(TrustAmount::None, Expiration::Never)],
         0,
         &[][..], &[][..], // Domain, regex.

src/common/types/userid_designator.rs

@@ -58,21 +58,39 @@ where
         let mut bad = None;
 
         if let Some(true) = self.all() {
-            let all_userids = vc.userids()
+            let mut revoked_userids = BTreeSet::new();
+            let valid_userids = vc.userids()
                 .filter_map(|ua| {
                     if let RevocationStatus::Revoked(_) = ua.revocation_status() {
+                        revoked_userids.insert(ua.userid());
                         None
                     } else {
                         Some(ua.userid().clone())
                     }
                 })
+                .collect::<BTreeSet<_>>();
+
+            let non_self_signed_userids = vc.cert().userids()
+                .filter(|_| self.all_matches_non_self_signed())
+                .filter(|u| ! revoked_userids.contains(u.userid()))
+                .filter(|u| ! valid_userids.contains(u.userid()))
+                .map(|u| u.userid().clone())
+                .collect::<Vec<_>>();
+
+            let all_userids = valid_userids.into_iter()
+                .chain(non_self_signed_userids.into_iter())
                 .map(|userid| ResolvedUserID::implicit(userid))
                 .collect::<Vec<_>>();
 
             if all_userids.is_empty() {
                 return Err(anyhow::anyhow!(
-                    "{} has no valid self-signed user IDs",
+                    "{} has no {}user IDs",
-                    vc.fingerprint()));
+                    vc.fingerprint(),
+                    if self.all_matches_non_self_signed() {
+                        ""
+                    } else {
+                        "valid self-signed "
+                    }));
             }
 
             userids.extend(all_userids);

tests/integration/common.rs

@@ -519,7 +519,20 @@ impl Sq {
     pub fn run<E>(&self, mut cmd: Command, expect: E) -> Output
         where E: Into<Option<bool>>
     {
-        eprintln!("Running: {:?}", cmd);
+        eprintln!("Running: {}",
+                  std::iter::once(cmd.get_program())
+                  .chain(cmd.get_args())
+                  .map(|arg| {
+                      let arg = arg.to_string_lossy();
+                      if arg.contains(" ") {
+                          format!("{:?}", arg)
+                      } else {
+                          arg.into_owned()
+                      }
+                  })
+                  .collect::<Vec<_>>()
+                  .join(" "));
+
         let output = cmd.output().expect("can run command");
         let expect = expect.into();
         match (output.status.success(), expect) {

tests/integration/sq_pki_link.rs

@@ -137,6 +137,9 @@ fn sq_retract(sq: &Sq, cert: &str, userids: &[&str], emails: &[&str])
     for email in emails {
         cmd.arg("--email").arg(email);
     }
+    if userids.is_empty() && emails.is_empty() {
+        cmd.arg("--all");
+    }
     eprintln!("{:?}", cmd);
     let output = sq.run(cmd, true);
 
@@ -838,9 +841,9 @@ fn special_names() {
     check("add", &["--all"], "xxx", false);
 
     for name in SPECIAL_STRINGS.iter() {
-        check("retract", &[], name, true);
+        check("retract", &["--all"], name, true);
     }
-    check("retract", &[], "xxx", false);
+    check("retract", &["--all"], "xxx", false);
 
     for name in SPECIAL_STRINGS.iter() {
         check("authorize", &["--all", "--unconstrained"], name, true);
@@ -848,7 +851,7 @@ fn special_names() {
     check("authorize", &["--all", "--unconstrained"], "xxx", false);
 
     for name in SPECIAL_STRINGS.iter() {
-        check("retract", &[], name, true);
+        check("retract", &["--all"], name, true);
     }
-    check("retract", &[], "xxx", false);
+    check("retract", &["--all"], "xxx", false);
 }

tests/integration/sq_pki_link_authorize.rs

@@ -459,7 +459,7 @@ fn retract_all() {
     // Retract all authorizations.  It should no longer be considered
     // a trusted introducer.
     sq.tick(1);
-    sq.pki_link_retract(&[], ca.key_handle(), NO_USERIDS);
+    sq.pki_link_retract(&["--all"], ca.key_handle(), NO_USERIDS);
     check(&sq, false);
 }