From 098830a95a63060402754a5043be3d2c8df85972 Mon Sep 17 00:00:00 2001 From: Hampus Date: Tue, 8 Sep 2026 22:18:10 +0200 Subject: [PATCH] fix(admin): compare the request origin against an origin (#2610) --- fluxer_admin/src/config/mod.rs | 8 +- fluxer_admin/src/middleware/csrf.rs | 99 +++++++++++++++++++++++- fluxer_admin/tests/csrf_legacy_cookie.rs | 20 ++++- 3 files changed, 119 insertions(+), 8 deletions(-) diff --git a/fluxer_admin/src/config/mod.rs b/fluxer_admin/src/config/mod.rs index 237d37106..5291bc20d 100644 --- a/fluxer_admin/src/config/mod.rs +++ b/fluxer_admin/src/config/mod.rs @@ -129,6 +129,11 @@ impl AdminConfig { pub fn secure_cookies(&self) -> bool { self.admin_endpoint.starts_with("https://") } + + pub fn admin_origin(&self) -> Option { + let origin = url::Url::parse(&self.admin_endpoint).ok()?.origin(); + origin.is_tuple().then(|| origin.ascii_serialization()) + } } impl RuntimeEnv { @@ -202,6 +207,7 @@ mod tests { unsafe { env::remove_var(name) }; } unsafe { env::remove_var("FLUXER_PUBLIC_PORT") }; + unsafe { env::remove_var("FLUXER_PUBLIC_ORIGIN") }; unsafe { env::set_var("FLUXER_ADMIN_SECRET_KEY_BASE", "test-secret") }; for (name, value) in vars { unsafe { env::set_var(name, value) }; @@ -350,7 +356,7 @@ mod tests { ("FLUXER_BASE_DOMAIN", "fluxer.example"), ("FLUXER_PUBLIC_PORT", "19080"), ("FLUXER_ADMIN_ENDPOINT", "http://fluxer.example/admin"), - ("FLUXER_APP_ENDPOINT", "http://fluxer.example:19080"), + ("FLUXER_APP_ENDPOINT", "http://fluxer.example"), ("FLUXER_MEDIA_ENDPOINT", "http://fluxer.example/media"), ("FLUXER_STATIC_CDN_ENDPOINT", "https://cdn.example.net"), ( diff --git a/fluxer_admin/src/middleware/csrf.rs b/fluxer_admin/src/middleware/csrf.rs index 2818c63ba..61f15cd29 100644 --- a/fluxer_admin/src/middleware/csrf.rs +++ b/fluxer_admin/src/middleware/csrf.rs @@ -27,7 +27,6 @@ pub async fn csrf_protection( ) -> Response { let config = state.config(); let secret = config.secret_key_base.clone(); - let admin_endpoint = config.admin_endpoint.clone(); let secure_cookies = config.secure_cookies(); let user_id = request @@ -50,7 +49,7 @@ pub async fn csrf_protection( .iter() .any(|suffix| path.ends_with(suffix)); if !is_ignored { - if !is_same_site_request(&request, &admin_endpoint) { + if !is_same_site_request(&request, config.admin_origin().as_deref()) { return StatusCode::FORBIDDEN.into_response(); } let header_token = extract_csrf_header(&request); @@ -167,7 +166,7 @@ async fn extract_csrf_from_form_body( Ok((request, token)) } -fn is_same_site_request(request: &Request, admin_endpoint: &str) -> bool { +fn is_same_site_request(request: &Request, admin_origin: Option<&str>) -> bool { if let Some(site) = request .headers() .get("sec-fetch-site") @@ -180,7 +179,7 @@ fn is_same_site_request(request: &Request, admin_endpoint: &str) -> bool { .get(header::ORIGIN) .and_then(|value| value.to_str().ok()) { - Some(origin) => origin == admin_endpoint, + Some(origin) => admin_origin.is_some_and(|expected| origin == expected), None => true, } } @@ -275,6 +274,98 @@ mod tests { ); } + async fn action_status(admin_endpoint: &str, origin: &str) -> StatusCode { + let state = state_with_admin_endpoint(admin_endpoint); + let app = Router::new() + .route("/", get(|| async { "ok" }).post(|| async { "ok" })) + .layer(from_fn_with_state(state, csrf_protection)); + let issued = app + .clone() + .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) + .await + .expect("router responds"); + let cookie = issued + .headers() + .get_all(header::SET_COOKIE) + .iter() + .filter_map(|value| value.to_str().ok()) + .filter_map(|value| value.split(';').next()) + .find(|pair| pair.contains("csrf_token=") && !pair.ends_with('=')) + .expect("a csrf cookie is issued") + .to_owned(); + let token = cookie.split_once('=').expect("a cookie value").1.to_owned(); + let response = app + .oneshot( + Request::builder() + .method(Method::POST) + .uri("/") + .header(header::COOKIE, cookie.as_str()) + .header(header::ORIGIN, origin) + .header(CSRF_HEADER_NAME, token.as_str()) + .body(Body::empty()) + .unwrap(), + ) + .await + .expect("router responds"); + response.status() + } + + #[tokio::test] + async fn a_matching_origin_passes_the_same_site_check() { + let status = action_status( + "https://admin.example.test/admin", + "https://admin.example.test", + ) + .await; + assert_eq!(status, StatusCode::OK); + } + + #[tokio::test] + async fn a_matching_origin_on_a_non_default_port_passes_the_same_site_check() { + let status = action_status( + "https://admin.example.test:19080/admin", + "https://admin.example.test:19080", + ) + .await; + assert_eq!(status, StatusCode::OK); + } + + #[tokio::test] + async fn a_foreign_origin_fails_the_same_site_check() { + let status = action_status( + "https://admin.example.test:19080/admin", + "https://evil.example.test:19080", + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn another_port_on_the_admin_host_fails_the_same_site_check() { + let status = action_status( + "https://admin.example.test:19080/admin", + "https://admin.example.test", + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn an_unparseable_admin_endpoint_fails_closed() { + let status = action_status("not-an-endpoint", "https://admin.example.test").await; + assert_eq!(status, StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn an_explicit_default_port_matches_a_portless_origin() { + let status = action_status( + "https://admin.example.test:443/admin", + "https://admin.example.test", + ) + .await; + assert_eq!(status, StatusCode::OK); + } + #[test] fn oauth2_callback_is_exempt() { let exempt = IGNORED_PATH_SUFFIXES diff --git a/fluxer_admin/tests/csrf_legacy_cookie.rs b/fluxer_admin/tests/csrf_legacy_cookie.rs index 871a5c6fb..4f9158b0d 100644 --- a/fluxer_admin/tests/csrf_legacy_cookie.rs +++ b/fluxer_admin/tests/csrf_legacy_cookie.rs @@ -18,6 +18,7 @@ use tower::ServiceExt; const SECRET_KEY: &str = "legacy-csrf-cookie-test-secret"; const ADMIN_ORIGIN: &str = "https://admin.example.test"; const LEGACY_HEX_TOKEN: &str = "8f14e45fceea167a5a36dedd4bea25438f14e45fceea167a5a36dedd4bea2543"; +const CREATED_KEY_SECRET: &str = "fa_1900000000000000001_OneTimeSecretForTests"; struct TestApp { router: Router, @@ -128,6 +129,10 @@ async fn load_page(app: &TestApp, cookie: &str) -> (String, String) { let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); let text = String::from_utf8(body.to_vec()).unwrap(); assert_eq!(status, StatusCode::OK, "{text}"); + assert!( + text.contains("AdminUser"), + "the page did not render the admin the mock API returns" + ); let cookie_token = host_csrf_cookie(&headers) .unwrap_or_else(|| panic!("no __Host-csrf_token in Set-Cookie: {headers:?}")); let page_token = form_csrf_value(&text).expect("no _csrf hidden input rendered"); @@ -166,7 +171,16 @@ async fn submit_action(app: &TestApp, cookie: &str, form_token: &str) -> StatusC ) .await .unwrap(); - response.status() + let status = response.status(); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + let text = String::from_utf8(body.to_vec()).unwrap(); + if status == StatusCode::OK { + assert!( + text.contains(CREATED_KEY_SECRET), + "the action did not render the key the mock API creates" + ); + } + status } fn host_csrf_cookie(headers: &HeaderMap) -> Option { @@ -207,11 +221,11 @@ async fn spawn_mock_api() -> String { async fn mock_api(method: Method, uri: Uri) -> Response { match (method, uri.path()) { - (Method::GET, "/admin/users/me") => Json(json!({ "user": admin_user() })).into_response(), + (Method::GET, "/admin/users/@me") => Json(json!({ "user": admin_user() })).into_response(), (Method::GET, "/admin/api-keys") => Json(json!([])).into_response(), (Method::POST, "/admin/api-keys") => Json(json!({ "key_id": "1900000000000000001", - "key": "fa_1900000000000000001_OneTimeSecretForTests", + "key": CREATED_KEY_SECRET, "name": "Legacy Cookie Key", "created_at": "2026-07-10T15:00:00.000Z", "expires_at": null,