Skip to content

Conversation

evanliu048
Copy link
Contributor

@evanliu048 evanliu048 commented Sep 30, 2025

Issue #, if available:

Description of changes:
This PR adds social login (Google/GitHub) to Kiro CLI, reusing existing PKCE infra and providing a browser-based flow with clear success/error handoff back to the terminal. It also enriches the codewhispererterminal_userLoggedIn telemetry event with the social provider metadata.

User Experience
q login (--social google|github)
(user chooses google/github login) Browser opens to provider consent.
If user is the first time to login use this method, they will be prompted to input a invitation code
On success or cancel, user is redirected to our local callback which:
Validates state, exchanges code for bearer token.
Serves index.html with a friendly success/denial message.
CLI stores tokens and (if present) profile ARN.

Changed UX: Updated the post-authorization page to use more natural wording, since we need to wait to confirm whether the token exchange is successful.
image

Telemetry example:

{
  "metric_name": "codewhispererterminal_userLoggedIn",
  "epoch_timestamp": 1759526407283,
  "unit": "None",
  "value": 1.0,
  "metadata": [
    {
      "key": "credentialStartUrl",
      "value": ""
    },
    {
      "key": "codewhispererterminal_inCloudshell",
      "value": ""
    },
    {
      "key": "socialProvider",
      "value": "Google"
    }
  ],
  "passive": false
}

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@evanliu048 evanliu048 marked this pull request as draft September 30, 2025 22:06
@evanliu048 evanliu048 marked this pull request as ready for review October 3, 2025 20:33
@kensave kensave changed the title feat: Support Kiro CLI social login feat: Support Qv2 CLI social login Oct 3, 2025
// Only show success message if explicitly marked as successful
// This prevents showing success when token exchange might still fail
if (success === 'true') {
const productName = 'KIRO CLI';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This logic could be extracted to their own method.

IdentityFuture::new_boxed(Box::pin(async {
let database = Database::new().await?;

if let Ok(Some(token)) = builder_id::BuilderIdToken::load(&database).await {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check both all times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The UnifiedBearerResolver needs to support two login paths — Builder ID and Social.
A user may complete login through either method. Therefore, during the resolution phase, we need attempt both methods

Self::recv_code_with_extra_accepts(listener, expected_state, 0).await
}

// NEW: extended version that accepts N extra connections to serve /index.html again
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: NEW not needed as part of the comment,

tokio::time::sleep(Duration::from_millis(200)).await;

// Let the server task finish (best-effort)
let _ = tokio::time::timeout(Duration::from_secs(2), server_handle).await;
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the reasoning behind the 2 seconds? why is this best effort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 2-second window is a small buffer for the browser to finish loading /index.html after the 302 redirect.
By the time we receive the code and state, authentication is already complete — this delay just prevents occasional “connection refused” or partial page load issues.
It’s best-effort only: even if it fails, the CLI flow remains successful (token exchanged).
The 2-second value is empirical — long enough for typical browser follow-up requests without delaying the CLI

// Check for auth on subcommands that require it.
if self.requires_auth() && !crate::auth::is_logged_in(&mut os.database).await {
if self.requires_auth()
&& !crate::auth::is_logged_in(&mut os.database).await
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems this two methods can be combined into one is_loged_in whuch checks is_builder_id_loged_in, is_social_loged_in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants