Skip to content

Commit 2d3d187

Browse files
authored
feat: validate OAuth authorization response issuer (#896)
* feat: validate OAuth authorization response issuer * fix: tighten issuer validation callbacks
1 parent f1ef2ec commit 2d3d187

3 files changed

Lines changed: 420 additions & 41 deletions

File tree

conformance/src/bin/client.rs

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use rmcp::{
33
model::*,
44
service::RequestContext,
55
transport::{
6-
AuthClient, AuthorizationManager, StreamableHttpClientTransport, auth::OAuthState,
6+
AuthClient, AuthorizationManager, StreamableHttpClientTransport,
7+
auth::{AuthorizationCallback, OAuthState},
78
streamable_http_client::StreamableHttpClientTransportConfig,
89
},
910
};
@@ -221,20 +222,16 @@ async fn perform_oauth_flow(
221222
.and_then(|v| v.to_str().ok())
222223
.ok_or_else(|| anyhow::anyhow!("No Location header in auth redirect"))?;
223224

224-
let redirect_url = url::Url::parse(location)?;
225-
let code = redirect_url
226-
.query_pairs()
227-
.find(|(k, _)| k == "code")
228-
.map(|(_, v)| v.to_string())
229-
.ok_or_else(|| anyhow::anyhow!("No code in redirect URL"))?;
230-
let state = redirect_url
231-
.query_pairs()
232-
.find(|(k, _)| k == "state")
233-
.map(|(_, v)| v.to_string())
234-
.ok_or_else(|| anyhow::anyhow!("No state in redirect URL"))?;
225+
let callback = AuthorizationCallback::from_redirect_url(location)?;
235226

236227
tracing::debug!("Got auth code, exchanging for token...");
237-
oauth.handle_callback(&code, &state).await?;
228+
oauth
229+
.handle_callback_with_issuer(
230+
&callback.code,
231+
&callback.csrf_token,
232+
callback.issuer.as_deref(),
233+
)
234+
.await?;
238235

239236
let am = oauth
240237
.into_authorization_manager()
@@ -334,8 +331,14 @@ async fn run_auth_scope_step_up_client(
334331
.await?;
335332

336333
let auth_url = oauth.get_authorization_url().await?;
337-
let (code, state) = headless_authorize(&auth_url).await?;
338-
oauth.handle_callback(&code, &state).await?;
334+
let callback = headless_authorize(&auth_url).await?;
335+
oauth
336+
.handle_callback_with_issuer(
337+
&callback.code,
338+
&callback.csrf_token,
339+
callback.issuer.as_deref(),
340+
)
341+
.await?;
339342

340343
let am = oauth
341344
.into_authorization_manager()
@@ -380,8 +383,14 @@ async fn run_auth_scope_step_up_client(
380383
)
381384
.await?;
382385
let auth_url2 = oauth2.get_authorization_url().await?;
383-
let (code2, state2) = headless_authorize(&auth_url2).await?;
384-
oauth2.handle_callback(&code2, &state2).await?;
386+
let callback2 = headless_authorize(&auth_url2).await?;
387+
oauth2
388+
.handle_callback_with_issuer(
389+
&callback2.code,
390+
&callback2.csrf_token,
391+
callback2.issuer.as_deref(),
392+
)
393+
.await?;
385394

386395
let am2 = oauth2.into_authorization_manager().unwrap();
387396
let auth_client2 = AuthClient::new(reqwest::Client::default(), am2);
@@ -422,8 +431,14 @@ async fn run_auth_scope_retry_limit_client(
422431
)
423432
.await?;
424433
let auth_url = oauth.get_authorization_url().await?;
425-
let (code, state) = headless_authorize(&auth_url).await?;
426-
oauth.handle_callback(&code, &state).await?;
434+
let callback = headless_authorize(&auth_url).await?;
435+
oauth
436+
.handle_callback_with_issuer(
437+
&callback.code,
438+
&callback.csrf_token,
439+
callback.issuer.as_deref(),
440+
)
441+
.await?;
427442

428443
let am = oauth.into_authorization_manager().unwrap();
429444
let auth_client = AuthClient::new(reqwest::Client::default(), am);
@@ -696,8 +711,8 @@ async fn run_cross_app_access_client(
696711

697712
// ─── Helpers ────────────────────────────────────────────────────────────────
698713

699-
/// Fetch an authorization URL headlessly, returning (code, state).
700-
async fn headless_authorize(auth_url: &str) -> anyhow::Result<(String, String)> {
714+
/// Fetch an authorization URL headlessly, returning the callback parameters.
715+
async fn headless_authorize(auth_url: &str) -> anyhow::Result<AuthorizationCallback> {
701716
let http = reqwest::Client::builder()
702717
.redirect(reqwest::redirect::Policy::none())
703718
.build()?;
@@ -707,18 +722,7 @@ async fn headless_authorize(auth_url: &str) -> anyhow::Result<(String, String)>
707722
.get("location")
708723
.and_then(|v| v.to_str().ok())
709724
.ok_or_else(|| anyhow::anyhow!("No Location header in auth redirect"))?;
710-
let redirect_url = url::Url::parse(location)?;
711-
let code = redirect_url
712-
.query_pairs()
713-
.find(|(k, _)| k == "code")
714-
.map(|(_, v)| v.to_string())
715-
.ok_or_else(|| anyhow::anyhow!("No code in redirect URL"))?;
716-
let state = redirect_url
717-
.query_pairs()
718-
.find(|(k, _)| k == "state")
719-
.map(|(_, v)| v.to_string())
720-
.ok_or_else(|| anyhow::anyhow!("No state in redirect URL"))?;
721-
Ok((code, state))
725+
AuthorizationCallback::from_redirect_url(location).map_err(Into::into)
722726
}
723727

724728
/// Build a `CallToolRequestParams` for a tool, optionally with arguments.

0 commit comments

Comments
 (0)