Skip to content

Commit 14512f1

Browse files
chenbachenba
authored
fix: do not validate exp on FxA event tokens (#2402)
1 parent c530cd4 commit 14512f1

4 files changed

Lines changed: 19 additions & 52 deletions

File tree

‎syncserver/src/tokenserver/extractors.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,7 +1381,6 @@ mod tests {
13811381
"quux",
13821382
"testo",
13831383
serde_json::json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
1384-
3600,
13851384
TEST_PRIVATE_KEY_PEM,
13861385
);
13871386
let req = TestRequest::default()
@@ -1404,7 +1403,6 @@ mod tests {
14041403
"quux",
14051404
"testo",
14061405
serde_json::json!({}),
1407-
3600,
14081406
OTHER_PRIVATE_KEY_PEM,
14091407
);
14101408
let req = TestRequest::default()
@@ -1425,13 +1423,7 @@ mod tests {
14251423
#[actix_rt::test]
14261424
async fn test_fxa_webhook_no_verifiers() {
14271425
let state = make_webhook_state(vec![]);
1428-
let token = make_set(
1429-
"quux",
1430-
"testo",
1431-
serde_json::json!({}),
1432-
3600,
1433-
TEST_PRIVATE_KEY_PEM,
1434-
);
1426+
let token = make_set("quux", "testo", serde_json::json!({}), TEST_PRIVATE_KEY_PEM);
14351427
let req = TestRequest::default()
14361428
.app_data(Data::new(state))
14371429
.insert_header(("Authorization", format!("Bearer {token}")))

‎syncserver/src/tokenserver/handlers.rs‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,6 @@ mod tests {
504504
"quux",
505505
"testo",
506506
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
507-
3600,
508507
OTHER_PRIVATE_KEY_PEM,
509508
);
510509
let req = TestRequest::post()
@@ -515,26 +514,6 @@ mod tests {
515514
assert_eq!(resp.status(), 401);
516515
}
517516

518-
#[actix_web::test]
519-
async fn test_expired_token() {
520-
let verifier =
521-
SETVerifierImpl::new(&test_jwk(), "testo", "https://accounts.firefox.com/").unwrap();
522-
let app = make_app(vec![verifier]).await;
523-
let token = make_set(
524-
"quux",
525-
"testo",
526-
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
527-
-3600,
528-
TEST_PRIVATE_KEY_PEM,
529-
);
530-
let req = TestRequest::post()
531-
.uri("/1.0/webhooks/fxa/events")
532-
.insert_header(("Authorization", format!("Bearer {token}")))
533-
.to_request();
534-
let resp = test::call_service(&app, req).await;
535-
assert_eq!(resp.status(), 401);
536-
}
537-
538517
#[actix_web::test]
539518
async fn test_wrong_audience() {
540519
let verifier =
@@ -544,7 +523,6 @@ mod tests {
544523
"quux",
545524
"some-other-RP",
546525
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
547-
3600,
548526
TEST_PRIVATE_KEY_PEM,
549527
);
550528
let req = TestRequest::post()
@@ -562,7 +540,6 @@ mod tests {
562540
"quux",
563541
"testo",
564542
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
565-
3600,
566543
TEST_PRIVATE_KEY_PEM,
567544
);
568545
let req = TestRequest::post()
@@ -584,7 +561,6 @@ mod tests {
584561
"quux",
585562
"testo",
586563
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
587-
3600,
588564
TEST_PRIVATE_KEY_PEM,
589565
);
590566
let req = TestRequest::post()
@@ -610,7 +586,6 @@ mod tests {
610586
"quux",
611587
"testo",
612588
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
613-
3600,
614589
TEST_PRIVATE_KEY_PEM,
615590
);
616591
let req = TestRequest::post()
@@ -638,7 +613,6 @@ mod tests {
638613
"quux",
639614
"testo",
640615
json!({"https://schemas.accounts.firefox.com/event/password-change": {"changeTime": change_time_ms}}),
641-
3600,
642616
TEST_PRIVATE_KEY_PEM,
643617
);
644618
let req = TestRequest::post()
@@ -673,7 +647,6 @@ mod tests {
673647
"quux",
674648
"testo",
675649
json!({"https://schemas.accounts.firefox.com/event/unknown-event": {}}),
676-
3600,
677650
TEST_PRIVATE_KEY_PEM,
678651
);
679652
let req = TestRequest::post()

‎tokenserver-auth/src/crypto.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ impl SETVerifierImpl {
196196
let mut validation = Validation::new(Algorithm::RS256);
197197
validation.set_audience(&[client_id]);
198198
validation.set_issuer(&[issuer_url]);
199-
validation.validate_exp = true;
199+
// jsonwebtoken validates `exp` by default
200+
validation.required_spec_claims.remove("exp");
200201
Ok(Self {
201202
key: decoding_key,
202203
validation,
@@ -223,7 +224,6 @@ mod tests {
223224
"quux",
224225
"testo",
225226
json!({"https://schemas.accounts.firefox.com/event/delete-user": {}}),
226-
3600,
227227
TEST_PRIVATE_KEY_PEM,
228228
);
229229
let claims: FxaWebhookClaims = verifier.verify(&token).unwrap();
@@ -232,19 +232,27 @@ mod tests {
232232
}
233233

234234
#[test]
235-
fn test_verify_expired_set() {
235+
fn test_verify_wrong_audience_set() {
236236
let verifier =
237237
SETVerifierImpl::new(&test_jwk(), "testo", "https://accounts.firefox.com/").unwrap();
238-
let token = make_set("quux", "testo", json!({}), -3600, TEST_PRIVATE_KEY_PEM);
239-
let err = verifier.verify::<FxaWebhookClaims>(&token).unwrap_err();
240-
assert!(matches!(err, JWTVerifyError::ExpiredSignature));
238+
let token = make_set("quux", "wrong-client-id", json!({}), TEST_PRIVATE_KEY_PEM);
239+
assert!(verifier.verify::<FxaWebhookClaims>(&token).is_err());
240+
}
241+
242+
#[test]
243+
fn test_verify_wrong_issuer_set() {
244+
let verifier =
245+
SETVerifierImpl::new(&test_jwk(), "testo", "https://accounts.stage.mozaws.net")
246+
.unwrap();
247+
let token = make_set("quux", "testo", json!({}), TEST_PRIVATE_KEY_PEM);
248+
assert!(verifier.verify::<FxaWebhookClaims>(&token).is_err());
241249
}
242250

243251
#[test]
244252
fn test_verify_wrong_key_set() {
245253
let verifier =
246254
SETVerifierImpl::new(&test_jwk(), "testo", "https://accounts.firefox.com/").unwrap();
247-
let token = make_set("quux", "testo", json!({}), 3600, OTHER_PRIVATE_KEY_PEM);
255+
let token = make_set("quux", "testo", json!({}), OTHER_PRIVATE_KEY_PEM);
248256
let err = verifier.verify::<FxaWebhookClaims>(&token).unwrap_err();
249257
assert!(matches!(err, JWTVerifyError::InvalidSignature));
250258
}

‎tokenserver-auth/src/test_utils.rs‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,9 @@ pub fn test_jwk() -> Jwk {
7474
serde_json::from_str(TEST_JWK).unwrap()
7575
}
7676

77-
/// Sign a SET with the given PEM.
78-
pub fn make_set(
79-
sub: &str,
80-
aud: &str,
81-
events: serde_json::Value,
82-
exp_offset_secs: i64,
83-
private_key_pem: &[u8],
84-
) -> String {
77+
/// Sign a SET with the given PEM. Mirrors the claims the FxA Event Broker emits: {iss, aud, sub,
78+
/// iat, jti, events}
79+
pub fn make_set(sub: &str, aud: &str, events: serde_json::Value, private_key_pem: &[u8]) -> String {
8580
let now = SystemTime::now()
8681
.duration_since(UNIX_EPOCH)
8782
.unwrap()
@@ -91,7 +86,6 @@ pub fn make_set(
9186
"sub": sub,
9287
"aud": aud,
9388
"iat": now,
94-
"exp": now + exp_offset_secs,
9589
"jti": "wibble",
9690
"events": events,
9791
});

0 commit comments

Comments
 (0)