fix/proxmox-reconnect-v1.2.3 #125

Merged
sarman merged 6 commits from fix/proxmox-reconnect-v1.2.3 into beta 2026-06-21 01:33:26 +00:00
6 changed files with 299 additions and 74 deletions
Showing only changes of commit b091602741 - Show all commits

View File

@ -44,7 +44,7 @@ impl Cli {
async fn main() {
let cli = Cli::parse();
let client = crate::proxmox::client::ProxmoxClient::new(&cli.url, 8006, &cli.username);
let mut client = crate::proxmox::client::ProxmoxClient::new(&cli.url, 8006, &cli.username);
let ticket = match client.authenticate(&cli.password).await {
Ok(t) => t,

View File

@ -41,10 +41,15 @@ pub async fn add_proxmox_cluster(
password: String,
state: State<'_, AppState>,
) -> Result<ClusterInfo, String> {
// Create client (no live auth — credentials stored and used on first connect)
let client = ProxmoxClient::new(&connection.url, connection.port, &username);
// Authenticate immediately — this verifies credentials and gives us a live
// ticketed client. If auth fails we return early before touching the DB.
let mut client = ProxmoxClient::new(&connection.url, connection.port, &username);
client
.authenticate(&password)
.await
.map_err(|e| format!("Failed to authenticate with Proxmox: {}", e))?;
// Encrypt raw password for storage; auth happens lazily on first API call
// Encrypt raw password so we can re-authenticate after app restart.
let credentials = serde_json::json!({
"password": password,
"username": username
@ -95,7 +100,7 @@ pub async fn add_proxmox_cluster(
.map_err(|e| format!("Failed to store cluster: {}", e))?;
}
// Store in memory connection pool (unauthenticated; ticket set on first use)
// Insert the authenticated client into the in-memory pool.
{
let mut clusters = state.proxmox_clusters.lock().await;
clusters.insert(id, Arc::new(Mutex::new(client)));
@ -1788,9 +1793,9 @@ pub async fn list_acls(
.await
.map_err(|e| format!("Failed to list ACLs: {}", e))?;
// handle_response already unwraps the Proxmox `{"data": ...}` envelope.
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -1811,8 +1816,7 @@ pub async fn list_users(
.map_err(|e| format!("Failed to list users: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -1857,8 +1861,7 @@ pub async fn get_cluster_notes(
.map_err(|e| format!("Failed to get cluster notes: {}", e))?;
Ok(response
.get("data")
.and_then(|d| d.get("notes"))
.get("notes")
.and_then(|n| n.as_str())
.unwrap_or("")
.to_string())
@ -1907,8 +1910,7 @@ pub async fn search_proxmox_resources(
.map_err(|e| format!("Failed to search resources: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -1931,10 +1933,7 @@ pub async fn get_node_status(
.await
.map_err(|e| format!("Failed to get node status: {}", e))?;
response
.get("data")
.cloned()
.ok_or_else(|| "Invalid response format: missing data field".to_string())
Ok(response)
}
// ─── Phase 11 - Syslog ────────────────────────────────────────────────────────
@ -1958,8 +1957,7 @@ pub async fn get_syslog(
.map_err(|e| format!("Failed to get syslog: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -1983,8 +1981,7 @@ pub async fn list_network_interfaces(
.map_err(|e| format!("Failed to list network interfaces: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -2080,10 +2077,7 @@ pub async fn get_subscription_status(
.await
.map_err(|e| format!("Failed to get subscription status: {}", e))?;
response
.get("data")
.cloned()
.ok_or_else(|| "Invalid response format: missing data field".to_string())
Ok(response)
}
// ─── Phase 15 - Cluster Task Log ─────────────────────────────────────────────
@ -2106,8 +2100,7 @@ pub async fn list_cluster_tasks(
.map_err(|e| format!("Failed to list cluster tasks: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
@ -2128,12 +2121,86 @@ pub async fn list_proxmox_containers(
.map_err(|e| format!("Failed to list containers: {}", e))?;
response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string())
}
/// Connect (or re-connect) to a Proxmox cluster that already exists in the DB.
/// Loads the stored credentials, authenticates, and inserts the ticketed client
/// into the in-memory pool. Returns `true` on success.
///
/// This is the action triggered by the "Connect" button in the Remotes UI and is
/// the path taken on every app restart for clusters that should be active.
#[tauri::command]
pub async fn connect_proxmox_cluster(
cluster_id: String,
state: State<'_, AppState>,
) -> Result<bool, String> {
let (url, port, username, encrypted_credentials) = {
let db = state
.db
.lock()
.map_err(|e| format!("Failed to lock database: {}", e))?;
let mut stmt = db
.prepare(
"SELECT url, port, username, encrypted_credentials \
FROM proxmox_clusters WHERE id = ?1",
)
.map_err(|e| format!("Failed to prepare query: {}", e))?;
stmt.query_row([&cluster_id], |row| {
Ok((
row.get::<_, String>(0)?,
row.get::<_, u16>(1)?,
row.get::<_, String>(2)?,
row.get::<_, String>(3)?,
))
})
.optional()
.map_err(|e| format!("Failed to query cluster: {}", e))?
.ok_or_else(|| format!("Cluster {} not found in database", cluster_id))?
};
let credentials_json = crate::integrations::auth::decrypt_token(&encrypted_credentials)
.map_err(|e| format!("Failed to decrypt credentials: {}", e))?;
let credentials: serde_json::Value = serde_json::from_str(&credentials_json)
.map_err(|e| format!("Failed to parse credentials: {}", e))?;
let password = credentials
.get("password")
.and_then(|v| v.as_str())
.ok_or_else(|| "Password not found in credentials".to_string())?;
let mut client = crate::proxmox::ProxmoxClient::new(&url, port, &username);
client
.authenticate(password)
.await
.map_err(|e| format!("Failed to authenticate with Proxmox: {}", e))?;
{
let mut clusters = state.proxmox_clusters.lock().await;
clusters.insert(cluster_id, Arc::new(Mutex::new(client)));
}
Ok(true)
}
/// Remove a Proxmox cluster's authenticated session from the in-memory pool.
/// The cluster record and credentials remain in the DB — use `connect_proxmox_cluster`
/// to reconnect.
#[tauri::command]
pub async fn disconnect_proxmox_cluster(
cluster_id: String,
state: State<'_, AppState>,
) -> Result<(), String> {
let mut clusters = state.proxmox_clusters.lock().await;
clusters.remove(&cluster_id);
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
@ -2171,17 +2238,20 @@ mod tests {
}
#[test]
fn test_list_proxmox_containers_error_message() {
fn test_cluster_not_found_error_message() {
let err = format!("Cluster {} not found", "missing-id");
assert_eq!(err, "Cluster missing-id not found");
}
// After the double-unwrap fix, handle_response returns the inner `data`
// value directly. Commands call `.as_array()` on the already-unwrapped value.
#[test]
fn test_list_proxmox_containers_invalid_response() {
let response = serde_json::json!({"other": "field"});
fn test_array_response_already_unwrapped_invalid() {
// The value returned by handle_response is not an array.
let response = serde_json::json!({"some": "object"});
let result: Result<Vec<serde_json::Value>, String> = response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string());
assert!(result.is_err());
@ -2189,16 +2259,14 @@ mod tests {
}
#[test]
fn test_list_proxmox_containers_valid_response() {
let response = serde_json::json!({
"data": [
{"vmid": 200, "name": "nginx-proxy", "node": "pve1", "status": "running"},
{"vmid": 201, "name": "redis-cache", "node": "pve2", "status": "running"}
]
});
fn test_array_response_already_unwrapped_valid() {
// handle_response strips {"data": [...]}, commands receive the raw array.
let response = serde_json::json!([
{"vmid": 200, "name": "nginx-proxy", "node": "pve1", "status": "running"},
{"vmid": 201, "name": "redis-cache", "node": "pve2", "status": "running"}
]);
let result: Result<Vec<serde_json::Value>, String> = response
.get("data")
.and_then(|d| d.as_array())
.as_array()
.map(|arr| arr.to_vec())
.ok_or_else(|| "Invalid response format".to_string());
assert!(result.is_ok());
@ -2211,6 +2279,35 @@ mod tests {
assert_eq!(err, "Cluster missing-id not found");
}
#[test]
fn test_cluster_notes_already_unwrapped_present() {
let response = serde_json::json!({"notes": "Important info", "name": "pve"});
let notes = response
.get("notes")
.and_then(|n| n.as_str())
.unwrap_or("")
.to_string();
assert_eq!(notes, "Important info");
}
#[test]
fn test_cluster_notes_already_unwrapped_missing_defaults_empty() {
let response = serde_json::json!({"name": "pve"});
let notes = response
.get("notes")
.and_then(|n| n.as_str())
.unwrap_or("")
.to_string();
assert_eq!(notes, "");
}
#[test]
fn test_connect_cluster_db_not_found_error_message() {
let msg = format!("Cluster {} not found in database", "unknown-id");
assert!(msg.contains("unknown-id"));
assert!(msg.contains("not found in database"));
}
#[test]
fn test_update_proxmox_cluster_rows_zero_means_not_found() {
let rows: usize = 0;

View File

@ -227,6 +227,8 @@ pub fn run() {
commands::proxmox::remove_proxmox_cluster,
commands::proxmox::update_proxmox_cluster,
commands::proxmox::ping_proxmox_cluster,
commands::proxmox::connect_proxmox_cluster,
commands::proxmox::disconnect_proxmox_cluster,
commands::proxmox::list_proxmox_clusters,
commands::proxmox::get_proxmox_cluster,
commands::proxmox::list_proxmox_vms,

View File

@ -12,16 +12,32 @@ pub struct ProxmoxClient {
username: String,
api_token: Option<String>,
pub ticket: Option<String>,
pub csrf_token: Option<String>,
client: Client,
}
/// Authentication response from Proxmox
/// Outer envelope wrapping every Proxmox API response.
#[derive(Debug, Deserialize)]
struct ProxmoxEnvelope<T> {
data: T,
}
/// Authentication response from Proxmox (inner `data` object).
#[derive(Debug, Deserialize)]
#[serde(rename_all = "PascalCase")]
pub struct AuthResponse {
/// Cookie value — `PVEAuthCookie=<ticket>`.
pub ticket: String,
pub username: String,
/// Seconds since epoch when the ticket expires.
#[serde(default)]
pub expire: u64,
pub cap: String,
/// Required on mutating requests as `CSRFPreventionToken` header.
#[serde(rename = "CSRFPreventionToken")]
pub csrf_prevention_token: Option<String>,
/// Capability map — structure varies, only needed for display/debug.
#[serde(default)]
pub cap: Option<serde_json::Value>,
}
/// API token for authentication
@ -42,21 +58,28 @@ impl ProxmoxClient {
username: username.to_string(),
api_token: None,
ticket: None,
csrf_token: None,
client: Client::builder()
.danger_accept_invalid_certs(true)
.timeout(Duration::from_secs(30))
.build()
.expect("Failed to create HTTP client"),
}
}
/// Set the ticket for authentication
/// Set the ticket for cookie-based authentication.
pub fn set_ticket(&mut self, ticket: &str) {
self.ticket = Some(ticket.to_string());
}
/// Authenticate with root username and password
/// Returns the API ticket for subsequent requests
pub async fn authenticate(&self, password: &str) -> Result<String> {
/// Set the CSRF prevention token (required for mutating requests).
pub fn set_csrf_token(&mut self, token: &str) {
self.csrf_token = Some(token.to_string());
}
/// Authenticate with username + password.
/// Stores the ticket and CSRF token on success; returns the ticket string.
pub async fn authenticate(&mut self, password: &str) -> Result<String> {
let url = format!(
"https://{}:{}/api2/json/access/ticket",
self.base_url, self.port
@ -82,11 +105,17 @@ impl ProxmoxClient {
));
}
let auth: AuthResponse = response
let envelope: ProxmoxEnvelope<AuthResponse> = response
.json()
.await
.map_err(|e| anyhow!("Failed to parse authentication response: {}", e))?;
let auth = envelope.data;
self.ticket = Some(auth.ticket.clone());
if let Some(csrf) = auth.csrf_prevention_token {
self.csrf_token = Some(csrf);
}
Ok(auth.ticket)
}
@ -105,12 +134,12 @@ impl ProxmoxClient {
)
}
/// Build request headers with authentication
fn build_headers(&self, ticket: Option<&str>) -> reqwest::header::HeaderMap {
/// Build request headers with authentication.
/// `include_csrf` should be true for POST / PUT / DELETE requests.
fn build_headers(&self, ticket: Option<&str>, include_csrf: bool) -> reqwest::header::HeaderMap {
let mut headers = reqwest::header::HeaderMap::new();
if let Some(token) = &self.api_token {
// API token format: user@realm!tokenid=tokenvalue
headers.insert(
reqwest::header::AUTHORIZATION,
format!("PVEAPIAuth {}", token)
@ -118,13 +147,20 @@ impl ProxmoxClient {
.expect("Invalid auth header"),
);
} else if let Some(ticket) = ticket {
// Cookie-based authentication
headers.insert(
"Cookie",
format!("PVEAuthCookie={}", ticket)
.parse()
.expect("Invalid cookie header"),
);
if include_csrf {
if let Some(csrf) = &self.csrf_token {
headers.insert(
"CSRFPreventionToken",
csrf.parse().expect("Invalid CSRF token header"),
);
}
}
}
headers.insert(
@ -144,7 +180,7 @@ impl ProxmoxClient {
ticket: Option<&str>,
) -> Result<T> {
let url = self.get_api_url(path);
let headers = self.build_headers(ticket);
let headers = self.build_headers(ticket, false);
let response = self
.client
@ -165,7 +201,7 @@ impl ProxmoxClient {
ticket: Option<&str>,
) -> Result<T> {
let url = self.get_api_url(path);
let headers = self.build_headers(ticket);
let headers = self.build_headers(ticket, true);
let response = self
.client
@ -187,7 +223,7 @@ impl ProxmoxClient {
ticket: Option<&str>,
) -> Result<T> {
let url = self.get_api_url(path);
let headers = self.build_headers(ticket);
let headers = self.build_headers(ticket, true);
let response = self
.client
@ -208,7 +244,7 @@ impl ProxmoxClient {
ticket: Option<&str>,
) -> Result<T> {
let url = self.get_api_url(path);
let headers = self.build_headers(ticket);
let headers = self.build_headers(ticket, true);
let response = self
.client
@ -280,6 +316,8 @@ mod tests {
assert_eq!(client.base_url(), "pve.example.com");
assert_eq!(client.port(), 8006);
assert_eq!(client.username(), "root@pam");
assert!(client.ticket.is_none());
assert!(client.csrf_token.is_none());
}
#[test]
@ -300,4 +338,73 @@ mod tests {
"https://pve.example.com:8006/api2/json/cluster/resources"
);
}
#[test]
fn test_auth_response_envelope_deserialization() {
// Validates that the `{"data": {...}}` envelope Proxmox uses is parsed
// correctly into ProxmoxEnvelope<AuthResponse>.
let json = r#"{
"data": {
"Ticket": "PVE:root@pam:12345",
"Username": "root@pam",
"Expire": 1800,
"CSRFPreventionToken": "abc123",
"Cap": null
}
}"#;
let envelope: ProxmoxEnvelope<AuthResponse> =
serde_json::from_str(json).expect("envelope should parse");
assert_eq!(envelope.data.ticket, "PVE:root@pam:12345");
assert_eq!(
envelope.data.csrf_prevention_token.as_deref(),
Some("abc123")
);
}
#[test]
fn test_auth_response_envelope_no_csrf() {
// Some Proxmox versions or API tokens may omit CSRFPreventionToken.
let json = r#"{
"data": {
"Ticket": "PVE:root@pam:99999",
"Username": "root@pam"
}
}"#;
let envelope: ProxmoxEnvelope<AuthResponse> =
serde_json::from_str(json).expect("envelope should parse without CSRF");
assert_eq!(envelope.data.ticket, "PVE:root@pam:99999");
assert!(envelope.data.csrf_prevention_token.is_none());
}
#[test]
fn test_build_headers_get_omits_csrf() {
let mut client = ProxmoxClient::new("pve.example.com", 8006, "root@pam");
client.set_ticket("my-ticket");
client.set_csrf_token("my-csrf");
let headers = client.build_headers(Some("my-ticket"), false);
assert!(!headers.contains_key("CSRFPreventionToken"));
assert!(headers.contains_key("Cookie"));
}
#[test]
fn test_build_headers_post_includes_csrf() {
let mut client = ProxmoxClient::new("pve.example.com", 8006, "root@pam");
client.set_ticket("my-ticket");
client.set_csrf_token("my-csrf");
let headers = client.build_headers(Some("my-ticket"), true);
assert!(headers.contains_key("CSRFPreventionToken"));
let csrf_val = headers.get("CSRFPreventionToken").unwrap().to_str().unwrap();
assert_eq!(csrf_val, "my-csrf");
}
#[test]
fn test_set_ticket_and_csrf_token() {
let mut client = ProxmoxClient::new("pve.example.com", 8006, "root@pam");
client.set_ticket("ticket-value");
client.set_csrf_token("csrf-value");
assert_eq!(client.ticket.as_deref(), Some("ticket-value"));
assert_eq!(client.csrf_token.as_deref(), Some("csrf-value"));
}
}

View File

@ -66,8 +66,25 @@ export async function updateProxmoxCluster(
* Ping a Proxmox cluster authenticates and calls the version endpoint to verify
* the API is reachable and credentials are valid.
*/
export async function pingProxmoxCluster(clusterId: string): Promise<any> {
return await invoke<any>("ping_proxmox_cluster", { clusterId });
export async function pingProxmoxCluster(clusterId: string): Promise<unknown> {
return await invoke("ping_proxmox_cluster", { clusterId });
}
/**
* Connect (or re-connect) to a cluster stored in the DB.
* Authenticates against the Proxmox API and populates the in-memory pool.
* Use after app restart or after an explicit disconnect.
*/
export async function connectProxmoxCluster(clusterId: string): Promise<boolean> {
return await invoke<boolean>("connect_proxmox_cluster", { clusterId });
}
/**
* Disconnect from a cluster by removing its authenticated session from the
* in-memory pool. Credentials are retained in the DB for later reconnection.
*/
export async function disconnectProxmoxCluster(clusterId: string): Promise<void> {
await invoke("disconnect_proxmox_cluster", { clusterId });
}
/**

View File

@ -6,7 +6,7 @@ import { AddRemoteForm } from '@/components/Proxmox';
import { EditRemoteForm } from '@/components/Proxmox';
import { RemoveRemoteDialog } from '@/components/Proxmox';
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/index';
import { listProxmoxClusters, addProxmoxCluster, removeProxmoxCluster, updateProxmoxCluster, pingProxmoxCluster } from '@/lib/proxmoxClient';
import { listProxmoxClusters, addProxmoxCluster, removeProxmoxCluster, updateProxmoxCluster, connectProxmoxCluster, disconnectProxmoxCluster } from '@/lib/proxmoxClient';
import { ClusterType } from '@/lib/domain';
import { toast } from 'sonner';
@ -134,8 +134,8 @@ export function ProxmoxRemotesPage() {
const handleConnectRemote = async (remote: RemoteInfo) => {
try {
toast.info(`Testing connection to ${remote.name}...`);
await pingProxmoxCluster(remote.id);
toast.info(`Connecting to ${remote.name}...`);
await connectProxmoxCluster(remote.id);
toast.success(`Connected to ${remote.name}`);
setRemotes((prev) =>
prev.map((r) => (r.id === remote.id ? { ...r, status: 'connected' } : r))
@ -149,11 +149,17 @@ export function ProxmoxRemotesPage() {
}
};
const handleDisconnectRemote = (remote: RemoteInfo) => {
setRemotes((prev) =>
prev.map((r) => (r.id === remote.id ? { ...r, status: 'disconnected' } : r))
);
toast.info(`Disconnected from ${remote.name}`);
const handleDisconnectRemote = async (remote: RemoteInfo) => {
try {
await disconnectProxmoxCluster(remote.id);
setRemotes((prev) =>
prev.map((r) => (r.id === remote.id ? { ...r, status: 'disconnected' } : r))
);
toast.info(`Disconnected from ${remote.name}`);
} catch (err) {
console.error('Failed to disconnect remote:', err);
toast.error('Disconnect failed: ' + String(err));
}
};
return (
@ -184,12 +190,8 @@ export function ProxmoxRemotesPage() {
onDelete={(remote) => {
setRemovingRemote(remote as RemoteInfo | null);
}}
onConnect={(remote) => {
void handleConnectRemote(remote as RemoteInfo);
}}
onDisconnect={(remote) => {
handleDisconnectRemote(remote as RemoteInfo);
}}
onConnect={(remote) => { void handleConnectRemote(remote as RemoteInfo); }}
onDisconnect={(remote) => { void handleDisconnectRemote(remote as RemoteInfo); }}
/>
{showAddDialog && (