mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Use 127.0.0.1 for local login redirects (#47927)
## What changed Use `127.0.0.1` instead of `localhost` in the OAuth callback URI and local login success URL. ## Testing Update login and app-server tests to check the callback address, verify that the success redirect shares the callback origin, and assert that the token exchange uses the same `redirect_uri` as the authorization request. GitOrigin-RevId: a27dd63dca34b9de81fb937da47c753d5dd1ac90
This commit is contained in:
committed by
copyberry
parent
0037b39cc1
commit
4b97832cfb
@@ -2554,8 +2554,8 @@ async fn login_account_chatgpt_start_can_be_cancelled() -> Result<()> {
|
||||
bail!("unexpected login response: {login:?}");
|
||||
};
|
||||
assert!(
|
||||
auth_url.contains("redirect_uri=http%3A%2F%2Flocalhost"),
|
||||
"auth_url should contain a redirect_uri to localhost"
|
||||
auth_url.contains("redirect_uri=http%3A%2F%2F127.0.0.1%3A"),
|
||||
"auth_url should contain a redirect_uri to 127.0.0.1"
|
||||
);
|
||||
|
||||
let cancel_id = mcp
|
||||
@@ -2646,6 +2646,7 @@ async fn login_account_chatgpt_uses_oauth_overrides() -> Result<()> {
|
||||
.query_pairs()
|
||||
.find_map(|(key, value)| (key == "state").then_some(value.into_owned()))
|
||||
.ok_or_else(|| anyhow::anyhow!("missing state"))?;
|
||||
let token_redirect_uri = callback_url.clone();
|
||||
let mut callback_url = Url::parse(&callback_url)?;
|
||||
callback_url
|
||||
.query_pairs_mut()
|
||||
@@ -2681,6 +2682,7 @@ async fn login_account_chatgpt_uses_oauth_overrides() -> Result<()> {
|
||||
token_form.get("client_id").map(String::as_str),
|
||||
Some("staging-client")
|
||||
);
|
||||
assert_eq!(token_form.get("redirect_uri"), Some(&token_redirect_uri));
|
||||
|
||||
let notification = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
|
||||
@@ -190,7 +190,7 @@ pub fn run_login_server(opts: ServerOptions) -> io::Result<LoginServer> {
|
||||
};
|
||||
let server = Arc::new(server);
|
||||
|
||||
let redirect_uri = format!("http://localhost:{actual_port}/auth/callback");
|
||||
let redirect_uri = format!("http://127.0.0.1:{actual_port}/auth/callback");
|
||||
let auth_url = build_authorize_url(
|
||||
&opts.issuer,
|
||||
&opts.client_id,
|
||||
|
||||
@@ -100,7 +100,7 @@ pub(crate) fn compose_success_url(
|
||||
.map(|(key, value)| format!("{key}={}", urlencoding::encode(&value)))
|
||||
.collect::<Vec<_>>()
|
||||
.join("&");
|
||||
LoginSuccessRedirect::Local(format!("http://localhost:{port}/success?{query}"))
|
||||
LoginSuccessRedirect::Local(format!("http://127.0.0.1:{port}/success?{query}"))
|
||||
}
|
||||
|
||||
pub(crate) fn jwt_auth_claims(jwt: &str) -> serde_json::Map<String, serde_json::Value> {
|
||||
|
||||
@@ -18,7 +18,7 @@ fn compose_success_url_uses_local_page_by_default() {
|
||||
};
|
||||
let url = Url::parse(&url).expect("success URL should parse");
|
||||
|
||||
assert_eq!(url.host_str(), Some("localhost"));
|
||||
assert_eq!(url.host_str(), Some("127.0.0.1"));
|
||||
assert_eq!(url.path(), "/success");
|
||||
assert_eq!(
|
||||
url.query_pairs()
|
||||
@@ -109,7 +109,7 @@ fn compose_success_url_keeps_setup_on_local_page() {
|
||||
};
|
||||
let url = Url::parse(&url).expect("success URL should parse");
|
||||
|
||||
assert_eq!(url.host_str(), Some("localhost"));
|
||||
assert_eq!(url.host_str(), Some("127.0.0.1"));
|
||||
assert_eq!(url.path(), "/success");
|
||||
assert_eq!(
|
||||
url.query_pairs()
|
||||
|
||||
@@ -160,6 +160,14 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> {
|
||||
"auth URL should include forced workspace parameter"
|
||||
);
|
||||
let login_port = server.actual_port;
|
||||
let callback_url = Url::parse(&server.auth_url)?
|
||||
.query_pairs()
|
||||
.find_map(|(key, value)| (key == "redirect_uri").then(|| value.into_owned()))
|
||||
.expect("auth URL should include a redirect URI");
|
||||
assert_eq!(
|
||||
callback_url,
|
||||
format!("http://127.0.0.1:{login_port}/auth/callback")
|
||||
);
|
||||
|
||||
// Simulate browser callback and assert the local success redirect before following it.
|
||||
let client = HttpClientBuilder::new()
|
||||
@@ -173,7 +181,7 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> {
|
||||
"test_state_123.extra=value.onboarding_entrypoint=life_sciences",
|
||||
] {
|
||||
let response = client
|
||||
.get(format!("http://127.0.0.1:{login_port}/auth/callback"))
|
||||
.get(&callback_url)
|
||||
.query(&[
|
||||
("state", state),
|
||||
("code", "untrusted"),
|
||||
@@ -190,14 +198,13 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> {
|
||||
)?)?,
|
||||
stale_auth
|
||||
);
|
||||
let url = format!(
|
||||
"http://127.0.0.1:{login_port}/auth/callback?code=abc&state=test_state_123.onboarding_entrypoint=life_sciences"
|
||||
);
|
||||
let url =
|
||||
format!("{callback_url}?code=abc&state=test_state_123.onboarding_entrypoint=life_sciences");
|
||||
let resp = client.get(&url).send().await?;
|
||||
assert_eq!(resp.status(), 302);
|
||||
let success_url = resp.headers()["location"].to_str()?;
|
||||
let success_url = Url::parse(success_url)?;
|
||||
assert_eq!(success_url.host_str(), Some("localhost"));
|
||||
assert_eq!(success_url.origin(), Url::parse(&callback_url)?.origin());
|
||||
assert_eq!(success_url.path(), "/success");
|
||||
|
||||
let success_resp = client.get(success_url).send().await?;
|
||||
@@ -644,7 +651,7 @@ async fn falls_back_to_registered_fallback_port_when_default_port_is_in_use() ->
|
||||
|
||||
assert_eq!(actual_port, FALLBACK_LOGIN_PORT);
|
||||
assert!(auth_url.contains(&format!(
|
||||
"redirect_uri=http%3A%2F%2Flocalhost%3A{FALLBACK_LOGIN_PORT}%2Fauth%2Fcallback"
|
||||
"redirect_uri=http%3A%2F%2F127.0.0.1%3A{FALLBACK_LOGIN_PORT}%2Fauth%2Fcallback"
|
||||
)));
|
||||
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user