feat(auth): add --port flag for orgs that block Desktop OAuth#559
feat(auth): add --port flag for orgs that block Desktop OAuth#559VictorML11 wants to merge 3 commits intogoogleworkspace:mainfrom
Conversation
Organizations with strict Google Workspace admin policies (admin_policy_enforced) block the "installed" (Desktop) OAuth flow that uses a random port. This adds a --port flag that uses yup_oauth2's HTTPPortRedirect with a fixed port, enabling authentication with Web Application type OAuth clients. Usage: gws auth login --port 8080 When --port is provided, uses HTTPPortRedirect(port) with a fixed redirect URI (http://localhost:<port>). When omitted, preserves current behavior (random port, Desktop flow). Closes googleworkspace#557 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a --port flag to gws auth login to support organizations that block the standard Desktop OAuth flow. The implementation correctly uses a fixed port when the flag is provided, while preserving the existing random-port behavior otherwise. However, there is a critical issue in src/oauth_config.rs where the generated client_secret.json is hardcoded to use port 8080, which is misleading and inconsistent with the default flow. This change is unnecessary and introduces scope creep, therefore it should be reverted.
Reverts the change to save_client_config that hardcoded port 8080 in the client_secret.json redirect_uris. This was unnecessary because: - handle_login overwrites redirect_uris based on the --port flag - The default Desktop flow expects http://localhost (no port) - Hardcoding 8080 would confuse users who run setup without --port The --port flag in auth_commands.rs handles the redirect URI dynamically — no config file changes needed.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a --port flag to the gws auth login command, allowing users to specify a fixed port for the OAuth redirect server. This is a valuable addition for users in organizations that block the default Desktop OAuth flow. The implementation correctly parses the new flag and conditionally adjusts the OAuth authenticator and redirect URI. My review identified one area for improvement: the provided port number is not validated to disallow port 0. Using port 0 would cause the operating system to select a random ephemeral port, which negates the purpose of this feature. I have provided a suggestion to add this validation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully adds the --port flag to support OAuth flows in environments that block the standard desktop flow. The implementation is clean and follows existing patterns. I've identified two high-severity issues: one related to code duplication in argument parsing that impacts maintainability, and another where the saved client_secret.json can contain an incorrect redirect_uris value when the new --port flag is used with environment variables. Addressing these will make the implementation more robust and maintainable.
| // 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; | ||
| } |
There was a problem hiding this comment.
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.
| redirect_uris: vec![match fixed_port { | ||
| Some(p) => format!("http://localhost:{p}"), | ||
| None => "http://localhost".to_string(), | ||
| }], |
There was a problem hiding this comment.
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.
Summary
Adds a
--portflag togws auth loginthat uses a fixed port for the OAuth redirect server, enabling authentication for users in organizations that block the Desktop/installed OAuth flow viaadmin_policy_enforced.Problem
Organizations with strict Google Workspace admin policies block the "installed" (Desktop) OAuth flow. The current implementation uses
yup_oauth2::InstalledFlowReturnMethod::HTTPRedirectwhich binds to a random port — Google identifies this as a Desktop flow pattern and some orgs block it.Solution
--port <PORT>flag togws auth loginHTTPPortRedirect(port)with a fixed redirect URI (http://localhost:<port>) — compatible with Web Application type OAuth clientsUsage
Users with
admin_policy_enforcedwould:http://localhost:8080gws auth login --port 8080Changes
src/auth_commands.rs: Parse--portflag, conditionally useHTTPPortRedirect(port)and fixed redirect URITest plan
cargo build --release)--port: existing behavior preserved (random port)--port 8080: fixed port used, Web Application OAuth client worksCloses #557