Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c6317b9
feat: DEV/TEST environments + OAuth scope step-up (DEVEX-719)
jpage-godaddy Jun 10, 2026
c9267bd
fix: address Copilot review on environments + scope step-up
jpage-godaddy Jun 10, 2026
89eece4
fix: address Copilot round 2 on PR #57
jpage-godaddy Jun 10, 2026
1834637
fix: address Copilot round 3 on PR #57
jpage-godaddy Jun 10, 2026
e9f28b5
fix: reject empty/whitespace api_url overrides (Copilot round 4)
jpage-godaddy Jun 10, 2026
28e584d
fix: exclude empty-api_url config entries from known list + env list …
jpage-godaddy Jun 10, 2026
f8c599f
fix: require http(s):// scheme for api_url overrides (Copilot round 6)
jpage-godaddy Jun 10, 2026
717e5ad
fix: validate auth/token overrides + show active env in list (round 7)
jpage-godaddy Jun 10, 2026
7735e76
fix: single --scope/--field value silently dropped (jgowdy review)
jpage-godaddy Jun 10, 2026
85a8db3
fix: clean_url rejects empty/query-only hosts (Copilot)
jpage-godaddy Jun 10, 2026
f894447
fix: don't hard-code config path in listable warning (Copilot)
jpage-godaddy Jun 10, 2026
35b5544
fix: 403 hint repeats --scope per scope (Copilot)
jpage-godaddy Jun 10, 2026
399aa06
docs: fix stale list_environments comment (Copilot)
jpage-godaddy Jun 10, 2026
be68264
docs: describe env config path as platform-dependent (Copilot)
jpage-godaddy Jun 10, 2026
277349e
fix: case-insensitive URL scheme in clean_url (Copilot)
jpage-godaddy Jun 10, 2026
2c08e03
docs: correct OAuth override var names + get_env survival caveat (Cop…
jpage-godaddy Jun 10, 2026
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
10 changes: 5 additions & 5 deletions rust/Cargo.lock

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

2 changes: 1 addition & 1 deletion rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ path = "src/main.rs"
async-trait = "0.1"
chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] }
clap = { version = "4.5", features = ["std", "string"] }
cli-engine = { features = ["pkce-auth"], version = "0.2" }
cli-engine = { features = ["pkce-auth"], version = "0.2.1" }
dirs = "6"
fancy-regex = "0.14"
regex = { version = "1", features = ["std"] }
Expand Down
92 changes: 75 additions & 17 deletions rust/src/api_explorer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,18 @@ fn find_endpoint<'a>(catalog: &'a [Domain], query: &str) -> Option<(&'a Domain,
})
}

/// Union of user-supplied `--scope` flags and a matched endpoint's declared
/// scopes, order-preserving and de-duplicated (flags first).
fn merge_required_scopes(flag_scopes: Vec<String>, endpoint_scopes: &[String]) -> Vec<String> {
let mut required = flag_scopes;
for scope in endpoint_scopes {
if !required.contains(scope) {
required.push(scope.clone());
}
}
required
}
Comment thread
jpage-godaddy marked this conversation as resolved.

fn search_endpoints<'a>(catalog: &'a [Domain], query: &str) -> Vec<(&'a Domain, &'a Endpoint)> {
let q = query.to_lowercase();
catalog
Expand Down Expand Up @@ -432,8 +444,11 @@ fn call_command() -> RuntimeCommandSpec {
.long("scope")
.short('s')
.value_name("SCOPE")
.num_args(0..)
.help("Required OAuth scope(s); on 403 a re-auth hint is shown"),
// One value per occurrence, repeatable (`--scope a --scope b`).
// Append (vs num_args(1..)) avoids greedily consuming the
// ENDPOINT positional and still rejects a bare `--scope`.
.action(clap::ArgAction::Append)
.help("Additional required OAuth scope(s), merged with the endpoint's"),
Comment thread
jpage-godaddy marked this conversation as resolved.
),
|ctx| async move {
let endpoint = ctx
Expand All @@ -446,7 +461,26 @@ fn call_command() -> RuntimeCommandSpec {
.get("method")
.and_then(|v| v.as_str())
.unwrap_or("GET");
let token = ctx.credential().await?.token;
// Required scopes = explicit --scope flags, plus the matched catalog
// endpoint's declared scopes (best-effort: a concrete request path
// may not match a templated catalog path, in which case only --scope
// contributes). These drive OAuth scope step-up at credential time.
let flag_scopes: Vec<String> = ctx
.args
.get("scope")
.and_then(|v| v.as_array())
.map(|arr| {
arr.iter()
.filter_map(|v| v.as_str().map(str::to_owned))
.collect()
})
.unwrap_or_default();
let endpoint_scopes = find_endpoint(catalog(), endpoint)
.map(|(_, ep)| ep.scopes.as_slice())
.unwrap_or(&[]);
let required = merge_required_scopes(flag_scopes, endpoint_scopes);

let token = ctx.credential_with_scopes(&required).await?.token;
let base_url = crate::application::client::api_url_for_env(&ctx.middleware.env);
let url = format!("{base_url}{endpoint}");

Expand Down Expand Up @@ -524,23 +558,17 @@ fn call_command() -> RuntimeCommandSpec {
None
};

let scopes: Vec<String> = ctx
.args
.get("scope")
.and_then(|v| v.as_array())
.map(|arr| {
arr.iter()
.filter_map(|v| v.as_str().map(str::to_owned))
.collect()
})
.unwrap_or_default();

let body: Value = resp.json().await.unwrap_or(json!(null));

if status == 403 && !scopes.is_empty() {
// Scope step-up already ran up front (the token was requested with
// `required`). A 403 here means the granted token still lacks a
// required scope — surface it rather than silently returning the body.
if status == 403 && !required.is_empty() {
return Err(cli_engine::CliCoreError::message(format!(
"403 Forbidden — your token may be missing scope(s): {}\nRun `gddy auth login` to re-authenticate.",
scopes.join(", ")
"403 Forbidden — the authorized token is missing required scope(s): {}. \
Re-run `gddy auth login --scope {}` and try again.",
required.join(", "),
required.join(" ")
)));
Comment thread
jpage-godaddy marked this conversation as resolved.
Comment thread
jpage-godaddy marked this conversation as resolved.
}

Expand All @@ -562,3 +590,33 @@ fn call_command() -> RuntimeCommandSpec {
},
)
}

#[cfg(test)]
mod tests {
use super::merge_required_scopes;

fn v(items: &[&str]) -> Vec<String> {
items.iter().map(|s| (*s).to_owned()).collect()
}

#[test]
fn merge_flags_only_when_no_endpoint_scopes() {
assert_eq!(merge_required_scopes(v(&["a", "b"]), &[]), v(&["a", "b"]));
}

#[test]
fn merge_endpoint_only_when_no_flags() {
assert_eq!(
merge_required_scopes(v(&[]), &v(&["x", "y"])),
v(&["x", "y"])
);
}

#[test]
fn merge_unions_and_dedupes_flags_first() {
assert_eq!(
merge_required_scopes(v(&["a", "b"]), &v(&["b", "c"])),
v(&["a", "b", "c"])
);
}
}
35 changes: 31 additions & 4 deletions rust/src/application/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,36 @@ impl ApplicationClient {
}
}

pub fn api_url_for_env(env: &str) -> &'static str {
match env {
"prod" => "https://api.godaddy.com",
_ => "https://api.ote-godaddy.com",
pub fn api_url_for_env(env: &str) -> String {
crate::environments::resolve(env)
.or_else(|_| crate::environments::resolve(crate::environments::DEFAULT_ENV))
.map(|e| e.api_url)
// Never return an empty base URL (e.g. if a malformed local config makes
// even the default fail to resolve) — fall back to the built-in default.
.unwrap_or_else(|_| crate::environments::default_api_url().to_owned())
}

#[cfg(test)]
mod tests {
use super::api_url_for_env;

#[test]
fn api_url_for_builtins_resolve_to_a_url() {
// The exact host mapping is covered deterministically in
// `environments::tests`. Here we only assert the built-ins resolve to a
// URL — a dev machine may legitimately override a built-in's URL via
// env var / local config, so don't hard-code the host.
for env in ["prod", "ote"] {
let url = api_url_for_env(env);
assert!(url.contains("://"), "{env} -> {url:?}");
}
}
Comment thread
jpage-godaddy marked this conversation as resolved.

#[test]
fn api_url_for_unknown_env_falls_back_to_default_and_is_never_empty() {
// Unknown env resolves to the default environment's URL (never empty).
let url = api_url_for_env("definitely-not-a-real-env-xyz");
assert!(!url.is_empty());
assert!(url.starts_with("https://"));
}
Comment thread
jpage-godaddy marked this conversation as resolved.
}
111 changes: 59 additions & 52 deletions rust/src/auth.rs
Original file line number Diff line number Diff line change
@@ -1,87 +1,94 @@
use async_trait::async_trait;
use cli_engine::{
CliCoreError, Credential, Result,
CliCoreError, Credential, CredentialRequest, Result,
auth::{AuthProvider, pkce::PkceAuthProvider},
};

const OTE_API_URL: &str = "https://api.ote-godaddy.com";
const PROD_API_URL: &str = "https://api.godaddy.com";

const OTE_CLIENT_ID: &str = "a502484b-d7b1-4509-aa88-08b391a54c28";
const PROD_CLIENT_ID: &str = "39489dee-4103-4284-9aab-9f2452142bce";

const SCOPES: &[&str] = &["apps.app-registry:read", "apps.app-registry:write"];
use crate::environments::{self, ResolvedEnv};

/// Single auth provider that dispatches to env-specific PKCE providers.
///
/// Env var overrides (per-env):
/// OTE: OTE_OAUTH_CLIENT_ID, OTE_OAUTH_AUTH_URL, OTE_OAUTH_TOKEN_URL
/// PROD: PROD_OAUTH_CLIENT_ID, PROD_OAUTH_AUTH_URL, PROD_OAUTH_TOKEN_URL
#[derive(Debug)]
pub struct GoDaddyAuthProvider {
ote: PkceAuthProvider,
prod: PkceAuthProvider,
}
/// Each env's provider is named after the env, so cli-engine's
/// `PkceAuthProvider` picks up its per-env overrides automatically:
/// `<PREFIX>_OAUTH_CLIENT_ID`, `<PREFIX>_OAUTH_AUTH_URL`, `<PREFIX>_OAUTH_TOKEN_URL`
/// where `<PREFIX>` is the env name uppercased with `-` replaced by `_`
/// (e.g. `OTE_OAUTH_CLIENT_ID`, `DEV_OAUTH_AUTH_URL`). The API base URL and the
/// per-env defaults come from [`crate::environments`], which also resolves
/// custom DEV/TEST environments from `~/.config/gddy/environments.toml`.
Comment thread
jpage-godaddy marked this conversation as resolved.
Outdated
#[derive(Debug, Default)]
pub struct GoDaddyAuthProvider;

impl GoDaddyAuthProvider {
pub fn new() -> Self {
let ote = PkceAuthProvider::new(
"ote",
format!("{OTE_API_URL}/v2/oauth2/authorize"),
format!("{OTE_API_URL}/v2/oauth2/token"),
OTE_CLIENT_ID,
SCOPES,
)
.with_app_id("gddy")
.with_redirect_uri("http://localhost:7443/callback");

let prod = PkceAuthProvider::new(
"prod",
format!("{PROD_API_URL}/v2/oauth2/authorize"),
format!("{PROD_API_URL}/v2/oauth2/token"),
PROD_CLIENT_ID,
SCOPES,
)
.with_app_id("gddy")
.with_redirect_uri("http://localhost:7443/callback");

Self { ote, prod }
Self
}

fn provider_for(&self, env: &str) -> Result<&PkceAuthProvider> {
match env {
"ote" => Ok(&self.ote),
"prod" => Ok(&self.prod),
_ => Err(CliCoreError::message(format!(
"unknown environment {env:?}; expected \"ote\" or \"prod\""
))),
}
/// Build a PKCE provider for the given env by resolving its endpoints.
///
/// Providers are constructed on demand (tokens persist in the OS keychain,
/// so there is nothing to cache across a one-shot CLI invocation). Works for
/// built-ins as well as any custom env defined via env var or local config.
fn provider_for(&self, env: &str) -> Result<PkceAuthProvider> {
let resolved =
environments::resolve(env).map_err(|e| CliCoreError::message(e.to_string()))?;
Ok(build_provider(&resolved))
}
}
Comment thread
jpage-godaddy marked this conversation as resolved.

fn build_provider(env: &ResolvedEnv) -> PkceAuthProvider {
PkceAuthProvider::new(
env.name.clone(),
env.auth_url.clone(),
env.token_url.clone(),
env.client_id.clone(),
environments::DEFAULT_OAUTH_SCOPES,
)
.with_app_id(environments::APP_ID)
.with_redirect_uri(environments::REDIRECT_URI)
}

#[async_trait]
impl AuthProvider for GoDaddyAuthProvider {
fn name(&self) -> &str {
"godaddy"
}

async fn get_credential(&self, env: &str, command: &str, tier: &str) -> Result<Credential> {
self.provider_for(env)?
.get_credential(env, command, tier)
.await
let provider = self.provider_for(env)?;
provider.get_credential(env, command, tier).await
}

async fn get_credential_for(&self, req: &CredentialRequest<'_>) -> Result<Credential> {
// Forward to the env's PKCE provider, which performs OAuth scope step-up
// when the cached token lacks the command's required scopes.
let provider = self.provider_for(req.env)?;
provider.get_credential_for(req).await
}

async fn status(&self, env: &str) -> Result<Credential> {
self.provider_for(env)?.status(env).await
let provider = self.provider_for(env)?;
provider.status(env).await
}

async fn logout(&self, env: &str) -> Result<()> {
self.provider_for(env)?.logout(env).await
let provider = self.provider_for(env)?;
provider.logout(env).await
}

async fn list_environments(&self) -> Result<Vec<String>> {
let mut envs = self.ote.list_environments().await.unwrap_or_default();
envs.extend(self.prod.list_environments().await.unwrap_or_default());
// Enumerate stored credentials across built-ins + locally-configured
// envs (env-var-only envs are excluded from `listable`, matching the
// `env list` contract).
// Surface a malformed local config rather than silently reporting zero
// environments (which would hide stored credentials, incl. built-ins).
let listable =
environments::listable().map_err(|e| CliCoreError::message(e.to_string()))?;
Comment thread
jpage-godaddy marked this conversation as resolved.
let mut envs = Vec::new();
for resolved in listable {
let provider = build_provider(&resolved);
envs.extend(provider.list_environments().await.unwrap_or_default());
}
Comment thread
jpage-godaddy marked this conversation as resolved.
envs.sort();
envs.dedup();
Ok(envs)
}
Expand Down
Loading
Loading