Skip to content

Commit b808ccf

Browse files
lovasoacursoragent
andauthored
Use HTTP 303 instead of 307 for oidc redirects (#1133)
* Use SeeOther for redirects, not TemporaryRedirect Co-authored-by: contact <contact@ophir.dev> * Fix OIDC login redirect to use HTTP 303 Co-authored-by: contact <contact@ophir.dev> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 747cc78 commit b808ccf

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# CHANGELOG.md
22

33
## 0.40.0 (unreleased)
4+
- 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.
45
- 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:
56
- `curl -H "Accept: application/json" http://example.com/page.sql`: returns a json array
67
- `curl -H "Accept: application/x-ndjson" http://example.com/page.sql`: returns one json object per line.

src/webserver/oidc.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,14 +488,14 @@ async fn build_auth_provider_redirect_response(
488488
) -> HttpResponse {
489489
let AuthUrl { url, params } = build_auth_url(oidc_state).await;
490490
let tmp_login_flow_state_cookie = create_tmp_login_flow_state_cookie(&params, initial_url);
491-
HttpResponse::TemporaryRedirect()
491+
HttpResponse::SeeOther()
492492
.append_header((header::LOCATION, url.to_string()))
493493
.cookie(tmp_login_flow_state_cookie)
494494
.body("Redirecting...")
495495
}
496496

497497
fn build_redirect_response(target_url: String) -> HttpResponse {
498-
HttpResponse::TemporaryRedirect()
498+
HttpResponse::SeeOther()
499499
.append_header(("Location", target_url))
500500
.body("Redirecting...")
501501
}
@@ -835,3 +835,22 @@ fn validate_redirect_url(url: String) -> String {
835835
log::warn!("Refusing to redirect to {url}");
836836
'/'.to_string()
837837
}
838+
839+
#[cfg(test)]
840+
mod tests {
841+
use super::*;
842+
use actix_web::http::StatusCode;
843+
844+
#[test]
845+
fn login_redirects_use_see_other() {
846+
let response = build_redirect_response("/foo".to_string());
847+
assert_eq!(response.status(), StatusCode::SEE_OTHER);
848+
let location = response
849+
.headers()
850+
.get(header::LOCATION)
851+
.expect("missing location header")
852+
.to_str()
853+
.expect("invalid location header");
854+
assert_eq!(location, "/foo");
855+
}
856+
}

0 commit comments

Comments
 (0)