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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/chat-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ jsonschema.workspace = true
zip.workspace = true
rmcp.workspace = true
serde_yaml.workspace = true
urlencoding = "2.1.3"

[target.'cfg(unix)'.dependencies]
nix.workspace = true
Expand Down
6 changes: 3 additions & 3 deletions crates/chat-cli/src/api_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use crate::api_client::model::{
};
use crate::api_client::opt_out::OptOutInterceptor;
use crate::api_client::send_message_output::SendMessageOutput;
use crate::auth::builder_id::BearerResolver;
use crate::auth::UnifiedBearerResolver;
use crate::aws_common::{
UserAgentOverrideInterceptor,
app_name,
Expand Down Expand Up @@ -122,7 +122,7 @@ impl ApiClient {
.http_client(crate::aws_common::http_client::client())
.interceptor(OptOutInterceptor::new(database))
.interceptor(UserAgentOverrideInterceptor::new())
.bearer_token_resolver(BearerResolver)
.bearer_token_resolver(UnifiedBearerResolver)
.app_name(app_name())
.endpoint_url(endpoint.url())
.build(),
Expand Down Expand Up @@ -183,7 +183,7 @@ impl ApiClient {
.interceptor(OptOutInterceptor::new(database))
.interceptor(UserAgentOverrideInterceptor::new())
.interceptor(DelayTrackingInterceptor::new())
.bearer_token_resolver(BearerResolver)
.bearer_token_resolver(UnifiedBearerResolver)
.app_name(app_name())
.endpoint_url(endpoint.url())
.retry_classifier(retry_classifier::QCliRetryClassifier::new())
Expand Down
2 changes: 2 additions & 0 deletions crates/chat-cli/src/auth/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub(crate) const SCOPES: &[&str] = &[

pub(crate) const CLIENT_TYPE: &str = "public";

pub const SOCIAL_AUTH_SERVICE_ENDPOINT: &str = "https://prod.us-east-1.auth.desktop.kiro.dev";

// The start URL for public builder ID users
pub const START_URL: &str = "https://view.awsapps.com/start";

Expand Down
52 changes: 40 additions & 12 deletions crates/chat-cli/src/auth/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -156,23 +156,51 @@ <h4>Request denied</h4>
const params = new URLSearchParams(window.location.search)

const error = params.get('error')
const success = params.get('success');
if (error) {
showErrorMessage(error)
return
}

const productName = 'Amazon Q for CLI'
document.getElementById(
'approvalMessage'
).innerText = `${productName} have been given requested permissions`
document.getElementById(
'footerText'
).innerText = `You can close this window and start using ${productName}`

function showErrorMessage(errorText) {
document.getElementById('approved-auth').classList.add('hidden')
document.getElementById('denied-auth').classList.remove('hidden')
document.getElementById('errorMessage').innerText = errorText

// 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.

document.getElementById('approvalMessage').innerText =
`${productName} has been granted given requested permissions.`;
document.getElementById('footerText').innerText =
`You can close this window and start using ${productName}`;
} else {
// Show neutral "processing" message
document.getElementById('approvalMessage').innerText =
'Processing authentication...';
document.getElementById('footerText').innerText =
'You can close this window and check the terminal for results.';
}

function showErrorMessage(errorParam) {
document.getElementById('approved-auth').classList.add('hidden');
document.getElementById('denied-auth').classList.remove('hidden');

// Map error codes to user-friendly messages
let errorMessage = '';
let retryMessage = 'You can close this window and try again.';

const decodedError = decodeURIComponent(errorParam);

if (decodedError === 'access_denied' || decodedError.includes('denied')) {
errorMessage = 'You cancelled the authentication or denied access. Kiro CLI requires the requested permissions to function properly.';
} else if (decodedError.includes('invalid') || decodedError.includes('invitation')) {
errorMessage = 'Invalid access code. Please check your invitation code and try again.';
} else if (decodedError.includes('timeout')) {
errorMessage = 'Authentication timed out. Please try again.';
} else {
errorMessage = `Authentication error: ${decodedError}`;
}

document.getElementById('errorMessage').innerText = errorMessage;
document.getElementById('retryText').innerText = retryMessage;
}
}
</script>
Expand Down
55 changes: 55 additions & 0 deletions crates/chat-cli/src/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ mod consts;
pub mod pkce;
mod scope;

pub mod social;
use aws_sdk_ssooidc::config::{
ConfigBag,
RuntimeComponents,
};
use aws_sdk_ssooidc::error::SdkError;
use aws_sdk_ssooidc::operation::create_token::CreateTokenError;
use aws_sdk_ssooidc::operation::register_client::RegisterClientError;
use aws_sdk_ssooidc::operation::start_device_authorization::StartDeviceAuthorizationError;
use aws_smithy_runtime_api::client::identity::http::Token;
use aws_smithy_runtime_api::client::identity::{
Identity,
IdentityFuture,
ResolveIdentity,
};
pub use builder_id::{
is_logged_in,
logout,
Expand All @@ -15,6 +26,7 @@ pub use consts::START_URL;
use thiserror::Error;

use crate::aws_common::SdkErrorDisplay;
use crate::database::Database;

#[derive(Debug, Error)]
pub enum AuthError {
Expand Down Expand Up @@ -48,6 +60,19 @@ pub enum AuthError {
OAuthCustomError(String),
#[error(transparent)]
DatabaseError(#[from] crate::database::DatabaseError),
#[error(transparent)]
Reqwest(#[from] reqwest::Error),
#[error("HTTP error: {0}")]
HttpStatus(reqwest::StatusCode),
// Social auth specific errors
#[error(
"Authentication failed: The identity provider denied access. Please ensure you grant all required permissions."
)]
SocialAuthProviderDeniedAccess,
#[error("Authentication failed: The identity provider reported an error: {0}")]
SocialAuthProviderFailure(String),
#[error("Invalid access code. Please check your invitation code and try again.")]
SocialInvalidInvitationCode,
}

impl From<aws_sdk_ssooidc::Error> for AuthError {
Expand All @@ -73,3 +98,33 @@ impl From<SdkError<StartDeviceAuthorizationError>> for AuthError {
Self::SdkStartDeviceAuthorization(Box::new(value))
}
}
/// Unified bearer token resolver that tries both social and builder ID tokens
#[derive(Debug, Clone)]
pub struct UnifiedBearerResolver;

impl ResolveIdentity for UnifiedBearerResolver {
fn resolve_identity<'a>(
&'a self,
_runtime_components: &'a RuntimeComponents,
_config_bag: &'a ConfigBag,
) -> IdentityFuture<'a> {
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

return Ok(Identity::new(
Token::new(token.access_token.0.clone(), Some(token.expires_at.into())),
Some(token.expires_at.into()),
));
}

if let Ok(Some(token)) = social::SocialToken::load(&database).await {
return Ok(Identity::new(
Token::new(token.access_token.0.clone(), Some(token.expires_at.into())),
Some(token.expires_at.into()),
));
}
Err(AuthError::NoToken.into())
}))
}
}
108 changes: 75 additions & 33 deletions crates/chat-cli/src/auth/pkce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;
use std::time::Duration;

pub use aws_sdk_ssooidc::client::Client;
Expand Down Expand Up @@ -282,42 +283,83 @@ impl PkceRegistration {
Ok(())
}

async fn recv_code(listener: TcpListener, expected_state: String) -> Result<String, AuthError> {
async fn recv_code(listener: tokio::net::TcpListener, expected_state: String) -> Result<String, AuthError> {
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,

pub async fn recv_code_with_extra_accepts(
listener: tokio::net::TcpListener,
expected_state: String,
extra_accepts: usize,
) -> Result<String, AuthError> {
// Channel for delivering (code, state) from any served connection
let (code_tx, mut code_rx) = tokio::sync::mpsc::channel::<Result<(String, String), AuthError>>(1);
let (stream, _) = listener.accept().await?;
let stream = TokioIo::new(stream); // Wrapper to implement Hyper IO traits for Tokio types.
let code_tx_arc = Arc::new(code_tx);

// Host used by the service to 302 -> /index.html
let host = listener.local_addr()?.to_string();
tokio::spawn(async move {
if let Err(err) = http1::Builder::new()
.serve_connection(stream, PkceHttpService {
code_tx: std::sync::Arc::new(code_tx),
host,
})
.await
{
error!(?err, "Error occurred serving the connection");

// Serve multiple connections to handle both /oauth/callback and /index.html
// Keep the behavior as close as possible to the previous version:
// first connection is typically the callback, but we allow extra accepts
// (callback + potential error redirect + index.html).
let max_accepts = 1 + extra_accepts;
let server_handle = tokio::spawn({
let code_tx_arc = code_tx_arc.clone();
let host = host.clone();
async move {
for _ in 0..max_accepts {
if let Ok((stream, _)) = listener.accept().await {
let stream = TokioIo::new(stream);
let service = PkceHttpService {
code_tx: code_tx_arc.clone(),
host: host.clone(),
};
tokio::spawn(async move {
if let Err(err) = http1::Builder::new().serve_connection(stream, service).await {
debug!(?err, "Error serving connection");
}
});
} else {
// Accept error; break to avoid a tight loop
break;
}
}
}
});
match code_rx.recv().await {
Some(Ok((code, state))) => {
debug!(code = "<redacted>", state, "Received code and state");
if state != expected_state {
return Err(AuthError::OAuthStateMismatch {
actual: state,
expected: expected_state,
});

// Wait for authorization code (or an error) with the same timeout semantics
let result = tokio::select! {
msg = code_rx.recv() => {
match msg {
Some(Ok((code, state))) => {
debug!(code = "<redacted>", state, "Received code and state");
if state != expected_state {
Err(AuthError::OAuthStateMismatch {
actual: state,
expected: expected_state,
})
} else {
Ok(code)
}
}
Some(Err(e)) => Err(e),
None => Err(AuthError::OAuthMissingCode),
}
// Give time for the user to be redirected to index.html.
tokio::time::sleep(Duration::from_millis(200)).await;
Ok(code)
},
Some(Err(err)) => {
// Give time for the user to be redirected to index.html.
tokio::time::sleep(Duration::from_millis(200)).await;
Err(err)
},
None => Err(AuthError::OAuthMissingCode),
}
}
_ = tokio::time::sleep(DEFAULT_AUTHORIZATION_TIMEOUT) => {
Err(AuthError::OAuthTimeout)
}
};

// Small grace period so the browser can load /index.html after the 302
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


result
}
}

Expand Down Expand Up @@ -459,14 +501,14 @@ impl PkceQueryParams {
/// Generates a random 43-octet URL safe string according to the RFC recommendation.
///
/// Reference: https://datatracker.ietf.org/doc/html/rfc7636#section-4.1
fn generate_code_verifier() -> String {
pub fn generate_code_verifier() -> String {
URL_SAFE.encode(rand::random::<[u8; 32]>()).replace('=', "")
}

/// Base64 URL encoded sha256 hash of the code verifier.
///
/// Reference: https://datatracker.ietf.org/doc/html/rfc7636#section-4.2
fn generate_code_challenge(code_verifier: &str) -> String {
pub fn generate_code_challenge(code_verifier: &str) -> String {
use sha2::{
Digest,
Sha256,
Expand Down
Loading
Loading