From 3b5a1c49ca43c95e54648c4f1106c5822c5794b7 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Mon, 3 Aug 2020 10:49:28 -0400 Subject: [PATCH 01/11] Add the ADM router settings --- autoendpoint/src/routers/adm/client.rs | 1 + autoendpoint/src/routers/adm/error.rs | 1 + autoendpoint/src/routers/adm/mod.rs | 6 ++++ autoendpoint/src/routers/adm/router.rs | 1 + autoendpoint/src/routers/adm/settings.rs | 38 ++++++++++++++++++++++++ autoendpoint/src/routers/mod.rs | 1 + 6 files changed, 48 insertions(+) create mode 100644 autoendpoint/src/routers/adm/client.rs create mode 100644 autoendpoint/src/routers/adm/error.rs create mode 100644 autoendpoint/src/routers/adm/mod.rs create mode 100644 autoendpoint/src/routers/adm/router.rs create mode 100644 autoendpoint/src/routers/adm/settings.rs diff --git a/autoendpoint/src/routers/adm/client.rs b/autoendpoint/src/routers/adm/client.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/autoendpoint/src/routers/adm/client.rs @@ -0,0 +1 @@ + diff --git a/autoendpoint/src/routers/adm/error.rs b/autoendpoint/src/routers/adm/error.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/autoendpoint/src/routers/adm/error.rs @@ -0,0 +1 @@ + diff --git a/autoendpoint/src/routers/adm/mod.rs b/autoendpoint/src/routers/adm/mod.rs new file mode 100644 index 000000000..51e0945b0 --- /dev/null +++ b/autoendpoint/src/routers/adm/mod.rs @@ -0,0 +1,6 @@ +//! A notification router for Amazon devices, using Amazon Device Messaging + +pub mod client; +pub mod error; +pub mod router; +pub mod settings; diff --git a/autoendpoint/src/routers/adm/router.rs b/autoendpoint/src/routers/adm/router.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/autoendpoint/src/routers/adm/router.rs @@ -0,0 +1 @@ + diff --git a/autoendpoint/src/routers/adm/settings.rs b/autoendpoint/src/routers/adm/settings.rs new file mode 100644 index 000000000..c23ff6e98 --- /dev/null +++ b/autoendpoint/src/routers/adm/settings.rs @@ -0,0 +1,38 @@ +use std::collections::HashMap; + +/// Settings for `AdmRouter` +#[derive(Clone, Debug, serde::Deserialize)] +#[serde(default)] +#[serde(deny_unknown_fields)] +pub struct AdmSettings { + /// A JSON dict of `AdmProfile`s. This must be a `String` because + /// environment variables cannot encode a `HashMap` + pub profiles: String, + /// The max size of notification data in bytes + pub max_data: usize, +} + +/// Settings for a specific ADM profile +#[derive(Clone, Debug, Default, serde::Deserialize)] +#[serde(default)] +#[serde(deny_unknown_fields)] +pub struct AdmProfile { + pub client_id: String, + pub client_secret: String, +} + +impl Default for AdmSettings { + fn default() -> Self { + Self { + profiles: "{}".to_string(), + max_data: 6000, + } + } +} + +impl AdmSettings { + /// Read the profiles from the JSON string + pub fn profiles(&self) -> serde_json::Result> { + serde_json::from_str(&self.profiles) + } +} diff --git a/autoendpoint/src/routers/mod.rs b/autoendpoint/src/routers/mod.rs index 41033e83f..7d52e2acc 100644 --- a/autoendpoint/src/routers/mod.rs +++ b/autoendpoint/src/routers/mod.rs @@ -12,6 +12,7 @@ use async_trait::async_trait; use std::collections::HashMap; use thiserror::Error; +pub mod adm; pub mod apns; mod common; pub mod fcm; From c720169a6471b82023f007b2555241d0d139265f Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Mon, 3 Aug 2020 11:00:39 -0400 Subject: [PATCH 02/11] Add the ADM router error --- autoendpoint/src/routers/adm/error.rs | 31 +++++++++++++++++++++++++++ autoendpoint/src/routers/mod.rs | 12 ++++++++--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/autoendpoint/src/routers/adm/error.rs b/autoendpoint/src/routers/adm/error.rs index 8b1378917..cd561ce89 100644 --- a/autoendpoint/src/routers/adm/error.rs +++ b/autoendpoint/src/routers/adm/error.rs @@ -1 +1,32 @@ +use crate::error::ApiErrorKind; +use crate::routers::RouterError; +use actix_web::http::StatusCode; +/// Errors that may occur in the Amazon Device Messaging router +#[derive(thiserror::Error, Debug)] +pub enum AdmError { + #[error("Failed to decode the profile settings")] + ProfileSettingsDecode(#[from] serde_json::Error), +} + +impl AdmError { + /// Get the associated HTTP status code + pub fn status(&self) -> StatusCode { + match self { + AdmError::ProfileSettingsDecode(_) => StatusCode::INTERNAL_SERVER_ERROR, + } + } + + /// Get the associated error number + pub fn errno(&self) -> Option { + match self { + AdmError::ProfileSettingsDecode(_) => None, + } + } +} + +impl From for ApiErrorKind { + fn from(e: AdmError) -> Self { + ApiErrorKind::Router(RouterError::Adm(e)) + } +} diff --git a/autoendpoint/src/routers/mod.rs b/autoendpoint/src/routers/mod.rs index 7d52e2acc..6488eeae9 100644 --- a/autoendpoint/src/routers/mod.rs +++ b/autoendpoint/src/routers/mod.rs @@ -4,6 +4,7 @@ use crate::db::error::DbError; use crate::error::ApiResult; use crate::extractors::notification::Notification; use crate::extractors::router_data_input::RouterDataInput; +use crate::routers::adm::error::AdmError; use crate::routers::apns::error::ApnsError; use crate::routers::fcm::error::FcmError; use actix_web::http::StatusCode; @@ -71,11 +72,14 @@ impl From for HttpResponse { #[derive(Debug, Error)] pub enum RouterError { #[error(transparent)] - Fcm(#[from] FcmError), + Adm(#[from] AdmError), #[error(transparent)] Apns(#[from] ApnsError), + #[error(transparent)] + Fcm(#[from] FcmError), + #[error("Database error while saving notification")] SaveDb(#[source] DbError), @@ -93,8 +97,9 @@ impl RouterError { /// Get the associated HTTP status code pub fn status(&self) -> StatusCode { match self { - RouterError::Fcm(e) => e.status(), + RouterError::Adm(e) => e.status(), RouterError::Apns(e) => e.status(), + RouterError::Fcm(e) => e.status(), RouterError::SaveDb(_) => StatusCode::SERVICE_UNAVAILABLE, RouterError::UserWasDeleted => StatusCode::GONE, RouterError::TooMuchData(_) => StatusCode::PAYLOAD_TOO_LARGE, @@ -104,8 +109,9 @@ impl RouterError { /// Get the associated error number pub fn errno(&self) -> Option { match self { - RouterError::Fcm(e) => e.errno(), + RouterError::Adm(e) => e.errno(), RouterError::Apns(e) => e.errno(), + RouterError::Fcm(e) => e.errno(), RouterError::TooMuchData(_) => Some(104), RouterError::SaveDb(_) => Some(201), RouterError::UserWasDeleted => Some(105), From a533788f5a3c7539260d31622eed37c7dc75122c Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 09:39:12 -0400 Subject: [PATCH 03/11] Add AdmClient and tests --- autoendpoint/src/routers/adm/client.rs | 345 +++++++++++++++++++++++ autoendpoint/src/routers/adm/error.rs | 52 +++- autoendpoint/src/routers/adm/settings.rs | 10 + 3 files changed, 405 insertions(+), 2 deletions(-) diff --git a/autoendpoint/src/routers/adm/client.rs b/autoendpoint/src/routers/adm/client.rs index 8b1378917..834e0f995 100644 --- a/autoendpoint/src/routers/adm/client.rs +++ b/autoendpoint/src/routers/adm/client.rs @@ -1 +1,346 @@ +use crate::routers::adm::error::AdmError; +use crate::routers::adm::settings::{AdmProfile, AdmSettings}; +use autopush_common::util::sec_since_epoch; +use futures::lock::Mutex; +use reqwest::StatusCode; +use serde::Deserialize; +use std::collections::HashMap; +use std::time::Duration; +use url::Url; +/// Holds profile-specific ADM data and authentication. This client handles +/// sending notifications to ADM. +pub struct AdmClient { + base_url: Url, + profile: AdmProfile, + timeout: Duration, + http: reqwest::Client, + token_info: Mutex, +} + +/// Holds information about the cached access token +struct TokenInfo { + token: String, + expiration_time: u64, +} + +/// A successful OAuth token response +#[derive(Deserialize)] +struct TokenResponse { + access_token: String, + expires_in: u64, +} + +/// A successful send response +#[derive(Deserialize)] +struct AdmSendResponse { + #[serde(rename = "registrationID")] + registration_id: String, +} + +/// An error returned by ADM +#[derive(Deserialize)] +struct AdmResponseError { + reason: Option, +} + +impl AdmClient { + /// Create an `AdmClient` using the provided profile + pub fn new(settings: &AdmSettings, profile: AdmProfile, http: reqwest::Client) -> Self { + AdmClient { + base_url: settings.base_url.clone(), + profile, + timeout: Duration::from_secs(settings.timeout as u64), + http, + token_info: Mutex::new(TokenInfo { + // Dummy values to trigger a token fetch + token: "".to_string(), + expiration_time: 0, + }), + } + } + + /// Get an ADM access token (from cache or request a new one) + async fn get_access_token(&self) -> Result { + let mut token_info = self.token_info.lock().await; + + if token_info.expiration_time > sec_since_epoch() + 60 { + trace!("Using cached access token"); + return Ok(token_info.token.clone()); + } + + trace!("Access token is out of date, requesting a new one"); + let oauth_url = self.base_url.join("auth/O2/token").unwrap(); + let response = self + .http + .post(oauth_url) + .form(&serde_json::json!({ + "grant_type": "client_credentials", + "scope": "messaging:push", + "client_id": &self.profile.client_id, + "client_secret": &self.profile.client_secret, + })) + .send() + .await?; + trace!("response = {:?}", response); + + if response.status() != 200 { + let status = response.status(); + let error_response: AdmResponseError = response.json().await?; + return Err(AdmError::Upstream { + status, + reason: error_response + .reason + .unwrap_or_else(|| "Unknown reason".to_string()), + }); + } + + let token_response: TokenResponse = response.json().await?; + token_info.token = token_response.access_token; + token_info.expiration_time = sec_since_epoch() + token_response.expires_in; + + Ok(token_info.token.clone()) + } + + /// Send the message data to ADM. The device's current registration ID is + /// returned. If it is different than the current stored ID, the stored ID + /// should be updated. + pub async fn send( + &self, + data: HashMap<&'static str, String>, + registration_id: String, + ttl: usize, + ) -> Result { + // Build the ADM message + let message = serde_json::json!({ + "data": data, + "expiresAfter": ttl, + }); + let access_token = self.get_access_token().await?; + let url = self.base_url.join(&format!( + "messaging/registrations/{}/messages", + registration_id + ))?; + + // Make the request + let response = self + .http + .post(url) + .header("Authorization", format!("Bearer {}", access_token.as_str())) + .header("Accept", "application/json") + .header( + "X-Amzn-Type-Version", + "com.amazon.device.messaging.ADMMessage@1.0", + ) + .header( + "X-Amzn-Accept-Type", + "com.amazon.device.messaging.ADMSendResult@1.0", + ) + .json(&message) + .timeout(self.timeout) + .send() + .await + .map_err(|e| { + if e.is_timeout() { + AdmError::RequestTimeout + } else { + AdmError::Connect(e) + } + })?; + + // Handle error + let status = response.status(); + if status != 200 { + let response_error: AdmResponseError = response + .json() + .await + .map_err(AdmError::DeserializeResponse)?; + + return Err(match (status, response_error.reason) { + (StatusCode::UNAUTHORIZED, _) => AdmError::Authentication, + (StatusCode::NOT_FOUND, _) => AdmError::NotFound, + (status, reason) => AdmError::Upstream { + status, + reason: reason.unwrap_or_else(|| "Unknown reason".to_string()), + }, + }); + } + + let send_response: AdmSendResponse = response.json().await?; + Ok(send_response.registration_id) + } +} + +#[cfg(test)] +pub mod tests { + use crate::routers::adm::client::AdmClient; + use crate::routers::adm::error::AdmError; + use crate::routers::adm::settings::{AdmProfile, AdmSettings}; + use std::collections::HashMap; + use url::Url; + + const REGISTRATION_ID: &str = "test-registration-id"; + const ACCESS_TOKEN: &str = "test-access-token"; + const CLIENT_ID: &str = "test-client-id"; + const CLIENT_SECRET: &str = "test-client-secret"; + + /// Mock the OAuth token endpoint to provide the access token + pub fn mock_token_endpoint() -> mockito::Mock { + mockito::mock("POST", "/auth/O2/token") + .with_body( + serde_json::json!({ + "access_token": ACCESS_TOKEN, + "expires_in": 3600, + "scope": "messaging:push", + "token_type": "Bearer" + }) + .to_string(), + ) + .create() + } + + /// Start building a mock for the ADM endpoint + pub fn mock_adm_endpoint_builder() -> mockito::Mock { + mockito::mock( + "POST", + format!("/messaging/registrations/{}/messages", REGISTRATION_ID).as_str(), + ) + } + + /// Make an AdmClient which uses the mock server + fn make_client() -> AdmClient { + AdmClient::new( + &AdmSettings { + base_url: Url::parse(&mockito::server_url()).unwrap(), + ..Default::default() + }, + AdmProfile { + client_id: CLIENT_ID.to_string(), + client_secret: CLIENT_SECRET.to_string(), + }, + reqwest::Client::new(), + ) + } + + /// The ADM client uses the access token and parameters to build the + /// expected request. + #[tokio::test] + async fn sends_correct_request() { + let client = make_client(); + let _token_mock = mock_token_endpoint(); + let adm_mock = mock_adm_endpoint_builder() + .match_header("Authorization", format!("Bearer {}", ACCESS_TOKEN).as_str()) + .match_header("Content-Type", "application/json") + .match_header("Accept", "application/json") + .match_header( + "X-Amzn-Type-Version", + "com.amazon.device.messaging.ADMMessage@1.0", + ) + .match_header( + "X-Amzn-Accept-Type", + "com.amazon.device.messaging.ADMSendResult@1.0", + ) + .match_body(r#"{"data":{"is_test":"true"},"expiresAfter":42}"#) + .with_body(r#"{"registrationID":"test-registration-id2"}"#) + .create(); + + let mut data = HashMap::new(); + data.insert("is_test", "true".to_string()); + + let result = client.send(data, REGISTRATION_ID.to_string(), 42).await; + assert!(result.is_ok(), "result = {:?}", result); + adm_mock.assert(); + } + + /// Authorization errors are handled + #[tokio::test] + async fn unauthorized() { + let client = make_client(); + let _token_mock = mock_token_endpoint(); + let _adm_mock = mock_adm_endpoint_builder() + .with_status(401) + .with_body(r#"{"reason":"test-message"}"#) + .create(); + + let result = client + .send(HashMap::new(), REGISTRATION_ID.to_string(), 42) + .await; + assert!(result.is_err()); + assert!( + matches!(result.as_ref().unwrap_err(), AdmError::Authentication), + "result = {:?}", + result + ); + } + + /// 404 errors are handled + #[tokio::test] + async fn not_found() { + let client = make_client(); + let _token_mock = mock_token_endpoint(); + let _adm_mock = mock_adm_endpoint_builder() + .with_status(404) + .with_body(r#"{"reason":"test-message"}"#) + .create(); + + let result = client + .send(HashMap::new(), REGISTRATION_ID.to_string(), 42) + .await; + assert!(result.is_err()); + assert!( + matches!(result.as_ref().unwrap_err(), AdmError::NotFound), + "result = {:?}", + result + ); + } + + /// Unhandled errors (where a reason is returned) are wrapped and returned + #[tokio::test] + async fn other_adm_error() { + let client = make_client(); + let _token_mock = mock_token_endpoint(); + let _fcm_mock = mock_adm_endpoint_builder() + .with_status(400) + .with_body(r#"{"reason":"test-message"}"#) + .create(); + + let result = client + .send(HashMap::new(), REGISTRATION_ID.to_string(), 42) + .await; + assert!(result.is_err()); + assert!( + matches!( + result.as_ref().unwrap_err(), + AdmError::Upstream { status, reason } + if *status == 400 && reason == "test-message" + ), + "result = {:?}", + result + ); + } + + /// Unknown errors (where a reason is NOT returned) are handled + #[tokio::test] + async fn unknown_adm_error() { + let client = make_client(); + let _token_mock = mock_token_endpoint(); + let _fcm_mock = mock_adm_endpoint_builder() + .with_status(400) + .with_body("{}") + .create(); + + let result = client + .send(HashMap::new(), REGISTRATION_ID.to_string(), 42) + .await; + assert!(result.is_err()); + assert!( + matches!( + result.as_ref().unwrap_err(), + AdmError::Upstream { status, reason } + if *status == 400 && reason == "Unknown reason" + ), + "result = {:?}", + result + ); + } +} diff --git a/autoendpoint/src/routers/adm/error.rs b/autoendpoint/src/routers/adm/error.rs index cd561ce89..54e6c0728 100644 --- a/autoendpoint/src/routers/adm/error.rs +++ b/autoendpoint/src/routers/adm/error.rs @@ -5,22 +5,70 @@ use actix_web::http::StatusCode; /// Errors that may occur in the Amazon Device Messaging router #[derive(thiserror::Error, Debug)] pub enum AdmError { + #[error("HTTP error: {0}")] + Http(#[from] reqwest::Error), + + #[error("Error while building a URL: {0}")] + ParseUrl(#[from] url::ParseError), + #[error("Failed to decode the profile settings")] ProfileSettingsDecode(#[from] serde_json::Error), + + #[error("Error while connecting to ADM")] + Connect(#[source] reqwest::Error), + + #[error("Unable to deserialize ADM response")] + DeserializeResponse(#[source] reqwest::Error), + + #[error("ADM authentication error")] + Authentication, + + #[error("ADM recipient no longer available")] + NotFound, + + #[error("ADM error, {status}: {reason}")] + Upstream { status: StatusCode, reason: String }, + + #[error("ADM request timed out")] + RequestTimeout, } impl AdmError { /// Get the associated HTTP status code pub fn status(&self) -> StatusCode { match self { - AdmError::ProfileSettingsDecode(_) => StatusCode::INTERNAL_SERVER_ERROR, + AdmError::ParseUrl(_) => StatusCode::BAD_REQUEST, + + AdmError::NotFound => StatusCode::GONE, + + AdmError::Http(_) | AdmError::ProfileSettingsDecode(_) => { + StatusCode::INTERNAL_SERVER_ERROR + } + + AdmError::Connect(_) + | AdmError::DeserializeResponse(_) + | AdmError::Authentication + | AdmError::Upstream { .. } + | AdmError::RequestTimeout => StatusCode::BAD_GATEWAY, } } /// Get the associated error number pub fn errno(&self) -> Option { match self { - AdmError::ProfileSettingsDecode(_) => None, + AdmError::NotFound => Some(106), + + AdmError::Authentication => Some(901), + + AdmError::Connect(_) => Some(902), + + AdmError::RequestTimeout => Some(903), + + AdmError::Http(_) + | AdmError::ParseUrl(_) + | AdmError::ProfileSettingsDecode(_) + | AdmError::DeserializeResponse(_) + | AdmError::Upstream { .. } => None, } } } diff --git a/autoendpoint/src/routers/adm/settings.rs b/autoendpoint/src/routers/adm/settings.rs index c23ff6e98..dbbd2e412 100644 --- a/autoendpoint/src/routers/adm/settings.rs +++ b/autoendpoint/src/routers/adm/settings.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use url::Url; /// Settings for `AdmRouter` #[derive(Clone, Debug, serde::Deserialize)] @@ -10,6 +11,12 @@ pub struct AdmSettings { pub profiles: String, /// The max size of notification data in bytes pub max_data: usize, + /// The base URL to use for ADM requests + pub base_url: Url, + /// The number of seconds to wait for ADM requests to complete + pub timeout: usize, + /// The minimum TTL to use for ADM notifications + pub min_ttl: usize, } /// Settings for a specific ADM profile @@ -26,6 +33,9 @@ impl Default for AdmSettings { Self { profiles: "{}".to_string(), max_data: 6000, + base_url: Url::parse("https://api.amazon.com").unwrap(), + timeout: 3, + min_ttl: 60, } } } From 26f66a7eb6506e77dda15f5cd3f6c522bab8b387 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 11:01:55 -0400 Subject: [PATCH 04/11] Add AdmRouter and extract common router code --- autoendpoint/src/routers/adm/client.rs | 68 +++++++----- autoendpoint/src/routers/adm/error.rs | 41 +++----- autoendpoint/src/routers/adm/router.rs | 133 ++++++++++++++++++++++++ autoendpoint/src/routers/apns/router.rs | 42 ++------ autoendpoint/src/routers/common.rs | 81 ++++++++++++++- autoendpoint/src/routers/fcm/client.rs | 43 +++++--- autoendpoint/src/routers/fcm/error.rs | 47 ++------- autoendpoint/src/routers/fcm/router.rs | 84 +++------------ autoendpoint/src/routers/mod.rs | 40 ++++++- 9 files changed, 362 insertions(+), 217 deletions(-) diff --git a/autoendpoint/src/routers/adm/client.rs b/autoendpoint/src/routers/adm/client.rs index 834e0f995..d84425cff 100644 --- a/autoendpoint/src/routers/adm/client.rs +++ b/autoendpoint/src/routers/adm/client.rs @@ -1,5 +1,6 @@ use crate::routers::adm::error::AdmError; use crate::routers::adm::settings::{AdmProfile, AdmSettings}; +use crate::routers::RouterError; use autopush_common::util::sec_since_epoch; use futures::lock::Mutex; use reqwest::StatusCode; @@ -61,7 +62,7 @@ impl AdmClient { } /// Get an ADM access token (from cache or request a new one) - async fn get_access_token(&self) -> Result { + async fn get_access_token(&self) -> Result { let mut token_info = self.token_info.lock().await; if token_info.expiration_time > sec_since_epoch() + 60 { @@ -81,21 +82,28 @@ impl AdmClient { "client_secret": &self.profile.client_secret, })) .send() - .await?; + .await + .map_err(AdmError::Http)?; trace!("response = {:?}", response); if response.status() != 200 { let status = response.status(); - let error_response: AdmResponseError = response.json().await?; - return Err(AdmError::Upstream { - status, - reason: error_response + let error_response: AdmResponseError = response + .json() + .await + .map_err(AdmError::DeserializeResponse)?; + return Err(RouterError::Upstream { + status: status.to_string(), + message: error_response .reason .unwrap_or_else(|| "Unknown reason".to_string()), }); } - let token_response: TokenResponse = response.json().await?; + let token_response: TokenResponse = response + .json() + .await + .map_err(AdmError::DeserializeResponse)?; token_info.token = token_response.access_token; token_info.expiration_time = sec_since_epoch() + token_response.expires_in; @@ -110,17 +118,20 @@ impl AdmClient { data: HashMap<&'static str, String>, registration_id: String, ttl: usize, - ) -> Result { + ) -> Result { // Build the ADM message let message = serde_json::json!({ "data": data, "expiresAfter": ttl, }); let access_token = self.get_access_token().await?; - let url = self.base_url.join(&format!( - "messaging/registrations/{}/messages", - registration_id - ))?; + let url = self + .base_url + .join(&format!( + "messaging/registrations/{}/messages", + registration_id + )) + .map_err(AdmError::ParseUrl)?; // Make the request let response = self @@ -142,9 +153,9 @@ impl AdmClient { .await .map_err(|e| { if e.is_timeout() { - AdmError::RequestTimeout + RouterError::RequestTimeout } else { - AdmError::Connect(e) + RouterError::Connect(e) } })?; @@ -157,16 +168,19 @@ impl AdmClient { .map_err(AdmError::DeserializeResponse)?; return Err(match (status, response_error.reason) { - (StatusCode::UNAUTHORIZED, _) => AdmError::Authentication, - (StatusCode::NOT_FOUND, _) => AdmError::NotFound, - (status, reason) => AdmError::Upstream { - status, - reason: reason.unwrap_or_else(|| "Unknown reason".to_string()), + (StatusCode::UNAUTHORIZED, _) => RouterError::Authentication, + (StatusCode::NOT_FOUND, _) => RouterError::NotFound, + (status, reason) => RouterError::Upstream { + status: status.to_string(), + message: reason.unwrap_or_else(|| "Unknown reason".to_string()), }, }); } - let send_response: AdmSendResponse = response.json().await?; + let send_response: AdmSendResponse = response + .json() + .await + .map_err(AdmError::DeserializeResponse)?; Ok(send_response.registration_id) } } @@ -174,8 +188,8 @@ impl AdmClient { #[cfg(test)] pub mod tests { use crate::routers::adm::client::AdmClient; - use crate::routers::adm::error::AdmError; use crate::routers::adm::settings::{AdmProfile, AdmSettings}; + use crate::routers::RouterError; use std::collections::HashMap; use url::Url; @@ -267,7 +281,7 @@ pub mod tests { .await; assert!(result.is_err()); assert!( - matches!(result.as_ref().unwrap_err(), AdmError::Authentication), + matches!(result.as_ref().unwrap_err(), RouterError::Authentication), "result = {:?}", result ); @@ -288,7 +302,7 @@ pub mod tests { .await; assert!(result.is_err()); assert!( - matches!(result.as_ref().unwrap_err(), AdmError::NotFound), + matches!(result.as_ref().unwrap_err(), RouterError::NotFound), "result = {:?}", result ); @@ -311,8 +325,8 @@ pub mod tests { assert!( matches!( result.as_ref().unwrap_err(), - AdmError::Upstream { status, reason } - if *status == 400 && reason == "test-message" + RouterError::Upstream { status, message } + if status == "400 Bad Request" && message == "test-message" ), "result = {:?}", result @@ -336,8 +350,8 @@ pub mod tests { assert!( matches!( result.as_ref().unwrap_err(), - AdmError::Upstream { status, reason } - if *status == 400 && reason == "Unknown reason" + RouterError::Upstream { status, message } + if status == "400 Bad Request" && message == "Unknown reason" ), "result = {:?}", result diff --git a/autoendpoint/src/routers/adm/error.rs b/autoendpoint/src/routers/adm/error.rs index 54e6c0728..7db3fd096 100644 --- a/autoendpoint/src/routers/adm/error.rs +++ b/autoendpoint/src/routers/adm/error.rs @@ -14,23 +14,17 @@ pub enum AdmError { #[error("Failed to decode the profile settings")] ProfileSettingsDecode(#[from] serde_json::Error), - #[error("Error while connecting to ADM")] - Connect(#[source] reqwest::Error), - #[error("Unable to deserialize ADM response")] DeserializeResponse(#[source] reqwest::Error), - #[error("ADM authentication error")] - Authentication, - - #[error("ADM recipient no longer available")] - NotFound, + #[error("No registration ID found for user")] + NoRegistrationId, - #[error("ADM error, {status}: {reason}")] - Upstream { status: StatusCode, reason: String }, + #[error("No ADM profile found for user")] + NoProfile, - #[error("ADM request timed out")] - RequestTimeout, + #[error("User has invalid ADM profile")] + InvalidProfile, } impl AdmError { @@ -39,36 +33,29 @@ impl AdmError { match self { AdmError::ParseUrl(_) => StatusCode::BAD_REQUEST, - AdmError::NotFound => StatusCode::GONE, + AdmError::NoRegistrationId | AdmError::NoProfile | AdmError::InvalidProfile => { + StatusCode::GONE + } AdmError::Http(_) | AdmError::ProfileSettingsDecode(_) => { StatusCode::INTERNAL_SERVER_ERROR } - AdmError::Connect(_) - | AdmError::DeserializeResponse(_) - | AdmError::Authentication - | AdmError::Upstream { .. } - | AdmError::RequestTimeout => StatusCode::BAD_GATEWAY, + AdmError::DeserializeResponse(_) => StatusCode::BAD_GATEWAY, } } /// Get the associated error number pub fn errno(&self) -> Option { match self { - AdmError::NotFound => Some(106), - - AdmError::Authentication => Some(901), - - AdmError::Connect(_) => Some(902), - - AdmError::RequestTimeout => Some(903), + AdmError::NoRegistrationId | AdmError::NoProfile | AdmError::InvalidProfile => { + Some(106) + } AdmError::Http(_) | AdmError::ParseUrl(_) | AdmError::ProfileSettingsDecode(_) - | AdmError::DeserializeResponse(_) - | AdmError::Upstream { .. } => None, + | AdmError::DeserializeResponse(_) => None, } } } diff --git a/autoendpoint/src/routers/adm/router.rs b/autoendpoint/src/routers/adm/router.rs index 8b1378917..69f1a8f3d 100644 --- a/autoendpoint/src/routers/adm/router.rs +++ b/autoendpoint/src/routers/adm/router.rs @@ -1 +1,134 @@ +use crate::db::client::DbClient; +use crate::error::ApiResult; +use crate::extractors::notification::Notification; +use crate::extractors::router_data_input::RouterDataInput; +use crate::routers::adm::client::AdmClient; +use crate::routers::adm::error::AdmError; +use crate::routers::adm::settings::AdmSettings; +use crate::routers::common::{build_message_data, handle_error, incr_success_metrics}; +use crate::routers::{Router, RouterError, RouterResponse}; +use async_trait::async_trait; +use cadence::StatsdClient; +use serde_json::Value; +use std::collections::HashMap; +use url::Url; +/// 31 days, specified by ADM +const MAX_TTL: usize = 2419200; + +/// Amazon Device Messaging router +pub struct AdmRouter { + settings: AdmSettings, + endpoint_url: Url, + metrics: StatsdClient, + ddb: Box, + /// A map from profile name to an authenticated ADM client + clients: HashMap, +} + +impl AdmRouter { + /// Create a new `AdmRouter` + pub async fn new( + settings: AdmSettings, + endpoint_url: Url, + http: reqwest::Client, + metrics: StatsdClient, + ddb: Box, + ) -> Result { + let profiles = settings.profiles()?; + let clients = profiles + .into_iter() + .map(|(name, profile)| (name, AdmClient::new(&settings, profile, http.clone()))) + .collect(); + + Ok(Self { + settings, + endpoint_url, + metrics, + ddb, + clients, + }) + } +} + +#[async_trait(?Send)] +impl Router for AdmRouter { + fn register( + &self, + router_input: &RouterDataInput, + app_id: &str, + ) -> Result, RouterError> { + if !self.clients.contains_key(app_id) { + return Err(AdmError::InvalidProfile.into()); + } + + let mut router_data = HashMap::new(); + router_data.insert( + "token".to_string(), + serde_json::to_value(&router_input.token).unwrap(), + ); + router_data.insert( + "creds".to_string(), + serde_json::json!({ "profile": app_id }), + ); + + Ok(router_data) + } + + async fn route_notification(&self, notification: &Notification) -> ApiResult { + debug!( + "Sending ADM notification to UAID {}", + notification.subscription.user.uaid + ); + trace!("Notification = {:?}", notification); + + let router_data = notification + .subscription + .user + .router_data + .as_ref() + .ok_or(AdmError::NoRegistrationId)?; + let registration_id = router_data + .get("token") + .and_then(Value::as_str) + .ok_or(AdmError::NoRegistrationId)?; + let profile = router_data + .get("creds") + .and_then(Value::as_object) + .and_then(|obj| obj.get("profile")) + .and_then(Value::as_str) + .ok_or(AdmError::NoProfile)?; + let ttl = MAX_TTL.min(self.settings.min_ttl.max(notification.headers.ttl as usize)); + let message_data = build_message_data(notification, self.settings.max_data)?; + + // Send the notification to ADM + let client = self.clients.get(profile).ok_or(AdmError::InvalidProfile)?; + trace!("Sending message to ADM: {:?}", message_data); + if let Err(e) = client + .send(message_data, registration_id.to_string(), ttl) + .await + { + return Err(handle_error( + e, + &self.metrics, + self.ddb.as_ref(), + "adm", + profile, + notification.subscription.user.uaid, + ) + .await); + } + + // Sent successfully, update metrics and make response + trace!("ADM request was successful"); + incr_success_metrics(&self.metrics, "adm", profile, notification); + + Ok(RouterResponse::success( + self.endpoint_url + .join(&format!("/m/{}", notification.message_id)) + .expect("Message ID is not URL-safe") + .to_string(), + notification.headers.ttl as usize, + )) + } +} diff --git a/autoendpoint/src/routers/apns/router.rs b/autoendpoint/src/routers/apns/router.rs index faf104fc3..1c1a5acc1 100644 --- a/autoendpoint/src/routers/apns/router.rs +++ b/autoendpoint/src/routers/apns/router.rs @@ -4,13 +4,13 @@ use crate::extractors::notification::Notification; use crate::extractors::router_data_input::RouterDataInput; use crate::routers::apns::error::ApnsError; use crate::routers::apns::settings::{ApnsChannel, ApnsSettings}; -use crate::routers::common::build_message_data; +use crate::routers::common::{build_message_data, incr_error_metric, incr_success_metrics}; use crate::routers::{Router, RouterError, RouterResponse}; use a2::request::notification::LocalizedAlert; use a2::request::payload::{APSAlert, Payload, APS}; use a2::{Endpoint, Error, NotificationOptions, Priority, Response}; use async_trait::async_trait; -use cadence::{Counted, StatsdClient}; +use cadence::StatsdClient; use futures::{StreamExt, TryStreamExt}; use serde::Deserialize; use serde_json::{Number, Value}; @@ -18,6 +18,7 @@ use std::collections::HashMap; use url::Url; use uuid::Uuid; +/// Apple Push Notification Service router pub struct ApnsRouter { /// A map from release channel to APNS client clients: HashMap, @@ -109,40 +110,13 @@ impl ApnsRouter { } } - /// Update metrics after successfully routing the notification - fn incr_success_metrics(&self, notification: &Notification, channel: &str) { - self.metrics - .incr_with_tags("notification.bridge.sent") - .with_tag("platform", "apns") - .with_tag("application", channel) - .send(); - self.metrics - .count_with_tags( - "notification.message_data", - notification.data.as_ref().map(String::len).unwrap_or(0) as i64, - ) - .with_tag("platform", "apns") - .with_tag("destination", "Direct") - .send(); - } - - /// Increment an error metric with some details - fn incr_error_metric(&self, reason: &str, channel: &str) { - self.metrics - .incr_with_tags("notification.bridge.connection.error") - .with_tag("platform", "apns") - .with_tag("application", channel) - .with_tag("reason", reason) - .send() - } - /// Handle an error by logging, updating metrics, etc async fn handle_error(&self, error: a2::Error, uaid: Uuid, channel: &str) -> ApiError { match &error { a2::Error::ResponseError(response) => { if response.code == 410 { debug!("APNS recipient has been unregistered, removing user"); - self.incr_error_metric("unregistered", channel); + incr_error_metric(&self.metrics, "apns", channel, "recipient_gone"); if let Err(e) = self.ddb.remove_user(uaid).await { warn!("Error while removing user due to APNS 410: {}", e); @@ -151,16 +125,16 @@ impl ApnsRouter { return ApiError::from(ApnsError::Unregistered); } else { warn!("APNS error: {:?}", response.error); - self.incr_error_metric("response_error", channel); + incr_error_metric(&self.metrics, "apns", channel, "server_error"); } } a2::Error::ConnectionError => { error!("APNS connection error"); - self.incr_error_metric("connection", channel); + incr_error_metric(&self.metrics, "apns", channel, "connection_unavailable"); } _ => { warn!("Unknown error while sending APNS request: {}", error); - self.incr_error_metric("unknown", channel); + incr_error_metric(&self.metrics, "apns", channel, "unknown"); } } @@ -291,7 +265,7 @@ impl Router for ApnsRouter { // Sent successfully, update metrics and make response trace!("APNS request was successful"); - self.incr_success_metrics(notification, channel); + incr_success_metrics(&self.metrics, "apns", channel, notification); Ok(RouterResponse::success( self.endpoint_url diff --git a/autoendpoint/src/routers/common.rs b/autoendpoint/src/routers/common.rs index 95bbf7080..b7394e913 100644 --- a/autoendpoint/src/routers/common.rs +++ b/autoendpoint/src/routers/common.rs @@ -1,8 +1,11 @@ -use crate::error::ApiResult; +use crate::db::client::DbClient; +use crate::error::{ApiError, ApiResult}; use crate::extractors::notification::Notification; use crate::routers::RouterError; use autopush_common::util::InsertOpt; +use cadence::{Counted, StatsdClient}; use std::collections::HashMap; +use uuid::Uuid; /// Convert a notification into a WebPush message pub fn build_message_data( @@ -30,6 +33,82 @@ pub fn build_message_data( Ok(message_data) } +/// Handle a bridge error by logging, updating metrics, etc +pub async fn handle_error( + error: RouterError, + metrics: &StatsdClient, + ddb: &dyn DbClient, + platform: &str, + app_id: &str, + uaid: Uuid, +) -> ApiError { + match &error { + RouterError::Authentication => { + error!("Bridge authentication error"); + incr_error_metric(metrics, platform, app_id, "authentication"); + } + RouterError::RequestTimeout => { + warn!("Bridge timeout"); + incr_error_metric(metrics, platform, app_id, "timeout"); + } + RouterError::Connect(e) => { + warn!("Bridge unavailable: {}", e); + incr_error_metric(metrics, platform, app_id, "connection_unavailable"); + } + RouterError::NotFound => { + debug!("Bridge recipient not found, removing user"); + incr_error_metric(metrics, platform, app_id, "recipient_gone"); + + if let Err(e) = ddb.remove_user(uaid).await { + warn!("Error while removing user due to bridge not_found: {}", e); + } + } + RouterError::Upstream { .. } => { + warn!("{}", error.to_string()); + incr_error_metric(metrics, platform, app_id, "server_error"); + } + _ => { + warn!("Unknown error while sending bridge request: {}", error); + incr_error_metric(metrics, platform, app_id, "unknown"); + } + } + + ApiError::from(error) +} + +/// Increment `notification.bridge.error` +pub fn incr_error_metric(metrics: &StatsdClient, platform: &str, app_id: &str, reason: &str) { + metrics + .incr_with_tags("notification.bridge.error") + .with_tag("platform", platform) + .with_tag("app_id", app_id) + .with_tag("reason", reason) + .send(); +} + +/// Update metrics after successfully routing the notification +pub fn incr_success_metrics( + metrics: &StatsdClient, + platform: &str, + app_id: &str, + notification: &Notification, +) { + metrics + .incr_with_tags("notification.bridge.sent") + .with_tag("platform", platform) + .with_tag("app_id", app_id) + .send(); + metrics + .count_with_tags( + "notification.message_data", + notification.data.as_ref().map(String::len).unwrap_or(0) as i64, + ) + .with_tag("platform", platform) + .with_tag("app_id", app_id) + .with_tag("destination", "Direct") + .send(); +} + /// Common router test code #[cfg(test)] pub mod tests { diff --git a/autoendpoint/src/routers/fcm/client.rs b/autoendpoint/src/routers/fcm/client.rs index 44b092115..842f0971a 100644 --- a/autoendpoint/src/routers/fcm/client.rs +++ b/autoendpoint/src/routers/fcm/client.rs @@ -1,5 +1,6 @@ use crate::routers::fcm::error::FcmError; use crate::routers::fcm::settings::{FcmCredential, FcmSettings}; +use crate::routers::RouterError; use reqwest::StatusCode; use serde::Deserialize; use std::collections::HashMap; @@ -51,7 +52,7 @@ impl FcmClient { data: HashMap<&'static str, String>, token: String, ttl: usize, - ) -> Result<(), FcmError> { + ) -> Result<(), RouterError> { // Build the FCM message let message = serde_json::json!({ "message": { @@ -62,7 +63,11 @@ impl FcmClient { } } }); - let access_token = self.auth.token(OAUTH_SCOPES).await?; + let access_token = self + .auth + .token(OAUTH_SCOPES) + .await + .map_err(FcmError::OAuthToken)?; // Make the request let response = self @@ -76,27 +81,31 @@ impl FcmClient { .await .map_err(|e| { if e.is_timeout() { - FcmError::FcmRequestTimeout + RouterError::RequestTimeout } else { - FcmError::FcmConnect(e) + RouterError::Connect(e) } })?; // Handle error - if let Err(e) = response.error_for_status_ref() { + let status = response.status(); + if status.is_client_error() || status.is_server_error() { let data: FcmResponse = response .json() .await .map_err(FcmError::DeserializeResponse)?; - return Err(match (e.status(), data.error) { - (Some(StatusCode::UNAUTHORIZED), _) => FcmError::FcmAuthentication, - (Some(StatusCode::NOT_FOUND), _) => FcmError::FcmNotFound, - (_, Some(error)) => FcmError::FcmUpstream { + return Err(match (status, data.error) { + (StatusCode::UNAUTHORIZED, _) => RouterError::Authentication, + (StatusCode::NOT_FOUND, _) => RouterError::NotFound, + (_, Some(error)) => RouterError::Upstream { status: error.status, message: error.message, }, - _ => FcmError::FcmUnknown, + (status, None) => RouterError::Upstream { + status: status.to_string(), + message: "Unknown reason".to_string(), + }, }); } @@ -118,8 +127,8 @@ struct FcmErrorResponse { #[cfg(test)] pub mod tests { use crate::routers::fcm::client::FcmClient; - use crate::routers::fcm::error::FcmError; use crate::routers::fcm::settings::{FcmCredential, FcmSettings}; + use crate::routers::RouterError; use std::collections::HashMap; use std::io::Write; use std::path::PathBuf; @@ -227,7 +236,7 @@ pub mod tests { .await; assert!(result.is_err()); assert!( - matches!(result.as_ref().unwrap_err(), FcmError::FcmAuthentication), + matches!(result.as_ref().unwrap_err(), RouterError::Authentication), "result = {:?}", result ); @@ -249,7 +258,7 @@ pub mod tests { .await; assert!(result.is_err()); assert!( - matches!(result.as_ref().unwrap_err(), FcmError::FcmNotFound), + matches!(result.as_ref().unwrap_err(), RouterError::NotFound), "result = {:?}", result ); @@ -273,7 +282,7 @@ pub mod tests { assert!( matches!( result.as_ref().unwrap_err(), - FcmError::FcmUpstream { status, message } + RouterError::Upstream { status, message } if status == "TEST_ERROR" && message == "test-message" ), "result = {:?}", @@ -297,7 +306,11 @@ pub mod tests { .await; assert!(result.is_err()); assert!( - matches!(result.as_ref().unwrap_err(), FcmError::FcmUnknown), + matches!( + result.as_ref().unwrap_err(), + RouterError::Upstream { status, message } + if status == "400 Bad Request" && message == "Unknown reason" + ), "result = {:?}", result ); diff --git a/autoendpoint/src/routers/fcm/error.rs b/autoendpoint/src/routers/fcm/error.rs index 73a500eab..8c2090df8 100644 --- a/autoendpoint/src/routers/fcm/error.rs +++ b/autoendpoint/src/routers/fcm/error.rs @@ -14,9 +14,6 @@ pub enum FcmError { #[error("Error while retrieving an OAuth token")] OAuthToken(#[from] yup_oauth2::Error), - #[error("Error while connecting to FCM")] - FcmConnect(#[source] reqwest::Error), - #[error("Unable to deserialize FCM response")] DeserializeResponse(#[source] reqwest::Error), @@ -28,65 +25,33 @@ pub enum FcmError { #[error("User has invalid app ID")] InvalidAppId, - - #[error("FCM authentication error")] - FcmAuthentication, - - #[error("FCM recipient no longer available")] - FcmNotFound, - - #[error("FCM request timed out")] - FcmRequestTimeout, - - #[error("FCM error, {status}: {message}")] - FcmUpstream { status: String, message: String }, - - #[error("Unknown FCM error")] - FcmUnknown, } impl FcmError { /// Get the associated HTTP status code pub fn status(&self) -> StatusCode { match self { - FcmError::NoRegistrationToken - | FcmError::NoAppId - | FcmError::InvalidAppId - | FcmError::FcmNotFound => StatusCode::GONE, + FcmError::NoRegistrationToken | FcmError::NoAppId | FcmError::InvalidAppId => { + StatusCode::GONE + } FcmError::CredentialDecode(_) | FcmError::OAuthClientBuild(_) | FcmError::OAuthToken(_) => StatusCode::INTERNAL_SERVER_ERROR, - FcmError::FcmConnect(_) - | FcmError::DeserializeResponse(_) - | FcmError::FcmAuthentication - | FcmError::FcmRequestTimeout - | FcmError::FcmUpstream { .. } - | FcmError::FcmUnknown => StatusCode::BAD_GATEWAY, + FcmError::DeserializeResponse(_) => StatusCode::BAD_GATEWAY, } } /// Get the associated error number pub fn errno(&self) -> Option { match self { - FcmError::NoRegistrationToken - | FcmError::NoAppId - | FcmError::InvalidAppId - | FcmError::FcmNotFound => Some(106), - - FcmError::FcmAuthentication => Some(901), - - FcmError::FcmConnect(_) => Some(902), - - FcmError::FcmRequestTimeout => Some(903), + FcmError::NoRegistrationToken | FcmError::NoAppId | FcmError::InvalidAppId => Some(106), FcmError::CredentialDecode(_) | FcmError::OAuthClientBuild(_) | FcmError::OAuthToken(_) - | FcmError::DeserializeResponse(_) - | FcmError::FcmUpstream { .. } - | FcmError::FcmUnknown => None, + | FcmError::DeserializeResponse(_) => None, } } } diff --git a/autoendpoint/src/routers/fcm/router.rs b/autoendpoint/src/routers/fcm/router.rs index 00d18c7a5..c36a76c3e 100644 --- a/autoendpoint/src/routers/fcm/router.rs +++ b/autoendpoint/src/routers/fcm/router.rs @@ -1,18 +1,17 @@ use crate::db::client::DbClient; -use crate::error::{ApiError, ApiResult}; +use crate::error::ApiResult; use crate::extractors::notification::Notification; use crate::extractors::router_data_input::RouterDataInput; -use crate::routers::common::build_message_data; +use crate::routers::common::{build_message_data, handle_error, incr_success_metrics}; use crate::routers::fcm::client::FcmClient; use crate::routers::fcm::error::FcmError; use crate::routers::fcm::settings::{FcmCredential, FcmSettings}; use crate::routers::{Router, RouterError, RouterResponse}; use async_trait::async_trait; -use cadence::{Counted, StatsdClient}; +use cadence::StatsdClient; use serde_json::Value; use std::collections::HashMap; use url::Url; -use uuid::Uuid; /// 28 days const MAX_TTL: usize = 28 * 24 * 60 * 60; @@ -67,67 +66,6 @@ impl FcmRouter { Ok(clients) } - - /// Handle an error by logging, updating metrics, etc - async fn handle_error(&self, error: FcmError, uaid: Uuid) -> ApiError { - match &error { - FcmError::FcmAuthentication => { - error!("FCM authentication error"); - self.incr_error_metric("authentication"); - } - FcmError::FcmRequestTimeout => { - warn!("FCM timeout"); - self.incr_error_metric("timeout"); - } - FcmError::FcmConnect(e) => { - warn!("FCM unavailable: {}", e); - self.incr_error_metric("connection_unavailable"); - } - FcmError::FcmNotFound => { - debug!("FCM recipient not found, removing user"); - self.incr_error_metric("recipient_gone"); - - if let Err(e) = self.ddb.remove_user(uaid).await { - warn!("Error while removing user due to FCM 404: {}", e); - } - } - FcmError::FcmUpstream { .. } | FcmError::FcmUnknown => { - warn!("{}", error.to_string()); - self.incr_error_metric("server_error"); - } - _ => { - warn!("Unknown error while sending FCM request: {}", error); - self.incr_error_metric("unknown"); - } - } - - ApiError::from(error) - } - - /// Update metrics after successfully routing the notification - fn incr_success_metrics(&self, notification: &Notification) { - self.metrics - .incr_with_tags("notification.bridge.sent") - .with_tag("platform", "fcmv1") - .send(); - self.metrics - .count_with_tags( - "notification.message_data", - notification.data.as_ref().map(String::len).unwrap_or(0) as i64, - ) - .with_tag("platform", "fcmv1") - .with_tag("destination", "Direct") - .send(); - } - - /// Increment `notification.bridge.error` with a reason - fn incr_error_metric(&self, reason: &str) { - self.metrics - .incr_with_tags("notification.bridge.error") - .with_tag("platform", "fcmv1") - .with_tag("reason", reason) - .send(); - } } #[async_trait(?Send)] @@ -179,14 +117,20 @@ impl Router for FcmRouter { let client = self.clients.get(app_id).ok_or(FcmError::InvalidAppId)?; trace!("Sending message to FCM: {:?}", message_data); if let Err(e) = client.send(message_data, fcm_token.to_string(), ttl).await { - return Err(self - .handle_error(e, notification.subscription.user.uaid) - .await); + return Err(handle_error( + e, + &self.metrics, + self.ddb.as_ref(), + "fcmv1", + app_id, + notification.subscription.user.uaid, + ) + .await); } // Sent successfully, update metrics and make response trace!("FCM request was successful"); - self.incr_success_metrics(notification); + incr_success_metrics(&self.metrics, "fcmv1", app_id, notification); Ok(RouterResponse::success( self.endpoint_url @@ -381,7 +325,7 @@ mod tests { assert!( matches!( result.as_ref().unwrap_err().kind, - ApiErrorKind::Router(RouterError::Fcm(FcmError::FcmNotFound)) + ApiErrorKind::Router(RouterError::NotFound) ), "result = {:?}", result diff --git a/autoendpoint/src/routers/mod.rs b/autoendpoint/src/routers/mod.rs index 6488eeae9..1cc5900bc 100644 --- a/autoendpoint/src/routers/mod.rs +++ b/autoendpoint/src/routers/mod.rs @@ -91,6 +91,21 @@ pub enum RouterError { size. Converted buffer is too long by {0} bytes" )] TooMuchData(usize), + + #[error("Bridge authentication error")] + Authentication, + + #[error("Bridge request timeout")] + RequestTimeout, + + #[error("Error while connecting to bridge service")] + Connect(#[source] reqwest::Error), + + #[error("Bridge reports user was not found")] + NotFound, + + #[error("Bridge error, {status}: {message}")] + Upstream { status: String, message: String }, } impl RouterError { @@ -100,9 +115,17 @@ impl RouterError { RouterError::Adm(e) => e.status(), RouterError::Apns(e) => e.status(), RouterError::Fcm(e) => e.status(), + RouterError::SaveDb(_) => StatusCode::SERVICE_UNAVAILABLE, - RouterError::UserWasDeleted => StatusCode::GONE, + + RouterError::UserWasDeleted | RouterError::NotFound => StatusCode::GONE, + RouterError::TooMuchData(_) => StatusCode::PAYLOAD_TOO_LARGE, + + RouterError::Authentication + | RouterError::RequestTimeout + | RouterError::Connect(_) + | RouterError::Upstream { .. } => StatusCode::BAD_GATEWAY, } } @@ -112,9 +135,22 @@ impl RouterError { RouterError::Adm(e) => e.errno(), RouterError::Apns(e) => e.errno(), RouterError::Fcm(e) => e.errno(), + RouterError::TooMuchData(_) => Some(104), - RouterError::SaveDb(_) => Some(201), + RouterError::UserWasDeleted => Some(105), + + RouterError::NotFound => Some(106), + + RouterError::SaveDb(_) => Some(201), + + RouterError::Authentication => Some(901), + + RouterError::Connect(_) => Some(902), + + RouterError::RequestTimeout => Some(903), + + RouterError::Upstream { .. } => None, } } } From 1f2e1033fa37538f68d4593f9a9c3f8cc004383e Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 11:25:54 -0400 Subject: [PATCH 05/11] Remove unnecessary Result return when initializing logging --- autoendpoint/src/logging.rs | 5 +---- autoendpoint/src/main.rs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/autoendpoint/src/logging.rs b/autoendpoint/src/logging.rs index f61bf48c6..1e02b45c2 100644 --- a/autoendpoint/src/logging.rs +++ b/autoendpoint/src/logging.rs @@ -1,12 +1,10 @@ use std::io; -use crate::error::ApiResult; - use slog::{self, slog_o, Drain}; use slog_mozlog_json::MozLogJson; // TODO: Merge back into common code? Removes hostname and adds envlogger -pub fn init_logging(json: bool) -> ApiResult<()> { +pub fn init_logging(json: bool) { let logger = if json { let drain = MozLogJson::new(io::stdout()) .logger_name(format!( @@ -33,7 +31,6 @@ pub fn init_logging(json: bool) -> ApiResult<()> { // https://github.com/slog-rs/slog/issues/169 slog_scope::set_global_logger(logger).cancel_reset(); slog_stdlog::init().ok(); - Ok(()) } pub fn reset_logging() { diff --git a/autoendpoint/src/main.rs b/autoendpoint/src/main.rs index f7bb04e1c..e4a56043f 100644 --- a/autoendpoint/src/main.rs +++ b/autoendpoint/src/main.rs @@ -41,7 +41,7 @@ async fn main() -> Result<(), Box> { .and_then(|d| d.deserialize()) .unwrap_or_else(|e| e.exit()); let settings = settings::Settings::with_env_and_config_file(&args.flag_config)?; - logging::init_logging(!settings.human_logs).expect("Logging failed to initialize"); + logging::init_logging(!settings.human_logs); debug!("Starting up..."); // Configure sentry error capture From e3bfa16b5808f18bfa78e2e9a5f2e21ab2f1f03d Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 11:40:45 -0400 Subject: [PATCH 06/11] Handle updating ADM registration ID and add router tests --- autoendpoint/src/routers/adm/client.rs | 7 +- autoendpoint/src/routers/adm/router.rs | 243 +++++++++++++++++++++++-- 2 files changed, 236 insertions(+), 14 deletions(-) diff --git a/autoendpoint/src/routers/adm/client.rs b/autoendpoint/src/routers/adm/client.rs index d84425cff..a3fac96b0 100644 --- a/autoendpoint/src/routers/adm/client.rs +++ b/autoendpoint/src/routers/adm/client.rs @@ -193,10 +193,10 @@ pub mod tests { use std::collections::HashMap; use url::Url; - const REGISTRATION_ID: &str = "test-registration-id"; + pub const REGISTRATION_ID: &str = "test-registration-id"; + pub const CLIENT_ID: &str = "test-client-id"; + pub const CLIENT_SECRET: &str = "test-client-secret"; const ACCESS_TOKEN: &str = "test-access-token"; - const CLIENT_ID: &str = "test-client-id"; - const CLIENT_SECRET: &str = "test-client-secret"; /// Mock the OAuth token endpoint to provide the access token pub fn mock_token_endpoint() -> mockito::Mock { @@ -263,6 +263,7 @@ pub mod tests { let result = client.send(data, REGISTRATION_ID.to_string(), 42).await; assert!(result.is_ok(), "result = {:?}", result); + assert_eq!(result.unwrap(), "test-registration-id2"); adm_mock.assert(); } diff --git a/autoendpoint/src/routers/adm/router.rs b/autoendpoint/src/routers/adm/router.rs index 69f1a8f3d..e7c2ed847 100644 --- a/autoendpoint/src/routers/adm/router.rs +++ b/autoendpoint/src/routers/adm/router.rs @@ -28,7 +28,7 @@ pub struct AdmRouter { impl AdmRouter { /// Create a new `AdmRouter` - pub async fn new( + pub fn new( settings: AdmSettings, endpoint_url: Url, http: reqwest::Client, @@ -104,19 +104,39 @@ impl Router for AdmRouter { // Send the notification to ADM let client = self.clients.get(profile).ok_or(AdmError::InvalidProfile)?; trace!("Sending message to ADM: {:?}", message_data); - if let Err(e) = client + + let new_registration_id = match client .send(message_data, registration_id.to_string(), ttl) .await { - return Err(handle_error( - e, - &self.metrics, - self.ddb.as_ref(), - "adm", - profile, - notification.subscription.user.uaid, - ) - .await); + Ok(id) => id, + Err(e) => { + return Err(handle_error( + e, + &self.metrics, + self.ddb.as_ref(), + "adm", + profile, + notification.subscription.user.uaid, + ) + .await) + } + }; + + // If the returned registration ID is different than the old one, update + // the user. + if new_registration_id != registration_id { + trace!("ADM reports a new registration ID for user, updating our copy"); + let mut user = notification.subscription.user.clone(); + let mut router_data = router_data.clone(); + + router_data.insert( + "token".to_string(), + serde_json::to_value(&new_registration_id).unwrap(), + ); + user.router_data = Some(router_data); + + self.ddb.update_user(&user).await?; } // Sent successfully, update metrics and make response @@ -132,3 +152,204 @@ impl Router for AdmRouter { )) } } + +#[cfg(test)] +mod tests { + use crate::db::client::DbClient; + use crate::db::mock::MockDbClient; + use crate::error::ApiErrorKind; + use crate::extractors::routers::RouterType; + use crate::routers::adm::client::tests::{ + mock_adm_endpoint_builder, mock_token_endpoint, CLIENT_ID, CLIENT_SECRET, REGISTRATION_ID, + }; + use crate::routers::adm::error::AdmError; + use crate::routers::adm::router::AdmRouter; + use crate::routers::adm::settings::AdmSettings; + use crate::routers::common::tests::{make_notification, CHANNEL_ID}; + use crate::routers::RouterError; + use crate::routers::{Router, RouterResponse}; + use autopush_common::db::DynamoDbUser; + use cadence::StatsdClient; + use mockall::predicate; + use serde_json::Value; + use std::collections::HashMap; + use url::Url; + + /// Create a router for testing + fn make_router(ddb: Box) -> AdmRouter { + AdmRouter::new( + AdmSettings { + base_url: Url::parse(&mockito::server_url()).unwrap(), + profiles: format!( + r#"{{ "dev": {{ "client_id": "{}", "client_secret": "{}" }} }}"#, + CLIENT_ID, CLIENT_SECRET + ), + ..Default::default() + }, + Url::parse("http://localhost:8080/").unwrap(), + reqwest::Client::new(), + StatsdClient::from_sink("autopush", cadence::NopMetricSink), + ddb, + ) + .unwrap() + } + + /// Create default user router data + fn default_router_data() -> HashMap { + let mut map = HashMap::new(); + map.insert( + "token".to_string(), + serde_json::to_value(REGISTRATION_ID).unwrap(), + ); + map.insert("creds".to_string(), serde_json::json!({ "profile": "dev" })); + map + } + + /// A notification with no data is sent to ADM + #[tokio::test] + async fn successful_routing_no_data() { + let ddb = MockDbClient::new().into_boxed_arc(); + let router = make_router(ddb); + let _token_mock = mock_token_endpoint(); + let adm_mock = mock_adm_endpoint_builder() + .match_body( + serde_json::json!({ + "data": { "chid": CHANNEL_ID }, + "expiresAfter": 60 + }) + .to_string() + .as_str(), + ) + .with_body(r#"{"registrationID":"test-registration-id"}"#) + .create(); + let notification = make_notification(default_router_data(), None, RouterType::ADM); + + let result = router.route_notification(¬ification).await; + assert!(result.is_ok(), "result = {:?}", result); + assert_eq!( + result.unwrap(), + RouterResponse::success("http://localhost:8080/m/test-message-id".to_string(), 0) + ); + adm_mock.assert(); + } + + /// A notification with data is sent to ADM + #[tokio::test] + async fn successful_routing_with_data() { + let ddb = MockDbClient::new().into_boxed_arc(); + let router = make_router(ddb); + let _token_mock = mock_token_endpoint(); + let adm_mock = mock_adm_endpoint_builder() + .match_body( + serde_json::json!({ + "data": { + "chid": CHANNEL_ID, + "body": "test-data", + "con": "test-encoding", + "enc": "test-encryption", + "cryptokey": "test-crypto-key", + "enckey": "test-encryption-key" + }, + "expiresAfter": 60 + }) + .to_string() + .as_str(), + ) + .with_body(r#"{"registrationID":"test-registration-id"}"#) + .create(); + let data = "test-data".to_string(); + let notification = make_notification(default_router_data(), Some(data), RouterType::ADM); + + let result = router.route_notification(¬ification).await; + assert!(result.is_ok(), "result = {:?}", result); + assert_eq!( + result.unwrap(), + RouterResponse::success("http://localhost:8080/m/test-message-id".to_string(), 0) + ); + adm_mock.assert(); + } + + /// If there is no client for the user's profile, an error is returned and + /// the ADM request is not sent. + #[tokio::test] + async fn missing_client() { + let ddb = MockDbClient::new().into_boxed_arc(); + let router = make_router(ddb); + let _token_mock = mock_token_endpoint(); + let adm_mock = mock_adm_endpoint_builder().expect(0).create(); + let mut router_data = default_router_data(); + router_data.insert( + "creds".to_string(), + serde_json::json!({ "profile": "unknown-profile" }), + ); + let notification = make_notification(router_data, None, RouterType::ADM); + + let result = router.route_notification(¬ification).await; + assert!(result.is_err()); + assert!( + matches!( + result.as_ref().unwrap_err().kind, + ApiErrorKind::Router(RouterError::Adm(AdmError::InvalidProfile)) + ), + "result = {:?}", + result + ); + adm_mock.assert(); + } + + /// If the ADM user no longer exists (404), we drop the user from our database + #[tokio::test] + async fn no_adm_user() { + let notification = make_notification(default_router_data(), None, RouterType::ADM); + let mut ddb = MockDbClient::new(); + ddb.expect_remove_user() + .with(predicate::eq(notification.subscription.user.uaid)) + .times(1) + .return_once(|_| Ok(())); + + let router = make_router(ddb.into_boxed_arc()); + let _token_mock = mock_token_endpoint(); + let _adm_mock = mock_adm_endpoint_builder() + .with_status(404) + .with_body(r#"{"reason":"test-message"}"#) + .create(); + + let result = router.route_notification(¬ification).await; + assert!(result.is_err()); + assert!( + matches!( + result.as_ref().unwrap_err().kind, + ApiErrorKind::Router(RouterError::NotFound) + ), + "result = {:?}", + result + ); + } + + /// If ADM returns a new registration token, update our copy to match + #[tokio::test] + async fn update_registration_token() { + let notification = make_notification(default_router_data(), None, RouterType::ADM); + let mut ddb = MockDbClient::new(); + ddb.expect_update_user() + .withf(|user: &DynamoDbUser| { + user.router_data + .as_ref() + .unwrap() + .get("token") + .and_then(Value::as_str) + == Some("test-registration-id2") + }) + .times(1) + .return_once(|_| Ok(())); + + let router = make_router(ddb.into_boxed_arc()); + let _token_mock = mock_token_endpoint(); + let _adm_mock = mock_adm_endpoint_builder() + .with_body(r#"{"registrationID":"test-registration-id2"}"#) + .create(); + + let result = router.route_notification(¬ification).await; + assert!(result.is_ok()); + } +} From 35d6f8a206db3d7de9a99eaa51ddbb62d0ddde89 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 11:46:12 -0400 Subject: [PATCH 07/11] Hook up the ADM router to the server --- autoendpoint/src/extractors/routers.rs | 5 ++++- autoendpoint/src/server.rs | 10 ++++++++++ autoendpoint/src/settings.rs | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/autoendpoint/src/extractors/routers.rs b/autoendpoint/src/extractors/routers.rs index 7e3bd9564..c257f4fcc 100644 --- a/autoendpoint/src/extractors/routers.rs +++ b/autoendpoint/src/extractors/routers.rs @@ -1,3 +1,4 @@ +use crate::routers::adm::router::AdmRouter; use crate::routers::apns::router::ApnsRouter; use crate::routers::fcm::router::FcmRouter; use crate::routers::webpush::WebPushRouter; @@ -51,6 +52,7 @@ pub struct Routers { webpush: WebPushRouter, fcm: Arc, apns: Arc, + adm: Arc, } impl FromRequest for Routers { @@ -72,6 +74,7 @@ impl FromRequest for Routers { }, fcm: state.fcm_router.clone(), apns: state.apns_router.clone(), + adm: state.adm_router.clone(), }) } } @@ -83,7 +86,7 @@ impl Routers { RouterType::WebPush => &self.webpush, RouterType::FCM => self.fcm.as_ref(), RouterType::APNS => self.apns.as_ref(), - RouterType::ADM => unimplemented!(), + RouterType::ADM => self.adm.as_ref(), } } } diff --git a/autoendpoint/src/server.rs b/autoendpoint/src/server.rs index 2fedbe5f5..0c5d2f270 100644 --- a/autoendpoint/src/server.rs +++ b/autoendpoint/src/server.rs @@ -4,6 +4,7 @@ use crate::db::client::{DbClient, DbClientImpl}; use crate::error::{ApiError, ApiResult}; use crate::metrics; use crate::middleware::sentry::sentry_middleware; +use crate::routers::adm::router::AdmRouter; use crate::routers::apns::router::ApnsRouter; use crate::routers::fcm::router::FcmRouter; use crate::routes::health::{health_route, lb_heartbeat_route, status_route, version_route}; @@ -32,6 +33,7 @@ pub struct ServerState { pub http: reqwest::Client, pub fcm_router: Arc, pub apns_router: Arc, + pub adm_router: Arc, } pub struct Server; @@ -66,6 +68,13 @@ impl Server { ) .await?, ); + let adm_router = Arc::new(AdmRouter::new( + settings.adm.clone(), + settings.endpoint_url.clone(), + http.clone(), + metrics.clone(), + ddb.clone(), + )?); let state = ServerState { metrics, settings, @@ -74,6 +83,7 @@ impl Server { http, fcm_router, apns_router, + adm_router, }; let server = HttpServer::new(move || { diff --git a/autoendpoint/src/settings.rs b/autoendpoint/src/settings.rs index 17260e1c0..cf4bed79d 100644 --- a/autoendpoint/src/settings.rs +++ b/autoendpoint/src/settings.rs @@ -1,5 +1,6 @@ //! Application settings +use crate::routers::adm::settings::AdmSettings; use crate::routers::apns::settings::ApnsSettings; use crate::routers::fcm::settings::FcmSettings; use config::{Config, ConfigError, Environment, File}; @@ -33,6 +34,7 @@ pub struct Settings { pub fcm: FcmSettings, pub apns: ApnsSettings, + pub adm: AdmSettings, } impl Default for Settings { @@ -53,6 +55,7 @@ impl Default for Settings { statsd_label: "autoendpoint".to_string(), fcm: FcmSettings::default(), apns: ApnsSettings::default(), + adm: AdmSettings::default(), } } } From ee7e1d004abc72c00af5ea938b993d00c4304b7c Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 4 Aug 2020 13:49:55 -0400 Subject: [PATCH 08/11] Record the successful message delivery before updating registration ID --- autoendpoint/src/routers/adm/router.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/autoendpoint/src/routers/adm/router.rs b/autoendpoint/src/routers/adm/router.rs index e7c2ed847..25c6cb068 100644 --- a/autoendpoint/src/routers/adm/router.rs +++ b/autoendpoint/src/routers/adm/router.rs @@ -104,7 +104,6 @@ impl Router for AdmRouter { // Send the notification to ADM let client = self.clients.get(profile).ok_or(AdmError::InvalidProfile)?; trace!("Sending message to ADM: {:?}", message_data); - let new_registration_id = match client .send(message_data, registration_id.to_string(), ttl) .await @@ -123,6 +122,10 @@ impl Router for AdmRouter { } }; + // Sent successfully, update metrics and make response + trace!("ADM request was successful"); + incr_success_metrics(&self.metrics, "adm", profile, notification); + // If the returned registration ID is different than the old one, update // the user. if new_registration_id != registration_id { @@ -139,10 +142,6 @@ impl Router for AdmRouter { self.ddb.update_user(&user).await?; } - // Sent successfully, update metrics and make response - trace!("ADM request was successful"); - incr_success_metrics(&self.metrics, "adm", profile, notification); - Ok(RouterResponse::success( self.endpoint_url .join(&format!("/m/{}", notification.message_id)) From e46608055a1c2b90dc056afde6ecddebedea2ad1 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Wed, 5 Aug 2020 09:39:36 -0400 Subject: [PATCH 09/11] Use Default for ADM client's TokenInfo --- autoendpoint/src/routers/adm/client.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/autoendpoint/src/routers/adm/client.rs b/autoendpoint/src/routers/adm/client.rs index a3fac96b0..581a2ff06 100644 --- a/autoendpoint/src/routers/adm/client.rs +++ b/autoendpoint/src/routers/adm/client.rs @@ -20,6 +20,7 @@ pub struct AdmClient { } /// Holds information about the cached access token +#[derive(Default)] struct TokenInfo { token: String, expiration_time: u64, @@ -53,11 +54,8 @@ impl AdmClient { profile, timeout: Duration::from_secs(settings.timeout as u64), http, - token_info: Mutex::new(TokenInfo { - // Dummy values to trigger a token fetch - token: "".to_string(), - expiration_time: 0, - }), + // The default TokenInfo has dummy values to trigger a token fetch + token_info: Mutex::default(), } } From 8b160e45e8ad9ba930df5f541de7bad9190c9b22 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Wed, 5 Aug 2020 09:45:21 -0400 Subject: [PATCH 10/11] Handle potential error in init_logging --- Cargo.lock | 1 + autoendpoint/Cargo.toml | 1 + autoendpoint/src/logging.rs | 4 ++-- autoendpoint/src/main.rs | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1cd2ae43e..d81f90bc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -380,6 +380,7 @@ dependencies = [ "hex 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "jsonwebtoken 7.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "mockall 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "mockito 0.26.0 (registry+https://github.com/rust-lang/crates.io-index)", "openssl 0.10.29 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/autoendpoint/Cargo.toml b/autoendpoint/Cargo.toml index 4f9e56c7a..28873be13 100644 --- a/autoendpoint/Cargo.toml +++ b/autoendpoint/Cargo.toml @@ -28,6 +28,7 @@ futures = "0.3" hex = "0.4.2" jsonwebtoken = "7.1.1" lazy_static = "1.4.0" +log = "0.4" openssl = "0.10" regex = "1.3" reqwest = "0.10.6" diff --git a/autoendpoint/src/logging.rs b/autoendpoint/src/logging.rs index 1e02b45c2..8a67b96f4 100644 --- a/autoendpoint/src/logging.rs +++ b/autoendpoint/src/logging.rs @@ -4,7 +4,7 @@ use slog::{self, slog_o, Drain}; use slog_mozlog_json::MozLogJson; // TODO: Merge back into common code? Removes hostname and adds envlogger -pub fn init_logging(json: bool) { +pub fn init_logging(json: bool) -> Result<(), log::SetLoggerError> { let logger = if json { let drain = MozLogJson::new(io::stdout()) .logger_name(format!( @@ -30,7 +30,7 @@ pub fn init_logging(json: bool) { // the global logger during shutdown anyway: // https://github.com/slog-rs/slog/issues/169 slog_scope::set_global_logger(logger).cancel_reset(); - slog_stdlog::init().ok(); + slog_stdlog::init() } pub fn reset_logging() { diff --git a/autoendpoint/src/main.rs b/autoendpoint/src/main.rs index e4a56043f..f7bb04e1c 100644 --- a/autoendpoint/src/main.rs +++ b/autoendpoint/src/main.rs @@ -41,7 +41,7 @@ async fn main() -> Result<(), Box> { .and_then(|d| d.deserialize()) .unwrap_or_else(|e| e.exit()); let settings = settings::Settings::with_env_and_config_file(&args.flag_config)?; - logging::init_logging(!settings.human_logs); + logging::init_logging(!settings.human_logs).expect("Logging failed to initialize"); debug!("Starting up..."); // Configure sentry error capture From 102715f54d9568b362e9cf8dae9ac080246da103 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Wed, 5 Aug 2020 13:30:28 -0400 Subject: [PATCH 11/11] Fix error after merge --- autoendpoint/src/server.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/autoendpoint/src/server.rs b/autoendpoint/src/server.rs index cda2cdf07..3fef2d107 100644 --- a/autoendpoint/src/server.rs +++ b/autoendpoint/src/server.rs @@ -63,7 +63,7 @@ impl Server { let apns_router = Arc::new( ApnsRouter::new( settings.apns.clone(), - endpoint_url, + endpoint_url.clone(), metrics.clone(), ddb.clone(), ) @@ -71,7 +71,7 @@ impl Server { ); let adm_router = Arc::new(AdmRouter::new( settings.adm.clone(), - settings.endpoint_url.clone(), + endpoint_url, http.clone(), metrics.clone(), ddb.clone(),