From 24c32c4f042262e19d685e9606b7d208b4d54210 Mon Sep 17 00:00:00 2001 From: 514sid Date: Sat, 6 Jun 2026 12:34:14 +0400 Subject: [PATCH 01/31] Add multi-profile auth: login --name, logout --name, auth list, auth switch --- docs/CommandLineHelp.md | 49 ++++++- src/authentication.rs | 304 +++++++++++++++++++++++++++++++++++----- src/cli.rs | 142 +++++++++++++++++-- 3 files changed, 448 insertions(+), 47 deletions(-) diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index 797429bd..50fceae4 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -7,6 +7,9 @@ This document contains the help content for the `screenly` command-line program. * [`screenly`↴](#screenly) * [`screenly login`↴](#screenly-login) * [`screenly logout`↴](#screenly-logout) +* [`screenly auth`↴](#screenly-auth) +* [`screenly auth list`↴](#screenly-auth-list) +* [`screenly auth switch`↴](#screenly-auth-switch) * [`screenly screen`↴](#screenly-screen) * [`screenly screen list`↴](#screenly-screen-list) * [`screenly screen get`↴](#screenly-screen-get) @@ -58,6 +61,7 @@ Command line interface is intended for quick interaction with Screenly through t * `login` — Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token * `logout` — Logs out and removes the stored token +* `auth` — Manage stored authentication profiles * `screen` — Screen related commands * `asset` — Asset related commands * `playlist` — Playlist related commands @@ -74,7 +78,11 @@ Command line interface is intended for quick interaction with Screenly through t Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token -**Usage:** `screenly login` +**Usage:** `screenly login [OPTIONS]` + +###### **Options:** + +* `--name ` — Profile name to store the token under. Required when other profiles already exist @@ -82,7 +90,44 @@ Logs in with the provided token and stores it for further use if valid. You can Logs out and removes the stored token -**Usage:** `screenly logout` +**Usage:** `screenly logout [OPTIONS]` + +###### **Options:** + +* `--name ` — Profile name to remove. Removes the active profile if not specified + + + +## `screenly auth` + +Manage stored authentication profiles + +**Usage:** `screenly auth ` + +###### **Subcommands:** + +* `list` — List stored authentication profiles +* `switch` — Switch the active authentication profile + + + +## `screenly auth list` + +List stored authentication profiles + +**Usage:** `screenly auth list` + + + +## `screenly auth switch` + +Switch the active authentication profile + +**Usage:** `screenly auth switch [NAME]` + +###### **Arguments:** + +* `` — Profile name to activate diff --git a/src/authentication.rs b/src/authentication.rs index ae14e673..c623887b 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -1,7 +1,10 @@ +use std::collections::HashMap; use std::{env, fs}; use reqwest::header::{HeaderMap, InvalidHeaderValue}; use reqwest::{header, StatusCode}; +use serde::{Deserialize, Serialize}; +use serde_yaml; use thiserror::Error; // For compatability reasons - let's leave build env as well. @@ -19,6 +22,8 @@ pub enum AuthenticationError { WrongCredentials, #[error("no credentials error")] NoCredentials, + #[error("profile not found: {0}")] + ProfileNotFound(String), #[error("request error")] Request(#[from] reqwest::Error), #[error("i/o error")] @@ -29,10 +34,18 @@ pub enum AuthenticationError { MissingHomeDir(), #[error("invalid header error")] InvalidHeader(#[from] InvalidHeaderValue), + #[error("yaml error")] + Yaml(#[from] serde_yaml::Error), #[error("unknown error")] Unknown, } +#[derive(Serialize, Deserialize, Default)] +struct TokenStore { + active: Option, + tokens: HashMap, +} + pub struct Authentication { pub config: Config, pub token: String, @@ -57,6 +70,35 @@ impl Config { } } +fn screenly_path() -> Result { + dirs::home_dir() + .map(|h| h.join(".screenly")) + .ok_or(AuthenticationError::MissingHomeDir()) +} + +fn read_store() -> Result { + let path = screenly_path()?; + if !path.exists() { + return Ok(TokenStore::default()); + } + let contents = fs::read_to_string(&path)?; + if let Ok(store) = serde_yaml::from_str::(&contents) { + return Ok(store); + } + // Backward compat: plain text token → migrate to "default" profile + let token = contents.trim().to_string(); + let mut store = TokenStore::default(); + store.tokens.insert("default".to_string(), token); + store.active = Some("default".to_string()); + Ok(store) +} + +fn write_store(store: &TokenStore) -> Result<(), AuthenticationError> { + let path = screenly_path()?; + fs::write(path, serde_yaml::to_string(store)?)?; + Ok(()) +} + impl Authentication { pub fn new() -> Result { Ok(Self { @@ -65,27 +107,59 @@ impl Authentication { }) } - pub fn remove_token() -> Result<(), AuthenticationError> { - match dirs::home_dir() { - Some(home) => { - fs::remove_file(home.join(".screenly"))?; - Ok(()) - } - None => Err(AuthenticationError::MissingHomeDir()), - } - } - fn read_token() -> Result { if let Ok(token) = env::var("API_TOKEN") { return Ok(token); } + let store = read_store()?; + let active = store.active.ok_or(AuthenticationError::NoCredentials)?; + store + .tokens + .get(&active) + .cloned() + .ok_or(AuthenticationError::NoCredentials) + } - match dirs::home_dir() { - Some(path) => { - fs::read_to_string(path.join(".screenly")).map_err(AuthenticationError::Io) - } - None => Err(AuthenticationError::NoCredentials), + pub fn remove_token(name: Option<&str>) -> Result<(), AuthenticationError> { + let mut store = read_store()?; + let target = match name { + Some(n) => n.to_string(), + None => store + .active + .clone() + .ok_or(AuthenticationError::NoCredentials)?, + }; + if !store.tokens.contains_key(&target) { + return Err(AuthenticationError::ProfileNotFound(target)); } + store.tokens.remove(&target); + if store.active.as_deref() == Some(&target) { + store.active = store.tokens.keys().next().cloned(); + } + write_store(&store) + } + + pub fn list_profiles() -> Result, AuthenticationError> { + let store = read_store()?; + let mut profiles: Vec<(String, String, bool)> = store + .tokens + .iter() + .map(|(name, token)| { + let is_active = store.active.as_deref() == Some(name.as_str()); + (name.clone(), token.clone(), is_active) + }) + .collect(); + profiles.sort_by(|a, b| a.0.cmp(&b.0)); + Ok(profiles) + } + + pub fn switch_profile(name: &str) -> Result<(), AuthenticationError> { + let mut store = read_store()?; + if !store.tokens.contains_key(name) { + return Err(AuthenticationError::ProfileNotFound(name.to_string())); + } + store.active = Some(name.to_string()); + write_store(&store) } #[cfg(test)] @@ -113,19 +187,54 @@ impl Authentication { } } +pub struct ProfileInfo { + pub email: String, + pub workspace: String, +} + +pub fn fetch_profile_info(token: &str, api_url: &str) -> Result { + let secret = format!("Token {token}"); + let client = reqwest::blocking::Client::builder().build()?; + + let user: serde_json::Value = client + .get(format!("{api_url}/v4.1/users/me")) + .header(header::AUTHORIZATION, &secret) + .send()? + .json()?; + + let email = user + .get(0) + .and_then(|u| u["email"].as_str()) + .unwrap_or("unknown") + .to_string(); + + let teams: serde_json::Value = client + .get(format!("{api_url}/v4.1/teams")) + .header(header::AUTHORIZATION, &secret) + .send()? + .json()?; + + let workspace = teams + .as_array() + .and_then(|arr| arr.iter().find(|t| t["is_current"].as_bool() == Some(true))) + .and_then(|t| t["name"].as_str()) + .unwrap_or("unknown") + .to_string(); + + Ok(ProfileInfo { email, workspace }) +} + pub fn verify_and_store_token( token: &str, + name: &str, api_url: &str, ) -> anyhow::Result<(), AuthenticationError> { verify_token(token, api_url)?; - match dirs::home_dir() { - Some(home) => { - fs::write(home.join(".screenly"), token)?; - Ok(()) - } - None => Err(AuthenticationError::MissingHomeDir()), - } + let mut store = read_store()?; + store.tokens.insert(name.to_string(), token.to_string()); + store.active = Some(name.to_string()); + write_store(&store) } fn verify_token(token: &str, api_url: &str) -> anyhow::Result<(), AuthenticationError> { @@ -180,11 +289,14 @@ mod tests { let config = Config::new(mock_server.base_url()); let authentication = Authentication::new_with_config(config, ""); - assert!(verify_and_store_token("correct_token", &authentication.config.url).is_ok()); + assert!( + verify_and_store_token("correct_token", "default", &authentication.config.url).is_ok() + ); let path = tmp_dir.path().join(".screenly"); assert!(path.exists()); - let contents = fs::read_to_string(path).unwrap(); - assert!(contents.eq("correct_token")); + let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(path).unwrap()).unwrap(); + assert_eq!(store.tokens.get("default").unwrap(), "correct_token"); + assert_eq!(store.active.unwrap(), "default"); } #[test] @@ -202,7 +314,7 @@ mod tests { }); let config = Config::new(mock_server.base_url()); - assert!(verify_and_store_token("wrong_token", &config.url).is_err()); + assert!(verify_and_store_token("wrong_token", "default", &config.url).is_err()); let path = tmp_dir.path().join(".screenly"); assert!(!path.exists()); @@ -214,8 +326,17 @@ mod tests { let _lock = lock_test(); let _token = set_env(OsString::from("API_TOKEN"), "env_token"); let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); - println!("{}", tmp_dir.path().join(".screenly").to_str().unwrap()); - fs::write(tmp_dir.path().join(".screenly").to_str().unwrap(), "token").unwrap(); + let store = TokenStore { + active: Some("default".to_string()), + tokens: [("default".to_string(), "token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); assert_eq!(Authentication::read_token().unwrap(), "env_token"); } @@ -224,20 +345,127 @@ mod tests { let tmp_dir = tempdir().unwrap(); let _lock = lock_test(); let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); - fs::write(tmp_dir.path().join(".screenly").to_str().unwrap(), "token").unwrap(); - + let store = TokenStore { + active: Some("default".to_string()), + tokens: [("default".to_string(), "token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); assert_eq!(Authentication::read_token().unwrap(), "token"); } #[test] - fn test_remove_token_should_remove_token_from_storage() { + fn test_read_token_backward_compat_plain_text() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + fs::write(tmp_dir.path().join(".screenly"), "legacy_token").unwrap(); + assert_eq!(Authentication::read_token().unwrap(), "legacy_token"); + } + + #[test] + fn test_remove_token_should_remove_active_profile() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("default".to_string()), + tokens: [("default".to_string(), "token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + Authentication::remove_token(None).unwrap(); + let store: TokenStore = + serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) + .unwrap(); + assert!(store.tokens.is_empty()); + assert!(store.active.is_none()); + } + + #[test] + fn test_switch_profile_should_change_active() { let tmp_dir = tempdir().unwrap(); let _lock = lock_test(); let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); - fs::write(tmp_dir.path().join(".screenly").to_str().unwrap(), "token").unwrap(); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [ + ("prod".to_string(), "prod_token".to_string()), + ("stage".to_string(), "stage_token".to_string()), + ] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + Authentication::switch_profile("stage").unwrap(); + let updated: TokenStore = + serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) + .unwrap(); + assert_eq!(updated.active.unwrap(), "stage"); + } - Authentication::remove_token().unwrap(); - assert!(!tmp_dir.path().join(".screenly").exists()); + #[test] + fn test_switch_profile_to_nonexistent_should_fail() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [("prod".to_string(), "prod_token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + assert!(Authentication::switch_profile("ghost").is_err()); + } + + #[test] + fn test_list_profiles_should_return_profiles_with_active_marked() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [ + ("prod".to_string(), "prod_token".to_string()), + ("stage".to_string(), "stage_token".to_string()), + ] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + let profiles = Authentication::list_profiles().unwrap(); + assert_eq!(profiles.len(), 2); + let prod = profiles.iter().find(|(n, _, _)| n == "prod").unwrap(); + let stage = profiles.iter().find(|(n, _, _)| n == "stage").unwrap(); + assert!(prod.2); + assert!(!stage.2); } #[test] @@ -257,11 +485,13 @@ mod tests { let config = Config::new(mock_server.base_url()); let authentication = Authentication::new_with_config(config, ""); - assert!(verify_and_store_token("correct_token", &authentication.config.url).is_ok()); + assert!( + verify_and_store_token("correct_token", "default", &authentication.config.url).is_ok() + ); let path = tmp_dir.path().join(".screenly"); assert!(path.exists()); - let contents = fs::read_to_string(path).unwrap(); + let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(path).unwrap()).unwrap(); group_call_mock.assert(); - assert!(contents.eq("correct_token")); + assert_eq!(store.tokens.get("default").unwrap(), "correct_token"); } } diff --git a/src/cli.rs b/src/cli.rs index d3ba88e5..0c316879 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,7 +9,9 @@ use reqwest::StatusCode; use rpassword::read_password; use thiserror::Error; -use crate::authentication::{verify_and_store_token, Authentication, AuthenticationError, Config}; +use crate::authentication::{ + fetch_profile_info, verify_and_store_token, Authentication, AuthenticationError, Config, +}; use crate::commands; use crate::commands::edge_app::instance_manifest::InstanceManifest; use crate::commands::edge_app::manifest::EdgeAppManifest; @@ -75,9 +77,20 @@ pub struct Cli { #[derive(Subcommand)] pub enum Commands { /// Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token. - Login {}, + Login { + /// Profile name to store the token under. Required when other profiles already exist. + #[arg(long)] + name: Option, + }, /// Logs out and removes the stored token. - Logout {}, + Logout { + /// Profile name to remove. Removes the active profile if not specified. + #[arg(long)] + name: Option, + }, + /// Manage stored authentication profiles. + #[command(subcommand)] + Auth(AuthCommands), /// Screen related commands. #[command(subcommand)] Screen(ScreenCommands), @@ -97,6 +110,17 @@ pub enum Commands { PrintHelpMarkdown {}, } +#[derive(Subcommand)] +pub enum AuthCommands { + /// List stored authentication profiles. + List {}, + /// Switch the active authentication profile. + Switch { + /// Profile name to activate. + name: Option, + }, +} + #[derive(Subcommand, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum ScreenCommands { /// Lists your screens. @@ -563,13 +587,27 @@ pub fn get_asset_title( pub fn handle_cli(cli: &Cli) { match &cli.command { - Commands::Login {} => { + Commands::Login { name } => { + let resolved_name = match name { + Some(n) => n.clone(), + None => { + let existing = Authentication::list_profiles().unwrap_or_default(); + if existing.is_empty() { + "default".to_string() + } else { + error!( + "Multiple profiles exist. Please specify a profile name with --name." + ); + std::process::exit(1); + } + } + }; print!("Enter your API Token: "); std::io::stdout().flush().unwrap(); let token = read_password().unwrap(); - match verify_and_store_token(&token, &Config::default().url) { + match verify_and_store_token(&token, &resolved_name, &Config::default().url) { Ok(()) => { - info!("Login credentials have been saved."); + info!("Login credentials have been saved under profile '{resolved_name}'."); std::process::exit(0); } @@ -589,11 +627,99 @@ pub fn handle_cli(cli: &Cli) { Commands::Asset(command) => handle_cli_asset_command(command), Commands::EdgeApp(command) => handle_cli_edge_app_command(command), Commands::Playlist(command) => handle_cli_playlist_command(command), - Commands::Logout {} => { - Authentication::remove_token().expect("Failed to remove token."); + Commands::Logout { name } => { + Authentication::remove_token(name.as_deref()).expect("Failed to remove token."); info!("Logout successful."); std::process::exit(0); } + Commands::Auth(auth_command) => match auth_command { + AuthCommands::List {} => match Authentication::list_profiles() { + Ok(profiles) if profiles.is_empty() => { + info!("No profiles stored. Run `screenly login` to add one."); + } + Ok(profiles) => { + let api_url = Config::default().url; + let rows: Vec<(String, bool, Option<(String, String)>)> = profiles + .into_iter() + .map(|(name, token, is_active)| { + let info = fetch_profile_info(&token, &api_url) + .ok() + .map(|i| (i.email, i.workspace)); + (name, is_active, info) + }) + .collect(); + + let name_w = rows.iter().map(|(n, _, _)| n.len()).max().unwrap_or(0); + let email_w = rows + .iter() + .filter_map(|(_, _, i)| i.as_ref().map(|(e, _)| e.len())) + .max() + .unwrap_or(0); + + println!(" {: { + println!("{marker} {name: println!("{marker} {name}"), + } + } + } + Err(e) => { + error!("Error occurred: {e:?}"); + std::process::exit(1); + } + }, + AuthCommands::Switch { name } => match name { + None => { + let api_url = Config::default().url; + if let Ok(profiles) = Authentication::list_profiles() { + let rows: Vec<(String, bool, Option<(String, String)>)> = profiles + .into_iter() + .map(|(name, token, is_active)| { + let info = fetch_profile_info(&token, &api_url) + .ok() + .map(|i| (i.email, i.workspace)); + (name, is_active, info) + }) + .collect(); + let name_w = rows.iter().map(|(n, _, _)| n.len()).max().unwrap_or(0); + let email_w = rows + .iter() + .filter_map(|(_, _, i)| i.as_ref().map(|(e, _)| e.len())) + .max() + .unwrap_or(0); + println!(" {: println!( + "{marker} {name: println!("{marker} {name}"), + } + } + } + } + Some(name) => match Authentication::switch_profile(name) { + Ok(()) => { + info!("Switched to profile '{name}'."); + } + Err(AuthenticationError::ProfileNotFound(_)) => { + error!("Profile '{name}' not found."); + std::process::exit(1); + } + Err(e) => { + error!("Error occurred: {e:?}"); + std::process::exit(1); + } + }, + }, + }, Commands::Mcp {} => { handle_cli_mcp_command(); } From 7062276d5be030abf63ce62741e46a5ce78eb6f6 Mon Sep 17 00:00:00 2001 From: 514sid Date: Sat, 6 Jun 2026 12:57:31 +0400 Subject: [PATCH 02/31] Return ProfileNotFound when active profile missing, show actionable error message --- src/authentication.rs | 2 +- src/cli.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/authentication.rs b/src/authentication.rs index c623887b..0b08896d 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -117,7 +117,7 @@ impl Authentication { .tokens .get(&active) .cloned() - .ok_or(AuthenticationError::NoCredentials) + .ok_or_else(|| AuthenticationError::ProfileNotFound(active)) } pub fn remove_token(name: Option<&str>) -> Result<(), AuthenticationError> { diff --git a/src/cli.rs b/src/cli.rs index 0c316879..a8defad4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -30,6 +30,9 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { AuthenticationError::Io(io_err) if io_err.kind() == std::io::ErrorKind::NotFound => { "Not logged in. Please run `screenly login` first to authenticate.".to_string() } + AuthenticationError::ProfileNotFound(name) => { + format!("Active profile '{name}' not found. Run `screenly auth switch` to pick a valid profile.") + } _ => { format!("Authentication error: {e}. Please run `screenly login` to authenticate.") } From ca9d932407b0277c09c21ac66927477d44d4faa5 Mon Sep 17 00:00:00 2001 From: 514sid Date: Sat, 6 Jun 2026 13:39:21 +0400 Subject: [PATCH 03/31] Feat: add 'me' command to show current profile info Displays the email and workspace for the active token, with proper error messages for unauthenticated and invalid-token cases. --- docs/CommandLineHelp.md | 14 +++++++++++ src/authentication.rs | 53 ++++++++++++++++++++++++++++++++++++--- src/cli.rs | 55 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index 50fceae4..f1141e50 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -7,6 +7,7 @@ This document contains the help content for the `screenly` command-line program. * [`screenly`↴](#screenly) * [`screenly login`↴](#screenly-login) * [`screenly logout`↴](#screenly-logout) +* [`screenly me`↴](#screenly-me) * [`screenly auth`↴](#screenly-auth) * [`screenly auth list`↴](#screenly-auth-list) * [`screenly auth switch`↴](#screenly-auth-switch) @@ -61,6 +62,7 @@ Command line interface is intended for quick interaction with Screenly through t * `login` — Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token * `logout` — Logs out and removes the stored token +* `me` — Show information about the currently authenticated profile * `auth` — Manage stored authentication profiles * `screen` — Screen related commands * `asset` — Asset related commands @@ -98,6 +100,18 @@ Logs out and removes the stored token +## `screenly me` + +Show information about the currently authenticated profile + +**Usage:** `screenly me [OPTIONS]` + +###### **Options:** + +* `-j`, `--json` — Enables JSON output + + + ## `screenly auth` Manage stored authentication profiles diff --git a/src/authentication.rs b/src/authentication.rs index 0b08896d..9cd45fcc 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -196,11 +196,16 @@ pub fn fetch_profile_info(token: &str, api_url: &str) -> Result Result Option { + read_store().ok().and_then(|s| s.active) +} + pub fn verify_and_store_token( token: &str, name: &str, @@ -494,4 +503,42 @@ mod tests { group_call_mock.assert(); assert_eq!(store.tokens.get("default").unwrap(), "correct_token"); } + + #[test] + fn test_fetch_profile_info_returns_email_and_workspace() { + let mock_server = MockServer::start(); + mock_server.mock(|when, then| { + when.method(GET) + .path("/v4.1/users/me") + .header("Authorization", "Token valid_token"); + then.status(200) + .json_body(serde_json::json!([{"email": "user@example.com"}])); + }); + mock_server.mock(|when, then| { + when.method(GET) + .path("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/v4.1/teams") + .header("Authorization", "Token valid_token"); + then.status(200).json_body( + serde_json::json!([{"name": "My Team", "is_current": true}]), + ); + }); + + let result = fetch_profile_info("valid_token", &mock_server.base_url()); + assert!(result.is_ok()); + let info = result.unwrap(); + assert_eq!(info.email, "user@example.com"); + assert_eq!(info.workspace, "My Team"); + } + + #[test] + fn test_fetch_profile_info_returns_wrong_credentials_on_401() { + let mock_server = MockServer::start(); + mock_server.mock(|when, then| { + when.method(GET).path("/v4.1/users/me"); + then.status(401); + }); + + let result = fetch_profile_info("bad_token", &mock_server.base_url()); + assert!(matches!(result, Err(AuthenticationError::WrongCredentials))); + } } diff --git a/src/cli.rs b/src/cli.rs index a8defad4..dcbbc740 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,7 +10,8 @@ use rpassword::read_password; use thiserror::Error; use crate::authentication::{ - fetch_profile_info, verify_and_store_token, Authentication, AuthenticationError, Config, + active_profile_name, fetch_profile_info, verify_and_store_token, Authentication, + AuthenticationError, Config, }; use crate::commands; use crate::commands::edge_app::instance_manifest::InstanceManifest; @@ -91,6 +92,12 @@ pub enum Commands { #[arg(long)] name: Option, }, + /// Show information about the currently authenticated profile. + Me { + /// Enables JSON output. + #[arg(short, long, action = clap::ArgAction::SetTrue)] + json: Option, + }, /// Manage stored authentication profiles. #[command(subcommand)] Auth(AuthCommands), @@ -630,6 +637,52 @@ pub fn handle_cli(cli: &Cli) { Commands::Asset(command) => handle_cli_asset_command(command), Commands::EdgeApp(command) => handle_cli_edge_app_command(command), Commands::Playlist(command) => handle_cli_playlist_command(command), + Commands::Me { json } => { + let auth = get_authentication(); + match fetch_profile_info(&auth.token, &auth.config.url) { + Ok(info) => { + let json_flag = json.unwrap_or(false); + if json_flag { + let mut obj = serde_json::Map::new(); + if let Some(name) = active_profile_name() { + obj.insert( + "profile".to_string(), + serde_json::Value::String(name), + ); + } + obj.insert( + "email".to_string(), + serde_json::Value::String(info.email), + ); + obj.insert( + "workspace".to_string(), + serde_json::Value::String(info.workspace), + ); + println!( + "{}", + serde_json::to_string_pretty(&serde_json::Value::Object(obj)) + .unwrap() + ); + } else { + if let Some(name) = active_profile_name() { + println!("Profile: {name}"); + } + println!("Email: {}", info.email); + println!("Workspace: {}", info.workspace); + } + } + Err(AuthenticationError::WrongCredentials) => { + error!( + "Token is invalid. Run `screenly login` to update your credentials." + ); + std::process::exit(1); + } + Err(e) => { + error!("Failed to fetch profile info: {e}"); + std::process::exit(1); + } + } + } Commands::Logout { name } => { Authentication::remove_token(name.as_deref()).expect("Failed to remove token."); info!("Logout successful."); From ffdf3e35b880bfde8fdb6527d6303d2f10bdc620 Mon Sep 17 00:00:00 2001 From: 514sid Date: Sat, 6 Jun 2026 16:26:51 +0400 Subject: [PATCH 04/31] Fix: apply rustfmt formatting --- src/authentication.rs | 5 ++--- src/cli.rs | 17 ++++------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 9cd45fcc..46c04efb 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -518,9 +518,8 @@ mod tests { when.method(GET) .path("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/v4.1/teams") .header("Authorization", "Token valid_token"); - then.status(200).json_body( - serde_json::json!([{"name": "My Team", "is_current": true}]), - ); + then.status(200) + .json_body(serde_json::json!([{"name": "My Team", "is_current": true}])); }); let result = fetch_profile_info("valid_token", &mock_server.base_url()); diff --git a/src/cli.rs b/src/cli.rs index dcbbc740..6b6f1a1e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -645,23 +645,16 @@ pub fn handle_cli(cli: &Cli) { if json_flag { let mut obj = serde_json::Map::new(); if let Some(name) = active_profile_name() { - obj.insert( - "profile".to_string(), - serde_json::Value::String(name), - ); + obj.insert("profile".to_string(), serde_json::Value::String(name)); } - obj.insert( - "email".to_string(), - serde_json::Value::String(info.email), - ); + obj.insert("email".to_string(), serde_json::Value::String(info.email)); obj.insert( "workspace".to_string(), serde_json::Value::String(info.workspace), ); println!( "{}", - serde_json::to_string_pretty(&serde_json::Value::Object(obj)) - .unwrap() + serde_json::to_string_pretty(&serde_json::Value::Object(obj)).unwrap() ); } else { if let Some(name) = active_profile_name() { @@ -672,9 +665,7 @@ pub fn handle_cli(cli: &Cli) { } } Err(AuthenticationError::WrongCredentials) => { - error!( - "Token is invalid. Run `screenly login` to update your credentials." - ); + error!("Token is invalid. Run `screenly login` to update your credentials."); std::process::exit(1); } Err(e) => { From 4b9d07c6dbe10313a91306e03f7e81f59008a014 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:01:31 +0400 Subject: [PATCH 05/31] Clean up authentication imports and error message Drop the redundant serde_yaml import (the crate path is already pulled in by the AuthenticationError enum) and surface the underlying parse error via {0} so YAML failures are diagnosable. --- src/authentication.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 46c04efb..a97a7c4e 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -4,7 +4,6 @@ use std::{env, fs}; use reqwest::header::{HeaderMap, InvalidHeaderValue}; use reqwest::{header, StatusCode}; use serde::{Deserialize, Serialize}; -use serde_yaml; use thiserror::Error; // For compatability reasons - let's leave build env as well. @@ -34,7 +33,7 @@ pub enum AuthenticationError { MissingHomeDir(), #[error("invalid header error")] InvalidHeader(#[from] InvalidHeaderValue), - #[error("yaml error")] + #[error("yaml error: {0}")] Yaml(#[from] serde_yaml::Error), #[error("unknown error")] Unknown, From 3920cb093fe68333c4869ee414bae12875e5187c Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:02:13 +0400 Subject: [PATCH 06/31] Make active-profile selection deterministic and handle logout errors remove_token now picks the alphabetically first remaining profile as the new active one instead of an arbitrary HashMap key, and returns the resulting active profile name. The Logout handler reports that name (or that no profiles remain) and prints a friendly message with exit(1) for NoCredentials, ProfileNotFound, and other errors instead of panicking. --- src/authentication.rs | 13 ++++++++++--- src/cli.rs | 27 ++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index a97a7c4e..9b5e7b10 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -119,7 +119,11 @@ impl Authentication { .ok_or_else(|| AuthenticationError::ProfileNotFound(active)) } - pub fn remove_token(name: Option<&str>) -> Result<(), AuthenticationError> { + /// Removes a profile. When `name` is `None` the active profile is removed. + /// If the removed profile was active, the new active profile is chosen + /// deterministically (the alphabetically first remaining profile). + /// Returns the name of the profile that is active after removal, if any. + pub fn remove_token(name: Option<&str>) -> Result, AuthenticationError> { let mut store = read_store()?; let target = match name { Some(n) => n.to_string(), @@ -133,9 +137,12 @@ impl Authentication { } store.tokens.remove(&target); if store.active.as_deref() == Some(&target) { - store.active = store.tokens.keys().next().cloned(); + let mut remaining: Vec<&String> = store.tokens.keys().collect(); + remaining.sort(); + store.active = remaining.first().map(|n| n.to_string()); } - write_store(&store) + write_store(&store)?; + Ok(store.active) } pub fn list_profiles() -> Result, AuthenticationError> { diff --git a/src/cli.rs b/src/cli.rs index 6b6f1a1e..104c9b9d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -674,11 +674,28 @@ pub fn handle_cli(cli: &Cli) { } } } - Commands::Logout { name } => { - Authentication::remove_token(name.as_deref()).expect("Failed to remove token."); - info!("Logout successful."); - std::process::exit(0); - } + Commands::Logout { name } => match Authentication::remove_token(name.as_deref()) { + Ok(new_active) => { + info!("Logout successful."); + match new_active { + Some(profile) => info!("Active profile is now '{profile}'."), + None => info!("No profiles remain."), + } + std::process::exit(0); + } + Err(AuthenticationError::NoCredentials) => { + error!("Not logged in."); + std::process::exit(1); + } + Err(AuthenticationError::ProfileNotFound(profile)) => { + error!("Profile '{profile}' not found."); + std::process::exit(1); + } + Err(e) => { + error!("Error occurred: {e}"); + std::process::exit(1); + } + }, Commands::Auth(auth_command) => match auth_command { AuthCommands::List {} => match Authentication::list_profiles() { Ok(profiles) if profiles.is_empty() => { From 84600794648a1a4a860e541dbcd18527e39e98a1 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:02:33 +0400 Subject: [PATCH 07/31] Harden token store writes: atomic rename and 0o600 permissions write_store now writes to a temp file and renames it over the target so a crash or concurrent read can't observe a truncated store. On Unix the file is chmod'd to 0o600 since it holds every profile's token. --- src/authentication.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/authentication.rs b/src/authentication.rs index 9b5e7b10..4d83f1bb 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -94,7 +94,28 @@ fn read_store() -> Result { fn write_store(store: &TokenStore) -> Result<(), AuthenticationError> { let path = screenly_path()?; - fs::write(path, serde_yaml::to_string(store)?)?; + let contents = serde_yaml::to_string(store)?; + + // Write to a temp file and rename over the target so a concurrent reader + // never observes a half-written store and a crash mid-write can't corrupt it. + let tmp_path = path.with_extension("tmp"); + fs::write(&tmp_path, contents)?; + restrict_permissions(&tmp_path)?; + fs::rename(&tmp_path, &path)?; + Ok(()) +} + +/// The token store holds every profile's credentials, so keep it readable +/// only by the owner. No-op on non-Unix platforms. +#[cfg(unix)] +fn restrict_permissions(path: &std::path::Path) -> Result<(), AuthenticationError> { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o600))?; + Ok(()) +} + +#[cfg(not(unix))] +fn restrict_permissions(_path: &std::path::Path) -> Result<(), AuthenticationError> { Ok(()) } From 4922ee9b411641537241aa9b6d069565b0637617 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:03:13 +0400 Subject: [PATCH 08/31] Share authenticated client and accept object-or-array in profile info Extract authenticated_client() so fetch_profile_info sends the same screenly-cli User-Agent as the rest of the CLI instead of a bare client. Parse /v4.1/users/me whether it returns a single object or a one-element array, so 'me' no longer silently prints 'unknown' if the API returns an object. --- src/authentication.rs | 53 ++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 4d83f1bb..49f8547e 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -198,35 +198,36 @@ impl Authentication { } pub fn build_client(&self) -> Result { - let token = self.token.clone(); - let secret = format!("Token {token}"); - let mut default_headers = HeaderMap::new(); - default_headers.insert(header::AUTHORIZATION, secret.parse()?); - default_headers.insert( - header::USER_AGENT, - format!("screenly-cli {}", env!("CARGO_PKG_VERSION")).parse()?, - ); - - reqwest::blocking::Client::builder() - .default_headers(default_headers) - .build() - .map_err(AuthenticationError::Request) + authenticated_client(&self.token) } } +/// Builds a blocking client that sends the auth token and the standard +/// `screenly-cli {version}` User-Agent on every request. +fn authenticated_client(token: &str) -> Result { + let secret = format!("Token {token}"); + let mut default_headers = HeaderMap::new(); + default_headers.insert(header::AUTHORIZATION, secret.parse()?); + default_headers.insert( + header::USER_AGENT, + format!("screenly-cli {}", env!("CARGO_PKG_VERSION")).parse()?, + ); + + reqwest::blocking::Client::builder() + .default_headers(default_headers) + .build() + .map_err(AuthenticationError::Request) +} + pub struct ProfileInfo { pub email: String, pub workspace: String, } pub fn fetch_profile_info(token: &str, api_url: &str) -> Result { - let secret = format!("Token {token}"); - let client = reqwest::blocking::Client::builder().build()?; + let client = authenticated_client(token)?; - let user_response = client - .get(format!("{api_url}/v4.1/users/me")) - .header(header::AUTHORIZATION, &secret) - .send()?; + let user_response = client.get(format!("{api_url}/v4.1/users/me")).send()?; if user_response.status() == StatusCode::UNAUTHORIZED { return Err(AuthenticationError::WrongCredentials); @@ -234,17 +235,11 @@ pub fn fetch_profile_info(token: &str, api_url: &str) -> Result Date: Thu, 23 Jul 2026 11:05:27 +0400 Subject: [PATCH 09/31] Extract profile table helper and stop exposing tokens from list_profiles list_profiles now returns (name, is_active) pairs; profile details are fetched via the new fetch_profiles_with_info, which keeps tokens inside the authentication module. The duplicated ~25-line table-rendering block in auth list and auth switch (no arg) is replaced by a single print_profiles_table helper. --- src/authentication.rs | 49 ++++++++++++++++++---- src/cli.rs | 96 +++++++++++++++---------------------------- 2 files changed, 72 insertions(+), 73 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 49f8547e..bbf73bf3 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -166,14 +166,17 @@ impl Authentication { Ok(store.active) } - pub fn list_profiles() -> Result, AuthenticationError> { + /// Returns the stored profiles as `(name, is_active)` pairs sorted by name. + /// Tokens are intentionally not exposed here to avoid accidental prints; + /// use `fetch_profiles_with_info` when profile details are needed. + pub fn list_profiles() -> Result, AuthenticationError> { let store = read_store()?; - let mut profiles: Vec<(String, String, bool)> = store + let mut profiles: Vec<(String, bool)> = store .tokens - .iter() - .map(|(name, token)| { + .keys() + .map(|name| { let is_active = store.active.as_deref() == Some(name.as_str()); - (name.clone(), token.clone(), is_active) + (name.clone(), is_active) }) .collect(); profiles.sort_by(|a, b| a.0.cmp(&b.0)); @@ -224,6 +227,34 @@ pub struct ProfileInfo { pub workspace: String, } +pub struct ProfileEntry { + pub name: String, + pub is_active: bool, + /// `None` when the profile's token could not be resolved against the API. + pub info: Option, +} + +/// Returns every stored profile together with its email/workspace fetched +/// from the API. Tokens stay inside this module and are never returned. +pub fn fetch_profiles_with_info(api_url: &str) -> Result, AuthenticationError> { + let store = read_store()?; + let mut names: Vec = store.tokens.keys().cloned().collect(); + names.sort(); + let entries = names + .into_iter() + .map(|name| { + let is_active = store.active.as_deref() == Some(name.as_str()); + let info = fetch_profile_info(&store.tokens[&name], api_url).ok(); + ProfileEntry { + name, + is_active, + info, + } + }) + .collect(); + Ok(entries) +} + pub fn fetch_profile_info(token: &str, api_url: &str) -> Result { let client = authenticated_client(token)?; @@ -493,10 +524,10 @@ mod tests { let profiles = Authentication::list_profiles().unwrap(); assert_eq!(profiles.len(), 2); - let prod = profiles.iter().find(|(n, _, _)| n == "prod").unwrap(); - let stage = profiles.iter().find(|(n, _, _)| n == "stage").unwrap(); - assert!(prod.2); - assert!(!stage.2); + let prod = profiles.iter().find(|(n, _)| n == "prod").unwrap(); + let stage = profiles.iter().find(|(n, _)| n == "stage").unwrap(); + assert!(prod.1); + assert!(!stage.1); } #[test] diff --git a/src/cli.rs b/src/cli.rs index 104c9b9d..3a616fb9 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,8 +10,8 @@ use rpassword::read_password; use thiserror::Error; use crate::authentication::{ - active_profile_name, fetch_profile_info, verify_and_store_token, Authentication, - AuthenticationError, Config, + active_profile_name, fetch_profile_info, fetch_profiles_with_info, verify_and_store_token, + Authentication, AuthenticationError, Config, ProfileEntry, }; use crate::commands; use crate::commands::edge_app::instance_manifest::InstanceManifest; @@ -40,6 +40,30 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { } } +/// Prints the stored profiles as a table with email and workspace columns, +/// marking the active profile with a `*`. +fn print_profiles_table(entries: &[ProfileEntry]) { + let name_w = entries.iter().map(|e| e.name.len()).max().unwrap_or(0); + let email_w = entries + .iter() + .filter_map(|e| e.info.as_ref().map(|i| i.email.len())) + .max() + .unwrap_or(0); + + println!(" {: println!( + "{marker} {: println!("{marker} {}", entry.name), + } + } +} + /// Creates an Authentication instance or exits with a user-friendly error message. fn get_authentication() -> Authentication { match Authentication::new() { @@ -697,76 +721,20 @@ pub fn handle_cli(cli: &Cli) { } }, Commands::Auth(auth_command) => match auth_command { - AuthCommands::List {} => match Authentication::list_profiles() { - Ok(profiles) if profiles.is_empty() => { + AuthCommands::List {} => match fetch_profiles_with_info(&Config::default().url) { + Ok(entries) if entries.is_empty() => { info!("No profiles stored. Run `screenly login` to add one."); } - Ok(profiles) => { - let api_url = Config::default().url; - let rows: Vec<(String, bool, Option<(String, String)>)> = profiles - .into_iter() - .map(|(name, token, is_active)| { - let info = fetch_profile_info(&token, &api_url) - .ok() - .map(|i| (i.email, i.workspace)); - (name, is_active, info) - }) - .collect(); - - let name_w = rows.iter().map(|(n, _, _)| n.len()).max().unwrap_or(0); - let email_w = rows - .iter() - .filter_map(|(_, _, i)| i.as_ref().map(|(e, _)| e.len())) - .max() - .unwrap_or(0); - - println!(" {: { - println!("{marker} {name: println!("{marker} {name}"), - } - } - } + Ok(entries) => print_profiles_table(&entries), Err(e) => { - error!("Error occurred: {e:?}"); + error!("Error occurred: {e}"); std::process::exit(1); } }, AuthCommands::Switch { name } => match name { None => { - let api_url = Config::default().url; - if let Ok(profiles) = Authentication::list_profiles() { - let rows: Vec<(String, bool, Option<(String, String)>)> = profiles - .into_iter() - .map(|(name, token, is_active)| { - let info = fetch_profile_info(&token, &api_url) - .ok() - .map(|i| (i.email, i.workspace)); - (name, is_active, info) - }) - .collect(); - let name_w = rows.iter().map(|(n, _, _)| n.len()).max().unwrap_or(0); - let email_w = rows - .iter() - .filter_map(|(_, _, i)| i.as_ref().map(|(e, _)| e.len())) - .max() - .unwrap_or(0); - println!(" {: println!( - "{marker} {name: println!("{marker} {name}"), - } - } + if let Ok(entries) = fetch_profiles_with_info(&Config::default().url) { + print_profiles_table(&entries); } } Some(name) => match Authentication::switch_profile(name) { From 562930effeb13b1f136765a715e92f55afa5fb9b Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:06:03 +0400 Subject: [PATCH 10/31] Fetch per-profile info in parallel auth list and auth switch (no arg) previously issued their per-profile API requests one profile at a time, so latency grew linearly with the number of profiles. Because the client is reqwest::blocking, use rayon (already a dependency) rather than futures::join_all to run them across threads. --- src/authentication.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/authentication.rs b/src/authentication.rs index bbf73bf3..c9308ab9 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -236,12 +236,16 @@ pub struct ProfileEntry { /// Returns every stored profile together with its email/workspace fetched /// from the API. Tokens stay inside this module and are never returned. +/// The per-profile requests are issued in parallel so the total latency +/// does not grow linearly with the number of profiles. pub fn fetch_profiles_with_info(api_url: &str) -> Result, AuthenticationError> { + use rayon::prelude::*; + let store = read_store()?; let mut names: Vec = store.tokens.keys().cloned().collect(); names.sort(); let entries = names - .into_iter() + .into_par_iter() .map(|name| { let is_active = store.active.as_deref() == Some(name.as_str()); let info = fetch_profile_info(&store.tokens[&name], api_url).ok(); From 45c492cd77f681a718ffb77e69cc7ad2f1978cad Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:06:31 +0400 Subject: [PATCH 11/31] Clarify switch no-arg behavior and me profile source Document that 'auth switch' without an argument prints the profile list, in both the subcommand and the argument help. When the token comes from the API_TOKEN env var, 'me' now shows 'Profile: (from API_TOKEN env)' in both human and JSON output instead of silently omitting the field. --- src/cli.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 3a616fb9..88a2dc67 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -149,8 +149,10 @@ pub enum AuthCommands { /// List stored authentication profiles. List {}, /// Switch the active authentication profile. + /// + /// Without an argument, prints the list of profiles instead of switching. Switch { - /// Profile name to activate. + /// Profile name to activate. Omit to print the profile list. name: Option, }, } @@ -666,11 +668,16 @@ pub fn handle_cli(cli: &Cli) { match fetch_profile_info(&auth.token, &auth.config.url) { Ok(info) => { let json_flag = json.unwrap_or(false); + let profile = match active_profile_name() { + Some(name) => name, + None => "(from API_TOKEN env)".to_string(), + }; if json_flag { let mut obj = serde_json::Map::new(); - if let Some(name) = active_profile_name() { - obj.insert("profile".to_string(), serde_json::Value::String(name)); - } + obj.insert( + "profile".to_string(), + serde_json::Value::String(profile), + ); obj.insert("email".to_string(), serde_json::Value::String(info.email)); obj.insert( "workspace".to_string(), @@ -681,9 +688,7 @@ pub fn handle_cli(cli: &Cli) { serde_json::to_string_pretty(&serde_json::Value::Object(obj)).unwrap() ); } else { - if let Some(name) = active_profile_name() { - println!("Profile: {name}"); - } + println!("Profile: {profile}"); println!("Email: {}", info.email); println!("Workspace: {}", info.workspace); } From 6acd7b3157ad2f5e389a41e2e55f0a3918bc68b2 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:07:08 +0400 Subject: [PATCH 12/31] Extract resolve_login_name for the --name safety check Move the login profile-name resolution out of handle_cli into a pure resolve_login_name helper so the '--name required when a profile already exists' rule can be unit-tested. Behavior is unchanged. --- src/cli.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 88a2dc67..6120ff3d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -40,6 +40,20 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { } } +/// Resolves the profile name a `login` should store under. +/// +/// An explicit name is always honored. With no name given, a fresh install +/// (no existing profiles) defaults to `"default"`; otherwise `None` is +/// returned to signal that `--name` is required so an existing profile is +/// not overwritten by accident. +fn resolve_login_name(name: Option<&str>, existing: &[(String, bool)]) -> Option { + match name { + Some(n) => Some(n.to_string()), + None if existing.is_empty() => Some("default".to_string()), + None => None, + } +} + /// Prints the stored profiles as a table with email and workspace columns, /// marking the active profile with a `*`. fn print_profiles_table(entries: &[ProfileEntry]) { @@ -624,18 +638,12 @@ pub fn get_asset_title( pub fn handle_cli(cli: &Cli) { match &cli.command { Commands::Login { name } => { - let resolved_name = match name { - Some(n) => n.clone(), + let existing = Authentication::list_profiles().unwrap_or_default(); + let resolved_name = match resolve_login_name(name.as_deref(), &existing) { + Some(resolved) => resolved, None => { - let existing = Authentication::list_profiles().unwrap_or_default(); - if existing.is_empty() { - "default".to_string() - } else { - error!( - "Multiple profiles exist. Please specify a profile name with --name." - ); - std::process::exit(1); - } + error!("A profile already exists. Please specify a profile name with --name."); + std::process::exit(1); } }; print!("Enter your API Token: "); From d137cdd45521b07bb8df1a22f7d8240c76e8a29b Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:08:58 +0400 Subject: [PATCH 13/31] Add tests for auth safety checks and profile-info parsing Cover the gaps called out in review: - resolve_login_name: fresh-install default, explicit name, and the --name-required rule when a profile already exists. - verify_and_store_token preserves existing profiles when storing a new one. - remove_token by explicit name (non-active), unknown name error, and deterministic new-active selection when the active profile is removed. - fetch_profile_info accepts an object response, not just an array. - write_store restricts the store to 0o600 on Unix. --- src/authentication.rs | 168 +++++++++++++++++++++++++++++++++++++++++- src/cli.rs | 20 +++++ 2 files changed, 187 insertions(+), 1 deletion(-) diff --git a/src/authentication.rs b/src/authentication.rs index c9308ab9..41fa9a51 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -365,6 +365,43 @@ mod tests { assert_eq!(store.active.unwrap(), "default"); } + #[test] + fn test_verify_and_store_token_preserves_existing_profiles() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + + let existing = TokenStore { + active: Some("prod".to_string()), + tokens: [("prod".to_string(), "prod_token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&existing).unwrap(), + ) + .unwrap(); + + let mock_server = MockServer::start(); + mock_server.mock(|when, then| { + when.method(GET) + .path("/v3/groups/11CF9Z3GZR0005XXKH00F8V20R/"); + then.status(404); + }); + + let config = Config::new(mock_server.base_url()); + assert!(verify_and_store_token("stage_token", "stage", &config.url).is_ok()); + + let store: TokenStore = + serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) + .unwrap(); + // The pre-existing profile is retained and the new one becomes active. + assert_eq!(store.tokens.get("prod").unwrap(), "prod_token"); + assert_eq!(store.tokens.get("stage").unwrap(), "stage_token"); + assert_eq!(store.active.as_deref(), Some("stage")); + } + #[test] fn test_verify_and_store_token_when_token_is_invalid() { let tmp_dir = tempdir().unwrap(); @@ -451,7 +488,8 @@ mod tests { ) .unwrap(); - Authentication::remove_token(None).unwrap(); + let new_active = Authentication::remove_token(None).unwrap(); + assert!(new_active.is_none()); let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) .unwrap(); @@ -459,6 +497,110 @@ mod tests { assert!(store.active.is_none()); } + #[test] + #[cfg(unix)] + fn test_write_store_restricts_permissions_to_owner() { + use std::os::unix::fs::PermissionsExt; + + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + + let mut store = TokenStore::default(); + store.tokens.insert("default".to_string(), "token".to_string()); + store.active = Some("default".to_string()); + write_store(&store).unwrap(); + + let mode = fs::metadata(tmp_dir.path().join(".screenly")) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600); + } + + #[test] + fn test_remove_token_with_explicit_name_keeps_active() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [ + ("prod".to_string(), "prod_token".to_string()), + ("stage".to_string(), "stage_token".to_string()), + ] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + // Removing a non-active profile by name leaves the active one intact. + let new_active = Authentication::remove_token(Some("stage")).unwrap(); + assert_eq!(new_active.as_deref(), Some("prod")); + let store: TokenStore = + serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) + .unwrap(); + assert!(!store.tokens.contains_key("stage")); + assert_eq!(store.active.as_deref(), Some("prod")); + } + + #[test] + fn test_remove_active_profile_picks_deterministic_new_active() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [ + ("prod".to_string(), "prod_token".to_string()), + ("alpha".to_string(), "alpha_token".to_string()), + ("stage".to_string(), "stage_token".to_string()), + ] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + // Removing the active profile picks the alphabetically first remaining one. + let new_active = Authentication::remove_token(None).unwrap(); + assert_eq!(new_active.as_deref(), Some("alpha")); + let store: TokenStore = + serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) + .unwrap(); + assert_eq!(store.active.as_deref(), Some("alpha")); + } + + #[test] + fn test_remove_token_with_unknown_name_errors() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [("prod".to_string(), "prod_token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + assert!(matches!( + Authentication::remove_token(Some("ghost")), + Err(AuthenticationError::ProfileNotFound(_)) + )); + } + #[test] fn test_switch_profile_should_change_active() { let tmp_dir = tempdir().unwrap(); @@ -586,6 +728,30 @@ mod tests { assert_eq!(info.workspace, "My Team"); } + #[test] + fn test_fetch_profile_info_accepts_object_response() { + let mock_server = MockServer::start(); + mock_server.mock(|when, then| { + when.method(GET) + .path("/v4.1/users/me") + .header("Authorization", "Token valid_token"); + // A single object, not wrapped in an array. + then.status(200) + .json_body(serde_json::json!({"email": "user@example.com"})); + }); + mock_server.mock(|when, then| { + when.method(GET) + .path("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/v4.1/teams") + .header("Authorization", "Token valid_token"); + then.status(200) + .json_body(serde_json::json!([{"name": "My Team", "is_current": true}])); + }); + + let info = fetch_profile_info("valid_token", &mock_server.base_url()).unwrap(); + assert_eq!(info.email, "user@example.com"); + assert_eq!(info.workspace, "My Team"); + } + #[test] fn test_fetch_profile_info_returns_wrong_credentials_on_401() { let mock_server = MockServer::start(); diff --git a/src/cli.rs b/src/cli.rs index 6120ff3d..2e066a21 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1437,6 +1437,26 @@ mod tests { use super::*; use crate::authentication::Config; + #[test] + fn test_resolve_login_name_defaults_to_default_on_fresh_install() { + assert_eq!(resolve_login_name(None, &[]), Some("default".to_string())); + } + + #[test] + fn test_resolve_login_name_honors_explicit_name() { + let existing = vec![("prod".to_string(), true)]; + assert_eq!( + resolve_login_name(Some("stage"), &existing), + Some("stage".to_string()) + ); + } + + #[test] + fn test_resolve_login_name_requires_name_when_profiles_exist() { + let existing = vec![("prod".to_string(), true)]; + assert_eq!(resolve_login_name(None, &existing), None); + } + #[test] fn test_get_screen_name_should_return_correct_screen_name() { let _tmp_dir = tempdir().unwrap(); From 4ae31a32f268f03050fcf6c8903120e0f7112871 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:10:00 +0400 Subject: [PATCH 14/31] Apply rustfmt --- src/authentication.rs | 4 +++- src/cli.rs | 5 +---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 41fa9a51..20d186e9 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -507,7 +507,9 @@ mod tests { let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); let mut store = TokenStore::default(); - store.tokens.insert("default".to_string(), "token".to_string()); + store + .tokens + .insert("default".to_string(), "token".to_string()); store.active = Some("default".to_string()); write_store(&store).unwrap(); diff --git a/src/cli.rs b/src/cli.rs index 2e066a21..7b938bcb 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -682,10 +682,7 @@ pub fn handle_cli(cli: &Cli) { }; if json_flag { let mut obj = serde_json::Map::new(); - obj.insert( - "profile".to_string(), - serde_json::Value::String(profile), - ); + obj.insert("profile".to_string(), serde_json::Value::String(profile)); obj.insert("email".to_string(), serde_json::Value::String(info.email)); obj.insert( "workspace".to_string(), From 0c69e9fa1be5c62eac2d7f5a8c2ae0be3fa58b6c Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 11:10:34 +0400 Subject: [PATCH 15/31] Regenerate CommandLineHelp.md for auth switch docs --- docs/CommandLineHelp.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index f1141e50..3b170831 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -135,13 +135,15 @@ List stored authentication profiles ## `screenly auth switch` -Switch the active authentication profile +Switch the active authentication profile. + +Without an argument, prints the list of profiles instead of switching. **Usage:** `screenly auth switch [NAME]` ###### **Arguments:** -* `` — Profile name to activate +* `` — Profile name to activate. Omit to print the profile list From 6297cf604ca72cf0f746b0c4e5f7f4c9d641ab7b Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 13:10:15 +0400 Subject: [PATCH 16/31] Harden the token store against data loss and tighten the write path Addresses review findings on the storage layer: - Blocking: read_store no longer reinterprets an unparseable store as a plain-text token. A malformed or truncated file (e.g. a mistyped key) now returns AuthenticationError::Yaml instead of collapsing every profile into a single bogus token that the next write would persist. Migration is gated on the file actually looking like a legacy token. - Create the temp file at 0600 from the start (no world-readable window) and give it a per-process name so concurrent writers can't interleave into a shared .screenly.tmp. - list_profiles returns a ProfileSummary struct instead of an opaque (String, bool) tuple. Tests: malformed YAML surfaces an error, a legacy plain-text file is rewritten as YAML on first write, and fetch_profiles_with_info returns per-profile details. --- src/authentication.rs | 183 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 150 insertions(+), 33 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 20d186e9..e372a200 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io::Write; use std::{env, fs}; use reqwest::header::{HeaderMap, InvalidHeaderValue}; @@ -81,42 +82,74 @@ fn read_store() -> Result { return Ok(TokenStore::default()); } let contents = fs::read_to_string(&path)?; - if let Ok(store) = serde_yaml::from_str::(&contents) { - return Ok(store); - } - // Backward compat: plain text token → migrate to "default" profile - let token = contents.trim().to_string(); - let mut store = TokenStore::default(); - store.tokens.insert("default".to_string(), token); - store.active = Some("default".to_string()); - Ok(store) + match serde_yaml::from_str::(&contents) { + Ok(store) => Ok(store), + Err(yaml_err) => { + // Backward compat: the original format was a single plain-text + // token. Only migrate when the file actually looks like one. + // Any other parse failure (a hand-edit typo, a truncated file, a + // future schema change) must surface as an error rather than be + // silently reinterpreted as a token, which would drop every stored + // profile on the next write. + let trimmed = contents.trim(); + if is_legacy_token(trimmed) { + let mut store = TokenStore::default(); + store + .tokens + .insert("default".to_string(), trimmed.to_string()); + store.active = Some("default".to_string()); + Ok(store) + } else { + Err(AuthenticationError::Yaml(yaml_err)) + } + } + } +} + +/// A legacy `~/.screenly` holds exactly one plain-text token: a single +/// non-empty line with no YAML mapping punctuation. +fn is_legacy_token(contents: &str) -> bool { + !contents.is_empty() && !contents.contains(':') && !contents.contains('\n') } fn write_store(store: &TokenStore) -> Result<(), AuthenticationError> { let path = screenly_path()?; let contents = serde_yaml::to_string(store)?; - // Write to a temp file and rename over the target so a concurrent reader - // never observes a half-written store and a crash mid-write can't corrupt it. - let tmp_path = path.with_extension("tmp"); - fs::write(&tmp_path, contents)?; - restrict_permissions(&tmp_path)?; + // Write to a per-process temp file and rename over the target so a + // concurrent reader never observes a half-written store and a crash + // mid-write can't corrupt it. The pid suffix keeps two concurrent writers + // from sharing (and interleaving into) the same temp file. + let tmp_path = path.with_extension(format!("tmp.{}", std::process::id())); + let mut file = create_private_file(&tmp_path)?; + file.write_all(contents.as_bytes())?; + file.sync_all()?; + drop(file); fs::rename(&tmp_path, &path)?; Ok(()) } -/// The token store holds every profile's credentials, so keep it readable -/// only by the owner. No-op on non-Unix platforms. +/// Creates (or truncates) a file that the token store can be written to, +/// owner-readable only from the moment it exists so the token is never +/// briefly world-readable. Permissions are a no-op on non-Unix platforms. #[cfg(unix)] -fn restrict_permissions(path: &std::path::Path) -> Result<(), AuthenticationError> { - use std::os::unix::fs::PermissionsExt; - fs::set_permissions(path, fs::Permissions::from_mode(0o600))?; - Ok(()) +fn create_private_file(path: &std::path::Path) -> Result { + use std::os::unix::fs::OpenOptionsExt; + Ok(fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path)?) } #[cfg(not(unix))] -fn restrict_permissions(_path: &std::path::Path) -> Result<(), AuthenticationError> { - Ok(()) +fn create_private_file(path: &std::path::Path) -> Result { + Ok(fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(path)?) } impl Authentication { @@ -166,20 +199,20 @@ impl Authentication { Ok(store.active) } - /// Returns the stored profiles as `(name, is_active)` pairs sorted by name. + /// Returns the stored profiles sorted by name, without their tokens. /// Tokens are intentionally not exposed here to avoid accidental prints; /// use `fetch_profiles_with_info` when profile details are needed. - pub fn list_profiles() -> Result, AuthenticationError> { + pub fn list_profiles() -> Result, AuthenticationError> { let store = read_store()?; - let mut profiles: Vec<(String, bool)> = store + let mut profiles: Vec = store .tokens .keys() - .map(|name| { - let is_active = store.active.as_deref() == Some(name.as_str()); - (name.clone(), is_active) + .map(|name| ProfileSummary { + is_active: store.active.as_deref() == Some(name.as_str()), + name: name.clone(), }) .collect(); - profiles.sort_by(|a, b| a.0.cmp(&b.0)); + profiles.sort_by(|a, b| a.name.cmp(&b.name)); Ok(profiles) } @@ -227,6 +260,12 @@ pub struct ProfileInfo { pub workspace: String, } +/// A stored profile without its token, for listing profile names offline. +pub struct ProfileSummary { + pub name: String, + pub is_active: bool, +} + pub struct ProfileEntry { pub name: String, pub is_active: bool, @@ -471,6 +510,84 @@ mod tests { assert_eq!(Authentication::read_token().unwrap(), "legacy_token"); } + #[test] + fn test_read_store_malformed_yaml_returns_error_not_token() { + // A store that fails to parse (here: the required `tokens` key is + // misspelled) must surface an error, not be silently reinterpreted as + // a plain-text token, which would drop the stored profiles. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + fs::write( + tmp_dir.path().join(".screenly"), + "active: prod\ntokenz:\n prod: prod_token\n", + ) + .unwrap(); + + assert!(matches!(read_store(), Err(AuthenticationError::Yaml(_)))); + } + + #[test] + fn test_legacy_plain_text_is_rewritten_as_yaml_on_first_write() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let path = tmp_dir.path().join(".screenly"); + fs::write(&path, "legacy_token").unwrap(); + + // Reading migrates the legacy token into a store; writing it back must + // persist YAML, not the original plain text. + let store = read_store().unwrap(); + write_store(&store).unwrap(); + + let contents = fs::read_to_string(&path).unwrap(); + let parsed: TokenStore = serde_yaml::from_str(&contents).unwrap(); + assert_eq!(parsed.tokens.get("default").unwrap(), "legacy_token"); + assert_eq!(parsed.active.as_deref(), Some("default")); + } + + #[test] + fn test_fetch_profiles_with_info_returns_details_per_profile() { + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("prod".to_string()), + tokens: [ + ("prod".to_string(), "prod_token".to_string()), + ("stage".to_string(), "stage_token".to_string()), + ] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + let mock_server = MockServer::start(); + mock_server.mock(|when, then| { + when.method(GET).path("/v4.1/users/me"); + then.status(200) + .json_body(serde_json::json!([{"email": "user@example.com"}])); + }); + mock_server.mock(|when, then| { + when.method(GET).path("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/v4.1/teams"); + then.status(200) + .json_body(serde_json::json!([{"name": "My Team", "is_current": true}])); + }); + + let entries = fetch_profiles_with_info(&mock_server.base_url()).unwrap(); + assert_eq!(entries.len(), 2); + // Sorted by name, so "prod" comes before "stage". + assert_eq!(entries[0].name, "prod"); + assert!(entries[0].is_active); + assert_eq!(entries[0].info.as_ref().unwrap().email, "user@example.com"); + assert_eq!(entries[0].info.as_ref().unwrap().workspace, "My Team"); + assert!(!entries[1].is_active); + } + #[test] fn test_remove_token_should_remove_active_profile() { let tmp_dir = tempdir().unwrap(); @@ -672,10 +789,10 @@ mod tests { let profiles = Authentication::list_profiles().unwrap(); assert_eq!(profiles.len(), 2); - let prod = profiles.iter().find(|(n, _)| n == "prod").unwrap(); - let stage = profiles.iter().find(|(n, _)| n == "stage").unwrap(); - assert!(prod.1); - assert!(!stage.1); + let prod = profiles.iter().find(|p| p.name == "prod").unwrap(); + let stage = profiles.iter().find(|p| p.name == "stage").unwrap(); + assert!(prod.is_active); + assert!(!stage.is_active); } #[test] From df627f5663c940161e433b038dd46c1601f37559 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 13:10:26 +0400 Subject: [PATCH 17/31] Fix login regression and profile command UX Addresses review findings on the command layer: - Blocking: plain 'screenly login' no longer errors for existing users. It now updates the active profile (falling back to 'default' on a fresh install), so the re-login-after-rotation flow works again. - 'me' drops its local --json flag and honors the global --output (table/json/csv) like other commands, via a Formatter impl. - 'me' derives its profile label with the same API_TOKEN-first precedence as read_token, so it no longer names the stored profile while authenticating as the env token. - 'auth list' honors --output too (table/json/csv). - The profile table accounts for header widths and shows an '(unavailable)' placeholder for unreachable profiles instead of misaligning or dropping columns; it returns a String and is unit-tested. - 'auth switch' with no argument is now a usage error (exit 1) that lists profile names offline instead of silently exiting 0 after 2N network calls. - NoCredentials gets the friendly 'Not logged in' message, which the logged-out path now produces since logout leaves an empty store. --- src/cli.rs | 308 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 231 insertions(+), 77 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 6529e5ff..4689500d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -7,6 +7,7 @@ use http_auth_basic::Credentials; use log::{error, info}; use reqwest::StatusCode; use rpassword::read_password; +use serde_json::json; use thiserror::Error; use crate::authentication::{ @@ -27,9 +28,13 @@ const DEFAULT_ASSET_DURATION: u32 = 15; /// Returns a user-friendly error message for authentication errors. fn get_authentication_error_message(e: &AuthenticationError) -> String { + let not_logged_in = "Not logged in. Please run `screenly login` first to authenticate."; match e { + // The logged-out state now leaves an empty store behind rather than + // deleting the file, so it surfaces as NoCredentials, not Io(NotFound). + AuthenticationError::NoCredentials => not_logged_in.to_string(), AuthenticationError::Io(io_err) if io_err.kind() == std::io::ErrorKind::NotFound => { - "Not logged in. Please run `screenly login` first to authenticate.".to_string() + not_logged_in.to_string() } AuthenticationError::ProfileNotFound(name) => { format!("Active profile '{name}' not found. Run `screenly auth switch` to pick a valid profile.") @@ -42,38 +47,148 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { /// Resolves the profile name a `login` should store under. /// -/// An explicit name is always honored. With no name given, a fresh install -/// (no existing profiles) defaults to `"default"`; otherwise `None` is -/// returned to signal that `--name` is required so an existing profile is -/// not overwritten by accident. -fn resolve_login_name(name: Option<&str>, existing: &[(String, bool)]) -> Option { +/// An explicit `--name` is always honored. With no name given, `login` +/// updates the currently active profile (the common re-login-after-rotation +/// flow), falling back to `"default"` on a fresh install with no active +/// profile. +fn resolve_login_name(name: Option<&str>, active: Option<&str>) -> String { match name { - Some(n) => Some(n.to_string()), - None if existing.is_empty() => Some("default".to_string()), - None => None, + Some(n) => n.to_string(), + None => active.unwrap_or("default").to_string(), } } -/// Prints the stored profiles as a table with email and workspace columns, -/// marking the active profile with a `*`. -fn print_profiles_table(entries: &[ProfileEntry]) { - let name_w = entries.iter().map(|e| e.name.len()).max().unwrap_or(0); - let email_w = entries +/// Renders the stored profiles as an aligned table, marking the active +/// profile with `*`. Returns a `String` so it can be unit-tested and reused +/// across output formats. Column widths account for the header labels and for +/// the `(unavailable)` placeholder shown when a profile's info can't be +/// fetched. +fn format_profiles_table(entries: &[ProfileEntry]) -> String { + // Resolve each row's cells first so the widths cover placeholders too. + let rows: Vec<(&str, String, String, bool)> = entries .iter() - .filter_map(|e| e.info.as_ref().map(|i| i.email.len())) + .map(|e| match &e.info { + Some(info) => ( + e.name.as_str(), + info.email.clone(), + info.workspace.clone(), + e.is_active, + ), + None => ( + e.name.as_str(), + "(unavailable)".to_string(), + "(unavailable)".to_string(), + e.is_active, + ), + }) + .collect(); + + let name_w = rows + .iter() + .map(|r| r.0.len()) + .max() + .unwrap_or(0) + .max("Profile".len()); + let email_w = rows + .iter() + .map(|r| r.1.len()) .max() - .unwrap_or(0); - - println!(" {: println!( - "{marker} {: bool { + true + } + + fn format(&self, output_type: OutputType) -> String { + match output_type { + OutputType::Json => serde_json::to_string_pretty(&json!({ + "profile": self.profile, + "email": self.email, + "workspace": self.workspace, + })) + .unwrap(), + OutputType::Csv => { + let mut wtr = csv::WriterBuilder::new().from_writer(vec![]); + wtr.write_record(["Profile", "Email", "Workspace"]).unwrap(); + wtr.write_record([ + self.profile.as_str(), + self.email.as_str(), + self.workspace.as_str(), + ]) + .unwrap(); + String::from_utf8(wtr.into_inner().unwrap()).unwrap() + } + OutputType::HumanReadable => format!( + "Profile: {}\nEmail: {}\nWorkspace: {}", + self.profile, self.email, self.workspace ), - None => println!("{marker} {}", entry.name), + } + } +} + +/// The stored profiles, rendered for the `auth list` command. +struct ProfilesTable(Vec); + +impl Formatter for ProfilesTable { + fn supports_csv() -> bool { + true + } + + fn format(&self, output_type: OutputType) -> String { + match output_type { + OutputType::HumanReadable => format_profiles_table(&self.0), + OutputType::Json => { + let arr: Vec = self + .0 + .iter() + .map(|e| { + json!({ + "profile": e.name, + "active": e.is_active, + "email": e.info.as_ref().map(|i| i.email.clone()), + "workspace": e.info.as_ref().map(|i| i.workspace.clone()), + }) + }) + .collect(); + serde_json::to_string_pretty(&serde_json::Value::Array(arr)).unwrap() + } + OutputType::Csv => { + let mut wtr = csv::WriterBuilder::new().from_writer(vec![]); + wtr.write_record(["Profile", "Active", "Email", "Workspace"]) + .unwrap(); + for e in &self.0 { + let active = e.is_active.to_string(); + let (email, workspace) = match &e.info { + Some(i) => (i.email.as_str(), i.workspace.as_str()), + None => ("", ""), + }; + wtr.write_record([e.name.as_str(), active.as_str(), email, workspace]) + .unwrap(); + } + String::from_utf8(wtr.into_inner().unwrap()).unwrap() + } } } } @@ -135,7 +250,8 @@ pub struct Cli { pub enum Commands { /// Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token. Login { - /// Profile name to store the token under. Required when other profiles already exist. + /// Profile name to store the token under. Defaults to the active + /// profile, or "default" on a fresh install. #[arg(long)] name: Option, }, @@ -146,11 +262,7 @@ pub enum Commands { name: Option, }, /// Show information about the currently authenticated profile. - Me { - /// Enables JSON output. - #[arg(short, long, action = clap::ArgAction::SetTrue)] - json: Option, - }, + Me {}, /// Manage stored authentication profiles. #[command(subcommand)] Auth(AuthCommands), @@ -178,10 +290,9 @@ pub enum AuthCommands { /// List stored authentication profiles. List {}, /// Switch the active authentication profile. - /// - /// Without an argument, prints the list of profiles instead of switching. Switch { - /// Profile name to activate. Omit to print the profile list. + /// Profile name to activate. If omitted, the available profiles are + /// listed and the command exits with an error. name: Option, }, } @@ -624,14 +735,8 @@ pub fn handle_cli(cli: &Cli) { match &cli.command { Commands::Login { name } => { - let existing = Authentication::list_profiles().unwrap_or_default(); - let resolved_name = match resolve_login_name(name.as_deref(), &existing) { - Some(resolved) => resolved, - None => { - error!("A profile already exists. Please specify a profile name with --name."); - std::process::exit(1); - } - }; + let active = active_profile_name(); + let resolved_name = resolve_login_name(name.as_deref(), active.as_deref()); print!("Enter your API Token: "); std::io::stdout().flush().unwrap(); let token = read_password().unwrap(); @@ -657,32 +762,24 @@ pub fn handle_cli(cli: &Cli) { Commands::Asset(command) => handle_cli_asset_command(command, output), Commands::EdgeApp(command) => handle_cli_edge_app_command(command, output), Commands::Playlist(command) => handle_cli_playlist_command(command, output), - Commands::Me { json } => { + Commands::Me {} => { let auth = get_authentication(); match fetch_profile_info(&auth.token, &auth.config.url) { Ok(info) => { - let json_flag = json.unwrap_or(false); - let profile = match active_profile_name() { - Some(name) => name, - None => "(from API_TOKEN env)".to_string(), - }; - if json_flag { - let mut obj = serde_json::Map::new(); - obj.insert("profile".to_string(), serde_json::Value::String(profile)); - obj.insert("email".to_string(), serde_json::Value::String(info.email)); - obj.insert( - "workspace".to_string(), - serde_json::Value::String(info.workspace), - ); - println!( - "{}", - serde_json::to_string_pretty(&serde_json::Value::Object(obj)).unwrap() - ); + // read_token() prefers API_TOKEN over the stored profile, + // so the label must follow the same precedence, otherwise + // it names the wrong profile when both are present. + let profile = if env::var("API_TOKEN").is_ok() { + "(from API_TOKEN env)".to_string() } else { - println!("Profile: {profile}"); - println!("Email: {}", info.email); - println!("Workspace: {}", info.workspace); - } + active_profile_name().unwrap_or_else(|| "unknown".to_string()) + }; + let details = ProfileDetails { + profile, + email: info.email, + workspace: info.workspace, + }; + handle_command_execution_result(Ok::<_, CommandError>(details), output); } Err(AuthenticationError::WrongCredentials) => { error!("Token is invalid. Run `screenly login` to update your credentials."); @@ -721,7 +818,12 @@ pub fn handle_cli(cli: &Cli) { Ok(entries) if entries.is_empty() => { info!("No profiles stored. Run `screenly login` to add one."); } - Ok(entries) => print_profiles_table(&entries), + Ok(entries) => { + handle_command_execution_result( + Ok::<_, CommandError>(ProfilesTable(entries)), + output, + ); + } Err(e) => { error!("Error occurred: {e}"); std::process::exit(1); @@ -729,9 +831,21 @@ pub fn handle_cli(cli: &Cli) { }, AuthCommands::Switch { name } => match name { None => { - if let Ok(entries) = fetch_profiles_with_info(&Config::default().url) { - print_profiles_table(&entries); + // A missing argument is a usage error, so exit non-zero + // (scripts can detect it) but still print the available + // profile names as a hint. Names come from the local store, + // so this needs no network round-trips. + error!("No profile name given. Specify one of the profiles below:"); + match Authentication::list_profiles() { + Ok(profiles) => { + for profile in profiles { + let marker = if profile.is_active { "*" } else { " " }; + println!("{marker} {}", profile.name); + } + } + Err(e) => error!("Could not read profiles: {e}"), } + std::process::exit(1); } Some(name) => match Authentication::switch_profile(name) { Ok(()) => { @@ -1410,22 +1524,62 @@ mod tests { #[test] fn test_resolve_login_name_defaults_to_default_on_fresh_install() { - assert_eq!(resolve_login_name(None, &[]), Some("default".to_string())); + assert_eq!(resolve_login_name(None, None), "default"); } #[test] fn test_resolve_login_name_honors_explicit_name() { - let existing = vec![("prod".to_string(), true)]; - assert_eq!( - resolve_login_name(Some("stage"), &existing), - Some("stage".to_string()) - ); + assert_eq!(resolve_login_name(Some("stage"), Some("prod")), "stage"); } #[test] - fn test_resolve_login_name_requires_name_when_profiles_exist() { - let existing = vec![("prod".to_string(), true)]; - assert_eq!(resolve_login_name(None, &existing), None); + fn test_resolve_login_name_defaults_to_active_profile() { + // Plain `login` with a profile already active updates that profile + // rather than failing (the re-login-after-rotation flow). + assert_eq!(resolve_login_name(None, Some("prod")), "prod"); + } + + #[test] + fn test_format_profiles_table_aligns_headers_and_placeholders() { + use crate::authentication::{ProfileEntry, ProfileInfo}; + + // A short name/email (shorter than the headers) and a profile with no + // info at all -- both used to break alignment. + let entries = vec![ + ProfileEntry { + name: "a".to_string(), + is_active: true, + info: Some(ProfileInfo { + email: "x@y.z".to_string(), + workspace: "Team".to_string(), + }), + }, + ProfileEntry { + name: "staging".to_string(), + is_active: false, + info: None, + }, + ]; + + let table = format_profiles_table(&entries); + let lines: Vec<&str> = table.lines().collect(); + + // Header column is at least as wide as "Profile" even though the + // widest name ("staging") is exactly 7 chars. + assert!(lines[0].starts_with(" Profile ")); + + // The Email and Workspace columns start at the same offset on the + // header and on every data row (lines[0] header, [1] rule, [2..] data). + let email_col = lines[0].find("Email").unwrap(); + let ws_col = lines[0].find("Workspace").unwrap(); + assert_eq!(lines[2].find("x@y.z"), Some(email_col)); + assert_eq!(lines[2].find("Team"), Some(ws_col)); + // Row with missing info renders a placeholder in the same columns + // rather than dropping them. + assert_eq!(lines[3].find("(unavailable)"), Some(email_col)); + + // Active profile is marked. + assert!(lines[2].starts_with("* a")); } #[test] From 52d67948bf1cd46779c556615b941238d6796577 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 13:10:31 +0400 Subject: [PATCH 18/31] Document profiles and refresh generated CLI help Add a README section covering named profiles, login/logout --name, auth list, auth switch, me, the API_TOKEN override, and legacy-file migration. Regenerate CommandLineHelp.md for the me/login/switch help changes. --- README.md | 30 ++++++++++++++++++++++++++++++ docs/CommandLineHelp.md | 14 ++++---------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 983b48e9..0cd32f56 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,36 @@ $ API_SERVER_NAME=local cargo build --release Explore available commands [here](https://developer.screenly.io/cli/#commands). +## Authentication and profiles + +Credentials are stored in `~/.screenly` as named profiles, with one profile active at a time. This lets you keep several tokens (for example one per workspace) and switch between them. + +```bash +# Log in. On a fresh install this creates the "default" profile; with a +# profile already active it updates that profile (e.g. after a token rotation). +$ screenly login + +# Log in under a specific profile name. +$ screenly login --name work + +# Show the profile you are currently authenticated as. +$ screenly me + +# List stored profiles (the active one is marked with *). Honors --output. +$ screenly auth list + +# Switch the active profile. +$ screenly auth switch work + +# Remove a profile. Without --name, removes the active one. +$ screenly logout +$ screenly logout --name work +``` + +The `API_TOKEN` environment variable overrides the stored profiles when set, so `me` and every other command authenticate with that token regardless of the active profile. + +Plain-text `~/.screenly` files from older versions are migrated to the profile format automatically on first write. + ## Output Formats All list and get commands support three output formats via the global `--output` (`-o`) flag: diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index 5f7efc62..90d97ec8 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -95,7 +95,7 @@ Logs in with the provided token and stores it for further use if valid. You can ###### **Options:** -* `--name ` — Profile name to store the token under. Required when other profiles already exist +* `--name ` — Profile name to store the token under. Defaults to the active profile, or "default" on a fresh install @@ -115,11 +115,7 @@ Logs out and removes the stored token Show information about the currently authenticated profile -**Usage:** `screenly me [OPTIONS]` - -###### **Options:** - -* `-j`, `--json` — Enables JSON output +**Usage:** `screenly me` @@ -146,15 +142,13 @@ List stored authentication profiles ## `screenly auth switch` -Switch the active authentication profile. - -Without an argument, prints the list of profiles instead of switching. +Switch the active authentication profile **Usage:** `screenly auth switch [NAME]` ###### **Arguments:** -* `` — Profile name to activate. Omit to print the profile list +* `` — Profile name to activate. If omitted, the available profiles are listed and the command exits with an error From a50ee24f733f299a87ad4c3fa3cfc271f58a11df Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 13:16:44 +0400 Subject: [PATCH 19/31] Give a readable, actionable error for a corrupt credentials file Instead of surfacing a raw serde 'missing field' error, a store that fails to parse now returns AuthenticationError::CorruptStore, which names the file's path and tells the user to fix it or delete it and run `screenly login`, and reassures that stored profiles are left untouched. No interactive reset: a corrupt file often still holds recoverable tokens, so the user decides whether to repair or discard it. Documented the behavior in the README. --- README.md | 2 ++ src/authentication.rs | 25 +++++++++++++++++++++++-- src/cli.rs | 4 +++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0cd32f56..eb78b515 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,8 @@ The `API_TOKEN` environment variable overrides the stored profiles when set, so Plain-text `~/.screenly` files from older versions are migrated to the profile format automatically on first write. +If `~/.screenly` becomes malformed (for example from a hand-edit), the CLI reports the problem and leaves the file untouched rather than discarding credentials. Fix the file's YAML, or delete it and run `screenly login` to start fresh. + ## Output Formats All list and get commands support three output formats via the global `--output` (`-o`) flag: diff --git a/src/authentication.rs b/src/authentication.rs index e372a200..c9cd53d8 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -36,6 +36,16 @@ pub enum AuthenticationError { InvalidHeader(#[from] InvalidHeaderValue), #[error("yaml error: {0}")] Yaml(#[from] serde_yaml::Error), + #[error( + "The credentials file at {path} could not be parsed ({source}).\n\ + Fix its contents, or delete it (`rm {path}`) and run `screenly login` to start fresh. \ + Your stored profiles are left untouched until you do." + )] + CorruptStore { + path: String, + #[source] + source: serde_yaml::Error, + }, #[error("unknown error")] Unknown, } @@ -100,7 +110,10 @@ fn read_store() -> Result { store.active = Some("default".to_string()); Ok(store) } else { - Err(AuthenticationError::Yaml(yaml_err)) + Err(AuthenticationError::CorruptStore { + path: path.display().to_string(), + source: yaml_err, + }) } } } @@ -524,7 +537,15 @@ mod tests { ) .unwrap(); - assert!(matches!(read_store(), Err(AuthenticationError::Yaml(_)))); + match read_store() { + Err(e @ AuthenticationError::CorruptStore { .. }) => { + // The message tells the user where the file is and what to do. + let msg = e.to_string(); + assert!(msg.contains(".screenly")); + assert!(msg.contains("screenly login")); + } + _ => panic!("expected CorruptStore error"), + } } #[test] diff --git a/src/cli.rs b/src/cli.rs index 4689500d..71d3818f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -39,6 +39,8 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { AuthenticationError::ProfileNotFound(name) => { format!("Active profile '{name}' not found. Run `screenly auth switch` to pick a valid profile.") } + // Already actionable and names the file; pass it through verbatim. + AuthenticationError::CorruptStore { .. } => e.to_string(), _ => { format!("Authentication error: {e}. Please run `screenly login` to authenticate.") } @@ -752,7 +754,7 @@ pub fn handle_cli(cli: &Cli) { std::process::exit(1); } _ => { - error!("Error occurred: {e:?}"); + error!("{e}"); std::process::exit(1); } }, From 855bca3be0f4e9fa18a60d7a1f79b262077e5726 Mon Sep 17 00:00:00 2001 From: 514sid Date: Thu, 23 Jul 2026 15:19:12 +0400 Subject: [PATCH 20/31] Treat an empty ~/.screenly as an empty store, not a corrupt one A zero-byte or whitespace-only credentials file previously took the CorruptStore path ("missing field tokens") and blocked `login` -- the very command the error suggests. read_store now returns an empty store for blank contents, matching the missing-file behavior. Test covers both empty and whitespace-only files. --- src/authentication.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/authentication.rs b/src/authentication.rs index c9cd53d8..b91b2870 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -92,6 +92,11 @@ fn read_store() -> Result { return Ok(TokenStore::default()); } let contents = fs::read_to_string(&path)?; + // An empty or whitespace-only file is treated like a missing one rather + // than a parse error, so it doesn't block `login` on a fresh/blank store. + if contents.trim().is_empty() { + return Ok(TokenStore::default()); + } match serde_yaml::from_str::(&contents) { Ok(store) => Ok(store), Err(yaml_err) => { @@ -548,6 +553,23 @@ mod tests { } } + #[test] + fn test_read_store_empty_file_is_treated_as_empty_store() { + // A zero-byte or whitespace-only file behaves like a missing one, so + // it doesn't take the CorruptStore path and block `login`. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let path = tmp_dir.path().join(".screenly"); + + for contents in ["", " \n\t"] { + fs::write(&path, contents).unwrap(); + let store = read_store().unwrap(); + assert!(store.tokens.is_empty()); + assert!(store.active.is_none()); + } + } + #[test] fn test_legacy_plain_text_is_rewritten_as_yaml_on_first_write() { let tmp_dir = tempdir().unwrap(); From b65363d8aea216fea7926645dc64d046c2169bb9 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:08:06 +0400 Subject: [PATCH 21/31] Make `auth list` honor --output when no profiles are stored The empty case short-circuited before the Formatter and printed a log line, so `auth list --output json` exited 0 with nothing on stdout and `| jq` failed on empty input. The "no profiles" hint is only useful to a human, so it now lives in the human-readable arm of the Formatter; JSON renders `[]` and CSV keeps its header. --- src/cli.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 71d3818f..12fc86ad 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -160,6 +160,12 @@ impl Formatter for ProfilesTable { fn format(&self, output_type: OutputType) -> String { match output_type { + // The "no profiles" hint is only useful to a human. The machine + // formats render an empty array / a bare header instead, so a + // consumer piping `--output json` always gets parseable output. + OutputType::HumanReadable if self.0.is_empty() => { + "No profiles stored. Run `screenly login` to add one.".to_string() + } OutputType::HumanReadable => format_profiles_table(&self.0), OutputType::Json => { let arr: Vec = self @@ -817,9 +823,6 @@ pub fn handle_cli(cli: &Cli) { }, Commands::Auth(auth_command) => match auth_command { AuthCommands::List {} => match fetch_profiles_with_info(&Config::default().url) { - Ok(entries) if entries.is_empty() => { - info!("No profiles stored. Run `screenly login` to add one."); - } Ok(entries) => { handle_command_execution_result( Ok::<_, CommandError>(ProfilesTable(entries)), @@ -1541,6 +1544,23 @@ mod tests { assert_eq!(resolve_login_name(None, Some("prod")), "prod"); } + #[test] + fn test_empty_profiles_table_still_renders_machine_formats() { + // With no profiles stored, `--output json` must stay parseable and + // `--output csv` must keep its header. Only the human-readable form + // switches to a hint. + let table = ProfilesTable(vec![]); + + assert_eq!(table.format(OutputType::Json), "[]"); + assert_eq!( + table.format(OutputType::Csv), + "Profile,Active,Email,Workspace\n" + ); + assert!(table + .format(OutputType::HumanReadable) + .contains("No profiles stored")); + } + #[test] fn test_format_profiles_table_aligns_headers_and_placeholders() { use crate::authentication::{ProfileEntry, ProfileInfo}; From 594e27073f6a05e6df7056f554f0cc02aff80832 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:10:22 +0400 Subject: [PATCH 22/31] Don't promote another profile when logging out of the active one `logout` picked the alphabetically first remaining profile as the new active one, so `screenly logout` while `prod` was active left `alpha` active and the next command talked to a different workspace. The determinism was the right fix for the arbitrary-HashMap-key version, but auto-selecting at all is the wrong default for a CLI that pushes content to physical screens. Removing the active profile now clears `active` and leaves the other tokens stored but unselected. `remove_token` returns a `Removal` so `logout` can report which of the three end states it reached, and `read_token` distinguishes that state (`NoActiveProfile`, "run auth switch") from an empty store (`NoCredentials`, "run login") instead of telling a user with stored profiles to log in again. --- README.md | 2 + src/authentication.rs | 96 +++++++++++++++++++++++++++++++++++-------- src/cli.rs | 19 ++++++--- 3 files changed, 94 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index f279da34..9ed0a6ce 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,8 @@ $ screenly logout $ screenly logout --name work ``` +Removing the active profile leaves no profile active, even when others are still stored. The CLI will not pick a replacement for you, because doing so would silently point the next command at a different workspace. Run `screenly auth switch ` to choose one. + The `API_TOKEN` environment variable overrides the stored profiles when set, so `me` and every other command authenticate with that token regardless of the active profile. Plain-text `~/.screenly` files from older versions are migrated to the profile format automatically on first write. diff --git a/src/authentication.rs b/src/authentication.rs index b91b2870..248708d3 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -22,6 +22,8 @@ pub enum AuthenticationError { WrongCredentials, #[error("no credentials error")] NoCredentials, + #[error("no active profile")] + NoActiveProfile, #[error("profile not found: {0}")] ProfileNotFound(String), #[error("request error")] @@ -183,7 +185,14 @@ impl Authentication { return Ok(token); } let store = read_store()?; - let active = store.active.ok_or(AuthenticationError::NoCredentials)?; + // Distinguish "nothing stored at all" from "profiles stored but none + // selected", which is the state `logout` leaves behind when it removes + // the active profile. The two need different advice. + let active = store.active.ok_or(if store.tokens.is_empty() { + AuthenticationError::NoCredentials + } else { + AuthenticationError::NoActiveProfile + })?; store .tokens .get(&active) @@ -192,10 +201,13 @@ impl Authentication { } /// Removes a profile. When `name` is `None` the active profile is removed. - /// If the removed profile was active, the new active profile is chosen - /// deterministically (the alphabetically first remaining profile). - /// Returns the name of the profile that is active after removal, if any. - pub fn remove_token(name: Option<&str>) -> Result, AuthenticationError> { + /// + /// Removing the active profile deliberately leaves *no* profile active + /// rather than promoting another one. Silently re-pointing the CLI at a + /// different account would make the next command talk to a different + /// workspace, so the user has to choose the next profile explicitly with + /// `auth switch`. + pub fn remove_token(name: Option<&str>) -> Result { let mut store = read_store()?; let target = match name { Some(n) => n.to_string(), @@ -209,12 +221,16 @@ impl Authentication { } store.tokens.remove(&target); if store.active.as_deref() == Some(&target) { - let mut remaining: Vec<&String> = store.tokens.keys().collect(); - remaining.sort(); - store.active = remaining.first().map(|n| n.to_string()); + store.active = None; } write_store(&store)?; - Ok(store.active) + let mut remaining: Vec = store.tokens.keys().cloned().collect(); + remaining.sort(); + Ok(Removal { + removed: target, + active: store.active.clone(), + remaining, + }) } /// Returns the stored profiles sorted by name, without their tokens. @@ -284,6 +300,17 @@ pub struct ProfileSummary { pub is_active: bool, } +/// The outcome of removing a profile, so `logout` can say what state the +/// store is in afterwards. +pub struct Removal { + pub removed: String, + /// The profile active after removal. `None` when the removed profile was + /// the active one, whether or not other profiles remain. + pub active: Option, + /// Profiles still stored after the removal, sorted by name. + pub remaining: Vec, +} + pub struct ProfileEntry { pub name: String, pub is_active: bool, @@ -648,8 +675,10 @@ mod tests { ) .unwrap(); - let new_active = Authentication::remove_token(None).unwrap(); - assert!(new_active.is_none()); + let removal = Authentication::remove_token(None).unwrap(); + assert_eq!(removal.removed, "default"); + assert!(removal.active.is_none()); + assert!(removal.remaining.is_empty()); let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) .unwrap(); @@ -701,8 +730,8 @@ mod tests { .unwrap(); // Removing a non-active profile by name leaves the active one intact. - let new_active = Authentication::remove_token(Some("stage")).unwrap(); - assert_eq!(new_active.as_deref(), Some("prod")); + let removal = Authentication::remove_token(Some("stage")).unwrap(); + assert_eq!(removal.active.as_deref(), Some("prod")); let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) .unwrap(); @@ -711,7 +740,7 @@ mod tests { } #[test] - fn test_remove_active_profile_picks_deterministic_new_active() { + fn test_remove_active_profile_leaves_no_profile_active() { let tmp_dir = tempdir().unwrap(); let _lock = lock_test(); let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); @@ -731,13 +760,44 @@ mod tests { ) .unwrap(); - // Removing the active profile picks the alphabetically first remaining one. - let new_active = Authentication::remove_token(None).unwrap(); - assert_eq!(new_active.as_deref(), Some("alpha")); + // Removing the active profile must not promote another one: the CLI + // would silently start talking to a different workspace. + let removal = Authentication::remove_token(None).unwrap(); + assert_eq!(removal.removed, "prod"); + assert!(removal.active.is_none()); + assert_eq!(removal.remaining, vec!["alpha", "stage"]); let store: TokenStore = serde_yaml::from_str(&fs::read_to_string(tmp_dir.path().join(".screenly")).unwrap()) .unwrap(); - assert_eq!(store.active.as_deref(), Some("alpha")); + assert!(store.active.is_none()); + // The other tokens are untouched, just unselected. + assert_eq!(store.tokens.len(), 2); + } + + #[test] + fn test_read_token_reports_no_active_profile_when_profiles_remain() { + // The state `logout` leaves behind when it removes the active profile: + // tokens are still stored, none is selected. That must not read as + // "not logged in", which would tell the user to log in again. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: None, + tokens: [("prod".to_string(), "prod_token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + + assert!(matches!( + Authentication::read_token(), + Err(AuthenticationError::NoActiveProfile) + )); } #[test] diff --git a/src/cli.rs b/src/cli.rs index 12fc86ad..f0cd9119 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -36,6 +36,9 @@ fn get_authentication_error_message(e: &AuthenticationError) -> String { AuthenticationError::Io(io_err) if io_err.kind() == std::io::ErrorKind::NotFound => { not_logged_in.to_string() } + AuthenticationError::NoActiveProfile => { + "No active profile. Run `screenly auth switch ` to choose one, or `screenly auth list` to see what is stored.".to_string() + } AuthenticationError::ProfileNotFound(name) => { format!("Active profile '{name}' not found. Run `screenly auth switch` to pick a valid profile.") } @@ -800,11 +803,17 @@ pub fn handle_cli(cli: &Cli) { } } Commands::Logout { name } => match Authentication::remove_token(name.as_deref()) { - Ok(new_active) => { - info!("Logout successful."); - match new_active { - Some(profile) => info!("Active profile is now '{profile}'."), - None => info!("No profiles remain."), + Ok(removal) => { + info!("Removed profile '{}'.", removal.removed); + match &removal.active { + // A non-active profile was removed, so the CLI still + // authenticates as before. + Some(profile) => info!("Active profile is still '{profile}'."), + None if removal.remaining.is_empty() => info!("No profiles remain."), + None => info!( + "No profile is active now. Run `screenly auth switch ` to pick one of: {}.", + removal.remaining.join(", ") + ), } std::process::exit(0); } From 5f580c73115e45c3b47d96d10262206a3a9c597d Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:12:02 +0400 Subject: [PATCH 23/31] Report authentication errors the same way from every auth command The four new command arms each formatted their fallback differently: `{e}` in login, `Error occurred: {e}` in logout and auth list, and `{e:?}` in auth switch. The Debug form is what an earlier commit removed from login, and the `Error occurred:` prefix made the corrupt-store message read worse from `auth list` than the identical error from `me`. They now share `exit_with_authentication_error`, which routes through `get_authentication_error_message`, as `get_authentication` already did. Command-specific arms (WrongCredentials on login, ProfileNotFound on logout and switch) keep their own better-targeted wording. `auth switch` with no argument also reads the store before reporting the usage error, so an unreadable store no longer prints "specify one of the profiles below" above a parse failure, and an empty store says to run login instead of listing nothing. --- src/cli.rs | 59 +++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index f0cd9119..824cba76 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -204,14 +204,22 @@ impl Formatter for ProfilesTable { } } +/// Reports an authentication error and exits. +/// +/// Every command that touches the credential store funnels its unhandled +/// authentication errors through here, so a corrupt store or a missing profile +/// reads the same whichever command hit it, and never surfaces as a `Debug` +/// dump. +fn exit_with_authentication_error(e: &AuthenticationError) -> ! { + error!("{}", get_authentication_error_message(e)); + std::process::exit(1); +} + /// Creates an Authentication instance or exits with a user-friendly error message. fn get_authentication() -> Authentication { match Authentication::new() { Ok(auth) => auth, - Err(e) => { - error!("{}", get_authentication_error_message(&e)); - std::process::exit(1); - } + Err(e) => exit_with_authentication_error(&e), } } @@ -762,10 +770,7 @@ pub fn handle_cli(cli: &Cli) { error!("Token verification failed."); std::process::exit(1); } - _ => { - error!("{e}"); - std::process::exit(1); - } + _ => exit_with_authentication_error(&e), }, } } @@ -825,10 +830,7 @@ pub fn handle_cli(cli: &Cli) { error!("Profile '{profile}' not found."); std::process::exit(1); } - Err(e) => { - error!("Error occurred: {e}"); - std::process::exit(1); - } + Err(e) => exit_with_authentication_error(&e), }, Commands::Auth(auth_command) => match auth_command { AuthCommands::List {} => match fetch_profiles_with_info(&Config::default().url) { @@ -838,26 +840,28 @@ pub fn handle_cli(cli: &Cli) { output, ); } - Err(e) => { - error!("Error occurred: {e}"); - std::process::exit(1); - } + Err(e) => exit_with_authentication_error(&e), }, AuthCommands::Switch { name } => match name { None => { // A missing argument is a usage error, so exit non-zero // (scripts can detect it) but still print the available // profile names as a hint. Names come from the local store, - // so this needs no network round-trips. + // so this needs no network round-trips. Read them before + // reporting the usage error: if the store itself is + // unreadable, that is the only message worth printing. + let profiles = match Authentication::list_profiles() { + Ok(profiles) => profiles, + Err(e) => exit_with_authentication_error(&e), + }; + if profiles.is_empty() { + error!("No profiles stored. Run `screenly login` to add one."); + std::process::exit(1); + } error!("No profile name given. Specify one of the profiles below:"); - match Authentication::list_profiles() { - Ok(profiles) => { - for profile in profiles { - let marker = if profile.is_active { "*" } else { " " }; - println!("{marker} {}", profile.name); - } - } - Err(e) => error!("Could not read profiles: {e}"), + for profile in profiles { + let marker = if profile.is_active { "*" } else { " " }; + println!("{marker} {}", profile.name); } std::process::exit(1); } @@ -869,10 +873,7 @@ pub fn handle_cli(cli: &Cli) { error!("Profile '{name}' not found."); std::process::exit(1); } - Err(e) => { - error!("Error occurred: {e:?}"); - std::process::exit(1); - } + Err(e) => exit_with_authentication_error(&e), }, }, }, From 966a250bfee13d97354d299e7e7e81633bf885af Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:12:54 +0400 Subject: [PATCH 24/31] Clean up the token temp file on a failed write and sync the directory `write_store` propagated errors with `?` after creating the temp file, so a failure in write_all, sync_all, or rename left `~/.screenly.tmp.` on disk holding every profile's token. The write path now removes the temp file on any error before propagating. The rename is also followed by a best-effort directory sync. The comment claimed a crash mid-write couldn't corrupt the store, but the rename only becomes durable once the directory entry is synced. The new test makes the rename fail (by putting a directory at the target path) and asserts no temp file survives; it fails without the cleanup. --- src/authentication.rs | 60 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 248708d3..9096a182 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -141,11 +141,37 @@ fn write_store(store: &TokenStore) -> Result<(), AuthenticationError> { // mid-write can't corrupt it. The pid suffix keeps two concurrent writers // from sharing (and interleaving into) the same temp file. let tmp_path = path.with_extension(format!("tmp.{}", std::process::id())); - let mut file = create_private_file(&tmp_path)?; - file.write_all(contents.as_bytes())?; + + // The temp file holds every profile's token, so it must not survive a + // failed write. Any error below removes it before propagating. + let result = write_tmp_and_rename(&tmp_path, &path, contents.as_bytes()); + if result.is_err() { + let _ = fs::remove_file(&tmp_path); + } + result +} + +fn write_tmp_and_rename( + tmp_path: &std::path::Path, + path: &std::path::Path, + contents: &[u8], +) -> Result<(), AuthenticationError> { + let mut file = create_private_file(tmp_path)?; + file.write_all(contents)?; file.sync_all()?; drop(file); - fs::rename(&tmp_path, &path)?; + fs::rename(tmp_path, path)?; + + // Renaming is atomic for a concurrent reader, but the directory entry + // itself is only durable once the directory is synced. Without this a + // crash right after the rename can leave the old file (or neither file) + // on disk. Best effort: some platforms refuse to open a directory for + // this, and a failure here does not make the store wrong. + if let Some(dir) = path.parent() { + if let Ok(dir_file) = fs::File::open(dir) { + let _ = dir_file.sync_all(); + } + } Ok(()) } @@ -686,6 +712,34 @@ mod tests { assert!(store.active.is_none()); } + #[test] + fn test_write_store_removes_temp_file_when_the_write_fails() { + // The temp file holds every token, so a failed write must not leave it + // behind. A directory at the target path makes the rename fail after + // the temp file has already been created and written. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + fs::create_dir(tmp_dir.path().join(".screenly")).unwrap(); + + let mut store = TokenStore::default(); + store + .tokens + .insert("default".to_string(), "token".to_string()); + assert!(write_store(&store).is_err()); + + let leftovers: Vec<_> = fs::read_dir(tmp_dir.path()) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().to_string()) + .filter(|name| name.starts_with(".screenly.tmp")) + .collect(); + assert!( + leftovers.is_empty(), + "temp file with tokens left behind: {leftovers:?}" + ); + } + #[test] #[cfg(unix)] fn test_write_store_restricts_permissions_to_owner() { From 5adecabc480b0881a1930b2c1c3b876d8eb673e6 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:13:47 +0400 Subject: [PATCH 25/31] Store tokens in a BTreeMap for stable file order A HashMap reshuffled the YAML keys on every write, so any command that touched the store produced a spurious whole-file rewrite, and three call sites (remove_token, list_profiles, fetch_profiles_with_info) each hand-sorted the keys to get presentable output. A BTreeMap iterates sorted, so the file order is stable and all three sorts go away. --- src/authentication.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 9096a182..76476034 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::BTreeMap; use std::io::Write; use std::{env, fs}; @@ -55,7 +55,10 @@ pub enum AuthenticationError { #[derive(Serialize, Deserialize, Default)] struct TokenStore { active: Option, - tokens: HashMap, + // A BTreeMap, not a HashMap: it keeps the serialized file in a stable key + // order (so rewriting the store doesn't reshuffle it) and it iterates + // sorted, which is the order every caller below wants to present. + tokens: BTreeMap, } pub struct Authentication { @@ -250,12 +253,10 @@ impl Authentication { store.active = None; } write_store(&store)?; - let mut remaining: Vec = store.tokens.keys().cloned().collect(); - remaining.sort(); Ok(Removal { removed: target, active: store.active.clone(), - remaining, + remaining: store.tokens.keys().cloned().collect(), }) } @@ -264,16 +265,14 @@ impl Authentication { /// use `fetch_profiles_with_info` when profile details are needed. pub fn list_profiles() -> Result, AuthenticationError> { let store = read_store()?; - let mut profiles: Vec = store + Ok(store .tokens .keys() .map(|name| ProfileSummary { is_active: store.active.as_deref() == Some(name.as_str()), name: name.clone(), }) - .collect(); - profiles.sort_by(|a, b| a.name.cmp(&b.name)); - Ok(profiles) + .collect()) } pub fn switch_profile(name: &str) -> Result<(), AuthenticationError> { @@ -352,8 +351,7 @@ pub fn fetch_profiles_with_info(api_url: &str) -> Result, Auth use rayon::prelude::*; let store = read_store()?; - let mut names: Vec = store.tokens.keys().cloned().collect(); - names.sort(); + let names: Vec = store.tokens.keys().cloned().collect(); let entries = names .into_par_iter() .map(|name| { From 6dca332e5d6083959102bf8009e0eddfd96676b0 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:15:14 +0400 Subject: [PATCH 26/31] Render `auth list` with prettytable like every other list command The hand-rolled table made `auth list` look unlike `screen list`, and its width math used str::len() (bytes) while the padding directive counts chars, so a non-ASCII profile name over-padded its column and a CJK or emoji name misaligned the row outright. prettytable already solves this and is what every other list command uses. The active marker moves into its own "Active" column, which drops the leading-space-versus-asterisk indent trick. The test no longer asserts hand-computed column offsets; it checks the marker, the placeholder cells, and that the rendered rows stay flush with a non-ASCII name. --- src/cli.rs | 119 ++++++++++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 66 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 824cba76..01d5e7d7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -5,6 +5,7 @@ use std::{env, fs, io}; use clap::{Parser, Subcommand}; use http_auth_basic::Credentials; use log::{error, info}; +use prettytable::{Cell, Row}; use reqwest::StatusCode; use rpassword::read_password; use serde_json::json; @@ -63,55 +64,31 @@ fn resolve_login_name(name: Option<&str>, active: Option<&str>) -> String { } } -/// Renders the stored profiles as an aligned table, marking the active -/// profile with `*`. Returns a `String` so it can be unit-tested and reused -/// across output formats. Column widths account for the header labels and for -/// the `(unavailable)` placeholder shown when a profile's info can't be -/// fetched. +/// Shown for a profile whose token could not be resolved against the API. The +/// profile is stored, it just can't be described right now. +const PROFILE_INFO_UNAVAILABLE: &str = "(unavailable)"; + +/// Renders the stored profiles as a table, marking the active profile with `*`. +/// +/// Uses `prettytable` like every other list command, so `auth list` looks like +/// `screen list` and column widths (including non-ASCII names) are the table's +/// problem rather than this function's. fn format_profiles_table(entries: &[ProfileEntry]) -> String { - // Resolve each row's cells first so the widths cover placeholders too. - let rows: Vec<(&str, String, String, bool)> = entries - .iter() - .map(|e| match &e.info { - Some(info) => ( - e.name.as_str(), - info.email.clone(), - info.workspace.clone(), - e.is_active, - ), - None => ( - e.name.as_str(), - "(unavailable)".to_string(), - "(unavailable)".to_string(), - e.is_active, - ), - }) - .collect(); - - let name_w = rows - .iter() - .map(|r| r.0.len()) - .max() - .unwrap_or(0) - .max("Profile".len()); - let email_w = rows - .iter() - .map(|r| r.1.len()) - .max() - .unwrap_or(0) - .max("Email".len()); - - let mut lines = vec![ - format!(" {: (info.email.as_str(), info.workspace.as_str()), + None => (PROFILE_INFO_UNAVAILABLE, PROFILE_INFO_UNAVAILABLE), + }; + table.add_row(Row::new(vec![ + Cell::new(if entry.is_active { "*" } else { "" }), + Cell::new(&entry.name), + Cell::new(email), + Cell::new(workspace), + ])); } - lines.join("\n") + table.to_string() } /// The current profile's details, rendered for the `me` command. @@ -1572,11 +1549,11 @@ mod tests { } #[test] - fn test_format_profiles_table_aligns_headers_and_placeholders() { + fn test_format_profiles_table_marks_active_and_shows_placeholders() { use crate::authentication::{ProfileEntry, ProfileInfo}; - // A short name/email (shorter than the headers) and a profile with no - // info at all -- both used to break alignment. + // A short name, a non-ASCII name (byte length != display width), and a + // profile whose info could not be fetched. let entries = vec![ ProfileEntry { name: "a".to_string(), @@ -1591,27 +1568,37 @@ mod tests { is_active: false, info: None, }, + ProfileEntry { + name: "работа".to_string(), + is_active: false, + info: Some(ProfileInfo { + email: "ru@y.z".to_string(), + workspace: "Команда".to_string(), + }), + }, ]; let table = format_profiles_table(&entries); let lines: Vec<&str> = table.lines().collect(); - // Header column is at least as wide as "Profile" even though the - // widest name ("staging") is exactly 7 chars. - assert!(lines[0].starts_with(" Profile ")); - - // The Email and Workspace columns start at the same offset on the - // header and on every data row (lines[0] header, [1] rule, [2..] data). - let email_col = lines[0].find("Email").unwrap(); - let ws_col = lines[0].find("Workspace").unwrap(); - assert_eq!(lines[2].find("x@y.z"), Some(email_col)); - assert_eq!(lines[2].find("Team"), Some(ws_col)); - // Row with missing info renders a placeholder in the same columns - // rather than dropping them. - assert_eq!(lines[3].find("(unavailable)"), Some(email_col)); - - // Active profile is marked. - assert!(lines[2].starts_with("* a")); + // prettytable draws the borders, so every line is the same display + // width regardless of how many bytes a name takes. + let widths: Vec = lines.iter().map(|l| l.chars().count()).collect(); + assert!( + widths.windows(2).all(|w| w[0] == w[1]), + "ragged table: {widths:?}\n{table}" + ); + + assert!(table.contains("Active")); + assert!(table.contains("Workspace")); + // The active profile is marked, the others are not. + let row_a = lines.iter().find(|l| l.contains(" a ")).unwrap(); + assert!(row_a.contains("*")); + let row_staging = lines.iter().find(|l| l.contains("staging")).unwrap(); + assert!(!row_staging.contains("*")); + // A profile with no info keeps its row and both columns. + assert_eq!(row_staging.matches("(unavailable)").count(), 2); + assert!(table.contains("работа")); } #[test] From 1f61080d4e184b9dc90fdb1b48fb83560dc62f3a Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:18:00 +0400 Subject: [PATCH 27/31] Bound the profile-info requests with a timeout reqwest sets no default timeout, so `auth list` fanned out two requests per profile with nothing to stop them. Verified against a server that accepts connections and never replies: the command hung indefinitely, so the `(unavailable)` placeholder never appeared for the failure most likely to produce it. It now caps at 30s and renders the placeholder. The timeout is passed per call site rather than baked into `authenticated_client`, because the general-purpose client that shares that constructor also carries asset uploads, which can legitimately run longer than any timeout worth setting for a metadata lookup. 30s is generous on purpose: a too-short cap would report a working profile as unavailable on a slow connection, which is worse than waiting. --- src/authentication.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 76476034..72af64e8 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -293,13 +293,26 @@ impl Authentication { } pub fn build_client(&self) -> Result { - authenticated_client(&self.token) + authenticated_client(&self.token, None) } } +/// How long a single profile-info request may take. Without a cap, `auth list` +/// fans out one request per profile and a single black-holed connection hangs +/// the whole command, so the `(unavailable)` placeholder would never appear for +/// the failure most likely to produce it. +const PROFILE_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + /// Builds a blocking client that sends the auth token and the standard /// `screenly-cli {version}` User-Agent on every request. -fn authenticated_client(token: &str) -> Result { +/// +/// `timeout` is per-request. It is deliberately `None` for the general-purpose +/// client, whose requests include asset uploads that can legitimately run far +/// longer than any timeout worth setting here. +fn authenticated_client( + token: &str, + timeout: Option, +) -> Result { let secret = format!("Token {token}"); let mut default_headers = HeaderMap::new(); default_headers.insert(header::AUTHORIZATION, secret.parse()?); @@ -308,10 +321,11 @@ fn authenticated_client(token: &str) -> Result Result, Auth } pub fn fetch_profile_info(token: &str, api_url: &str) -> Result { - let client = authenticated_client(token)?; + let client = authenticated_client(token, Some(PROFILE_REQUEST_TIMEOUT))?; let user_response = client.get(format!("{api_url}/v4.1/users/me")).send()?; From 8c4973e8c48113ca4bc2c5e700422c013903a0be Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:19:02 +0400 Subject: [PATCH 28/31] Treat an empty API_TOKEN as unset `API_TOKEN=` (set but empty) took precedence over the stored profiles and authenticated with an empty token, so the user got a 401 far from the cause instead of falling back to `~/.screenly`. `me` compounded it by reporting "(from API_TOKEN env)" for a token that was empty. Exporting an empty variable is what a shell does when it interpolates a missing secret, which is exactly what this repo's own GitHub Action does when `screenly_api_token` is not configured. An empty value now counts as unset for both the token lookup and the `me` label, so that case reports "Not logged in" instead. --- src/authentication.rs | 49 ++++++++++++++++++++++++++++++++++++++++++- src/cli.rs | 6 +++--- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/authentication.rs b/src/authentication.rs index 72af64e8..a2c3689d 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -85,6 +85,16 @@ impl Config { } } +/// The token from `API_TOKEN`, which overrides the stored profiles. +/// +/// An empty value counts as unset. Exporting an empty variable is what a shell +/// does when it interpolates a missing secret (the CLI's own GitHub Action does +/// exactly this when `screenly_api_token` is not configured), and authenticating +/// with an empty token only produces a confusing 401 far from the cause. +pub fn api_token_from_env() -> Option { + env::var("API_TOKEN").ok().filter(|t| !t.is_empty()) +} + fn screenly_path() -> Result { dirs::home_dir() .map(|h| h.join(".screenly")) @@ -210,7 +220,7 @@ impl Authentication { } fn read_token() -> Result { - if let Ok(token) = env::var("API_TOKEN") { + if let Some(token) = api_token_from_env() { return Ok(token); } let store = read_store()?; @@ -565,6 +575,43 @@ mod tests { assert_eq!(Authentication::read_token().unwrap(), "env_token"); } + #[test] + fn test_read_token_ignores_an_empty_env_token() { + // A shell interpolating a missing secret exports an empty value. That + // must fall back to the stored profile instead of authenticating with + // an empty token and failing with a distant 401. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _token = set_env(OsString::from("API_TOKEN"), ""); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + let store = TokenStore { + active: Some("default".to_string()), + tokens: [("default".to_string(), "stored_token".to_string())] + .into_iter() + .collect(), + }; + fs::write( + tmp_dir.path().join(".screenly"), + serde_yaml::to_string(&store).unwrap(), + ) + .unwrap(); + assert_eq!(Authentication::read_token().unwrap(), "stored_token"); + } + + #[test] + fn test_read_token_with_empty_env_token_and_no_store_is_not_logged_in() { + // The CI case: empty API_TOKEN, nothing stored. Should say "not logged + // in" rather than authenticate as the empty token. + let tmp_dir = tempdir().unwrap(); + let _lock = lock_test(); + let _token = set_env(OsString::from("API_TOKEN"), ""); + let _test = set_env(OsString::from("HOME"), tmp_dir.path().to_str().unwrap()); + assert!(matches!( + Authentication::read_token(), + Err(AuthenticationError::NoCredentials) + )); + } + #[test] fn test_read_token_correct_token_is_returned() { let tmp_dir = tempdir().unwrap(); diff --git a/src/cli.rs b/src/cli.rs index 01d5e7d7..bed47469 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -12,8 +12,8 @@ use serde_json::json; use thiserror::Error; use crate::authentication::{ - active_profile_name, fetch_profile_info, fetch_profiles_with_info, verify_and_store_token, - Authentication, AuthenticationError, Config, ProfileEntry, + active_profile_name, api_token_from_env, fetch_profile_info, fetch_profiles_with_info, + verify_and_store_token, Authentication, AuthenticationError, Config, ProfileEntry, }; use crate::commands; use crate::commands::edge_app::instance_manifest::InstanceManifest; @@ -762,7 +762,7 @@ pub fn handle_cli(cli: &Cli) { // read_token() prefers API_TOKEN over the stored profile, // so the label must follow the same precedence, otherwise // it names the wrong profile when both are present. - let profile = if env::var("API_TOKEN").is_ok() { + let profile = if api_token_from_env().is_some() { "(from API_TOKEN env)".to_string() } else { active_profile_name().unwrap_or_else(|| "unknown".to_string()) From 93ff49882576e5dd35993fc95d55182e30d79dfa Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:19:27 +0400 Subject: [PATCH 29/31] Update the logout help text for profiles The top-level help still described logout as removing "the stored token", from before there was more than one. It now says what the command does: removes a profile, and removing the active one leaves nothing active without touching the others. Regenerated CommandLineHelp.md. --- docs/CommandLineHelp.md | 4 ++-- src/cli.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index 90d97ec8..b1c3174e 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -61,7 +61,7 @@ Command line interface is intended for quick interaction with Screenly through t ###### **Subcommands:** * `login` — Logs in with the provided token and stores it for further use if valid. You can set the API_TOKEN environment variable to override the stored token -* `logout` — Logs out and removes the stored token +* `logout` — Removes a stored authentication profile. Removing the active profile leaves no profile active; other profiles are kept * `me` — Show information about the currently authenticated profile * `auth` — Manage stored authentication profiles * `screen` — Screen related commands @@ -101,7 +101,7 @@ Logs in with the provided token and stores it for further use if valid. You can ## `screenly logout` -Logs out and removes the stored token +Removes a stored authentication profile. Removing the active profile leaves no profile active; other profiles are kept **Usage:** `screenly logout [OPTIONS]` diff --git a/src/cli.rs b/src/cli.rs index bed47469..a5e2a404 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -251,7 +251,7 @@ pub enum Commands { #[arg(long)] name: Option, }, - /// Logs out and removes the stored token. + /// Removes a stored authentication profile. Removing the active profile leaves no profile active; other profiles are kept. Logout { /// Profile name to remove. Removes the active profile if not specified. #[arg(long)] From 3e368f3124ab4105efa33621f9c1dddb01d83415 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:20:02 +0400 Subject: [PATCH 30/31] Document the store's concurrency and Windows permission limits Two behaviors were implicit in the code. The atomic rename protects a reader from a torn file but does not prevent two concurrent logins from losing one of the two profiles, and the 0600 permissions are Unix-only while the file now holds every token rather than one. Both are acceptable for a hand-driven CLI, but they should be stated rather than inferred, so they are now in the write path's doc comment and the README. --- README.md | 2 ++ src/authentication.rs | 14 +++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9ed0a6ce..6793f9dc 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,8 @@ Plain-text `~/.screenly` files from older versions are migrated to the profile f If `~/.screenly` becomes malformed (for example from a hand-edit), the CLI reports the problem and leaves the file untouched rather than discarding credentials. Fix the file's YAML, or delete it and run `screenly login` to start fresh. +The file holds every profile's token in plain text. On Linux and macOS it is created with `0600` permissions, so only your user can read it. On Windows it inherits the permissions of your home directory, so treat it the way you would any other credentials file. Avoid running two `login` commands at the same time: the CLI replaces the file atomically, but it does not lock it, so simultaneous writes can drop one of the two profiles. + ## Output Formats All list and get commands support three output formats via the global `--output` (`-o`) flag: diff --git a/src/authentication.rs b/src/authentication.rs index a2c3689d..6963fc32 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -145,6 +145,14 @@ fn is_legacy_token(contents: &str) -> bool { !contents.is_empty() && !contents.contains(':') && !contents.contains('\n') } +/// Writes the store, replacing the file atomically. +/// +/// Not safe against a concurrent writer: two `login` processes racing here both +/// read the old store and the second rename wins, so one of the two profiles is +/// lost. The rename keeps any *reader* from seeing a torn file, which is the +/// case that would corrupt credentials; losing one of two simultaneous logins +/// needs file locking, which is not worth the portability cost for a CLI a +/// person drives by hand. fn write_store(store: &TokenStore) -> Result<(), AuthenticationError> { let path = screenly_path()?; let contents = serde_yaml::to_string(store)?; @@ -190,7 +198,11 @@ fn write_tmp_and_rename( /// Creates (or truncates) a file that the token store can be written to, /// owner-readable only from the moment it exists so the token is never -/// briefly world-readable. Permissions are a no-op on non-Unix platforms. +/// briefly world-readable. +/// +/// On non-Unix platforms there is no equivalent here and the store inherits +/// whatever the directory's ACLs give it, which now covers every profile's +/// token rather than one. Documented in the README. #[cfg(unix)] fn create_private_file(path: &std::path::Path) -> Result { use std::os::unix::fs::OpenOptionsExt; From 607324fded6d7b6638fd20d151f873f58b489f41 Mon Sep 17 00:00:00 2001 From: 514sid Date: Tue, 25 Aug 2026 13:46:37 +0400 Subject: [PATCH 31/31] Support non-interactive login instead of panicking on a non-tty `read_password().unwrap()` panicked with exit 101 when stdin was not a terminal, because rpassword fails outright rather than degrading when it has no terminal to disable echo on. That made scripted login impossible for a feature whose point is managing several credentials, and it aborted with a Rust backtrace rather than an error. `login` now reads stdin as a plain line when stdin is not a terminal, so `echo "$TOKEN" | screenly login --name ci` works. `--token-stdin` forces that path on a terminal too, matching `docker login --password-stdin`. The interactive prompt is unchanged when stdin is a tty. The token is trimmed, since the newline from `echo` would otherwise be stored and sent in the Authorization header, and an empty read reports "no token provided" instead of failing later against the API. Remaining I/O errors are reported and exit 1 rather than unwrapping. --- README.md | 5 +++ docs/CommandLineHelp.md | 1 + src/cli.rs | 79 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 6793f9dc..431b780b 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,11 @@ $ screenly login # Log in under a specific profile name. $ screenly login --name work +# Log in without a prompt, for scripts and CI. --token-stdin is optional when +# stdin is already a pipe or a file, and required to skip the prompt on a +# terminal. +$ echo "$SCREENLY_TOKEN" | screenly login --token-stdin --name ci + # Show the profile you are currently authenticated as. $ screenly me diff --git a/docs/CommandLineHelp.md b/docs/CommandLineHelp.md index b1c3174e..2b1388a6 100644 --- a/docs/CommandLineHelp.md +++ b/docs/CommandLineHelp.md @@ -96,6 +96,7 @@ Logs in with the provided token and stores it for further use if valid. You can ###### **Options:** * `--name ` — Profile name to store the token under. Defaults to the active profile, or "default" on a fresh install +* `--token-stdin` — Read the token from stdin instead of prompting, for scripts: `echo "$TOKEN" | screenly login --token-stdin`. Implied when stdin is not a terminal diff --git a/src/cli.rs b/src/cli.rs index a5e2a404..d0707f32 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,4 @@ -use std::io::{Read, Write}; +use std::io::{BufRead, IsTerminal, Read, Write}; use std::path::PathBuf; use std::{env, fs, io}; @@ -64,6 +64,33 @@ fn resolve_login_name(name: Option<&str>, active: Option<&str>) -> String { } } +/// Reads the API token for `login`. +/// +/// With stdin on a terminal, prompts and reads without echoing. With stdin +/// redirected, reads it as a plain line: there is no terminal to disable echo +/// on, and `read_password` fails outright rather than degrading, which used to +/// panic. `--token-stdin` forces the non-interactive path even on a terminal, +/// for scripts that want no prompt at all. +fn read_login_token(force_stdin: bool) -> io::Result { + if force_stdin || !io::stdin().is_terminal() { + return read_token_line(&mut io::stdin().lock()); + } + print!("Enter your API Token: "); + io::stdout().flush()?; + Ok(read_password()?.trim().to_string()) +} + +/// Reads one line as a token. +/// +/// Trimmed because a token never contains surrounding whitespace, and the +/// trailing newline from `echo` or a here-doc would otherwise be stored as part +/// of it and sent in the Authorization header. +fn read_token_line(reader: &mut impl BufRead) -> io::Result { + let mut line = String::new(); + reader.read_line(&mut line)?; + Ok(line.trim().to_string()) +} + /// Shown for a profile whose token could not be resolved against the API. The /// profile is stored, it just can't be described right now. const PROFILE_INFO_UNAVAILABLE: &str = "(unavailable)"; @@ -250,6 +277,9 @@ pub enum Commands { /// profile, or "default" on a fresh install. #[arg(long)] name: Option, + /// Read the token from stdin instead of prompting, for scripts: `echo "$TOKEN" | screenly login --token-stdin`. Implied when stdin is not a terminal. + #[arg(long)] + token_stdin: bool, }, /// Removes a stored authentication profile. Removing the active profile leaves no profile active; other profiles are kept. Logout { @@ -730,12 +760,20 @@ pub fn handle_cli(cli: &Cli) { }; match &cli.command { - Commands::Login { name } => { + Commands::Login { name, token_stdin } => { let active = active_profile_name(); let resolved_name = resolve_login_name(name.as_deref(), active.as_deref()); - print!("Enter your API Token: "); - std::io::stdout().flush().unwrap(); - let token = read_password().unwrap(); + let token = match read_login_token(*token_stdin) { + Ok(token) => token, + Err(e) => { + error!("Could not read the API token: {e}"); + std::process::exit(1); + } + }; + if token.is_empty() { + error!("No API token provided. Pipe one in (`echo \"$TOKEN\" | screenly login --token-stdin`) or run `screenly login` on a terminal to be prompted."); + std::process::exit(1); + } match verify_and_store_token(&token, &resolved_name, &Config::default().url) { Ok(()) => { info!("Login credentials have been saved under profile '{resolved_name}'."); @@ -1514,6 +1552,37 @@ mod tests { use super::*; use crate::authentication::Config; + #[test] + fn test_read_token_line_trims_the_trailing_newline() { + // `echo "$TOKEN" |` appends a newline. Storing it would send it in the + // Authorization header. + let mut input = "tok_abc123\n".as_bytes(); + assert_eq!(read_token_line(&mut input).unwrap(), "tok_abc123"); + + // A here-doc or a Windows-authored file can add a CR too. + let mut crlf = "tok_abc123\r\n".as_bytes(); + assert_eq!(read_token_line(&mut crlf).unwrap(), "tok_abc123"); + + // Surrounding whitespace is not part of a token either. + let mut padded = " tok_abc123 \n".as_bytes(); + assert_eq!(read_token_line(&mut padded).unwrap(), "tok_abc123"); + } + + #[test] + fn test_read_token_line_takes_only_the_first_line() { + // Extra lines on stdin must not end up in the token. + let mut input = "tok_abc123\nnot_the_token\n".as_bytes(); + assert_eq!(read_token_line(&mut input).unwrap(), "tok_abc123"); + } + + #[test] + fn test_read_token_line_on_empty_input_is_empty_not_an_error() { + // Closed/empty stdin reads as empty, which the caller reports as + // "no token provided" rather than failing obscurely. + let mut input = "".as_bytes(); + assert_eq!(read_token_line(&mut input).unwrap(), ""); + } + #[test] fn test_resolve_login_name_defaults_to_default_on_fresh_install() { assert_eq!(resolve_login_name(None, None), "default");