Show OAuth error descriptions in callback responses (#9654)
### Motivation - The local OAuth callback server returned a generic "Invalid OAuth callback" on failures even when the query contained an `error_description`, making it hard to debug OAuth failures. ### Description - Update `codex-rs/rmcp-client/src/perform_oauth_login.rs` to surface `error_description` values from the callback query in the HTTP response. - Introduce a `CallbackOutcome` enum and change `parse_oauth_callback` to return it, parsing `code`, `state`, and `error_description` from the query string. - Change `spawn_callback_server` to match on `CallbackOutcome` and return `OAuth error: <description>` with a 400 status when `error_description` is present, while preserving the existing success and invalid flows. - Use inline formatting for the error response string. ### Testing - Ran `just fmt` in the `codex-rs` workspace to format changes successfully. - Ran `cargo test -p codex-rmcp-client` and all tests passed. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_6971aadc68d0832e93159efea8cd48a9)
This commit is contained in:
parent
337643b00a
commit
c7c2b3cf8d
1 changed files with 53 additions and 22 deletions
|
|
@ -103,21 +103,32 @@ fn spawn_callback_server(server: Arc<Server>, tx: oneshot::Sender<(String, Strin
|
|||
tokio::task::spawn_blocking(move || {
|
||||
while let Ok(request) = server.recv() {
|
||||
let path = request.url().to_string();
|
||||
if let Some(OauthCallbackResult { code, state }) = parse_oauth_callback(&path) {
|
||||
let response =
|
||||
Response::from_string("Authentication complete. You may close this window.");
|
||||
if let Err(err) = request.respond(response) {
|
||||
eprintln!("Failed to respond to OAuth callback: {err}");
|
||||
match parse_oauth_callback(&path) {
|
||||
CallbackOutcome::Success(OauthCallbackResult { code, state }) => {
|
||||
let response = Response::from_string(
|
||||
"Authentication complete. You may close this window.",
|
||||
);
|
||||
if let Err(err) = request.respond(response) {
|
||||
eprintln!("Failed to respond to OAuth callback: {err}");
|
||||
}
|
||||
if let Err(err) = tx.send((code, state)) {
|
||||
eprintln!("Failed to send OAuth callback: {err:?}");
|
||||
}
|
||||
break;
|
||||
}
|
||||
if let Err(err) = tx.send((code, state)) {
|
||||
eprintln!("Failed to send OAuth callback: {err:?}");
|
||||
CallbackOutcome::Error(description) => {
|
||||
let response = Response::from_string(format!("OAuth error: {description}"))
|
||||
.with_status_code(400);
|
||||
if let Err(err) = request.respond(response) {
|
||||
eprintln!("Failed to respond to OAuth callback: {err}");
|
||||
}
|
||||
}
|
||||
break;
|
||||
} else {
|
||||
let response =
|
||||
Response::from_string("Invalid OAuth callback").with_status_code(400);
|
||||
if let Err(err) = request.respond(response) {
|
||||
eprintln!("Failed to respond to OAuth callback: {err}");
|
||||
CallbackOutcome::Invalid => {
|
||||
let response =
|
||||
Response::from_string("Invalid OAuth callback").with_status_code(400);
|
||||
if let Err(err) = request.respond(response) {
|
||||
eprintln!("Failed to respond to OAuth callback: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -129,29 +140,49 @@ struct OauthCallbackResult {
|
|||
state: String,
|
||||
}
|
||||
|
||||
fn parse_oauth_callback(path: &str) -> Option<OauthCallbackResult> {
|
||||
let (route, query) = path.split_once('?')?;
|
||||
enum CallbackOutcome {
|
||||
Success(OauthCallbackResult),
|
||||
Error(String),
|
||||
Invalid,
|
||||
}
|
||||
|
||||
fn parse_oauth_callback(path: &str) -> CallbackOutcome {
|
||||
let Some((route, query)) = path.split_once('?') else {
|
||||
return CallbackOutcome::Invalid;
|
||||
};
|
||||
if route != "/callback" {
|
||||
return None;
|
||||
return CallbackOutcome::Invalid;
|
||||
}
|
||||
|
||||
let mut code = None;
|
||||
let mut state = None;
|
||||
let mut error_description = None;
|
||||
|
||||
for pair in query.split('&') {
|
||||
let (key, value) = pair.split_once('=')?;
|
||||
let decoded = decode(value).ok()?.into_owned();
|
||||
let Some((key, value)) = pair.split_once('=') else {
|
||||
continue;
|
||||
};
|
||||
let Ok(decoded) = decode(value) else {
|
||||
continue;
|
||||
};
|
||||
let decoded = decoded.into_owned();
|
||||
match key {
|
||||
"code" => code = Some(decoded),
|
||||
"state" => state = Some(decoded),
|
||||
"error_description" => error_description = Some(decoded),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
Some(OauthCallbackResult {
|
||||
code: code?,
|
||||
state: state?,
|
||||
})
|
||||
if let (Some(code), Some(state)) = (code, state) {
|
||||
return CallbackOutcome::Success(OauthCallbackResult { code, state });
|
||||
}
|
||||
|
||||
if let Some(description) = error_description {
|
||||
return CallbackOutcome::Error(description);
|
||||
}
|
||||
|
||||
CallbackOutcome::Invalid
|
||||
}
|
||||
|
||||
pub struct OauthLoginHandle {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue