diff --git a/CHANGELOG.md b/CHANGELOG.md index 93640d3e5..bc77d3265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # CHANGELOG.md ## 0.40.0 (unreleased) + - OIDC login redirects now use HTTP 303 responses so POST submissions are converted to safe GET requests before reaching the identity provider, fixing incorrect reuse of the original POST (HTTP 307) that could break standard auth flows. - SQLPage now respects [HTTP accept headers](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Accept) for JSON. You can now easily process the contents of any existing sql page programmatically with: - `curl -H "Accept: application/json" http://example.com/page.sql`: returns a json array - `curl -H "Accept: application/x-ndjson" http://example.com/page.sql`: returns one json object per line. diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index 4d04077c7..9d7885812 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -488,14 +488,14 @@ async fn build_auth_provider_redirect_response( ) -> HttpResponse { let AuthUrl { url, params } = build_auth_url(oidc_state).await; let tmp_login_flow_state_cookie = create_tmp_login_flow_state_cookie(¶ms, initial_url); - HttpResponse::TemporaryRedirect() + HttpResponse::SeeOther() .append_header((header::LOCATION, url.to_string())) .cookie(tmp_login_flow_state_cookie) .body("Redirecting...") } fn build_redirect_response(target_url: String) -> HttpResponse { - HttpResponse::TemporaryRedirect() + HttpResponse::SeeOther() .append_header(("Location", target_url)) .body("Redirecting...") } @@ -835,3 +835,22 @@ fn validate_redirect_url(url: String) -> String { log::warn!("Refusing to redirect to {url}"); '/'.to_string() } + +#[cfg(test)] +mod tests { + use super::*; + use actix_web::http::StatusCode; + + #[test] + fn login_redirects_use_see_other() { + let response = build_redirect_response("/foo".to_string()); + assert_eq!(response.status(), StatusCode::SEE_OTHER); + let location = response + .headers() + .get(header::LOCATION) + .expect("missing location header") + .to_str() + .expect("invalid location header"); + assert_eq!(location, "/foo"); + } +}