Skip to content

Commit 8dec037

Browse files
committed
simplify test: use token_endpoint_delay instead of Notify gate
1 parent f028f71 commit 8dec037

1 file changed

Lines changed: 24 additions & 43 deletions

File tree

tests/oidc/mod.rs

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use serde_json::json;
1212
use sqlpage::webserver::http::create_app;
1313
use std::collections::HashMap;
1414
use std::sync::{Arc, Mutex};
15-
use tokio::sync::Notify;
15+
use std::time::Duration;
1616
use tokio_util::sync::{CancellationToken, DropGuard};
1717

1818
fn base64url_encode(data: &[u8]) -> String {
@@ -51,7 +51,7 @@ struct ProviderState<'a> {
5151
client_id: String,
5252
auth_codes: HashMap<String, String>, // code -> nonce
5353
jwt_customizer: Option<Box<JwtCustomizer<'a>>>,
54-
token_endpoint_gate: Option<Arc<Notify>>,
54+
token_endpoint_delay: Duration,
5555
}
5656

5757
type ProviderStateWithLifetime<'a> = ProviderState<'a>;
@@ -119,18 +119,6 @@ async fn token_endpoint(
119119
state: Data<SharedProviderState>,
120120
req: web::Form<HashMap<String, String>>,
121121
) -> impl Responder {
122-
let gate = state.lock().unwrap().token_endpoint_gate.clone();
123-
if let Some(gate) = gate {
124-
// Simulate a provider that stalls mid-response: send headers
125-
// immediately but never complete the body.
126-
let body = futures_util::stream::once(async move {
127-
gate.notified().await;
128-
Ok::<web::Bytes, actix_web::Error>(web::Bytes::new())
129-
});
130-
return HttpResponse::Ok()
131-
.insert_header((header::CONTENT_TYPE, "application/json"))
132-
.streaming(body);
133-
}
134122
let mut state = state.lock().unwrap();
135123
let Some(code) = req.get("code") else {
136124
return HttpResponse::BadRequest().body("Missing code");
@@ -156,16 +144,24 @@ async fn token_endpoint(
156144
.map(|customizer| customizer(claims.clone(), &state.secret))
157145
.unwrap_or_else(|| make_jwt(&claims, &state.secret));
158146

147+
let delay = state.token_endpoint_delay;
148+
drop(state);
149+
159150
let response = TokenResponse {
160151
access_token: "test_access_token".to_string(),
161152
token_type: "Bearer".to_string(),
162153
id_token,
163154
expires_in: 3600,
164155
};
165156

157+
let json_bytes = serde_json::to_vec(&response).unwrap();
158+
let body = futures_util::stream::once(async move {
159+
tokio::time::sleep(delay).await;
160+
Ok::<web::Bytes, actix_web::Error>(web::Bytes::from(json_bytes))
161+
});
166162
HttpResponse::Ok()
167163
.insert_header((header::CONTENT_TYPE, "application/json"))
168-
.json(response)
164+
.streaming(body)
169165
}
170166

171167
pub struct FakeOidcProvider {
@@ -199,7 +195,7 @@ impl FakeOidcProvider {
199195
client_id: client_id.clone(),
200196
auth_codes: HashMap::new(),
201197
jwt_customizer: None,
202-
token_endpoint_gate: None,
198+
token_endpoint_delay: Duration::ZERO,
203199
}));
204200

205201
let state_for_server = Arc::clone(&state);
@@ -241,10 +237,8 @@ impl FakeOidcProvider {
241237
f(&mut state)
242238
}
243239

244-
pub fn gate_token_endpoint(&self) -> Arc<Notify> {
245-
let gate = Arc::new(Notify::new());
246-
self.with_state_mut(|s| s.token_endpoint_gate = Some(gate.clone()));
247-
gate
240+
pub fn set_token_endpoint_delay(&self, delay: Duration) {
241+
self.with_state_mut(|s| s.token_endpoint_delay = delay);
248242
}
249243

250244
pub fn store_auth_code(&self, code: String, nonce: String) {
@@ -562,14 +556,13 @@ async fn test_oidc_logout_uses_correct_scheme() {
562556
assert_eq!(post_logout, "https://example.com/logged_out");
563557
}
564558

565-
/// Regression test: a stalled OIDC provider must not freeze the server.
559+
/// A slow OIDC provider must not freeze the server.
566560
/// See https://github.com/sqlpage/SQLPage/issues/1231
567561
#[actix_web::test]
568-
async fn test_stalled_token_endpoint_does_not_freeze_server() {
562+
async fn test_slow_token_endpoint_does_not_freeze_server() {
569563
let (app, provider) = setup_oidc_test(|_| {}).await;
570564
let mut cookies: Vec<Cookie<'static>> = Vec::new();
571565

572-
// Step 1: initiate login — get redirected to auth provider
573566
let resp = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies);
574567
assert_eq!(resp.status(), StatusCode::SEE_OTHER);
575568
let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap();
@@ -578,46 +571,34 @@ async fn test_stalled_token_endpoint_does_not_freeze_server() {
578571
let redirect_uri = get_query_param(&auth_url, "redirect_uri");
579572
provider.store_auth_code("test_auth_code".to_string(), nonce);
580573

581-
// Step 2: gate the token endpoint so it sends headers but stalls the body
582-
let _gate = provider.gate_token_endpoint();
574+
provider.set_token_endpoint_delay(Duration::from_secs(999));
583575

584-
// Step 3: hit the OIDC callback — the server will try to exchange the auth
585-
// code for a token. The token endpoint sends headers but the body never
586-
// arrives. With the bug, response.body().await hangs forever (no timeout).
587576
let callback_uri = format!(
588577
"{}?code=test_auth_code&state={}",
589578
Url::parse(&redirect_uri).unwrap().path(),
590579
state_param
591580
);
592581

593-
// Detect the hang without wall-clock delays using a two-phase approach:
594-
// Phase 1 (yields): let the localhost TCP round trip complete so the awc
595-
// client receives HTTP headers and enters body().await.
596-
// Phase 2 (pause + sleep): freeze tokio time then sleep past the body
597-
// timeout. With auto-advance this resolves instantly. If there IS a body
598-
// timeout (fix applied), auto-advance fires it and the request completes.
599-
// If there is NO body timeout (bug), nothing wakes the body read, and
600-
// the sleep branch completes first → panic.
582+
// Race the callback request against a deadline to detect hangs without
583+
// wall-clock delays. Phase 1 (yields) lets the localhost TCP round trip
584+
// complete. Phase 2 (pause + sleep) uses tokio auto-advance to instantly
585+
// skip past any body-read timeout that may be set on the HTTP client.
601586
tokio::select! {
602587
_resp = async {
603588
let mut req = test::TestRequest::get().uri(&callback_uri);
604589
for cookie in cookies.iter() {
605590
req = req.cookie(cookie.clone());
606591
}
607592
test::call_service(&app, req.to_request()).await
608-
} => {} // request completed — bug is fixed
593+
} => {}
609594
_ = async {
610595
for _ in 0..1000 {
611596
tokio::task::yield_now().await;
612597
}
613598
tokio::time::pause();
614-
tokio::time::sleep(std::time::Duration::from_secs(60)).await;
599+
tokio::time::sleep(Duration::from_secs(60)).await;
615600
} => {
616-
panic!(
617-
"OIDC callback request hung — the server froze because \
618-
response.body().await has no timeout when the OIDC provider \
619-
stalls after sending headers"
620-
);
601+
panic!("OIDC callback hung on a slow token endpoint");
621602
}
622603
}
623604
}

0 commit comments

Comments
 (0)