diff --git a/NEWS b/NEWS index 4f32b2b6..ca1e36ba 100644 --- a/NEWS +++ b/NEWS @@ -78,6 +78,9 @@ - `sq pki link retract`'s positional argument for specifying the certificate to unlink must now be specified using a named argument, `--cert`. + - Removed `sq pki link add`'s positional argument for specifying a + user ID directly or by email address. Use the named arguments, + `--userid` or `--email` instead. * Changes in 0.38.0 ** Notable changes diff --git a/src/cli/pki/link.rs b/src/cli/pki/link.rs index 4b533b1f..1e5a8c4f 100644 --- a/src/cli/pki/link.rs +++ b/src/cli/pki/link.rs @@ -283,7 +283,7 @@ to force the signature to be re-created anyway.", #[clap( long = "all", - conflicts_with_all = &[ "userid", "email", "petname", "pattern" ], + conflicts_with_all = &[ "userid", "email", "petname" ], required = false, help = "Link all valid self-signed User ID to the certificate.", long_help = "Link all valid self-signed User ID to the certificate.", @@ -327,19 +327,6 @@ to force the signature to be re-created anyway.", silently converted to ``.", )] pub petname: Vec, - - #[clap( - value_name = "USERID|EMAIL", - required = false, - help = "A User ID or email address to accept.", - long_help = "A User ID or email address to link to the certificate. \ - This must match a self-signed User ID. To link a User ID \ - to the certificate that does not have a self-signature, \ - use `--petname`. Scripts should prefer to use \ - `--email` or `--userid`, as `sq` does not need to \ - guess if a value is a User ID or an email address.", - )] - pub pattern: Vec, } const ADD_EXAMPLES: Actions = Actions { diff --git a/src/commands.rs b/src/commands.rs index 41542a19..777871f6 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -498,7 +498,7 @@ impl<'c, 'store, 'rstore> VHelper<'c, 'store, 'rstore> { cert_fpr, signer_userid)) .sq().arg("pki").arg("link").arg("add") .arg("--cert").arg(cert_fpr) - .arg(signer_userid) + .arg_value("--userid", signer_userid) .done(); } (false, false) => { @@ -513,7 +513,7 @@ impl<'c, 'store, 'rstore> VHelper<'c, 'store, 'rstore> { cert_fpr, signer_userid)) .sq().arg("pki").arg("link").arg("add") .arg("--cert").arg(cert_fpr) - .arg(signer_userid) + .arg_value("--userid", signer_userid) .done(); } }; diff --git a/src/commands/pki/link.rs b/src/commands/pki/link.rs index 5bfee7a3..85fd5282 100644 --- a/src/commands/pki/link.rs +++ b/src/commands/pki/link.rs @@ -215,7 +215,7 @@ pub fn add(sq: Sq, c: link::AddCommand) = sq.resolve_cert(&c.cert, sequoia_wot::FULLY_TRUSTED)?; let mut userids = - check_userids(&sq, &cert, true, &c.userid, &c.email, &c.pattern) + check_userids(&sq, &cert, true, &c.userid, &c.email, &Vec::new()) .context("sq pki link add: Invalid User IDs")?; userids.extend(c.petname.iter().map(|petname| { // If it is a bare email, we wrap it in angle brackets. diff --git a/tests/integration/common.rs b/tests/integration/common.rs index e8d82063..985c7dc5 100644 --- a/tests/integration/common.rs +++ b/tests/integration/common.rs @@ -1499,7 +1499,7 @@ impl Sq { cmd.arg(arg); } cmd.arg("--cert").arg(cert.to_string()); - cmd.arg(userid); + cmd.arg("--userid").arg(userid); let output = self.run(cmd, None); if output.status.success() { diff --git a/tests/integration/sq_key_approvals_update.rs b/tests/integration/sq_key_approvals_update.rs index 961ef663..abcbe367 100644 --- a/tests/integration/sq_key_approvals_update.rs +++ b/tests/integration/sq_key_approvals_update.rs @@ -219,7 +219,9 @@ fn update_authenticated() -> Result<()> { // Link Bob's cert to his user ID. let mut link = sq.command(); - link.args(&["pki", "link", "add", "--cert", &bob_fp, BOB_USERID]); + link.args(&["pki", "link", "add", + "--cert", &bob_fp, + "--userid", BOB_USERID]); sq.run(link, true); // Attest Bob's certification. diff --git a/tests/integration/sq_pki_link.rs b/tests/integration/sq_pki_link.rs index 3e629c3c..2a2f9cad 100644 --- a/tests/integration/sq_pki_link.rs +++ b/tests/integration/sq_pki_link.rs @@ -88,14 +88,19 @@ fn sq_verify(sq: &Sq, // Links a User ID and a certificate. fn sq_link(sq: &Sq, - cert: &str, userids: &[&str], more_args: &[&str], + cert: &str, userids: &[&str], emails: &[&str], more_args: &[&str], success: bool) -> (ExitStatus, String, String) { let mut cmd = sq.command(); cmd.args(&["pki", "link", "add", "--time", &tick()]); cmd.arg("--cert").arg(cert); - cmd.args(userids); + for userid in userids { + cmd.arg("--userid").arg(userid); + } + for email in emails { + cmd.arg("--email").arg(email); + } cmd.args(more_args); let output = sq.run(cmd, None); @@ -304,14 +309,14 @@ fn sq_pki_link_add_retract() -> Result<()> { // Let's accept Alice, but not (yet) as a trusted introducer. We // should now be able to verify Alice's signature, but not Bob's. - sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], true); + sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], &[], true); sq_verify(&sq, None, &[], &[], &alice.sig_file, 1, 0); sq_verify(&sq, None, &[], &[], &bob.sig_file, 0, 1); // Accept Alice as a trusted introducer. We should be able to // verify Alice, Bob, and Carol's signatures. - sq_link(&sq, &alice_fpr, &[ &alice_userid ], &["--ca", "*"], true); + sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], &["--ca", "*"], true); sq_verify(&sq, None, &[], &[], &alice.sig_file, 1, 0); sq_verify(&sq, None, &[], &[], &bob.sig_file, 1, 0); @@ -328,7 +333,7 @@ fn sq_pki_link_add_retract() -> Result<()> { // Accept Alice as a trusted introducer again. We should be able // to verify Alice, Bob, and Carol's signatures. - sq_link(&sq, &alice_fpr, &[ &alice_userid ], &["--ca", "*"], true); + sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], &["--ca", "*"], true); sq_verify(&sq, None, &[], &[], &alice.sig_file, 1, 0); sq_verify(&sq, None, &[], &[], &bob.sig_file, 1, 0); @@ -348,7 +353,7 @@ fn sq_pki_link_add_retract() -> Result<()> { // Change Alice's acceptance to just be a normal certification. // We should only be able to verify her signature. - sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], true); + sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], &[], true); sq_verify(&sq, None, &[], &[], &alice.sig_file, 1, 0); sq_verify(&sq, None, &[], &[], &bob.sig_file, 0, 1); @@ -357,7 +362,7 @@ fn sq_pki_link_add_retract() -> Result<()> { // Change Alice's acceptance to be a ca, but only for example.org, // i.e., not for Dave. - sq_link(&sq, &alice_fpr, &[ &alice_userid ], &["--ca", "example.org"], + sq_link(&sq, &alice_fpr, &[ &alice_userid ], &[], &["--ca", "example.org"], true); sq_verify(&sq, None, &[], &[], &alice.sig_file, 1, 0); @@ -395,45 +400,29 @@ fn sq_pki_link_add_retract() -> Result<()> { // If we don't use --petname, than a self-signed User ID must // exist. - sq_link(&sq, &ed_fpr, &[ "--userid", "bob@example.com" ], &[], false); + sq_link(&sq, &ed_fpr, &[ "bob@example.com" ], &[], &[], false); sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); - sq_link(&sq, &ed_fpr, &[ "--email", "bob@example.com" ], &[], false); - sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); - - sq_link(&sq, &ed_fpr, &[ "bob@example.com" ], &[], false); + sq_link(&sq, &ed_fpr, &[], &[ "bob@example.com" ], &[], false); sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); // We should only create links if all the supplied User IDs are // valid. - sq_link(&sq, &ed_fpr, &[ - "--userid", "ed@some.org", "--userid", "bob@example.com" - ], &[], false); + sq_link(&sq, &ed_fpr, &["ed@some.org", "bob@example.com"], &[], &[], false); sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); - sq_link(&sq, &ed_fpr, &[ - "--userid", "ed@some.org", "--email", "bob@example.com" - ], &[], false); - sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); - - sq_link(&sq, &ed_fpr, &[ - "--userid", "ed@some.org", "bob@example.com" - ], &[], false); + sq_link(&sq, &ed_fpr, &["ed@some.org"], &["bob@example.com"], &[], false); sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); // Pass an email address to --userid. This shouldn't match // either. - sq_link(&sq, &ed_fpr, &[ - "--userid", "ed@other.org" - ], &[], false); + sq_link(&sq, &ed_fpr, &["ed@other.org"], &[], &[], false); sq_verify(&sq, None, &[], &[], &ed_sig_file, 0, 1); // Link all User IDs individually. - sq_link(&sq, &ed_fpr, &[ - "--email", "ed@other.org", - "--email", "ed@example.org", - "--userid", "ed@some.org", - ], &[], true); + sq_link(&sq, &ed_fpr, + &["ed@some.org"], &["ed@other.org", "ed@example.org"], + &[], true); sq_verify(&sq, None, &[], &[], &ed_sig_file, 1, 0); // Retract the links one at a time. @@ -488,37 +477,37 @@ fn sq_pki_link_update_detection() -> Result<()> { let bytes = compare(bytes, &alice_cert_pgp, true); // Link it. - sq_link(&sq, &alice_fpr, &[], &["--all"], true); + sq_link(&sq, &alice_fpr, &[], &[], &["--all"], true); let bytes = compare(bytes, &alice_cert_pgp, false); // As no parameters changed, this should succeeded, but no // certification should be written. - let output = sq_link(&sq, &alice_fpr, &[], &["--all"], true); + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, true); // Make Alice a CA. - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--ca", "*", "--all"], true); assert!(output.2.contains("was previously"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--ca", "*", "--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, true); // Make her a partially trusted CA. - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--amount", "30", "--all"], true); assert!(output.2.contains("was previously"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--amount", "30", "--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); @@ -537,38 +526,38 @@ fn sq_pki_link_update_detection() -> Result<()> { // Link it again. - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--depth", "10", "--amount", "10", "--all"], true); assert!(output.2.contains("was retracted"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--depth", "10", "--amount", "10", "--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, true); // Use a notation. - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--notation", "foo", "10", "--all"], true); assert!(output.2.contains("was previously"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); - let output = sq_link(&sq, &alice_fpr, &[], + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--notation", "foo", "10", "--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, true); // The default link again. - let output = sq_link(&sq, &alice_fpr, &[], &["--all"], true); + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--all"], true); assert!(output.2.contains("was previously"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); - let output = sq_link(&sq, &alice_fpr, &[], &["--all"], true); + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--all"], true); assert!(output.2.contains("Certification parameters are unchanged"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, true); @@ -616,7 +605,7 @@ fn sq_pki_link_add_temporary() -> Result<()> { sq_verify(&sq, None, &[], &[], &alice_sig_file, 0, 1); - let output = sq_link(&sq, &alice_fpr, &[], &["--temporary", "--all"], true); + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--temporary", "--all"], true); assert!(output.2.contains("Certifying "), "stdout:\n{}\nstderr:\n{}", output.1, output.2); let bytes = compare(bytes, &alice_cert_pgp, false); @@ -637,7 +626,7 @@ fn sq_pki_link_add_temporary() -> Result<()> { // Now mark it as fully trusted. It should be trusted now, in 6 // days and in 8 days. - let output = sq_link(&sq, &alice_fpr, &[], &["--all"], true); + let output = sq_link(&sq, &alice_fpr, &[], &[], &["--all"], true); assert!(output.2.contains("was previously"), "stdout:\n{}\nstderr:\n{}", output.1, output.2); eprintln!("{:?}", output);