From 8ce98a00846699dc0c19fdf768f09604945b1ac9 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 1 Aug 2024 13:18:39 +0200 Subject: [PATCH] ref(token): Separate validation warning from parsing Separate logic for issuing the invalid auth token format validation warning from the parsing logic. This will allow us in the future to test whether a certain string might be an auth token without logging a warning. --- Cargo.lock | 10 ------- Cargo.toml | 1 - src/commands/derive_parser.rs | 6 ++--- src/commands/mod.rs | 3 ++- src/utils/auth_token/auth_token_impl.rs | 9 ++++--- src/utils/auth_token/test.rs | 26 +++---------------- src/utils/value_parsers.rs | 12 +++++++++ .../warn-invalid-auth-token.trycmd | 7 +++-- 8 files changed, 30 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a16c3d975f..b1324ff726 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2474,7 +2474,6 @@ dependencies = [ "sourcemap", "symbolic", "tempfile", - "testing_logger", "thiserror", "trycmd", "unix-daemonize", @@ -2930,15 +2929,6 @@ dependencies = [ "windows-sys 0.48.0", ] -[[package]] -name = "testing_logger" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d92b727cb45d33ae956f7f46b966b25f1bc712092aeef9dba5ac798fc89f720" -dependencies = [ - "log", -] - [[package]] name = "thiserror" version = "1.0.61" diff --git a/Cargo.toml b/Cargo.toml index a8f9db43d0..ec73eb5aaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,7 +87,6 @@ mockito = "0.31.1" predicates = "2.1.5" rstest = "0.18.2" tempfile = "3.8.1" -testing_logger = "0.1.1" trycmd = "0.14.11" [features] diff --git a/src/commands/derive_parser.rs b/src/commands/derive_parser.rs index 219fc3052e..ee6c3a389e 100644 --- a/src/commands/derive_parser.rs +++ b/src/commands/derive_parser.rs @@ -1,6 +1,6 @@ use crate::utils::auth_token::AuthToken; -use crate::utils::value_parsers::kv_parser; -use clap::{command, value_parser, ArgAction::SetTrue, Parser, Subcommand}; +use crate::utils::value_parsers::{auth_token_parser, kv_parser}; +use clap::{command, ArgAction::SetTrue, Parser, Subcommand}; use super::send_metric::SendMetricArgs; @@ -13,7 +13,7 @@ pub(super) struct SentryCLI { #[arg(help = "Custom headers that should be attached to all requests{n}in key:value format")] pub(super) headers: Vec<(String, String)>, - #[arg(global=true, long, value_parser=value_parser!(AuthToken))] + #[arg(global=true, long, value_parser=auth_token_parser)] #[arg(help = "Use the given Sentry auth token")] pub(super) auth_token: Option, diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 8e31b6ed83..c167dd8901 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -17,6 +17,7 @@ use crate::utils::logging::set_quiet_mode; use crate::utils::logging::Logger; use crate::utils::system::{init_backtrace, load_dotenv, print_error, QuietExit}; use crate::utils::update::run_sentrycli_update_nagger; +use crate::utils::value_parsers::auth_token_parser; mod derive_parser; @@ -165,7 +166,7 @@ fn app() -> Command { .value_name("AUTH_TOKEN") .long("auth-token") .global(true) - .value_parser(value_parser!(AuthToken)) + .value_parser(auth_token_parser) .help("Use the given Sentry auth token."), ) .arg( diff --git a/src/utils/auth_token/auth_token_impl.rs b/src/utils/auth_token/auth_token_impl.rs index 294aebaa08..749ff78bd9 100644 --- a/src/utils/auth_token/auth_token_impl.rs +++ b/src/utils/auth_token/auth_token_impl.rs @@ -24,6 +24,12 @@ impl AuthToken { fn as_str(&self) -> &str { self.0.as_str() } + + /// Returns whether the auth token follows a recognized format. If this function returns false, + /// that indicates that the auth token might not be valid, since it failed our soft validation. + pub fn format_recognized(&self) -> bool { + !matches!(self.0, AuthTokenInner::Unknown(_)) + } } impl From for AuthToken { @@ -72,9 +78,6 @@ impl AuthTokenInner { } else if let Ok(user_auth_token) = UserAuthToken::try_from(auth_string.clone()) { AuthTokenInner::User(user_auth_token) } else { - log::warn!( - "Unrecognized auth token format!\n\tHint: Did you copy your token correctly?" - ); AuthTokenInner::Unknown(auth_string) } } diff --git a/src/utils/auth_token/test.rs b/src/utils/auth_token/test.rs index bbecb257c6..0ffd5c9833 100644 --- a/src/utils/auth_token/test.rs +++ b/src/utils/auth_token/test.rs @@ -2,20 +2,6 @@ use super::AuthToken; use rstest::rstest; -use testing_logger::CapturedLog; - -/// Asserts that the logs vector is empty. -#[allow(clippy::ptr_arg)] // This function signature is required by testing_logger -fn assert_no_logs(logs: &Vec) { - assert!(logs.is_empty()); -} - -/// Asserts that the logs vector contains exactly one warning. -#[allow(clippy::ptr_arg)] // This function signature is required by testing_logger -fn assert_one_warning(logs: &Vec) { - assert_eq!(logs.len(), 1); - assert_eq!(logs[0].level, log::Level::Warn); -} // Org auth token tests ----------------------------------------------------- @@ -28,7 +14,6 @@ fn test_valid_org_auth_token() { lQ5ETt61cHhvJa35fxvxARsDXeVrd0pu4/smF4sRieA", ); - testing_logger::setup(); let token = AuthToken::from(good_token.clone()); assert!(token.payload().is_some()); @@ -39,7 +24,7 @@ fn test_valid_org_auth_token() { assert_eq!(good_token, token.to_string()); - testing_logger::validate(assert_no_logs); + assert!(token.format_recognized()); } #[test] @@ -51,7 +36,6 @@ fn test_valid_org_auth_token_missing_url() { lQ5ETt61cHhvJa35fxvxARsDXeVrd0pu4/smF4sRieA", ); - testing_logger::setup(); let token = AuthToken::from(good_token.clone()); assert!(token.payload().is_some()); @@ -62,7 +46,7 @@ fn test_valid_org_auth_token_missing_url() { assert_eq!(good_token, token.to_string()); - testing_logger::validate(assert_no_logs); + assert!(token.format_recognized()); } // User auth token tests ---------------------------------------------------- @@ -73,13 +57,12 @@ fn test_valid_org_auth_token_missing_url() { fn test_valid_user_auth_token(#[case] token_str: &'static str) { let good_token = String::from(token_str); - testing_logger::setup(); let token = AuthToken::from(good_token.clone()); assert!(token.payload().is_none()); assert_eq!(good_token, token.to_string()); - testing_logger::validate(assert_no_logs); + assert!(token.format_recognized()); } // Unknown auth token tests ------------------------------------------------- @@ -145,11 +128,10 @@ fn test_valid_user_auth_token(#[case] token_str: &'static str) { )] #[case::wrong_prefix("sntryt_c66aee1348a6e7a0993145d71cf8fa529ed09ee13dd5177b5f692e9f6ca38c30")] fn test_unknown_auth_token(#[case] token_str: &'static str) { - testing_logger::setup(); let token = AuthToken::from(token_str.to_owned()); assert_eq!(token_str, token.to_string()); assert!(token.payload().is_none()); - testing_logger::validate(assert_one_warning); + assert!(!token.format_recognized()); } diff --git a/src/utils/value_parsers.rs b/src/utils/value_parsers.rs index f63a18cc0c..11cd5e4965 100644 --- a/src/utils/value_parsers.rs +++ b/src/utils/value_parsers.rs @@ -1,4 +1,6 @@ +use crate::utils::auth_token::AuthToken; use anyhow::{anyhow, Result}; +use std::convert::Infallible; /// Parse key:value pair from string, used as a value_parser for Clap arguments pub fn kv_parser(s: &str) -> Result<(String, String)> { @@ -6,3 +8,13 @@ pub fn kv_parser(s: &str) -> Result<(String, String)> { .map(|(k, v)| (k.into(), v.into())) .ok_or_else(|| anyhow!("`{s}` is missing a `:`")) } + +/// Parse an AuthToken, and warn if the format is unrecognized +pub fn auth_token_parser(s: &str) -> Result { + let token = AuthToken::from(s); + if !token.format_recognized() { + log::warn!("Unrecognized auth token format. Ensure you copied your token correctly."); + } + + Ok(token) +} diff --git a/tests/integration/_cases/token_validation/warn-invalid-auth-token.trycmd b/tests/integration/_cases/token_validation/warn-invalid-auth-token.trycmd index bec101ddfd..33cd00abf3 100644 --- a/tests/integration/_cases/token_validation/warn-invalid-auth-token.trycmd +++ b/tests/integration/_cases/token_validation/warn-invalid-auth-token.trycmd @@ -1,8 +1,7 @@ ``` $ sentry-cli --auth-token not_a_valid_auth_token --version ? success -[..]WARN[..]Unrecognized auth token format! - Hint: Did you copy your token correctly? -sentry-cli [..] +[..]WARN[..] Unrecognized auth token format. Ensure you copied your token correctly. +... -``` \ No newline at end of file +```