fix: use gateway owner_id for relay OAuth nonce storage (#2473)

henrypark133 · claude · web-flow · commit 8973d1b53426 · 2026-04-14T16:20:42.000-07:00
* fix: use gateway owner_id for relay OAuth nonce storage

The relay OAuth nonce was stored under the authenticated user's ID
(a DB user UUID) but the callback handler looked it up under
state.owner_id (the gateway owner, typically "default"). This
mismatch caused the nonce lookup to silently fail, returning
"Invalid or expired authorization" on every Slack OAuth callback.

Use self.user_id (which holds config.owner_id) in auth_channel_relay
for nonce storage so both sides use the same scope.

Also adds tracing to the callback handler's get_decrypted error path
to make future auth failures diagnosable.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;

* test: add test for nonce user scope mismatch

Verifies that a nonce stored under a DB user UUID (different from the
gateway owner_id) is not found by the callback handler, reproducing
the bug that caused "Invalid or expired authorization" on hosted
instances.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;

* fix: address PR review comments

- Also delete legacy caller-scoped nonce on upgrade (Copilot)
- Include redacted state param in tracing log (Gemini)

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;

---------

Co-authored-by: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs
@@ -2119,7 +2119,14 @@ async fn slack_relay_oauth_callback_handler(
         .await
     {
         Ok(secret) => secret.expose().to_string(),
-        Err(_) => {
+        Err(e) => {
+            tracing::warn!(
+                owner_id = %state.owner_id,
+                state_key = %state_key,
+                state = %redact_oauth_state_for_logs(&state_param),
+                error = %e,
+                "relay OAuth callback: failed to retrieve stored nonce"
+            );
             return axum::response::Html(
                 "<html><body style='font-family: system-ui; text-align: center; padding: 60px;'>\
                  <h2>Error</h2><p>Invalid or expired authorization.</p></body></html>"
@@ -6644,6 +6651,59 @@ mod tests {
         assert!(!exists, "CSRF nonce should be deleted after use");
     }
 
+    #[tokio::test]
+    async fn test_relay_oauth_callback_nonce_under_different_user_fails() {
+        // why: In hosted mode, the DB user's UUID differs from the gateway
+        //      owner_id. If the nonce is stored under the DB user's scope,
+        //      the callback handler (which uses owner_id) cannot find it.
+        use axum::body::Body;
+        use tower::ServiceExt;
+
+        let secrets = test_secrets_store();
+        let nonce = "nonce-stored-under-wrong-user";
+
+        // given: nonce stored under a DB user UUID, NOT the gateway owner ("test")
+        secrets
+            .create(
+                "b50a4a66-ba1b-439c-907b-cc6b371871b0",
+                crate::secrets::CreateSecretParams::new(
+                    format!("relay:{}:oauth_state", DEFAULT_RELAY_NAME),
+                    nonce,
+                ),
+            )
+            .await
+            .expect("store nonce");
+
+        // ext_mgr.user_id = "test", gateway owner_id = "test"
+        let (ext_mgr, _wasm_tools_dir, _wasm_channels_dir) = test_ext_mgr(secrets);
+        let state = test_gateway_state(Some(ext_mgr));
+        let app = test_relay_oauth_router(state);
+
+        // when: callback arrives with the correct nonce value
+        let req = axum::http::Request::builder()
+            .uri(format!(
+                "/oauth/slack/callback?team_id=T123&provider=slack&state={}",
+                nonce
+            ))
+            .body(Body::empty())
+            .expect("request");
+
+        let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
+            .await
+            .expect("response");
+
+        // then: fails because nonce is under a different user scope
+        let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
+            .await
+            .expect("body");
+        let html = String::from_utf8_lossy(&body);
+        assert!(
+            html.contains("Invalid or expired authorization"),
+            "Nonce stored under wrong user scope should fail lookup, got: {}",
+            &html[..html.len().min(300)]
+        );
+    }
+
     #[test]
     fn test_is_local_origin_localhost() {
         assert!(is_local_origin("http://localhost:3001"));
diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs
@@ -6072,10 +6072,20 @@ impl ExtensionManager {
         // state and appends it to the post-OAuth redirect URL.
         let state_nonce = uuid::Uuid::new_v4().to_string();
         let state_key = format!("relay:{}:oauth_state", name);
-        // Delete any stale nonce before storing the new one
+        // Delete any stale nonce before storing the new one.
+        // Use self.user_id (the gateway owner) — NOT the caller's user_id —
+        // because the OAuth callback handler looks up the nonce under
+        // state.owner_id which matches self.user_id.
+        //
+        // Also best-effort delete any legacy caller-scoped entry so older
+        // per-user nonces don't remain in the secrets table after upgrading.
         let _ = self.secrets.delete(user_id, &state_key).await;
+        let _ = self.secrets.delete(&self.user_id, &state_key).await;
         self.secrets
-            .create(user_id, CreateSecretParams::new(&state_key, &state_nonce))
+            .create(
+                &self.user_id,
+                CreateSecretParams::new(&state_key, &state_nonce),
+            )
             .await
             .map_err(|e| {
                 tracing::warn!(
