ref(crons): Prefer DSN based auth (#1545)

Co-authored-by: Evan Purkhiser <evanpurkhiser@gmail.com>
getsentry · Mar 24, 2023 · ac6532f · ac6532f
1 parent d325a3f
commit ac6532f
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 17 deletions.
diff --git a/src/api.rs b/src/api.rs
@@ -28,6 +28,7 @@ use parking_lot::{Mutex, RwLock};
 use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
 use regex::{Captures, Regex};
 use sentry::protocol::{Exception, Values};
+use sentry::types::Dsn;
 use serde::de::{DeserializeOwned, Deserializer};
 use serde::{Deserialize, Serialize};
 use sha1_smol::Digest;
@@ -433,6 +434,24 @@ impl Api {
         ApiRequest::create(handle, &method, &url, auth, env, headers)
     }
 
+    /// Convenience method that performs a request using DSN as authentication method.
+    pub fn request_with_dsn_auth(
+        &self,
+        method: Method,
+        url: &str,
+        dsn: Dsn,
+    ) -> ApiResult<ApiResponse> {
+        // We resolve an absolute URL to skip default authentication flow.
+        let url = self
+            .config
+            .get_api_endpoint(url)
+            .map_err(|err| ApiError::with_source(ApiErrorKind::BadApiUrl, err))?;
+
+        self.request(method, &url)?
+            .with_header("Authorization", &format!("DSN {dsn}"))?
+            .send()
+    }
+
     /// Convenience method that performs a `GET` request.
     pub fn get(&self, path: &str) -> ApiResult<ApiResponse> {
         self.request(Method::Get, path)?.send()
@@ -1442,11 +1461,16 @@ impl Api {
     /// Create a new checkin for a monitor
     pub fn create_monitor_checkin(
         &self,
+        dsn: Option<Dsn>,
         monitor_slug: &String,
         checkin: &CreateMonitorCheckIn,
     ) -> ApiResult<MonitorCheckIn> {
         let path = &format!("/monitors/{}/checkins/", PathArg(monitor_slug),);
-        let resp = self.post(path, checkin)?;
+        let resp = if let Some(dsn) = dsn {
+            self.request_with_dsn_auth(Method::Post, path, dsn)?
+        } else {
+            self.post(path, checkin)?
+        };
         if resp.status() == 404 {
             return Err(ApiErrorKind::ResourceNotFound.into());
         }
@@ -1456,6 +1480,7 @@ impl Api {
     /// Update a checkin for a monitor
     pub fn update_monitor_checkin(
         &self,
+        dsn: Option<Dsn>,
         monitor_slug: &String,
         checkin_id: &Uuid,
         checkin: &UpdateMonitorCheckIn,
@@ -1465,7 +1490,12 @@ impl Api {
             PathArg(monitor_slug),
             PathArg(checkin_id),
         );
-        let resp = self.put(path, checkin)?;
+        let resp = if let Some(dsn) = dsn {
+            self.request_with_dsn_auth(Method::Put, path, dsn)?
+        } else {
+            self.put(path, checkin)?
+        };
+
         if resp.status() == 404 {
             return Err(ApiErrorKind::ResourceNotFound.into());
         }
@@ -1812,10 +1842,6 @@ impl ApiRequest {
                 debug!("using token authentication");
                 self.with_header("Authorization", &format!("Bearer {token}"))
             }
-            Auth::Dsn(ref public_key) => {
-                debug!("using dsn authentication");
-                self.with_header("Authorization", &format!("DSN {public_key}"))
-            }
         }
     }
 
@@ -2463,7 +2489,7 @@ pub enum MonitorCheckinStatus {
 #[derive(Debug, Deserialize)]
 pub struct MonitorCheckIn {
     pub id: Uuid,
-    pub status: MonitorCheckinStatus,
+    pub status: Option<MonitorCheckinStatus>,
     pub duration: Option<u64>,
 }
 

diff --git a/src/commands/info.rs b/src/commands/info.rs
@@ -60,7 +60,6 @@ fn describe_auth(auth: Option<&Auth>) -> &str {
         None => "Unauthorized",
         Some(&Auth::Token(_)) => "Auth Token",
         Some(&Auth::Key(_)) => "API Key",
-        Some(&Auth::Dsn(_)) => "DSN",
     }
 }
 
@@ -76,7 +75,6 @@ fn get_config_status_json() -> Result<()> {
     rv.auth.auth_type = config.get_auth().map(|val| match val {
         Auth::Token(_) => "token".into(),
         Auth::Key(_) => "api_key".into(),
-        Auth::Dsn(_) => "dsn".into(),
     });
     rv.auth.successful = config.get_auth().is_some() && Api::current().get_auth_info().is_ok();
     rv.have_dsn = config.get_dsn().is_ok();

diff --git a/src/commands/monitors/run.rs b/src/commands/monitors/run.rs
@@ -1,3 +1,4 @@
+use log::warn;
 use std::process;
 use std::time::Instant;
 
@@ -6,6 +7,7 @@ use clap::{Arg, ArgAction, ArgMatches, Command};
 use console::style;
 
 use crate::api::{Api, CreateMonitorCheckIn, MonitorCheckinStatus, UpdateMonitorCheckIn};
+use crate::config::Config;
 use crate::utils::system::{print_error, QuietExit};
 
 pub fn make_command(command: Command) -> Command {
@@ -35,13 +37,22 @@ pub fn make_command(command: Command) -> Command {
 
 pub fn execute(matches: &ArgMatches) -> Result<()> {
     let api = Api::current();
+    let config = Config::current();
+    let dsn = config.get_dsn().ok();
+
+    // Token based auth is deprecated, prefer DSN style auth for monitor checkins.
+    // Using token based auth *DOES NOT WORK* when using slugs.
+    if dsn.is_none() {
+        warn!("Token auth is deprecated for cron monitor checkins. Please use DSN auth.");
+    }
 
     let monitor_slug = matches.get_one::<String>("monitor_slug").unwrap();
 
     let allow_failure = matches.get_flag("allow_failure");
     let args: Vec<_> = matches.get_many::<String>("args").unwrap().collect();
 
     let monitor_checkin = api.create_monitor_checkin(
+        dsn.clone(),
         monitor_slug,
         &CreateMonitorCheckIn {
             status: MonitorCheckinStatus::InProgress,
@@ -69,6 +80,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     match monitor_checkin {
         Ok(checkin) => {
             api.update_monitor_checkin(
+                dsn,
                 monitor_slug,
                 &checkin.id,
                 &UpdateMonitorCheckIn {

diff --git a/src/config.rs b/src/config.rs
@@ -24,7 +24,6 @@ use crate::utils::http::is_absolute_url;
 pub enum Auth {
     Key(String),
     Token(String),
-    Dsn(String),
 }
 
 lazy_static! {
@@ -154,9 +153,6 @@ impl Config {
                 self.ini
                     .set_to(Some("auth"), "api_key".into(), val.to_string());
             }
-            Some(Auth::Dsn(ref val)) => {
-                self.ini.set_to(Some("auth"), "dsn".into(), val.to_string());
-            }
             None => {}
         }
     }
@@ -588,14 +584,10 @@ fn get_default_auth(ini: &Ini) -> Option<Auth> {
         Some(Auth::Token(val))
     } else if let Ok(val) = env::var("SENTRY_API_KEY") {
         Some(Auth::Key(val))
-    } else if let Ok(val) = env::var("SENTRY_DSN") {
-        Some(Auth::Dsn(val))
     } else if let Some(val) = ini.get_from(Some("auth"), "token") {
         Some(Auth::Token(val.to_owned()))
     } else if let Some(val) = ini.get_from(Some("auth"), "api_key") {
         Some(Auth::Key(val.to_owned()))
-    } else if let Some(val) = ini.get_from(Some("auth"), "dsn") {
-        Some(Auth::Dsn(val.to_owned()))
     } else {
         None
     }

diff --git a/tests/integration/_cases/monitors/monitors-run-token-auth-win.trycmd b/tests/integration/_cases/monitors/monitors-run-token-auth-win.trycmd
@@ -0,0 +1,7 @@
+```
+$ sentry-cli monitors run foo-monitor -- cmd.exe /C echo 123
+? success
+  WARN    [..] Token auth is deprecated for cron monitor checkins. Please use DSN auth.
+123
+
+```
diff --git a/tests/integration/_cases/monitors/monitors-run-token-auth.trycmd b/tests/integration/_cases/monitors/monitors-run-token-auth.trycmd
@@ -0,0 +1,7 @@
+```
+$ sentry-cli monitors run foo-monitor -- echo 123
+? success
+  WARN    [..] Token auth is deprecated for cron monitor checkins. Please use DSN auth.
+123
+
+```
diff --git a/tests/integration/monitors/run.rs b/tests/integration/monitors/run.rs
@@ -6,13 +6,27 @@ fn command_monitors_run() {
         EndpointOptions::new("POST", "/api/0/monitors/foo-monitor/checkins/", 200)
             .with_response_file("monitors/post-monitors.json"),
     );
+
     if cfg!(windows) {
         register_test("monitors/monitors-run-win.trycmd");
     } else {
         register_test("monitors/monitors-run.trycmd");
     }
 }
 
+#[test]
+fn command_monitors_run_token_auth() {
+    let _server = mock_endpoint(
+        EndpointOptions::new("POST", "/api/0/monitors/foo-monitor/checkins/", 200)
+            .with_response_file("monitors/post-monitors.json"),
+    );
+    if cfg!(windows) {
+        register_test("monitors/monitors-run-token-auth-win.trycmd").env("SENTRY_DSN", "");
+    } else {
+        register_test("monitors/monitors-run-token-auth.trycmd").env("SENTRY_DSN", "");
+    }
+}
+
 #[test]
 fn command_monitors_run_env() {
     let _server = mock_endpoint(