Skip to content
Merged
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
41 changes: 41 additions & 0 deletions cli/src/auth/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,26 @@ impl IntegrationConfig {
self.overlay.source
}

/// Whether the resolved client is the bundled first-party app (no BYO
/// profile/env client_id). Used to gate behaviors that only the bundled app
/// can't do — e.g. the bundled Google Desktop client can't do device-code,
/// but a BYO Google client may. (#151)
pub fn is_first_party(&self) -> bool {
self.overlay.source == AppSource::FirstParty
}

pub fn app_source_label(&self) -> &'static str {
self.overlay.source.label()
}

/// Whether the resolved client id is still a bundled placeholder — i.e. no
/// real first-party app is registered for this integration and no BYO override
/// (profile/env) supplied one. Provider OAuth flows can't work with it, so the
/// caller should point the user at a BYO profile (#146).
pub fn is_placeholder_client(&self) -> bool {
self.client_id().starts_with("AWARE_AECO_PLACEHOLDER")
}

/// Resolve the client id: profile ▸ env var ▸ bundled default.
pub fn client_id(&self) -> String {
if let Some(p) = &self.overlay.profile
Expand Down Expand Up @@ -393,6 +409,31 @@ mod tests {
assert!(m.client_secret().is_none());
}

#[test]
fn trimble_client_is_placeholder_until_registered() {
// Trimble has no bundled app yet (#153); M365/Google got real apps in #145.
// Guard against a BYO env override that would resolve a real client_id.
if std::env::var("AWARE_OAUTH_TRIMBLE_CLIENT_ID").is_err() {
assert!(
for_integration("trimble-connect")
.unwrap()
.is_placeholder_client()
);
}
assert!(
!for_integration("microsoft-365")
.unwrap()
.is_placeholder_client()
);
if std::env::var("AWARE_OAUTH_GOOGLE_CLIENT_ID").is_err() {
assert!(
!for_integration("google-workspace")
.unwrap()
.is_placeholder_client()
);
}
}

#[test]
fn client_id_falls_back_to_default_when_env_unset() {
// Use an integration not normally in tests' env
Expand Down
69 changes: 61 additions & 8 deletions cli/src/auth/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,29 @@ fn device_endpoints_for(cfg: &IntegrationConfig, tenant: Option<&str>) -> Device
// For the remaining providers there is no tenant substitution, so
// `token_url()` already yields the resolved value (profile override ▸
// bundled). A profile `device_authorization_url` override is honored too.
"google-workspace" => DeviceEndpoints {
device_authorization_url: cfg
"google-workspace" => {
// The *bundled* Google client is a Desktop/loopback client; Google's
// device-authorization flow requires a separate "TV & Limited Input"
// client, so the bundled client just 401s on `/device/code`. For it we
// report device-code unsupported and point at --oauth, instead of
// leaking a raw 401 (#151). A BYO client (profile/env) may be
// device-capable, so it gets an explicit override or Google's standard
// device endpoint.
let device_authorization_url = cfg
.device_authorization_url_override()
.unwrap_or("https://oauth2.googleapis.com/device/code")
.to_string(),
token_url: cfg.token_url().to_string(),
},
.map(String::from)
.unwrap_or_else(|| {
if cfg.is_first_party() {
String::new()
} else {
"https://oauth2.googleapis.com/device/code".to_string()
}
});
DeviceEndpoints {
device_authorization_url,
token_url: cfg.token_url().to_string(),
}
}
"trimble-connect" => DeviceEndpoints {
// Trimble Identity supports OAuth device-code per the docs, but the
// endpoint URL isn't published as a stable identifier — the standard
Expand Down Expand Up @@ -346,10 +362,47 @@ mod tests {
}

#[test]
fn google_has_fixed_endpoints() {
fn google_device_code_unsupported_by_default() {
// Bundled Google client can't do device-code (Desktop client → 401), so the
// default device endpoint is empty and the flow reports it unsupported (#151).
let cfg = config::for_integration("google-workspace").unwrap();
let e = device_endpoints_for(&cfg, None);
assert_eq!(e.device_authorization_url, "https://oauth2.googleapis.com/device/code");
assert!(e.device_authorization_url.is_empty());
}

#[test]
fn google_byo_client_uses_standard_device_endpoint() {
// A BYO Google client (profile client_id, no explicit device endpoint) is
// not the bundled Desktop client, so it gets Google's standard device
// endpoint rather than being disabled. (#151 review)
let tmp = tempfile::tempdir().unwrap();
let dir = tmp.path().join("oauth");
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("google-workspace.yaml"), "client_id: org-google\n").unwrap();
let cfg = config::for_integration("google-workspace")
.unwrap()
.with_profile(tmp.path(), None)
.unwrap();
let e = device_endpoints_for(&cfg, None);
assert_eq!(
e.device_authorization_url,
"https://oauth2.googleapis.com/device/code"
);
}

#[test]
fn google_device_code_flow_errors_with_use_oauth_hint() {
// End-to-end: the bundled Google client surfaces a clean "use --oauth"
// error, not a raw 401 (#151).
let cfg = config::for_integration("google-workspace").unwrap();
let err = run_device_code_flow(&cfg, None, &[], false).unwrap_err();
match err {
AwareError::Validation(msg) => {
assert!(msg.contains("does not support device-code"));
assert!(msg.contains("--oauth"));
}
other => panic!("expected Validation, got {other:?}"),
}
}

#[test]
Expand Down
9 changes: 9 additions & 0 deletions cli/src/commands/connect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,15 @@ pub fn run_connect(args: ConnectArgs, ctx: &Context) -> Result<(), AwareError> {
let cfg = crate::auth::config::for_integration(integration)?
.with_profile(&ctx.paths.aware_home, args.r#as.as_deref())?
.with_tenant(args.tenant.as_deref());
// No real bundled app (still the placeholder client_id) and no BYO override
// → fail fast instead of sending a placeholder to the provider, which just
// returns "Unregistered client" (#153).
if cfg.is_placeholder_client() {
return Err(AwareError::Validation(format!(
"{integration} has no bundled OAuth app yet — register your own and configure a \
BYO profile at ~/.aware/oauth/{integration}.yaml (see 10-core/oauth-registration.md, #146)"
)));
}
let extra_scopes = parse_scopes(args.scopes.as_deref());
crate::auth::pkce::run_pkce_flow(&cfg, &extra_scopes)?
} else {
Expand Down
37 changes: 37 additions & 0 deletions cli/tests/connect_byo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,43 @@
use assert_cmd::Command;
use predicates::prelude::*;

/// `trimble-connect --oauth` has no bundled app (placeholder client_id), so it
/// must fail fast with a BYO hint rather than sending the placeholder to Trimble
/// ("Unregistered client"). (#153)
#[test]
fn trimble_oauth_without_bundled_app_fails_with_byo_hint() {
let tmp = tempfile::tempdir().unwrap();
Command::cargo_bin("aware")
.unwrap()
.env("AWARE_HOME", tmp.path())
.env("AWARE_DISABLE_KEYRING", "1")
.env_remove("AWARE_OAUTH_TRIMBLE_CLIENT_ID")
.args(["connect", "trimble-connect", "--oauth"])
.assert()
.failure()
.stderr(predicate::str::contains("no bundled OAuth app"));
}

/// `google-workspace --device-code` must report device-code unsupported (point at
/// --oauth) rather than leaking a raw Google 401. (#151)
#[test]
fn google_device_code_reports_unsupported_not_raw_401() {
let tmp = tempfile::tempdir().unwrap();
Command::cargo_bin("aware")
.unwrap()
.env("AWARE_HOME", tmp.path())
.env("AWARE_DISABLE_KEYRING", "1")
// Exercise the bundled default client — clear any BYO env overrides so the
// test doesn't turn Google into a (device-capable) BYO client and hit the net.
.env_remove("AWARE_OAUTH_GOOGLE_CLIENT_ID")
.env_remove("AWARE_OAUTH_GOOGLE_CLIENT_SECRET")
.args(["--json", "connect", "google-workspace", "--device-code"])
.assert()
.success()
.stdout(predicate::str::contains("does not support device-code"))
.stdout(predicate::str::contains("--oauth"));
}

/// A malformed BYO OAuth profile must NOT block the non-OAuth token-import paths
/// (`--from-file`), since those don't use OAuth config at all. Regression for the
/// eager-profile-load issue found in review.
Expand Down