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
151 changes: 100 additions & 51 deletions tvc/src/commands/login.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//! Login command for authenticating with Turnkey.

use crate::config::turnkey::{
Config, KeyCurve, OrgConfig, StoredApiKey, StoredQosOperatorKey, API_BASE_URL_DEV,
API_BASE_URL_LOCAL, API_BASE_URL_PREPROD, API_BASE_URL_PROD,
Config, KeyCurve, OrgConfig, StoredApiKey, StoredQosOperatorKey, API_BASE_URL_PROD,
};
use crate::prompts;
use anyhow::{anyhow, bail, Context, Result};
Expand All @@ -13,37 +12,6 @@ use tracing::debug;
use turnkey_api_key_stamper::TurnkeyP256ApiKey;
use turnkey_client::generated::GetWhoamiRequest;

/// Turnkey API environment selectable during login.
#[derive(Debug, Clone, Copy)]
enum ApiEnv {
Prod,
Preprod,
Dev,
Local,
}

impl ApiEnv {
fn url(self) -> &'static str {
match self {
ApiEnv::Prod => API_BASE_URL_PROD,
ApiEnv::Preprod => API_BASE_URL_PREPROD,
ApiEnv::Dev => API_BASE_URL_DEV,
ApiEnv::Local => API_BASE_URL_LOCAL,
}
}
}

impl std::fmt::Display for ApiEnv {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ApiEnv::Prod => write!(f, "prod ({API_BASE_URL_PROD})"),
ApiEnv::Preprod => write!(f, "preprod ({API_BASE_URL_PREPROD})"),
ApiEnv::Dev => write!(f, "dev ({API_BASE_URL_DEV})"),
ApiEnv::Local => write!(f, "local ({API_BASE_URL_LOCAL})"),
}
}
}

/// Authenticate with Turnkey and set up local credentials.
#[derive(Debug, ClapArgs)]
#[command(about, long_about = None)]
Expand All @@ -52,20 +20,29 @@ pub struct Args {
/// If not provided, will prompt interactively.
#[arg(long, env = "TVC_ORG")]
pub org: Option<String>,
/// Turnkey API base URL. Defaults to production for newly configured orgs.
#[arg(long, env = "TVC_API_BASE_URL", value_name = "URL")]
pub api_base_url: Option<String>,
Comment on lines +24 to +25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: "BASE_URL" seems redundant to customers?

Suggested change
#[arg(long, env = "TVC_API_BASE_URL", value_name = "URL")]
pub api_base_url: Option<String>,
#[arg(long, env = "TVC_API_URL", value_name = "URL")]
pub api_url: Option<String>,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

URL on its own can include paths, query-params, etc. BASE_URL implies that this is a full scheme + host.

}

/// Run the login command.
pub async fn run(args: Args) -> anyhow::Result<()> {
debug!(
org_arg_present = args.org.is_some(),
api_base_url_override_present = args.api_base_url.is_some(),
"running login command"
);

// Load existing config
let mut config = Config::load().await?;

// Select or create org
let (alias, org_config) = select_or_create_org(&mut config, args.org.as_deref()).await?;
let (alias, org_config) = select_or_create_org(
&mut config,
args.org.as_deref(),
args.api_base_url.as_deref(),
)
.await?;
Comment on lines 36 to +45
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think code and logic would be much clearer if we resolve the base_url here before going into the select_or_create_org() and nested logic.

Some ideas:

  • we could add a field/enum to tvc::config::turnkey::Config that defaults to API_BASE_URL_PROD, or the actual URL is fine too. Then, pass the config into select_or_create_org(). That seems clean
  • if you dont want to do that, call update_api_base_url_from_override() here instead of within the func, and then you don't have to do it later

either approach reads cleaner to me than the nested if let Some() in current logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

100% agree. I poked around and tried to make a small change, but a larger refactor in order:

https://linear.app/turnkey/issue/SYS-100/flatten-tvc-login-org-selection-and-api-base-url-override-flow


println!("Selected org: {} ({})", alias, org_config.id);

Expand Down Expand Up @@ -112,19 +89,24 @@ pub async fn run(args: Args) -> anyhow::Result<()> {
async fn select_or_create_org(
config: &mut Config,
org_arg: Option<&str>,
api_base_url_override: Option<&str>,
) -> Result<(String, OrgConfig)> {
debug!(
org_arg_present = org_arg.is_some(),
api_base_url_override_present = api_base_url_override.is_some(),
configured_org_count = config.orgs.len(),
active_org = ?config.active_org,
"selecting organization"
);

// If --org provided, try to find it by alias or ID
if let Some(org) = org_arg {
if let Some((alias, org_config)) = find_org(config, org) {
if let Some((alias, _)) = find_org(config, org) {
let alias = alias.clone();
debug!(org_alias = %alias, "selected existing organization from argument");
return Ok((alias.clone(), org_config.clone()));
update_api_base_url_from_override(config, &alias, api_base_url_override);
let org_config = config.orgs.get(&alias).unwrap().clone();
return Ok((alias, org_config));
}
debug!("organization argument did not match configured organizations");
bail!("Organization '{org}' not found. Run `tvc login` without --org to set up a new organization.");
Expand All @@ -140,7 +122,7 @@ async fn select_or_create_org(
// No orgs configured - prompt for new org
debug!("no organizations configured; prompting for new organization");
println!("No organization configured.");
return prompt_for_new_org(config).await;
return prompt_for_new_org(config, api_base_url_override).await;
}

// Show existing orgs in a `Select` list
Expand All @@ -163,10 +145,11 @@ async fn select_or_create_org(

match prompts::select("Select organization", options)? {
OrgChoice::Existing { alias, .. } => {
update_api_base_url_from_override(config, &alias, api_base_url_override);
let org_config = config.orgs.get(&alias).unwrap().clone();
Ok((alias, org_config))
}
OrgChoice::New => prompt_for_new_org(config).await,
OrgChoice::New => prompt_for_new_org(config, api_base_url_override).await,
}
}

Expand All @@ -186,7 +169,10 @@ impl std::fmt::Display for OrgChoice {
}

/// Prompt the user to enter a new organization ID and alias.
async fn prompt_for_new_org(config: &mut Config) -> Result<(String, OrgConfig)> {
async fn prompt_for_new_org(
config: &mut Config,
api_base_url_override: Option<&str>,
) -> Result<(String, OrgConfig)> {
debug!("prompting for new organization");

println!("You can find your Organization ID at: https://app.turnkey.com/dashboard/welcome");
Expand All @@ -200,25 +186,31 @@ async fn prompt_for_new_org(config: &mut Config) -> Result<(String, OrgConfig)>
let alias = prompts::text("Organization alias", Some("default"))?;
debug!(org_alias = %alias, "user selected organization");

// Prompt for API base URL
let api_base_url = prompt_for_api_url()?;
let api_base_url = new_org_api_base_url(api_base_url_override);
debug!(org_alias = %alias, %api_base_url, "adding prompted organization");

config.add_org(&alias, org_id, api_base_url)?;
let org_config = config.orgs.get(&alias).unwrap().clone();
Ok((alias, org_config))
}

/// Prompt the user to select a Turnkey API URL.
///
/// Only reachable in TTY mode — `select_or_create_org` calls
/// `bail_if_non_interactive` upstream before we get here.
fn prompt_for_api_url() -> Result<String> {
let env = prompts::select(
"Select Turnkey API URL",
vec![ApiEnv::Prod, ApiEnv::Preprod, ApiEnv::Dev, ApiEnv::Local],
)?;
Ok(env.url().to_string())
fn new_org_api_base_url(api_base_url_override: Option<&str>) -> String {
api_base_url_override
.unwrap_or(API_BASE_URL_PROD)
.to_string()
}

fn update_api_base_url_from_override(
config: &mut Config,
alias: &str,
api_base_url_override: Option<&str>,
) {
if let Some(api_base_url) = api_base_url_override {
debug!(org_alias = alias, %api_base_url, "updating organization API base URL from override");
if let Some(org_config) = config.orgs.get_mut(alias) {
org_config.api_base_url = api_base_url.to_string();
}
}
}

/// Get an existing API key or generate a new one.
Expand Down Expand Up @@ -390,3 +382,60 @@ async fn verify_credentials(
user_id: response.user_id,
})
}

#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
use std::path::PathBuf;

const OVERRIDE_URL: &str = "http://127.0.0.1:8081";

#[test]
fn new_org_api_base_url_defaults_to_prod() {
assert_eq!(new_org_api_base_url(None), API_BASE_URL_PROD);
}

#[test]
fn new_org_api_base_url_uses_override() {
assert_eq!(new_org_api_base_url(Some(OVERRIDE_URL)), OVERRIDE_URL);
}

#[test]
fn absent_override_preserves_existing_org_api_base_url() {
let mut config = config_with_org("http://existing.example");

update_api_base_url_from_override(&mut config, "default", None);

assert_eq!(
config.orgs["default"].api_base_url,
"http://existing.example"
);
}

#[test]
fn explicit_override_updates_existing_org_api_base_url() {
let mut config = config_with_org(API_BASE_URL_PROD);

update_api_base_url_from_override(&mut config, "default", Some(OVERRIDE_URL));

assert_eq!(config.orgs["default"].api_base_url, OVERRIDE_URL);
}

fn config_with_org(api_base_url: &str) -> Config {
Config {
active_org: Some("default".to_string()),
orgs: HashMap::from([(
"default".to_string(),
OrgConfig {
id: "org-test".to_string(),
api_key_path: PathBuf::from("api_key.json"),
operator_key_path: PathBuf::from("operator.json"),
api_base_url: api_base_url.to_string(),
},
)]),
last_created_app_id: HashMap::new(),
last_operator_ids: HashMap::new(),
}
}
}
11 changes: 11 additions & 0 deletions tvc/tests/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,14 @@ fn login_errors_when_provided_org_not_found() {
"Organization 'does-not-exist' not found",
));
}

#[test]
fn login_help_shows_api_base_url_override() {
cargo_bin_cmd!("tvc")
.arg("login")
.arg("--help")
.assert()
.success()
.stdout(predicate::str::contains("--api-base-url"))
.stdout(predicate::str::contains("TVC_API_BASE_URL"));
}
Loading