Skip to content

Commit

Permalink
feat: Route notifications to FCM (Android) (#171)
Browse files Browse the repository at this point in the history
* Move RouterType to extractors/routers.rs

* Remove GCM as a router type

Google Cloud Messaging has been deprecated in favor of FCM, and as of
May 29, 2019 it no longer functions.

* Add FCM router stub

* Remove some unnecessary FCM settings

* Implement insert_opt on HashMap for more concise code

Also renamed the Content-Encoding header field to "encoding", which is
what the rest of autopush/webpush expects.

* Implement (most of) FcmRouter

* Implement FcmClient

* Differentiate between a connection error and a timeout in FcmClient

* Log and increment metrics in FcmRouter when a FcmClient error occurs

* Add RouterResponse::success to avoid code duplication between routers

* Clarify how long MAX_TTL is

* Move the base FCM url to settings

This will make testing easier, for example with mockito.

* Error if unknown settings are provided

* Add tests to FcmClient

* Add .gitguardian.yml

* Add some logging to FcmRouter

* Fix returning an invalid URL after routing a notification

* Add tests to FcmRouter

* Make router_data optional

This fixes the integration tests, because sometimes users would not have
any router data in DynamoDB. Also, just to be safe, router_data uses
rusoto's AttributeValue instead of serde_json::Value.

* Switch router_data back to serde_json::Value

This allows us to use DynamoDbUser with any version of rusoto.

* Make FCM request timeout a config option

* Use a more obvious UUID when testing

Closes #162
  • Loading branch information
AzureMarker committed Jul 20, 2020
1 parent 2d4d08f commit d9a0d9d
Show file tree
Hide file tree
Showing 21 changed files with 1,377 additions and 242 deletions.
3 changes: 3 additions & 0 deletions .gitguardian.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
paths-ignore:
# This is a fake key to make the OAuth client happy.
- "autoendpoint/src/server/routers/fcm/client.rs"
548 changes: 377 additions & 171 deletions Cargo.lock

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions autoendpoint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,9 @@ url = "2.1"
uuid = { version = "0.8.1", features = ["serde", "v4"] }
validator = "0.10.0"
validator_derive = "0.10.0"
yup-oauth2 = "4.1.2"

[dev-dependencies]
mockito = "0.26.0"
tempfile = "3.1.0"
tokio = { version = "0.2.12", features = ["macros"] }
4 changes: 3 additions & 1 deletion autoendpoint/docs/operation_notes.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Operation Notes / Runbook (WIP)
- Logging level can be controlled via the `RUST_LOG` env variable on a per-module
basis: https://docs.rs/slog-envlogger/2.2.0/slog_envlogger/
basis: https://docs.rs/slog-envlogger/2.2.0/slog_envlogger/
- Nested settings (ex. `FcmSettings`) can be set with environment variables. For
example, to set `settings.fcm.ttl`, use `AUTOEND_FCM.TTL=60`
2 changes: 1 addition & 1 deletion autoendpoint/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl ApiErrorKind {
/// Get the associated error number
pub fn errno(&self) -> Option<usize> {
match self {
ApiErrorKind::Router(e) => Some(e.errno()),
ApiErrorKind::Router(e) => e.errno(),

ApiErrorKind::InvalidToken => Some(102),

Expand Down
4 changes: 3 additions & 1 deletion autoendpoint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ async fn main() -> Result<(), Box<dyn Error>> {
let _sentry_guard = configure_sentry();

// Run server...
let server = server::Server::with_settings(settings).expect("Could not start server");
let server = server::Server::with_settings(settings)
.await
.expect("Could not start server");
info!("Server started");
server.await?;

Expand Down
23 changes: 7 additions & 16 deletions autoendpoint/src/server/extractors/notification_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::server::headers::crypto_key::CryptoKeyHeader;
use crate::server::headers::util::{get_header, get_owned_header};
use actix_web::HttpRequest;
use autopush_common::util::InsertOpt;
use lazy_static::lazy_static;
use regex::Regex;
use std::cmp::min;
Expand Down Expand Up @@ -49,21 +50,11 @@ impl From<NotificationHeaders> for HashMap<String, String> {
let mut map = HashMap::new();

map.insert("ttl".to_string(), headers.ttl.to_string());
if let Some(h) = headers.topic {
map.insert("topic".to_string(), h);
}
if let Some(h) = headers.encoding {
map.insert("encoding".to_string(), h);
}
if let Some(h) = headers.encryption {
map.insert("encryption".to_string(), h);
}
if let Some(h) = headers.encryption_key {
map.insert("encryption_key".to_string(), h);
}
if let Some(h) = headers.crypto_key {
map.insert("crypto_key".to_string(), h);
}
map.insert_opt("topic", headers.topic);
map.insert_opt("encoding", headers.encoding);
map.insert_opt("encryption", headers.encryption);
map.insert_opt("encryption_key", headers.encryption_key);
map.insert_opt("crypto_key", headers.crypto_key);

map
}
Expand Down Expand Up @@ -250,7 +241,7 @@ mod tests {
assert!(result.is_err());
let error = match result.unwrap_err().kind {
ApiErrorKind::InvalidEncryption(error) => error,
_ => panic!("Expected an ecryption error"),
_ => panic!("Expected an encryption error"),
};

assert_eq!(error, expected_error);
Expand Down
34 changes: 30 additions & 4 deletions autoendpoint/src/server/extractors/routers.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,42 @@
use crate::server::extractors::user::RouterType;
use crate::server::routers::fcm::router::FcmRouter;
use crate::server::routers::webpush::WebPushRouter;
use crate::server::routers::Router;
use crate::server::ServerState;
use actix_web::dev::{Payload, PayloadStream};
use actix_web::web::Data;
use actix_web::{FromRequest, HttpRequest};
use futures::future;
use std::str::FromStr;
use std::sync::Arc;

/// Valid `DynamoDbUser::router_type` values
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum RouterType {
WebPush,
FCM,
APNS,
ADM,
}

impl FromStr for RouterType {
type Err = ();

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"webpush" => Ok(RouterType::WebPush),
"fcm" => Ok(RouterType::FCM),
"apns" => Ok(RouterType::APNS),
"adm" => Ok(RouterType::ADM),
_ => Err(()),
}
}
}

/// Holds the various notification routers. The routers use resources from the
/// server state, which is why `Routers` is an extractor.
pub struct Routers {
pub webpush: WebPushRouter,
webpush: WebPushRouter,
fcm: Arc<FcmRouter>,
}

impl FromRequest for Routers {
Expand All @@ -30,6 +56,7 @@ impl FromRequest for Routers {
http: state.http.clone(),
endpoint_url: state.settings.endpoint_url.clone(),
},
fcm: state.fcm_router.clone(),
})
}
}
Expand All @@ -39,8 +66,7 @@ impl Routers {
pub fn get(&self, router_type: RouterType) -> &dyn Router {
match router_type {
RouterType::WebPush => &self.webpush,
RouterType::GCM => unimplemented!(),
RouterType::FCM => unimplemented!(),
RouterType::FCM => self.fcm.as_ref(),
RouterType::APNS => unimplemented!(),
RouterType::ADM => unimplemented!(),
}
Expand Down
3 changes: 2 additions & 1 deletion autoendpoint/src/server/extractors/subscription.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::server::extractors::routers::RouterType;
use crate::server::extractors::token_info::{ApiVersion, TokenInfo};
use crate::server::extractors::user::{validate_user, RouterType};
use crate::server::extractors::user::validate_user;
use crate::server::headers::crypto_key::CryptoKeyHeader;
use crate::server::headers::vapid::{VapidHeader, VapidHeaderWithKey, VapidVersionData};
use crate::server::{ServerState, VapidError};
Expand Down
27 changes: 1 addition & 26 deletions autoendpoint/src/server/extractors/user.rs
Original file line number Diff line number Diff line change
@@ -1,38 +1,13 @@
//! User validations

use crate::error::{ApiErrorKind, ApiResult};
use crate::server::extractors::routers::RouterType;
use crate::server::ServerState;
use autopush_common::db::{DynamoDbUser, DynamoStorage};
use cadence::{Counted, StatsdClient};
use futures::compat::Future01CompatExt;
use std::str::FromStr;
use uuid::Uuid;

/// Valid `DynamoDbUser::router_type` values
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum RouterType {
WebPush,
GCM,
FCM,
APNS,
ADM,
}

impl FromStr for RouterType {
type Err = ();

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"webpush" => Ok(RouterType::WebPush),
"gcm" => Ok(RouterType::GCM),
"fcm" => Ok(RouterType::FCM),
"apns" => Ok(RouterType::APNS),
"adm" => Ok(RouterType::ADM),
_ => Err(()),
}
}
}

/// Perform some validations on the user, including:
/// - Validate router type
/// - (WebPush) Check that the subscription/channel exists
Expand Down
15 changes: 14 additions & 1 deletion autoendpoint/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::metrics;
use crate::server::routers::fcm::router::FcmRouter;
use crate::server::routes::health::{
health_route, lb_heartbeat_route, status_route, version_route,
};
Expand All @@ -22,6 +23,7 @@ mod routers;
mod routes;

pub use headers::vapid::VapidError;
pub use routers::fcm::settings::FcmSettings;
pub use routers::RouterError;

#[derive(Clone)]
Expand All @@ -32,12 +34,13 @@ pub struct ServerState {
pub fernet: Arc<MultiFernet>,
pub ddb: DynamoStorage,
pub http: reqwest::Client,
pub fcm_router: Arc<FcmRouter>,
}

pub struct Server;

impl Server {
pub fn with_settings(settings: Settings) -> ApiResult<dev::Server> {
pub async fn with_settings(settings: Settings) -> ApiResult<dev::Server> {
let metrics = metrics::metrics_from_opts(&settings)?;
let bind_address = format!("{}:{}", settings.host, settings.port);
let fernet = Arc::new(settings.make_fernet());
Expand All @@ -48,12 +51,22 @@ impl Server {
)
.map_err(ApiErrorKind::Database)?;
let http = reqwest::Client::new();
let fcm_router = Arc::new(
FcmRouter::new(
settings.fcm.clone(),
settings.endpoint_url.clone(),
http.clone(),
metrics.clone(),
)
.await?,
);
let state = ServerState {
metrics,
settings,
fernet,
ddb,
http,
fcm_router,
};

let server = HttpServer::new(move || {
Expand Down
Loading

0 comments on commit d9a0d9d

Please sign in to comment.