From 50819a9d208917344d913800e818fe37e71974a8 Mon Sep 17 00:00:00 2001
From: Marshall Bowers <git@maxdeviant.com>
Date: Mon, 18 Aug 2025 15:57:28 -0400
Subject: [PATCH] client: Parse auth callback query parameters before showing
 sign-in success page (#36440)

This PR fixes an issue where we would redirect the user's browser to the
sign-in success page even if the OAuth callback was malformed.

We now parse the OAuth callback parameters from the query string and
only redirect to the sign-in success page when they are valid.

Release Notes:

- Updated the sign-in flow to not show the sign-in success page
prematurely.
---
 Cargo.lock                  |  1 +
 Cargo.toml                  |  1 +
 crates/client/Cargo.toml    |  1 +
 crates/client/src/client.rs | 24 +++++++++++++-----------
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 98f10eff41..3158a61ad8 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3070,6 +3070,7 @@ dependencies = [
  "schemars",
  "serde",
  "serde_json",
+ "serde_urlencoded",
  "settings",
  "sha2",
  "smol",
diff --git a/Cargo.toml b/Cargo.toml
index 83d6da5cd7..914f9e6837 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -582,6 +582,7 @@ serde_json_lenient = { version = "0.2", features = [
     "raw_value",
 ] }
 serde_repr = "0.1"
+serde_urlencoded = "0.7"
 sha2 = "0.10"
 shellexpand = "2.1.0"
 shlex = "1.3.0"
diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml
index 365625b445..5c6d1157fd 100644
--- a/crates/client/Cargo.toml
+++ b/crates/client/Cargo.toml
@@ -44,6 +44,7 @@ rpc = { workspace = true, features = ["gpui"] }
 schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+serde_urlencoded.workspace = true
 settings.workspace = true
 sha2.workspace = true
 smol.workspace = true
diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs
index f09c012a85..0f00471356 100644
--- a/crates/client/src/client.rs
+++ b/crates/client/src/client.rs
@@ -1410,6 +1410,12 @@ impl Client {
 
                     open_url_tx.send(url).log_err();
 
+                    #[derive(Deserialize)]
+                    struct CallbackParams {
+                        pub user_id: String,
+                        pub access_token: String,
+                    }
+
                     // Receive the HTTP request from the user's browser. Retrieve the user id and encrypted
                     // access token from the query params.
                     //
@@ -1420,17 +1426,13 @@ impl Client {
                             for _ in 0..100 {
                                 if let Some(req) = server.recv_timeout(Duration::from_secs(1))? {
                                     let path = req.url();
-                                    let mut user_id = None;
-                                    let mut access_token = None;
                                     let url = Url::parse(&format!("http://example.com{}", path))
                                         .context("failed to parse login notification url")?;
-                                    for (key, value) in url.query_pairs() {
-                                        if key == "access_token" {
-                                            access_token = Some(value.to_string());
-                                        } else if key == "user_id" {
-                                            user_id = Some(value.to_string());
-                                        }
-                                    }
+                                    let callback_params: CallbackParams =
+                                        serde_urlencoded::from_str(url.query().unwrap_or_default())
+                                            .context(
+                                                "failed to parse sign-in callback query parameters",
+                                            )?;
 
                                     let post_auth_url =
                                         http.build_url("/native_app_signin_succeeded");
@@ -1445,8 +1447,8 @@ impl Client {
                                     )
                                     .context("failed to respond to login http request")?;
                                     return Ok((
-                                        user_id.context("missing user_id parameter")?,
-                                        access_token.context("missing access_token parameter")?,
+                                        callback_params.user_id,
+                                        callback_params.access_token,
                                     ));
                                 }
                             }