Skip to content

Commit

Permalink
feat: add metrics for vapid errors (#340)
Browse files Browse the repository at this point in the history 
from the work for:
SYNC-3389

Co-authored-by: JR Conlin <[email protected]>

Co-authored-by: JR Conlin <[email protected]>
Co-authored-by: JR Conlin <[email protected]>
  • Loading branch information
@pjenvey @jrconlin
3 people authored Jan 20, 2023
1 parent 65ac1a3 commit 922fcf8
Showing 1 changed file with 64 additions and 18 deletions.
82 changes: 64 additions & 18 deletions autoendpoint/src/extractors/subscription.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::extractors::token_info::{ApiVersion, TokenInfo};
use crate::extractors::user::validate_user;
use crate::headers::crypto_key::CryptoKeyHeader;
use crate::headers::vapid::{VapidError, VapidHeader, VapidHeaderWithKey, VapidVersionData};
use crate::metrics::Metrics;
use crate::server::ServerState;
use crate::tags::Tags;
use actix_web::dev::{Payload, PayloadStream};
use actix_web::web::Data;
use actix_web::{FromRequest, HttpRequest};
Expand Down Expand Up @@ -62,6 +64,7 @@ impl FromRequest for Subscription {
trace!("Token info: {:?}", &token_info);
let state: Data<ServerState> =
Data::extract(&req).await.expect("No server state found");
let metrics = Metrics::from(&state);

// Decrypt the token
let token = state
Expand Down Expand Up @@ -103,7 +106,7 @@ impl FromRequest for Subscription {

// Validate the VAPID JWT token and record the version
if let Some(vapid) = &vapid {
validate_vapid_jwt(vapid, &state.settings.endpoint_url())?;
validate_vapid_jwt(vapid, &state.settings.endpoint_url(), &metrics)?;

state
.metrics
Expand Down Expand Up @@ -239,7 +242,11 @@ fn version_2_validation(token: &[u8], vapid: Option<&VapidHeaderWithKey>) -> Api
/// - Make sure the expiration isn't too far into the future
///
/// This is mostly taken care of by the jsonwebtoken library
fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()> {
fn validate_vapid_jwt(
vapid: &VapidHeaderWithKey,
domain: &Url,
metrics: &Metrics,
) -> ApiResult<()> {
let VapidHeaderWithKey { vapid, public_key } = vapid;

let public_key = decode_public_key(public_key)?;
Expand All @@ -252,6 +259,20 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()>
Err(e) => match e.kind() {
// NOTE: This will fail if `exp` is specified as anything instead of a numeric or if a required field is empty
jsonwebtoken::errors::ErrorKind::Json(e) => {
let mut tags = Tags::default();
tags.tags.insert(
"error".to_owned(),
match e.classify() {
serde_json::error::Category::Io => "IO_ERROR",
serde_json::error::Category::Syntax => "SYNTAX_ERROR",
serde_json::error::Category::Data => "DATA_ERROR",
serde_json::error::Category::Eof => "EOF_ERROR",
}
.to_owned(),
);
metrics
.clone()
.incr_with_tags("notification.auth.bad_vapid.json", Some(tags));
if e.is_data() {
return Err(VapidError::InvalidVapid(
"A value in the vapid claims is either missing or incorrectly specified (e.g. \"exp\":\"12345\" or \"sub\":null). Please correct and retry.".to_owned(),
Expand All @@ -262,11 +283,13 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()>
// the Json parse error.
return Err(VapidError::InvalidVapid(e.to_string()).into());
}
_ => return Err(e.into()),
_ => {
metrics.clone().incr("notification.auth.bad_vapid.other");
return Err(e.into());
}
},
};

// Check the signature and make sure the expiration is in the future, but not too far
if token_data.claims.exp > (sec_since_epoch() + ONE_DAY_IN_SECONDS) {
// The expiration is too far in the future
return Err(VapidError::FutureExpirationToken.into());
Expand All @@ -276,12 +299,14 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()>
Ok(v) => v,
Err(_) => {
error!("Bad Aud: Invalid audience {:?}", &token_data.claims.aud);
metrics.clone().incr("notification.auth.bad_vapid.aud");
return Err(VapidError::InvalidAudience.into());
}
};

if domain != &aud {
error!("Bad Aud: I am <{:?}>, asked for <{:?}> ", domain, aud);
metrics.clone().incr("notification.auth.bad_vapid.domain");
return Err(VapidError::InvalidAudience.into());
}

Expand All @@ -294,6 +319,7 @@ mod tests {
use crate::error::ApiErrorKind;
use crate::extractors::subscription::repad_base64;
use crate::headers::vapid::{VapidError, VapidHeader, VapidHeaderWithKey, VapidVersionData};
use crate::metrics::Metrics;
use autopush_common::endpoint::STANDARD_NO_PAD;
use autopush_common::util::sec_since_epoch;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -339,7 +365,7 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
let result = validate_vapid_jwt(&header, &Url::from_str(domain).unwrap());
let result = validate_vapid_jwt(&header, &Url::from_str(domain).unwrap(), &Metrics::noop());
assert!(result.is_ok());
}

Expand Down Expand Up @@ -371,9 +397,13 @@ mod tests {
},
};
assert!(matches!(
validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap())
.unwrap_err()
.kind,
validate_vapid_jwt(
&header,
&Url::from_str("http://example.org").unwrap(),
&Metrics::noop()
)
.unwrap_err()
.kind,
ApiErrorKind::VapidError(VapidError::InvalidAudience)
));
}
Expand Down Expand Up @@ -412,9 +442,13 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap())
.unwrap_err()
.kind;
let vv = validate_vapid_jwt(
&header,
&Url::from_str("http://example.org").unwrap(),
&Metrics::noop(),
)
.unwrap_err()
.kind;
assert!(matches![
vv,
ApiErrorKind::VapidError(VapidError::InvalidVapid(_))
Expand Down Expand Up @@ -458,7 +492,9 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
assert!(validate_vapid_jwt(&header, &Url::from_str(domain).unwrap()).is_ok());
assert!(
validate_vapid_jwt(&header, &Url::from_str(domain).unwrap(), &Metrics::noop()).is_ok()
);
// try standard form with no padding
let header = VapidHeaderWithKey {
public_key: public_key_standard.trim_end_matches('=').to_owned(),
Expand All @@ -468,7 +504,9 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
assert!(validate_vapid_jwt(&header, &Url::from_str(domain).unwrap()).is_ok());
assert!(
validate_vapid_jwt(&header, &Url::from_str(domain).unwrap(), &Metrics::noop()).is_ok()
);
// try URL safe form with padding
let header = VapidHeaderWithKey {
public_key: public_key_url_safe.clone(),
Expand All @@ -478,7 +516,9 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
assert!(validate_vapid_jwt(&header, &Url::from_str(domain).unwrap()).is_ok());
assert!(
validate_vapid_jwt(&header, &Url::from_str(domain).unwrap(), &Metrics::noop()).is_ok()
);
// try URL safe form without padding
let header = VapidHeaderWithKey {
public_key: public_key_url_safe.trim_end_matches('=').to_owned(),
Expand All @@ -488,7 +528,9 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
assert!(validate_vapid_jwt(&header, &Url::from_str(domain).unwrap()).is_ok());
assert!(
validate_vapid_jwt(&header, &Url::from_str(domain).unwrap(), &Metrics::noop()).is_ok()
);
}

#[test]
Expand Down Expand Up @@ -525,9 +567,13 @@ mod tests {
version_data: VapidVersionData::Version1,
},
};
let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap())
.unwrap_err()
.kind;
let vv = validate_vapid_jwt(
&header,
&Url::from_str("http://example.org").unwrap(),
&Metrics::noop(),
)
.unwrap_err()
.kind;
assert!(matches![
vv,
ApiErrorKind::VapidError(VapidError::InvalidVapid(_))
Expand Down

0 comments on commit 922fcf8

Please sign in to comment.