| ... | @@ -156,16 +156,12 @@ fn signed_claims( | ... | @@ -156,16 +156,12 @@ fn signed_claims( |
| 156 | keys: &Value, | 156 | keys: &Value, |
| 157 | client: &str, | 157 | client: &str, |
| 158 | nonce: &str, | 158 | nonce: &str, |
| 159 | require_nonce: bool, | 159 | id_token: bool, |
| 160 | ) -> Result<Value> { | 160 | ) -> Result<Value> { |
| 161 | let reject = |reason: &str| { | 161 | let reject = |reason: &str| { |
| 162 | eprintln!( | 162 | eprintln!( |
| 163 | "guest token verification: {} {reason}", | 163 | "guest token verification: {} {reason}", |
| 164 | if require_nonce { | 164 | if id_token { "id_token" } else { "userinfo" } |
| 165 | "id_token" | | |
| 166 | } else { | | |
| 167 | "userinfo" | | |
| 168 | } | | |
| 169 | ); | 165 | ); |
| 170 | Error::new( | 166 | Error::new( |
| 171 | 502, | 167 | 502, |
| ... | @@ -272,11 +268,13 @@ fn signed_claims( | ... | @@ -272,11 +268,13 @@ fn signed_claims( |
| 272 | ), | 268 | ), |
| 273 | ( | 269 | ( |
| 274 | "expiration", | 270 | "expiration", |
| 275 | claims["exp"].as_i64().is_none_or(|t| t <= time), | 271 | (id_token || claims.get("exp").is_some()) |
| | 272 | && claims["exp"].as_i64().is_none_or(|t| t <= time), |
| 276 | ), | 273 | ), |
| 277 | ( | 274 | ( |
| 278 | "issued_at", | 275 | "issued_at", |
| 279 | claims["iat"].as_i64().is_none_or(|t| t > time + 60), | 276 | (id_token || claims.get("iat").is_some()) |
| | 277 | && claims["iat"].as_i64().is_none_or(|t| t > time + 60), |
| 280 | ), | 278 | ), |
| 281 | ( | 279 | ( |
| 282 | "not_before", | 280 | "not_before", |
| ... | @@ -284,10 +282,7 @@ fn signed_claims( | ... | @@ -284,10 +282,7 @@ fn signed_claims( |
| 284 | .get("nbf") | 282 | .get("nbf") |
| 285 | .is_some_and(|t| t.as_i64().is_none_or(|n| n > time + 60)), | 283 | .is_some_and(|t| t.as_i64().is_none_or(|n| n > time + 60)), |
| 286 | ), | 284 | ), |
| 287 | ( | 285 | ("nonce", id_token && claims["nonce"].as_str() != Some(nonce)), |
| 288 | "nonce", | | |
| 289 | require_nonce && claims["nonce"].as_str() != Some(nonce), | | |
| 290 | ), | | |
| 291 | ] { | 286 | ] { |
| 292 | if invalid { | 287 | if invalid { |
| 293 | return Err(reject(reason)); | 288 | return Err(reject(reason)); |
| ... | @@ -387,14 +382,14 @@ async fn exchange( | ... | @@ -387,14 +382,14 @@ async fn exchange( |
| 387 | } | 382 | } |
| 388 | let keys = | 383 | let keys = |
| 389 | json_bytes(&response_bytes(http.get(format!("{ASTHENO}/api/jwks")).send().await?).await?)?; | 384 | json_bytes(&response_bytes(http.get(format!("{ASTHENO}/api/jwks")).send().await?).await?)?; |
| 390 | let claims = signed_claims( | 385 | let claims = token |
| 391 | string(&token["id_token"]), | 386 | .get("id_token") |
| 392 | &keys, | 387 | .map(|token| signed_claims(string(token), &keys, client, string(&flow["nonce"]), true)) |
| 393 | client, | 388 | .transpose()?; |
| 394 | string(&flow["nonce"]), | 389 | if let Some(hash) = claims |
| 395 | true, | 390 | .as_ref() |
| 396 | )?; | 391 | .and_then(|claims| claims["at_hash"].as_str()) |
| 397 | if let Some(hash) = claims["at_hash"].as_str() { | 392 | { |
| 398 | if hash | 393 | if hash |
| 399 | != URL_SAFE_NO_PAD | 394 | != URL_SAFE_NO_PAD |
| 400 | .encode(&Sha256::digest(string(&token["access_token"]).as_bytes())[..16]) | 395 | .encode(&Sha256::digest(string(&token["access_token"]).as_bytes())[..16]) |
| ... | @@ -417,7 +412,12 @@ async fn exchange( | ... | @@ -417,7 +412,12 @@ async fn exchange( |
| 417 | } else { | 412 | } else { |
| 418 | signed_claims(std::str::from_utf8(&bytes)?, &keys, client, "", false)? | 413 | signed_claims(std::str::from_utf8(&bytes)?, &keys, client, "", false)? |
| 419 | }; | 414 | }; |
| 420 | if profile["sub"] != claims["sub"] { | 415 | if string(&profile["sub"]).is_empty() |
| | 416 | || string(&profile["sub"]).len() > 512 |
| | 417 | || claims |
| | 418 | .as_ref() |
| | 419 | .is_some_and(|claims| profile["sub"] != claims["sub"]) |
| | 420 | { |
| 421 | return Err(Error::new( | 421 | return Err(Error::new( |
| 422 | 502, | 422 | 502, |
| 423 | "Astheno couldn't verify your account. Return to Shale and try again.", | 423 | "Astheno couldn't verify your account. Return to Shale and try again.", |
| ... | @@ -430,7 +430,7 @@ async fn exchange( | ... | @@ -430,7 +430,7 @@ async fn exchange( |
| 430 | .chars() | 430 | .chars() |
| 431 | .take(128) | 431 | .take(128) |
| 432 | .collect(); | 432 | .collect(); |
| 433 | Ok((string(&claims["sub"]).to_owned(), name)) | 433 | Ok((string(&profile["sub"]).to_owned(), name)) |
| 434 | } | 434 | } |
| 435 | | 435 | |
| 436 | fn account(auth: &auth::Store, provider: &str, subject: &str, name: &str) -> Result<String> { | 436 | fn account(auth: &auth::Store, provider: &str, subject: &str, name: &str) -> Result<String> { |
| ... | @@ -738,6 +738,33 @@ mod tests { | ... | @@ -738,6 +738,33 @@ mod tests { |
| 738 | ); | 738 | ); |
| 739 | } | 739 | } |
| 740 | | 740 | |
| | 741 | #[test] |
| | 742 | fn signed_userinfo_requires_identity_and_signature_but_not_id_token_times_or_nonce() { |
| | 743 | let vector: Value = serde_json::from_str(include_str!("../tests/guest-jwt.json")).unwrap(); |
| | 744 | let userinfo = &vector["userinfo"]; |
| | 745 | let token = string(&userinfo["token"]); |
| | 746 | assert_eq!( |
| | 747 | signed_claims(token, &userinfo["keys"], "fixture", "", false).unwrap()["sub"], |
| | 748 | "external-123" |
| | 749 | ); |
| | 750 | assert!(signed_claims(token, &userinfo["keys"], "fixture", "fixture-nonce", true).is_err()); |
| | 751 | assert!(signed_claims(token, &userinfo["keys"], "other", "", false).is_err()); |
| | 752 | assert!(signed_claims(token, &vector["keys"], "fixture", "", false).is_err()); |
| | 753 | for reason in ["issuer", "audience", "expiry", "future", "subject"] { |
| | 754 | assert!( |
| | 755 | signed_claims( |
| | 756 | string(&vector["invalid"][reason]), |
| | 757 | &vector["keys"], |
| | 758 | "fixture", |
| | 759 | "", |
| | 760 | false |
| | 761 | ) |
| | 762 | .is_err(), |
| | 763 | "{reason}" |
| | 764 | ); |
| | 765 | } |
| | 766 | } |
| | 767 | |
| 741 | #[test] | 768 | #[test] |
| 742 | fn identities_never_link_by_name_or_email_and_cannot_gain_credentials_or_groups() { | 769 | fn identities_never_link_by_name_or_email_and_cannot_gain_credentials_or_groups() { |
| 743 | let path = std::env::temp_dir().join(format!("guest-test-{}", uuid::Uuid::new_v4())); | 770 | let path = std::env::temp_dir().join(format!("guest-test-{}", uuid::Uuid::new_v4())); |