Skip to content
Closed
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
37 changes: 34 additions & 3 deletions src/auth_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> {
" --scopes Comma-separated custom scopes\n",
" -s, --services Comma-separated service names to limit scope picker\n",
" (e.g. -s drive,gmail,sheets)\n",
" --port <PORT> Use a fixed port for the OAuth redirect server\n",
" (use with Web Application type OAuth clients for orgs\n",
" that block Desktop OAuth via admin_policy_enforced)\n",
" setup Configure GCP project + OAuth client (requires gcloud)\n",
" --project Use a specific GCP project\n",
" --login Run `gws auth login` after successful setup\n",
Expand Down Expand Up @@ -211,15 +214,37 @@ impl yup_oauth2::authenticator_delegate::InstalledFlowDelegate for CliFlowDelega
}

async fn handle_login(args: &[String]) -> Result<(), GwsError> {
// Extract -s/--services from args
// Extract -s/--services and --port from args
let mut services_filter: Option<HashSet<String>> = None;
let mut fixed_port: Option<u16> = None;
let mut filtered_args: Vec<String> = Vec::new();
let mut skip_next = false;
for i in 0..args.len() {
if skip_next {
skip_next = false;
continue;
}

// Parse --port <PORT> or --port=<PORT>
let port_str = if args[i] == "--port" && i + 1 < args.len() {
skip_next = true;
Some(args[i + 1].as_str())
} else {
args[i].strip_prefix("--port=")
};
if let Some(value) = port_str {
let port = value.parse::<u16>().map_err(|_| {
GwsError::Validation(format!("Invalid port number: {value}"))
})?;
if port == 0 {
return Err(GwsError::Validation(
"Port number must be a non-zero value between 1 and 65535.".to_string(),
));
}
fixed_port = Some(port);
continue;
}
Comment on lines +228 to +246
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The argument parsing logic for --port is very similar to the existing logic for --services that follows this block. This duplication makes the code harder to read and maintain. Future changes, like adding more flags or fixing a bug in the parsing, would require modifications in multiple places.

To improve maintainability, consider refactoring this logic. A helper function could encapsulate the shared pattern of parsing flags that accept a value (e.g., --flag <value> or --flag=<value>). This would make handle_login cleaner and less prone to errors as it evolves.


let services_str = if (args[i] == "-s" || args[i] == "--services") && i + 1 < args.len() {
skip_next = true;
Some(args[i + 1].as_str())
Expand Down Expand Up @@ -271,7 +296,10 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> {
client_secret: client_secret.clone(),
auth_uri: "https://accounts.google.com/o/oauth2/auth".to_string(),
token_uri: "https://oauth2.googleapis.com/token".to_string(),
redirect_uris: vec!["http://localhost".to_string()],
redirect_uris: vec![match fixed_port {
Some(p) => format!("http://localhost:{p}"),
None => "http://localhost".to_string(),
}],
Comment on lines +299 to +302
Copy link
Contributor

Choose a reason for hiding this comment

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

high

While this block correctly configures the redirect_uris for the current authentication flow, there's a related issue with how the client configuration is persisted. When credentials are provided via environment variables, gws saves them to client_secret.json (around line 276). The save_client_config function used for this hardcodes redirect_uris to ["http://localhost"]. If a user provides a fixed port via --port, this saved configuration will be incorrect for their "Web Application" type client.

To fix this, save_client_config could be updated to accept the fixed_port and generate the correct redirect_uris in the saved file. This would involve changes in oauth_config.rs and its call sites in auth_commands.rs and setup.rs.

..Default::default()
};

Expand Down Expand Up @@ -304,7 +332,10 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> {

let auth = yup_oauth2::InstalledFlowAuthenticator::builder(
secret,
yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect,
match fixed_port {
Some(p) => yup_oauth2::InstalledFlowReturnMethod::HTTPPortRedirect(p),
None => yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect,
},
)
.with_storage(Box::new(crate::token_storage::EncryptedTokenStorage::new(
temp_path.clone(),
Expand Down
Loading