fix(oauth): require CSRF token as part of the OAuth authorization flow. (#435)

* Require CSRF token as part of the authorization flow.

* Update auth example.

* Update docs/OAUTH_SUPPORT.md.

Author: vorporeal

crates/rmcp/src/transport/auth.rs

@@ -147,7 +147,7 @@ pub struct AuthorizationManager {
     metadata: Option<AuthorizationMetadata>,
     oauth_client: Option<OAuthClient>,
     credentials: RwLock<Option<OAuthTokenResponse>>,
-    pkce_verifier: RwLock<Option<PkceCodeVerifier>>,
+    state: RwLock<Option<AuthorizationState>>,
     expires_at: RwLock<Option<Instant>>,
     base_url: Url,
 }
@@ -172,6 +172,12 @@ pub struct ClientRegistrationResponse {
     pub additional_fields: HashMap<String, serde_json::Value>,
 }
 
+#[derive(Debug)]
+struct AuthorizationState {
+    pkce_verifier: PkceCodeVerifier,
+    csrf_token: CsrfToken,
+}
+
 impl AuthorizationManager {
     /// create new auth manager with base url
     pub async fn new<U: IntoUrl>(base_url: U) -> Result<Self, AuthError> {
@@ -186,7 +192,7 @@ impl AuthorizationManager {
             metadata: None,
             oauth_client: None,
             credentials: RwLock::new(None),
-            pkce_verifier: RwLock::new(None),
+            state: RwLock::new(None),
             expires_at: RwLock::new(None),
             base_url,
         };
@@ -405,11 +411,14 @@ impl AuthorizationManager {
             auth_request = auth_request.add_scope(Scope::new(scope.to_string()));
         }
 
-        let (auth_url, _csrf_token) = auth_request.url();
+        let (auth_url, csrf_token) = auth_request.url();
 
         // store pkce verifier for later use
-        *self.pkce_verifier.write().await = Some(pkce_verifier);
-        debug!("set pkce verifier: {:?}", self.pkce_verifier.read().await);
+        *self.state.write().await = Some(AuthorizationState {
+            pkce_verifier,
+            csrf_token,
+        });
+        debug!("set authorization state: {:?}", self.state.read().await);
 
         Ok(auth_url.to_string())
     }
@@ -418,19 +427,25 @@ impl AuthorizationManager {
     pub async fn exchange_code_for_token(
         &self,
         code: &str,
+        csrf_token: &str,
     ) -> Result<StandardTokenResponse<EmptyExtraTokenFields, BasicTokenType>, AuthError> {
         debug!("start exchange code for token: {:?}", code);
         let oauth_client = self
             .oauth_client
             .as_ref()
             .ok_or_else(|| AuthError::InternalError("OAuth client not configured".to_string()))?;
 
-        let pkce_verifier = self
-            .pkce_verifier
-            .write()
-            .await
-            .take()
-            .ok_or_else(|| AuthError::InternalError("PKCE verifier not found".to_string()))?;
+        let AuthorizationState {
+            pkce_verifier,
+            csrf_token: expected_csrf_token,
+        } =
+            self.state.write().await.take().ok_or_else(|| {
+                AuthError::InternalError("Authorization state not found".to_string())
+            })?;
+
+        if csrf_token != expected_csrf_token.secret() {
+            return Err(AuthError::InternalError("CSRF token mismatch".to_string()));
+        }
 
         let http_client = reqwest::ClientBuilder::new()
             .redirect(reqwest::redirect::Policy::none())
@@ -601,8 +616,11 @@ impl AuthorizationSession {
     pub async fn handle_callback(
         &self,
         code: &str,
+        csrf_token: &str,
     ) -> Result<StandardTokenResponse<EmptyExtraTokenFields, BasicTokenType>, AuthError> {
-        self.auth_manager.exchange_code_for_token(code).await
+        self.auth_manager
+            .exchange_code_for_token(code, csrf_token)
+            .await
     }
 }
 
@@ -787,10 +805,10 @@ impl OAuthState {
     }
 
     /// handle authorization callback
-    pub async fn handle_callback(&mut self, code: &str) -> Result<(), AuthError> {
+    pub async fn handle_callback(&mut self, code: &str, csrf_token: &str) -> Result<(), AuthError> {
         match self {
             OAuthState::Session(session) => {
-                session.handle_callback(code).await?;
+                session.handle_callback(code, csrf_token).await?;
                 self.complete_authorization().await
             }
             OAuthState::Unauthorized(_) => {

docs/OAUTH_SUPPORT.md

@@ -44,8 +44,9 @@ rmcp = { version = "0.1", features = ["auth", "transport-sse-client"] }
     println!("Please open the following URL in your browser for authorization:\n{}", auth_url);
 
     // Handle callback - In real applications, this is typically done in a callback server
-    let auth_code = "Authorization code obtained from browser after user authorization";
-    let credentials = oauth_state.handle_callback(auth_code).await?;
+    let auth_code = "Authorization code (`code` param) obtained from browser after user authorization";
+    let csrf_token = "CSRF token (`state` param) obtained from browser after user authorization";
+    let credentials = oauth_state.handle_callback(auth_code, csrf_token).await?;
 
     println!("Authorization successful, access token: {}", credentials.access_token);
 

examples/clients/src/auth/oauth_client.rs

@@ -31,25 +31,24 @@ const CALLBACK_HTML: &str = include_str!("callback.html");
 
 #[derive(Clone)]
 struct AppState {
-    code_receiver: Arc<Mutex<Option<oneshot::Sender<String>>>>,
+    code_receiver: Arc<Mutex<Option<oneshot::Sender<CallbackParams>>>>,
 }
 
 #[derive(Debug, Deserialize)]
 struct CallbackParams {
     code: String,
-    #[allow(dead_code)]
-    state: Option<String>,
+    state: String,
 }
 
 async fn callback_handler(
     Query(params): Query<CallbackParams>,
     State(state): State<AppState>,
 ) -> Html<String> {
-    tracing::info!("Received callback with code: {}", params.code);
+    tracing::info!("Received callback: {params:?}");
 
     // Send the code to the main thread
     if let Some(sender) = state.code_receiver.lock().await.take() {
-        let _ = sender.send(params.code);
+        let _ = sender.send(params);
     }
     // Return success page
     Html(CALLBACK_HTML.to_string())
@@ -67,7 +66,7 @@ async fn main() -> Result<()> {
         .init();
     // it is a http server for handling callback
     // Create channel for receiving authorization code
-    let (code_sender, code_receiver) = oneshot::channel::<String>();
+    let (code_sender, code_receiver) = oneshot::channel::<CallbackParams>();
 
     // Create app state
     let app_state = AppState {
@@ -121,14 +120,17 @@ async fn main() -> Result<()> {
 
     // Wait for authorization code
     tracing::info!("Waiting for authorization code...");
-    let auth_code = code_receiver
+    let CallbackParams {
+        code: auth_code,
+        state: csrf_token,
+    } = code_receiver
         .await
         .context("Failed to get authorization code")?;
     tracing::info!("Received authorization code: {}", auth_code);
     // Exchange code for access token
     tracing::info!("Exchanging authorization code for access token...");
     oauth_state
-        .handle_callback(&auth_code)
+        .handle_callback(&auth_code, &csrf_token)
         .await
         .context("Failed to handle callback")?;
     tracing::info!("Successfully obtained access token");