Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/oauth-callback-host-port.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": minor
---

Add `--callback-port` flag to `gws auth login` so users can configure the OAuth callback server host and port. Both flags also read from environment variable `GOOGLE_WORKSPACE_CLI_CALLBACK_PORT` respectively (CLI flags take precedence). This is useful when the OAuth app is registered with a fixed redirect URI or when running in Docker/CI with port-forwarding.
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ See [`src/helpers/README.md`](crates/google-workspace-cli/src/helpers/README.md)
|---|---|
| `GOOGLE_WORKSPACE_CLI_CLIENT_ID` | OAuth client ID (for `gws auth login` when no `client_secret.json` is saved) |
| `GOOGLE_WORKSPACE_CLI_CLIENT_SECRET` | OAuth client secret (paired with `CLIENT_ID` above) |
| `GOOGLE_WORKSPACE_CLI_CALLBACK_PORT` | Port for the local OAuth callback server during `gws auth login` (default: `0` = OS-assigned; overridden by `--callback-port`) |

### Sanitization (Model Armor)

Expand Down
2 changes: 1 addition & 1 deletion crates/google-workspace-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ google-workspace = { version = "0.22.5", path = "../google-workspace" }
tempfile = "3"
aes-gcm = "0.10"
anyhow = "1"
clap = { version = "4", features = ["derive", "string"] }
clap = { version = "4", features = ["derive", "string", "env"] }
dirs = "5"
dotenvy = "0.15"
hostname = "0.4"
Expand Down
115 changes: 99 additions & 16 deletions crates/google-workspace-cli/src/auth_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,12 @@ async fn login_with_proxy_support(
client_id: &str,
client_secret: &str,
scopes: &[String],
callback_port: u16,
) -> Result<(String, String), GwsError> {
// Start local server to receive OAuth callback
let listener = TcpListener::bind("127.0.0.1:0")
// Start local server to receive OAuth callback.
// Bind to all interfaces so port-forwarding works in Docker/CI environments.
let host = if callback_port == 0 { "127.0.0.1" } else { "0.0.0.0" };
let listener = TcpListener::bind(format!("{host}:{callback_port}"))
Comment on lines +121 to +122
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Binding to 0.0.0.0 by default when a custom port is specified is a security risk, as it exposes the temporary OAuth callback server to the entire network. While this is intended to support Docker/CI environments, it should not be forced on all users. To address this security concern while avoiding scope creep, consider defaulting to 127.0.0.1 and only binding to 0.0.0.0 if explicitly necessary for the environment.

References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

.map_err(|e| GwsError::Auth(format!("Failed to start local server: {e}")))?;
let port = listener
.local_addr()
Expand Down Expand Up @@ -392,6 +395,15 @@ fn build_login_subcommand() -> clap::Command {
)
.value_name("services"),
)
.arg(
clap::Arg::new("callback-port")
.long("callback-port")
.env("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT")
.help("Port for the local OAuth callback server (0 = OS-assigned)")
.value_name("PORT")
.value_parser(clap::value_parser!(u16))
.default_value("0"),
)
}

/// Build the clap Command for `gws auth`.
Expand Down Expand Up @@ -448,9 +460,10 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> {

match matches.subcommand() {
Some(("login", sub_m)) => {
let (scope_mode, services_filter) = parse_login_args(sub_m);
let (scope_mode, services_filter, callback_port) =
parse_login_args(sub_m);

handle_login_inner(scope_mode, services_filter).await
handle_login_inner(scope_mode, services_filter, callback_port).await
}
Some(("setup", sub_m)) => {
// Collect remaining args and delegate to setup's own clap parser.
Expand Down Expand Up @@ -482,8 +495,10 @@ fn login_command() -> clap::Command {
build_login_subcommand()
}

/// Extract `ScopeMode` and optional services filter from parsed login args.
fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option<HashSet<String>>) {
/// Extract `ScopeMode`, optional services filter, and OAuth callback port from parsed login args.
fn parse_login_args(
matches: &clap::ArgMatches,
) -> (ScopeMode, Option<HashSet<String>>, u16) {
let scope_mode = if let Some(scopes_str) = matches.get_one::<String>("scopes") {
ScopeMode::Custom(
scopes_str
Expand All @@ -508,7 +523,11 @@ fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option<HashSet<St
.collect()
});

(scope_mode, services_filter)
let callback_port = *matches
.get_one::<u16>("callback-port")
.expect("callback-port has a default_value and is always present");

(scope_mode, services_filter, callback_port)
}

/// Run the `auth login` flow.
Expand All @@ -532,9 +551,9 @@ pub async fn run_login(args: &[String]) -> Result<(), GwsError> {
Err(e) => return Err(GwsError::Validation(e.to_string())),
};

let (scope_mode, services_filter) = parse_login_args(&matches);
let (scope_mode, services_filter, callback_port) = parse_login_args(&matches);

handle_login_inner(scope_mode, services_filter).await
handle_login_inner(scope_mode, services_filter, callback_port).await
}
/// Custom delegate that prints the OAuth URL on its own line for easy copying.
/// Optionally includes `login_hint` in the URL for account pre-selection.
Expand Down Expand Up @@ -576,6 +595,7 @@ impl yup_oauth2::authenticator_delegate::InstalledFlowDelegate for CliFlowDelega
async fn handle_login_inner(
scope_mode: ScopeMode,
services_filter: Option<HashSet<String>>,
callback_port: u16,
) -> Result<(), GwsError> {
// Resolve client_id and client_secret:
// 1. Env vars (highest priority)
Expand Down Expand Up @@ -618,13 +638,21 @@ async fn handle_login_inner(
std::fs::create_dir_all(&config)
.map_err(|e| GwsError::Validation(format!("Failed to create config directory: {e}")))?;

// If proxy env vars are set, use proxy-aware OAuth flow (reqwest)
// Otherwise use yup-oauth2 (faster, but doesn't support proxy)
let (access_token, refresh_token) = if crate::auth::has_proxy_env() {
login_with_proxy_support(&client_id, &client_secret, &scopes).await?
} else {
login_with_yup_oauth(&config, &client_id, &client_secret, &scopes).await?
};
// If proxy env vars are set, or a custom callback port is requested,
// use proxy-aware OAuth flow (reqwest). Otherwise use yup-oauth2 (faster,
// but doesn't support proxy or custom callback configuration).
let (access_token, refresh_token) =
if crate::auth::has_proxy_env() || callback_port != 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Forcing the use of login_with_proxy_support whenever a custom port is specified is a regression in reliability. The custom one-shot TCP listener fails on any non-OAuth request (like a favicon request). To maintain the primary goal of supporting custom ports without introducing unnecessary complexity or maintenance burden, it is recommended to use yup_oauth2::InstalledFlowAuthenticatorBuilder::with_port() instead of implementing custom listener logic.

References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

login_with_proxy_support(
&client_id,
&client_secret,
&scopes,
callback_port,
)
.await?
} else {
login_with_yup_oauth(&config, &client_id, &client_secret, &scopes).await?
};

// Build credentials in the standard authorized_user format
let creds_json = json!({
Expand Down Expand Up @@ -2532,4 +2560,59 @@ mod tests {
let err = read_refresh_token_from_cache(file.path()).unwrap_err();
assert!(err.to_string().contains("no refresh token was returned"));
}

#[test]
#[serial_test::serial]
fn parse_login_args_defaults_callback_port() {
unsafe {
std::env::remove_var("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT");
}
let matches = build_login_subcommand()
.try_get_matches_from(["login"])
.unwrap();
let (_, _, callback_port) = parse_login_args(&matches);
assert_eq!(callback_port, 0);
}

#[test]
fn parse_login_args_custom_callback_port() {
let matches = build_login_subcommand()
.try_get_matches_from(["login", "--callback-port", "9090"])
.unwrap();
let (_, _, callback_port) = parse_login_args(&matches);
assert_eq!(callback_port, 9090u16);
}

#[test]
#[serial_test::serial]
fn parse_login_args_callback_port_from_env() {
unsafe {
std::env::set_var("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT", "8888");
}
let matches = build_login_subcommand()
.try_get_matches_from(["login"])
.unwrap();
let (_, _, callback_port) = parse_login_args(&matches);
unsafe {
std::env::remove_var("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT");
}
assert_eq!(callback_port, 8888u16);
}

#[test]
#[serial_test::serial]
fn parse_login_args_cli_arg_overrides_env_for_callback() {
// CLI arg takes precedence even when env var is set
unsafe {
std::env::set_var("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT", "7777");
}
let matches = build_login_subcommand()
.try_get_matches_from(["login", "--callback-port", "5555"])
.unwrap();
let (_, _, callback_port) = parse_login_args(&matches);
unsafe {
std::env::remove_var("GOOGLE_WORKSPACE_CLI_CALLBACK_PORT");
}
assert_eq!(callback_port, 5555u16);
}
}
Loading