diff --git a/clients/openframe-client/src/clients/.registration_client.md b/clients/openframe-client/src/clients/.registration_client.md index f69fb4451f..b0ab42da53 100644 --- a/clients/openframe-client/src/clients/.registration_client.md +++ b/clients/openframe-client/src/clients/.registration_client.md @@ -4,10 +4,10 @@ HTTP client for agent registration and reinstallation against the OpenFrame plat ## Key Components ### `RegistrationClient` -The primary struct wrapping a `reqwest::Client` and base URL. Cloneable for shared use across async tasks. +The primary struct wrapping a `reqwest::Client`, base URL and the optional local machine id. Cloneable for shared use across async tasks. -- **`new(base_url, http_client)`** — Constructs a new client instance. -- **`register(initial_key, machine_info, request)`** — Posts to `/register` (no `machine_info`) or `/reinstall` (with `machine_info`). Attaches `X-Initial-Key`, and optionally `X-Client-Secret` / `X-Machine-Id` headers. Returns `AgentRegistrationResponse` on success. +- **`new(base_url, http_client, local_machine_id)`** — Constructs a new client instance. `local_machine_id` is the locally generated id from `MachineIdService` (`Option`; `None` for callers that never register, such as `DeregistrationService`). +- **`register(initial_key, machine_info, request)`** — Posts to `/register` (no `machine_info`) or `/reinstall` (with `machine_info`). Attaches `X-Initial-Key`, plus `X-Client-Secret` / `X-Machine-Id` (server-assigned) on reinstall, or `X-Machine-Id` with the local machine id on fresh registration, so a `/register` call carries a machine id even before the server assigns one, like every other `/clients/**` call. Returns `AgentRegistrationResponse` on success. ### `RegistrationError` Typed error enum allowing callers to branch on failure cause: @@ -29,6 +29,7 @@ use crate::registration_client::RegistrationClient; let client = RegistrationClient::new( "https://api.openframe.ai".to_string(), Client::new(), + Some(local_machine_id), )?; match client.register("initial-key-here", None, request).await { diff --git a/clients/openframe-client/src/clients/registration_client.rs b/clients/openframe-client/src/clients/registration_client.rs index c8bad1bfe8..b671f3e5b3 100644 --- a/clients/openframe-client/src/clients/registration_client.rs +++ b/clients/openframe-client/src/clients/registration_client.rs @@ -42,13 +42,20 @@ pub enum DeregistrationOutcome { pub struct RegistrationClient { http_client: Client, base_url: String, + /// Local id from `MachineIdService`, sent on fresh `/register` calls that have no server-assigned id yet. + local_machine_id: Option, } impl RegistrationClient { - pub fn new(base_url: String, http_client: Client) -> Result { + pub fn new( + base_url: String, + http_client: Client, + local_machine_id: Option, + ) -> Result { Ok(Self { http_client, base_url, + local_machine_id, }) } @@ -64,26 +71,8 @@ impl RegistrationClient { format!("{}/clients/api/agents/register", self.base_url) }; - let mut headers = HeaderMap::new(); - headers.insert( - "X-Initial-Key", - initial_key - .parse() - .context("Failed to parse initial key header")?, - ); - if let Some(machine_info) = machine_info { - let parsed_client_secret = machine_info - .client_secret - .parse() - .context("Failed to parse client secret header")?; - let parsed_machine_id = machine_info - .machine_id - .parse() - .context("Failed to parse machine id header")?; - headers.insert("X-Client-Secret", parsed_client_secret); - headers.insert("X-Machine-Id", parsed_machine_id); - } - headers.insert("Content-Type", HeaderValue::from_static("application/json")); + let headers = + build_register_headers(initial_key, machine_info, self.local_machine_id.as_deref())?; let response = self .http_client @@ -171,6 +160,42 @@ impl RegistrationClient { } } +/// Reinstalls send the server-assigned id + secret; fresh registrations send the local machine id. +fn build_register_headers( + initial_key: &str, + machine_info: Option, + local_machine_id: Option<&str>, +) -> Result { + let mut headers = HeaderMap::new(); + headers.insert( + "X-Initial-Key", + initial_key + .parse() + .context("Failed to parse initial key header")?, + ); + if let Some(machine_info) = machine_info { + let parsed_client_secret = machine_info + .client_secret + .parse() + .context("Failed to parse client secret header")?; + let parsed_machine_id = machine_info + .machine_id + .parse() + .context("Failed to parse machine id header")?; + headers.insert("X-Client-Secret", parsed_client_secret); + headers.insert("X-Machine-Id", parsed_machine_id); + } else if let Some(local_machine_id) = local_machine_id { + headers.insert( + "X-Machine-Id", + local_machine_id + .parse() + .context("Failed to parse local machine id header")?, + ); + } + headers.insert("Content-Type", HeaderValue::from_static("application/json")); + Ok(headers) +} + /// Statuses proving a retry cannot help: the platform already forgot this machine /// (401/403/410) or does not expose the endpoint yet (404). fn is_already_gone(status: StatusCode) -> bool { diff --git a/clients/openframe-client/src/clients/registration_client_tests.rs b/clients/openframe-client/src/clients/registration_client_tests.rs index 18265d4bff..d62c2c92c7 100644 --- a/clients/openframe-client/src/clients/registration_client_tests.rs +++ b/clients/openframe-client/src/clients/registration_client_tests.rs @@ -1,5 +1,42 @@ use super::*; +fn persisted_info() -> PersistedMachineInfo { + PersistedMachineInfo { + machine_id: "server-machine-id".to_string(), + client_secret: "secret".to_string(), + user_id: None, + } +} + +#[test] +fn fresh_register_sends_local_machine_id_header() { + let headers = build_register_headers("key", None, Some("local-machine-id")).unwrap(); + assert_eq!(headers.get("X-Machine-Id").unwrap(), "local-machine-id"); + assert_eq!(headers.get("X-Initial-Key").unwrap(), "key"); + assert_eq!(headers.get("Content-Type").unwrap(), "application/json"); + assert!(headers.get("X-Client-Secret").is_none()); +} + +#[test] +fn fresh_register_without_local_machine_id_omits_header() { + let headers = build_register_headers("key", None, None).unwrap(); + assert!(headers.get("X-Machine-Id").is_none()); +} + +#[test] +fn reinstall_sends_server_assigned_credentials() { + let headers = + build_register_headers("key", Some(persisted_info()), Some("local-machine-id")).unwrap(); + assert_eq!(headers.get("X-Machine-Id").unwrap(), "server-machine-id"); + assert_eq!(headers.get("X-Client-Secret").unwrap(), "secret"); + assert_eq!(headers.get("X-Initial-Key").unwrap(), "key"); +} + +#[test] +fn rejects_unparseable_local_machine_id() { + assert!(build_register_headers("key", None, Some("bad\nid")).is_err()); +} + #[test] fn detects_client_secret_invalid() { let body = r#"{"code":"CLIENT_SECRET_INVALID","message":"Invalid client secret"}"#; diff --git a/clients/openframe-client/src/doctor/checks.rs b/clients/openframe-client/src/doctor/checks.rs index b93f15aac1..735c1b4bd0 100644 --- a/clients/openframe-client/src/doctor/checks.rs +++ b/clients/openframe-client/src/doctor/checks.rs @@ -5,7 +5,7 @@ use std::time::Duration; use super::{CheckCategory, CheckResult}; use crate::installation_initial_config_service::InstallConfigParams; use crate::platform::permissions::PermissionUtils; -use crate::services::MACHINE_ID_HEADER; +use crate::services::with_machine_id; pub fn check_required_args(params: &InstallConfigParams) -> CheckResult { let mut missing = Vec::new(); @@ -209,17 +209,6 @@ pub fn check_tcp_connect(server_url: &str) -> CheckResult { } } -/// The platform firewall drops client requests without it, so probes must carry it to reach the gateway. -fn with_machine_id( - request: reqwest::RequestBuilder, - machine_id: Option<&str>, -) -> reqwest::RequestBuilder { - match machine_id { - Some(id) => request.header(MACHINE_ID_HEADER, id), - None => request, - } -} - pub async fn check_tls_handshake(server_url: &str, machine_id: Option<&str>) -> CheckResult { let url = ensure_https(server_url); diff --git a/clients/openframe-client/src/lib.rs b/clients/openframe-client/src/lib.rs index b8ccaacf80..e6bf804950 100644 --- a/clients/openframe-client/src/lib.rs +++ b/clients/openframe-client/src/lib.rs @@ -252,8 +252,12 @@ impl Client { ); // Initialize registration client - let registration_client = RegistrationClient::new(http_url.clone(), http_client.clone()) - .context("Failed to create registration client")?; + let registration_client = RegistrationClient::new( + http_url.clone(), + http_client.clone(), + Some(machine_id.clone()), + ) + .context("Failed to create registration client")?; // Initialize device data fetcher let device_data_fetcher = DeviceDataFetcher::new(); @@ -409,8 +413,11 @@ impl Client { .context("Failed to initialize OpenFrame client info service")?; // Initialize GitHub download service (used by update and installation services) - let github_download_service = - GithubDownloadService::new(download_client.clone(), DmgExtractor::new()); + let github_download_service = GithubDownloadService::new( + download_client.clone(), + Some(machine_id), + DmgExtractor::new(), + ); // Initialize update state and cleanup services (needed by update service) let update_state_service = UpdateStateService::new(directory_manager.clone()) diff --git a/clients/openframe-client/src/services/.github_download_service.md b/clients/openframe-client/src/services/.github_download_service.md index 5fd225e96a..b2331bd65e 100644 --- a/clients/openframe-client/src/services/.github_download_service.md +++ b/clients/openframe-client/src/services/.github_download_service.md @@ -4,13 +4,13 @@ Handles downloading, extracting, and saving agent binaries from GitHub releases, ## Key Components ### `GithubDownloadService` -The main service struct holding an `reqwest::Client` and a `DmgExtractor`. Implements `Clone`. +The main service struct holding a `reqwest::Client`, the optional local machine id and a `DmgExtractor`. Implements `Clone`. ### Primary Methods | Method | Description | |--------|-------------| -| `new(http_client, dmg_extractor)` | Constructor | +| `new(http_client, local_machine_id, dmg_extractor)` | Constructor; `local_machine_id` (`Option`, from `MachineIdService`) is sent as `X-Machine-Id` on every download so gateway-fronted asset links pass the machine-id check | | `download_and_extract(config)` | Downloads archive and returns extracted binary bytes | | `download_and_save(config, tool_folder_path, default_agent_path)` | Downloads, extracts, writes to disk; returns executable path for folder extractions | | `find_config_for_current_os(configs)` | Selects the matching `DownloadConfiguration` for the current OS | @@ -31,7 +31,8 @@ use reqwest::Client; let client = Client::new(); let extractor = DmgExtractor::default(); -let service = GithubDownloadService::new(client, extractor); +let local_machine_id = machine_id_service.get_or_create()?; +let service = GithubDownloadService::new(client, Some(local_machine_id), extractor); let configs = fetch_download_configurations().await?; let config = service.find_config_for_current_os(&configs)?; diff --git a/clients/openframe-client/src/services/deregistration_service.rs b/clients/openframe-client/src/services/deregistration_service.rs index 080fe6e14d..0c535f2968 100644 --- a/clients/openframe-client/src/services/deregistration_service.rs +++ b/clients/openframe-client/src/services/deregistration_service.rs @@ -36,7 +36,8 @@ impl DeregistrationService { .context("Failed to create HTTP client")?; let base_url = format!("https://{}", initial_config_service.get_server_url()?); - let registration_client = RegistrationClient::new(base_url, http_client) + // No local machine id: deregister() only ever sends the saved server-assigned credentials. + let registration_client = RegistrationClient::new(base_url, http_client, None) .context("Failed to create registration client")?; // Loaded up front so the final retry still has them once the on-disk copy is wiped. diff --git a/clients/openframe-client/src/services/github_download_service.rs b/clients/openframe-client/src/services/github_download_service.rs index 7b8b09881b..17b359855d 100644 --- a/clients/openframe-client/src/services/github_download_service.rs +++ b/clients/openframe-client/src/services/github_download_service.rs @@ -3,6 +3,7 @@ use crate::config::update_config::{ }; use crate::models::download_configuration::DownloadConfiguration; use crate::platform::binary_writer; +use crate::services::with_machine_id; use anyhow::{anyhow, Context, Result}; use bytes::Bytes; use reqwest::Client; @@ -14,14 +15,21 @@ use tracing::{info, warn}; #[derive(Clone)] pub struct GithubDownloadService { http_client: Client, + /// Sent on every download so gateway-fronted asset links (`/v0/api/assets/download`) carry the machine id. + local_machine_id: Option, #[allow(dead_code)] // read only by macos-only dmg extraction path dmg_extractor: crate::platform::DmgExtractor, } impl GithubDownloadService { - pub fn new(http_client: Client, dmg_extractor: crate::platform::DmgExtractor) -> Self { + pub fn new( + http_client: Client, + local_machine_id: Option, + dmg_extractor: crate::platform::DmgExtractor, + ) -> Self { Self { http_client, + local_machine_id, dmg_extractor, } } @@ -173,9 +181,7 @@ impl GithubDownloadService { /// Downloads file from URL and returns bytes async fn download(&self, url: &str) -> Result { - let response = self - .http_client - .get(url) + let response = with_machine_id(self.http_client.get(url), self.local_machine_id.as_deref()) .send() .await .context("Failed to send download request")?; diff --git a/clients/openframe-client/src/services/machine_id_service.rs b/clients/openframe-client/src/services/machine_id_service.rs index 9dc6d2d279..a8358470a4 100644 --- a/clients/openframe-client/src/services/machine_id_service.rs +++ b/clients/openframe-client/src/services/machine_id_service.rs @@ -9,6 +9,17 @@ use crate::platform::DirectoryManager; pub const MACHINE_ID_HEADER: &str = "x-machine-id"; +/// Sets the machine id header explicitly so the request does not depend on the client's default headers. +pub fn with_machine_id( + request: reqwest::RequestBuilder, + machine_id: Option<&str>, +) -> reqwest::RequestBuilder { + match machine_id { + Some(id) => request.header(MACHINE_ID_HEADER, id), + None => request, + } +} + // Locally generated machine identity, distinct from the server-assigned machine_id in agent config. // Persisted in the shared app-support dir so integrated tool agents (mesh, fleet) can read it. #[derive(Clone)] @@ -71,3 +82,7 @@ impl MachineIdService { Ok(()) } } + +#[cfg(test)] +#[path = "machine_id_service_tests.rs"] +mod tests; diff --git a/clients/openframe-client/src/services/machine_id_service_tests.rs b/clients/openframe-client/src/services/machine_id_service_tests.rs new file mode 100644 index 0000000000..3551eba513 --- /dev/null +++ b/clients/openframe-client/src/services/machine_id_service_tests.rs @@ -0,0 +1,23 @@ +use super::*; + +fn build(machine_id: Option<&str>) -> reqwest::Request { + let client = reqwest::Client::new(); + with_machine_id(client.get("https://example.invalid/"), machine_id) + .build() + .unwrap() +} + +#[test] +fn with_machine_id_sets_header() { + let request = build(Some("local-machine-id")); + assert_eq!( + request.headers().get(MACHINE_ID_HEADER).unwrap(), + "local-machine-id" + ); +} + +#[test] +fn with_machine_id_none_leaves_request_untouched() { + let request = build(None); + assert!(request.headers().get(MACHINE_ID_HEADER).is_none()); +} diff --git a/clients/openframe-client/src/services/mod.rs b/clients/openframe-client/src/services/mod.rs index ad18aedf6b..cfdd75aa0d 100644 --- a/clients/openframe-client/src/services/mod.rs +++ b/clients/openframe-client/src/services/mod.rs @@ -59,7 +59,7 @@ pub use last_known_good_service::LastKnownGoodService; pub use local_tls_config_provider::LocalTlsConfigProvider; pub use machine_heartbeat_publisher::MachineHeartbeatPublisher; pub use machine_heartbeat_run_manager::MachineHeartbeatRunManager; -pub use machine_id_service::{MachineIdService, MACHINE_ID_HEADER}; +pub use machine_id_service::{with_machine_id, MachineIdService, MACHINE_ID_HEADER}; pub use nats_connection_manager::NatsConnectionManager; pub use nats_message_publisher::NatsMessagePublisher; pub use openframe_client_info_service::OpenFrameClientInfoService;