diff --git a/src/libslic3r/AppConfig.cpp b/src/libslic3r/AppConfig.cpp index d7aaac9280..0156751d60 100644 --- a/src/libslic3r/AppConfig.cpp +++ b/src/libslic3r/AppConfig.cpp @@ -1374,6 +1374,26 @@ bool AppConfig::write_config_file(const std::string &path, std::string body, con return true; } +void AppConfig::clear_local_machine_access_code(const std::string& dev_id, const std::string& agent_id) +{ + // Unscoped credentials predate printer agents and belong to the legacy BBL agent. + const bool is_bbl_agent = agent_id.empty() || agent_id == "bbl"; + auto it = m_local_machines.find(dev_id); + if (it != m_local_machines.end()) { + const std::string& stored_agent = it->second.printer_agent_id; + if (stored_agent == agent_id || (is_bbl_agent && (stored_agent.empty() || stored_agent == "bbl"))) { + if (!it->second.access_code.empty()) { + it->second.access_code.clear(); + m_dirty = true; + } + } + } + if (is_bbl_agent) { + erase("access_code", dev_id); + erase("user_access_code", dev_id); + } +} + bool AppConfig::get_variant(const std::string &vendor, const std::string &model, const std::string &variant) const { const auto it_v = m_vendors.find(vendor); diff --git a/src/libslic3r/AppConfig.hpp b/src/libslic3r/AppConfig.hpp index 3afb2bb138..61dceda87a 100644 --- a/src/libslic3r/AppConfig.hpp +++ b/src/libslic3r/AppConfig.hpp @@ -306,6 +306,8 @@ public: } } + void clear_local_machine_access_code(const std::string& dev_id, const std::string& agent_id); + const std::vector &get_filament_presets() const { return m_filament_presets; } void set_filament_presets(const std::vector &filament_presets){ m_filament_presets = filament_presets; diff --git a/src/slic3r/GUI/ConnectPrinter.cpp b/src/slic3r/GUI/ConnectPrinter.cpp index 5d7bf67e03..4a1d8c7da9 100644 --- a/src/slic3r/GUI/ConnectPrinter.cpp +++ b/src/slic3r/GUI/ConnectPrinter.cpp @@ -1,5 +1,6 @@ #include "ConnectPrinter.hpp" #include "GUI_App.hpp" +#include "slic3r/Utils/NetworkAgentFactory.hpp" #include "slic3r/GUI/GUI_Utils.hpp" #include "slic3r/GUI/GUI.hpp" #include "slic3r/GUI/DeviceCore/DevConfigUtil.h" @@ -169,7 +170,7 @@ void ConnectPrinterDialog::on_input_enter(wxCommandEvent& evt) void ConnectPrinterDialog::on_button_confirm(wxCommandEvent &event) { wxString code = m_textCtrl_code->GetTextCtrl()->GetValue(); - if (code.empty()) + if (code.empty() && (!m_obj || m_obj->printer_agent_id != MOONRAKER_PRINTER_AGENT_ID)) code = "88888888"; for (char c : code) { if (!(('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'))) { diff --git a/src/slic3r/GUI/DeviceCore/DevManager.cpp b/src/slic3r/GUI/DeviceCore/DevManager.cpp index c04321e71f..d4282074ba 100644 --- a/src/slic3r/GUI/DeviceCore/DevManager.cpp +++ b/src/slic3r/GUI/DeviceCore/DevManager.cpp @@ -276,6 +276,18 @@ namespace Slic3r std::string connect_type = j["connect_type"].get(); std::string bind_state = j["bind_state"].get(); + // Passive discovery must not replace another agent's saved binding. + if (get_current_printer_agent_id() == MOONRAKER_PRINTER_AGENT_ID) { + if (AppConfig* config = get_app_config()) { + const auto& machines = config->get_local_machines(); + auto it = machines.find(dev_id); + if (it != machines.end() && it->second.printer_agent_id != MOONRAKER_PRINTER_AGENT_ID) { + BOOST_LOG_TRIVIAL(warning) << "Ignoring Moonraker discovery for a device owned by another agent: " << dev_id; + return; + } + } + } + if (connect_type == "farm") { connect_type ="lan"; bind_state = "free"; diff --git a/src/slic3r/GUI/DeviceManager.cpp b/src/slic3r/GUI/DeviceManager.cpp index c1c357a2d5..7c75ee105a 100644 --- a/src/slic3r/GUI/DeviceManager.cpp +++ b/src/slic3r/GUI/DeviceManager.cpp @@ -4,6 +4,7 @@ #include "HMS.hpp" #include "I18N.hpp" #include "libslic3r/Thread.hpp" +#include "libslic3r/PresetBundle.hpp" #include "slic3r/Utils/Http.hpp" #include "slic3r/Utils/NetworkAgent.hpp" #include "slic3r/Utils/NetworkAgentFactory.hpp" @@ -66,6 +67,7 @@ #include #include #include +#include #include #include #include @@ -527,6 +529,18 @@ bool MachineObject::HasRecentLanMessage() return diff.count() < 5000; } +bool MachineObject::has_access_right() const +{ + return !m_access_revoked && (printer_agent_id == MOONRAKER_PRINTER_AGENT_ID || !get_access_code().empty()); +} + +void MachineObject::revoke_access() +{ + // UI panels retain this object; revoke its binding without invalidating their pointers. + access_code.clear(); + m_access_revoked = true; +} + std::string MachineObject::get_access_code() const { return access_code; @@ -535,6 +549,7 @@ std::string MachineObject::get_access_code() const void MachineObject::set_access_code(std::string code, bool only_refresh) { this->access_code = code; + m_access_revoked = false; if (only_refresh) { AppConfig* config = m_manager ? m_manager->get_app_config() : GUI::wxGetApp().app_config; if (config) { @@ -548,25 +563,10 @@ void MachineObject::set_access_code(std::string code, bool only_refresh) // session boundary, since parse_user_print_info() always overwrites their code // fresh from the cloud API's current response, so there's no cross-agent leakage // risk to guard against there. - if (!code.empty()) { + if (has_access_right()) { DeviceManager::update_local_machine(*this, config); } else { - // Only patch an existing record's code - don't persist a brand-new - // never-bound entry just because set_access_code("") was called on it. - const auto& machines = config->get_local_machines(); - auto it = machines.find(get_dev_id()); - if (it != machines.end()) { - BBLocalMachine local_machine = it->second; - local_machine.access_code = ""; - config->update_local_machine(local_machine); - } - // Also clear the pre-scoping flat legacy key when unbinding under BBL, so an - // old BBL-era code can't silently "re-bind" this device again via - // get_access_code_with_legacy_fallback()'s legacy fallback. - if (printer_agent_id == BBL_PRINTER_AGENT_ID || printer_agent_id.empty()) { - config->erase("access_code", get_dev_id()); - config->erase("user_access_code", get_dev_id()); - } + config->clear_local_machine_access_code(get_dev_id(), printer_agent_id); } } else { if (!code.empty()) @@ -2678,7 +2678,7 @@ void MachineObject::update_print_progress(const json& value) int MachineObject::connect() { - if (get_dev_ip().empty()) return -1; + if (m_access_revoked || get_dev_ip().empty()) return -1; std::string username = m_agent ? m_agent->default_lan_username() : std::string(); std::string password = get_access_code(); @@ -2691,17 +2691,25 @@ int MachineObject::connect() std::string host = Http::get_host_from_url(input, &port); std::string ca_file; - if (GUI::wxGetApp().preset_bundle) { + if (wxTheApp && GUI::wxGetApp().preset_bundle) { const auto& config = GUI::wxGetApp().preset_bundle->printers.get_edited_preset().config; if (port.empty()) port = config.opt_string("printhost_port"); ca_file = config.opt_string("printhost_cafile"); + + // Refresh credentials from the matching preset for connections that bypass GUI_App::select_machine(). + const std::string print_host = config.opt_string("print_host"); + if (printer_agent_id == MOONRAKER_PRINTER_AGENT_ID && config.opt_string("printer_agent") == MOONRAKER_PRINTER_AGENT_ID && + !print_host.empty() && dev_id_from_address(print_host, config.opt_string("printhost_port")) == get_dev_id()) { + password = config.opt_string("printhost_apikey"); + if (password != get_access_code()) + set_access_code(password); + } } if (host.empty()) host = get_dev_ip(); - if (m_agent) { try { PrinterConnectionParams params{ diff --git a/src/slic3r/GUI/DeviceManager.hpp b/src/slic3r/GUI/DeviceManager.hpp index f41a32ada7..75bc545f64 100644 --- a/src/slic3r/GUI/DeviceManager.hpp +++ b/src/slic3r/GUI/DeviceManager.hpp @@ -119,6 +119,7 @@ private: std::string dev_name; std::string dev_ip; std::string access_code; + bool m_access_revoked = false; // type, time stamp, delay std::vector> message_delay; @@ -229,7 +230,8 @@ public: std::string dev_connection_name; /* lan | eth */ /*access code*/ - bool has_access_right() const { return !get_access_code().empty(); } + bool has_access_right() const; + void revoke_access(); std::string get_access_code() const; void set_access_code(std::string code, bool only_refresh = true); diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 7f4ddfadb7..fd2a8a2a3a 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -1149,14 +1149,8 @@ void GUI_App::post_init() if(!m_networking_need_update && m_agent) { m_agent->set_on_ssdp_msg_fn( [this](std::string json_str) { - if (is_closing()) { - return; - } - GUI::wxGetApp().CallAfter([this, json_str] { - if (m_device_manager) { - m_device_manager->on_machine_alive(json_str); - } - }); + if (!is_closing() && m_device_manager) + m_device_manager->on_machine_alive(json_str); } ); m_agent->set_on_http_error_fn([this](CloudEvent event, unsigned int status, std::string body) { @@ -1814,14 +1808,8 @@ void GUI_App::restart_networking() init_networking_callbacks(); m_agent->set_on_ssdp_msg_fn( [this](std::string json_str) { - if (is_closing()) { - return; - } - GUI::wxGetApp().CallAfter([this, json_str] { - if (m_device_manager) { - m_device_manager->on_machine_alive(json_str); - } - }); + if (!is_closing() && m_device_manager) + m_device_manager->on_machine_alive(json_str); } ); m_agent->set_on_http_error_fn([this](CloudEvent event, unsigned int status, std::string body) { @@ -4231,8 +4219,10 @@ void GUI_App::switch_printer_agent() const std::string print_host = config.opt_string("print_host"); if (!print_host.empty()) { const std::string dev_id = MachineObject::dev_id_from_address(print_host, config.opt_string("printhost_port")); - MachineObject* sel = m_device_manager->get_selected_machine(); - if (!sel || sel->get_dev_id() != dev_id) + MachineObject* sel = m_device_manager->get_selected_machine(); + const bool access_code_changed = effective_agent_id == MOONRAKER_PRINTER_AGENT_ID && sel && + sel->get_access_code() != config.opt_string("printhost_apikey"); + if (!sel || sel->get_dev_id() != dev_id || access_code_changed) select_machine(effective_agent_id); } } @@ -4274,6 +4264,9 @@ void GUI_App::select_machine(const std::string& agent_id) return; } std::string port = host_cfg->opt_string("printhost_port"); + std::string access_code = host_cfg->opt_string("printhost_apikey"); + if (access_code.empty() && agent_id != MOONRAKER_PRINTER_AGENT_ID) + access_code = "88888888"; // Generate dev_id from host and port std::string dev_id = MachineObject::dev_id_from_address(print_host, port); @@ -4300,11 +4293,6 @@ void GUI_App::select_machine(const std::string& agent_id) machine.dev_ip = dev_id; machine.dev_name = dev_id; machine.printer_type = preset.config.opt_string("printer_model"); - auto access_code = preset.config.opt_string("printhost_apikey"); - // Orca expect non empty access code - if (access_code.empty()) { - access_code = "88888888"; - } existing = m_device_manager->insert_local_device( machine, "lan", "free", "", access_code); @@ -4314,6 +4302,9 @@ void GUI_App::select_machine(const std::string& agent_id) return; } BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": created new machine dev_id=" << dev_id; + } else if (agent_id == MOONRAKER_PRINTER_AGENT_ID && existing->printer_agent_id == agent_id && + (!existing->has_access_right() || existing->get_access_code() != access_code)) { + existing->set_access_code(access_code); } existing->local_use_ssl = boost::istarts_with(print_host, "https://"); diff --git a/src/slic3r/GUI/SelectMachinePop.cpp b/src/slic3r/GUI/SelectMachinePop.cpp index 47c05143d2..b7c8d48ccf 100644 --- a/src/slic3r/GUI/SelectMachinePop.cpp +++ b/src/slic3r/GUI/SelectMachinePop.cpp @@ -732,10 +732,11 @@ void SelectMachinePopup::update_user_devices() if (mobj) { AppConfig* config = wxGetApp().app_config; if (config) { + config->clear_local_machine_access_code(mobj->get_dev_id(), mobj->printer_agent_id); config->erase_local_machine(mobj->get_dev_id()); } - mobj->set_access_code(""); + mobj->revoke_access(); } if (GUI::wxGetApp().plater()) diff --git a/src/slic3r/Utils/Moonraker.cpp b/src/slic3r/Utils/Moonraker.cpp index faa245bb28..1388772857 100644 --- a/src/slic3r/Utils/Moonraker.cpp +++ b/src/slic3r/Utils/Moonraker.cpp @@ -52,9 +52,6 @@ std::string Moonraker::make_url(const std::string &path) const void Moonraker::set_auth(Http &http) const { - //ORCA: Moonraker accepts unauthenticated requests by default; X-Api-Key is the only auth header - // defined by the Moonraker spec. HTTP Basic / Digest do NOT belong here even if the user - // filled the user/password fields — those are PrusaLink/OctoPrint conventions. if (!m_apikey.empty()) http.header("X-Api-Key", m_apikey); if (!m_cafile.empty()) diff --git a/src/slic3r/Utils/Moonraker.hpp b/src/slic3r/Utils/Moonraker.hpp index a7f1d8dd68..5629cde066 100644 --- a/src/slic3r/Utils/Moonraker.hpp +++ b/src/slic3r/Utils/Moonraker.hpp @@ -27,9 +27,10 @@ class Http; // plateindex) // POST /printer/print/start (json) -- {"filename":".gcode"} starts print // -// Auth: X-Api-Key header if `printhost_apikey` is non-empty; Moonraker accepts -// unauthenticated LAN access by default, so the key is optional. HTTP Basic / -// Digest are not part of the Moonraker spec and are not sent. +// Auth: Send X-Api-Key when `printhost_apikey` is non-empty. Without a key, +// requests rely on Moonraker's `[authorization] trusted_clients`; `force_logins` +// overrides trusted-client access when enabled and at least one user exists. +// HTTP Basic / Digest credentials are not sent. class Moonraker : public PrintHost { public: diff --git a/src/slic3r/Utils/MoonrakerPrinterAgent.cpp b/src/slic3r/Utils/MoonrakerPrinterAgent.cpp index 3e2f346b0b..81a25165a2 100644 --- a/src/slic3r/Utils/MoonrakerPrinterAgent.cpp +++ b/src/slic3r/Utils/MoonrakerPrinterAgent.cpp @@ -1,6 +1,7 @@ #include "MoonrakerPrinterAgent.hpp" #include "Http.hpp" #include "IPrinterAgent.hpp" +#include "NetworkAgentFactory.hpp" #include "bambu_networking.hpp" #include "libslic3r/Preset.hpp" #include "libslic3r/PresetBundle.hpp" @@ -142,7 +143,7 @@ MoonrakerPrinterAgent::~MoonrakerPrinterAgent() AgentInfo MoonrakerPrinterAgent::get_agent_info_static() { - return AgentInfo{"moonraker", "Moonraker", MoonrakerPrinterAgent_VERSION, "Klipper/Moonraker printer agent"}; + return AgentInfo{MOONRAKER_PRINTER_AGENT_ID, "Moonraker", MoonrakerPrinterAgent_VERSION, "Klipper/Moonraker printer agent"}; } void MoonrakerPrinterAgent::set_cloud_agent(std::shared_ptr cloud) @@ -1372,7 +1373,7 @@ void MoonrakerPrinterAgent::announce_printhost_device() { std::lock_guard lock(state_mutex); ssdp_fn = on_ssdp_msg_fn; - if (!ssdp_fn) { + if (!ssdp_fn || device_info.dev_id.empty() || device_info.dev_ip.empty() || device_info.base_url.empty()) { return; } if (ssdp_announced_host == device_info.base_url && !ssdp_announced_id.empty()) { @@ -1393,11 +1394,6 @@ void MoonrakerPrinterAgent::announce_printhost_device() const std::string model_id = device_info.model_id; - if (auto* app_config = GUI::wxGetApp().app_config) { - const std::string access_code = device_info.api_key.empty() ? "88888888" : device_info.api_key; - app_config->set_str("access_code", device_info.dev_id, access_code); - } - nlohmann::json payload; payload["dev_name"] = dev_name; payload["dev_id"] = device_info.dev_id; diff --git a/src/slic3r/Utils/NetworkAgent.cpp b/src/slic3r/Utils/NetworkAgent.cpp index 4e5154fd42..576e983134 100644 --- a/src/slic3r/Utils/NetworkAgent.cpp +++ b/src/slic3r/Utils/NetworkAgent.cpp @@ -128,8 +128,8 @@ void NetworkAgent::add_cloud_agent(const std::string& provider, std::shared_ptr< void NetworkAgent::set_printer_agent(std::shared_ptr printer_agent) { m_user_machine_list_generation.fetch_add(1); - - // Disconnect all callbacks from the old agent + // Invalidate queued discovery even when a cached agent instance is reused. + m_discovery_session = std::make_shared(0); auto old_printer_agent = m_printer_agent; m_printer_agent = std::move(printer_agent); @@ -155,7 +155,7 @@ void NetworkAgent::apply_printer_callbacks(const std::shared_ptr& return; } - printer_agent->set_on_ssdp_msg_fn(callbacks.on_ssdp_msg_fn); + apply_discovery_callback(printer_agent, callbacks); printer_agent->set_on_printer_connected_fn(callbacks.on_printer_connected_fn); printer_agent->set_on_subscribe_failure_fn(callbacks.on_subscribe_failure_fn); printer_agent->set_on_message_fn(callbacks.on_message_fn); @@ -166,6 +166,26 @@ void NetworkAgent::apply_printer_callbacks(const std::shared_ptr& printer_agent->set_server_callback(callbacks.on_server_err_fn); } +int NetworkAgent::apply_discovery_callback(const std::shared_ptr& printer_agent, + const PrinterCallbacks& callbacks) +{ + if (!callbacks.on_ssdp_msg_fn) + return printer_agent->set_on_ssdp_msg_fn(nullptr); + + const std::weak_ptr session = m_discovery_session; + return printer_agent->set_on_ssdp_msg_fn( + [session, fn = callbacks.on_ssdp_msg_fn, queue = callbacks.queue_on_main_fn](std::string payload) { + // Without a UI queue this would run on the agent's thread, so drop it. + if (session.expired() || !queue) + return; + queue([session, fn, payload = std::move(payload)] { + // The agent may have changed while this event waited on the UI queue. + if (!session.expired()) + fn(payload); + }); + }); +} + std::shared_ptr NetworkAgent::get_cloud_agent(const std::string& provider) const { const auto& key = (provider.empty() || provider == ORCA_CLOUD_PROVIDER) ? ORCA_CLOUD_PROVIDER : provider; @@ -185,8 +205,10 @@ int NetworkAgent::set_queue_on_main_fn(QueueOnMainFn fn, const std::string& prov int ret = -1; if (cloud_agent) ret = cloud_agent->set_queue_on_main_fn(fn); - if (m_printer_agent) + if (m_printer_agent) { m_printer_agent->set_queue_on_main_fn(fn); + apply_discovery_callback(m_printer_agent, m_printer_callbacks); + } return ret; } @@ -737,9 +759,10 @@ int NetworkAgent::get_mw_user_4ulist(int seed, int limit, std::function(0); m_printer_callbacks.on_ssdp_msg_fn = fn; if (m_printer_agent) - return m_printer_agent->set_on_ssdp_msg_fn(fn); + return apply_discovery_callback(m_printer_agent, m_printer_callbacks); return -1; } diff --git a/src/slic3r/Utils/NetworkAgent.hpp b/src/slic3r/Utils/NetworkAgent.hpp index c7813f2bb7..7a141fa0d4 100644 --- a/src/slic3r/Utils/NetworkAgent.hpp +++ b/src/slic3r/Utils/NetworkAgent.hpp @@ -144,6 +144,7 @@ public: int get_mw_user_4ulist(int seed, int limit, std::function callback, const std::string& provider = ORCA_CLOUD_PROVIDER); // Printer agent methods + // Discovery is delivered through queue_on_main_fn, scoped to the live agent session. int set_on_ssdp_msg_fn(OnMsgArrivedFn fn); int set_on_printer_connected_fn(OnPrinterConnectedFn fn); int set_on_subscribe_failure_fn(GetSubscribeFailureFn fn); @@ -210,6 +211,9 @@ private: void apply_printer_callbacks(const std::shared_ptr& printer_agent, const PrinterCallbacks& callbacks); + int apply_discovery_callback(const std::shared_ptr& printer_agent, + const PrinterCallbacks& callbacks); + std::shared_ptr m_discovery_session = std::make_shared(0); PrinterCallbacks m_printer_callbacks; bool enable_track = false; diff --git a/src/slic3r/Utils/NetworkAgentFactory.hpp b/src/slic3r/Utils/NetworkAgentFactory.hpp index 0053a42466..ea2758618a 100644 --- a/src/slic3r/Utils/NetworkAgentFactory.hpp +++ b/src/slic3r/Utils/NetworkAgentFactory.hpp @@ -18,6 +18,7 @@ namespace Slic3r { static constexpr char ORCA_PRINTER_AGENT_ID[] = "orca"; static constexpr char BBL_PRINTER_AGENT_ID[] = "bbl"; +static constexpr char MOONRAKER_PRINTER_AGENT_ID[] = "moonraker"; // Factory function type for creating printer agents using PrinterAgentFactory = diff --git a/tests/libslic3r/test_appconfig.cpp b/tests/libslic3r/test_appconfig.cpp index b99d44745c..01ceebc538 100644 --- a/tests/libslic3r/test_appconfig.cpp +++ b/tests/libslic3r/test_appconfig.cpp @@ -3,6 +3,8 @@ #include #include #include "libslic3r/AppConfig.hpp" +#include "libslic3r/Thread.hpp" +#include "../slic3rutils/plugin_test_utils.hpp" using namespace Slic3r; @@ -46,6 +48,105 @@ TEST_CASE("AppConfig network version helpers", "[AppConfig]") { } } +TEST_CASE("Local machine agent replacements survive save and reload", "[AppConfig]") +{ + ScopedDataDir data_dir("local-machine"); + save_main_thread_id(); + AppConfig config; + BBLocalMachine saved; + saved.dev_id = "127.0.0.1:7125"; + saved.dev_ip = saved.dev_id; + saved.dev_name = "Test printer"; + saved.printer_type = "test-model"; + saved.printer_agent_id = "orca"; + saved.access_code = "old-key"; + config.update_local_machine(saved); + config.save(); + REQUIRE_FALSE(config.dirty()); + + BBLocalMachine replacement = saved; + replacement.printer_agent_id = "moonraker"; + replacement.access_code = GENERATE("", "new-key", "88888888"); + config.update_local_machine(replacement); + REQUIRE(config.dirty()); + REQUIRE(config.get_local_machines().at(saved.dev_id) == replacement); + config.save(); + + AppConfig reloaded; + REQUIRE(reloaded.load().empty()); + REQUIRE(reloaded.get_local_machines().at(saved.dev_id) == replacement); + REQUIRE_FALSE(reloaded.dirty()); + reloaded.update_local_machine(replacement); + REQUIRE_FALSE(reloaded.dirty()); +} + +TEST_CASE("Clearing local credentials preserves another agent's record", "[AppConfig]") +{ + AppConfig config; + BBLocalMachine saved; + saved.dev_id = "test_dev"; + saved.printer_agent_id = GENERATE("orca", "qidi", "bbl", ""); + saved.access_code = "saved-key"; + config.update_local_machine(saved); + config.set_str("access_code", saved.dev_id, "legacy-key"); + config.set_str("user_access_code", saved.dev_id, "legacy-user-key"); + + config.clear_local_machine_access_code(saved.dev_id, "moonraker"); + + REQUIRE(config.get_local_machines().at(saved.dev_id) == saved); + REQUIRE(config.get("access_code", saved.dev_id) == "legacy-key"); + REQUIRE(config.get("user_access_code", saved.dev_id) == "legacy-user-key"); +} + +TEST_CASE("Clearing local credentials changes only the owning agent's key", "[AppConfig]") +{ + ScopedDataDir data_dir("local-machine-clear"); + save_main_thread_id(); + AppConfig config; + BBLocalMachine saved; + saved.dev_id = "test_dev"; + saved.dev_name = "Test printer"; + saved.printer_agent_id = GENERATE("moonraker", "qidi"); + saved.access_code = "saved-key"; + config.update_local_machine(saved); + config.save(); + REQUIRE_FALSE(config.dirty()); + + config.clear_local_machine_access_code(saved.dev_id, saved.printer_agent_id); + REQUIRE(config.dirty()); + config.save(); + + AppConfig reloaded; + REQUIRE(reloaded.load().empty()); + saved.access_code.clear(); + REQUIRE(reloaded.get_local_machines().at(saved.dev_id) == saved); + REQUIRE_FALSE(reloaded.dirty()); + reloaded.clear_local_machine_access_code(saved.dev_id, saved.printer_agent_id); + reloaded.clear_local_machine_access_code("unknown_dev", saved.printer_agent_id); + REQUIRE(reloaded.get_local_machines().size() == 1); + REQUIRE_FALSE(reloaded.dirty()); +} + +TEST_CASE("BBL credential clearing removes legacy fallback without changing foreign records", "[AppConfig]") +{ + AppConfig config; + BBLocalMachine saved; + saved.dev_id = "test_dev"; + saved.printer_agent_id = GENERATE("bbl", "", "moonraker"); + saved.access_code = "saved-key"; + config.update_local_machine(saved); + config.set_str("access_code", saved.dev_id, "legacy-key"); + config.set_str("user_access_code", saved.dev_id, "legacy-user-key"); + + config.clear_local_machine_access_code(saved.dev_id, GENERATE("bbl", "")); + + if (saved.printer_agent_id != "moonraker") + saved.access_code.clear(); + REQUIRE(config.get_local_machines().at(saved.dev_id) == saved); + REQUIRE(config.get("access_code", saved.dev_id).empty()); + REQUIRE(config.get("user_access_code", saved.dev_id).empty()); +} + TEST_CASE("Remembered checkbox settings retain both selections", "[AppConfig][Regression]") { AppConfig config; const bool checked = GENERATE(false, true); diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index 2fe6512d9e..298610644b 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -5,6 +5,9 @@ add_executable(${_TEST_NAME}_tests test_config_value_formatter.cpp test_creality_cfs_match.cpp test_dev_mapping.cpp + test_device_access.cpp + test_moonraker_discovery.cpp + test_network_discovery.cpp test_filament_bitmap_utils.cpp test_device_progress.cpp test_device_manager_integration.cpp diff --git a/tests/slic3rutils/test_device_access.cpp b/tests/slic3rutils/test_device_access.cpp new file mode 100644 index 0000000000..e45d867750 --- /dev/null +++ b/tests/slic3rutils/test_device_access.cpp @@ -0,0 +1,128 @@ +#ifdef WIN32 + #ifndef WIN32_LEAN_AND_MEAN + #define WIN32_LEAN_AND_MEAN + #endif + #ifndef NOMINMAX + #define NOMINMAX + #endif + #include +#endif + +#include +#include +#include + +#include "libslic3r/AppConfig.hpp" +#include "slic3r/GUI/DeviceCore/DevManager.h" +#include "slic3r/GUI/DeviceManager.hpp" +#include "slic3r/Utils/IPrinterAgent.hpp" +#include "slic3r/Utils/NetworkAgentFactory.hpp" +#include "slic3r/Utils/MoonrakerPrinterAgent.hpp" +#include "slic3r/Utils/NetworkAgent.hpp" + +using namespace Slic3r; + +TEST_CASE("Moonraker permits an empty access code", "[DeviceAccess]") +{ + MachineObject machine(nullptr, nullptr, "test", "test_dev", "127.0.0.1"); + machine.printer_agent_id = MOONRAKER_PRINTER_AGENT_ID; + machine.set_access_code("", false); + + REQUIRE(machine.has_access_right()); +} + +TEST_CASE("Moonraker logout revokes access until explicitly rebound", "[DeviceAccess]") +{ + MachineObject machine(nullptr, nullptr, "test", "test_dev", "127.0.0.1"); + machine.printer_agent_id = MOONRAKER_PRINTER_AGENT_ID; + const std::string code = GENERATE("", "configured-key"); + machine.set_access_code(code, false); + REQUIRE(machine.has_access_right()); + + machine.revoke_access(); + REQUIRE(machine.get_access_code().empty()); + REQUIRE_FALSE(machine.has_access_right()); + + machine.set_access_code(code, false); + REQUIRE(machine.has_access_right()); + REQUIRE(machine.get_access_code() == code); +} + +TEST_CASE("Revoked access prevents calls to the printer agent", "[DeviceAccess]") +{ + class RecordingAgent : public MoonrakerPrinterAgent + { + public: + RecordingAgent() : MoonrakerPrinterAgent("") {} + int connections = 0; + int connect_printer(const PrinterConnectionParams&) override + { + ++connections; + return 0; + } + int disconnect_printer() override { return 0; } + }; + + auto agent = std::make_shared(); + NetworkAgent network(nullptr, agent); + MachineObject machine(nullptr, &network, "test", "test_dev", "127.0.0.1"); + machine.printer_agent_id = GENERATE(BBL_PRINTER_AGENT_ID, ORCA_PRINTER_AGENT_ID, MOONRAKER_PRINTER_AGENT_ID); + machine.set_access_code("configured-key", false); + REQUIRE(machine.connect() == 0); + REQUIRE(agent->connections == 1); + + machine.revoke_access(); + REQUIRE(machine.connect() == -1); + REQUIRE(agent->connections == 1); + + machine.set_access_code("configured-key", false); + REQUIRE(machine.connect() == 0); + REQUIRE(agent->connections == 2); +} + +TEST_CASE("Other printer agents require an access code", "[DeviceAccess]") +{ + MachineObject machine(nullptr, nullptr, "test", "test_dev", "127.0.0.1"); + machine.printer_agent_id = GENERATE( + BBL_PRINTER_AGENT_ID, ORCA_PRINTER_AGENT_ID, "qidi", "snapmaker", "crealityprint", "plugin", "unknown"); + + machine.set_access_code("", false); + REQUIRE_FALSE(machine.has_access_right()); + + machine.set_access_code("88888888", false); + REQUIRE(machine.has_access_right()); +} + +TEST_CASE("Rebinding Moonraker with an empty key saves the machine", "[DeviceAccess]") +{ + AppConfig config; + DeviceManager manager(nullptr, false, &config); + MachineObject machine(&manager, nullptr, "test", "test_dev", "127.0.0.1"); + machine.printer_agent_id = MOONRAKER_PRINTER_AGENT_ID; + machine.dev_connection_type = "lan"; + machine.revoke_access(); + + machine.set_access_code(""); + const auto& machines = config.get_local_machines(); + REQUIRE(machines.count("test_dev") == 1); + REQUIRE(machines.at("test_dev").printer_agent_id == MOONRAKER_PRINTER_AGENT_ID); + REQUIRE(machines.at("test_dev").access_code.empty()); +} + +TEST_CASE("Moonraker discovery leaves another agent's saved machine alone", "[DeviceAccess]") +{ + AppConfig config; + NetworkAgent network(nullptr, std::make_shared("")); + DeviceManager manager(&network, false, &config); + + const std::string owner = GENERATE(as{}, ORCA_PRINTER_AGENT_ID, MOONRAKER_PRINTER_AGENT_ID); + BBLocalMachine saved; + saved.dev_id = "192.0.2.10:7125"; + saved.dev_ip = saved.dev_id; + saved.printer_agent_id = owner; + config.update_local_machine(saved); + + manager.on_machine_alive(R"({"dev_name":"Klipper","dev_id":"192.0.2.10:7125","dev_ip":"192.0.2.10:7125", + "dev_type":"","dev_signal":"0","connect_type":"lan","bind_state":"free"})"); + REQUIRE((manager.get_local_machine(saved.dev_id) != nullptr) == (owner == MOONRAKER_PRINTER_AGENT_ID)); +} diff --git a/tests/slic3rutils/test_moonraker_discovery.cpp b/tests/slic3rutils/test_moonraker_discovery.cpp new file mode 100644 index 0000000000..f4df147225 --- /dev/null +++ b/tests/slic3rutils/test_moonraker_discovery.cpp @@ -0,0 +1,51 @@ +#include + +#include +#include "slic3r/Utils/MoonrakerPrinterAgent.hpp" + +using namespace Slic3r; + +namespace { + +class DiscoveryAgent : public MoonrakerPrinterAgent +{ +public: + using MoonrakerPrinterAgent::MoonrakerPrinterAgent; + using MoonrakerPrinterAgent::device_info; + mutable unsigned fetch_count = 0; + +protected: + bool fetch_device_info(const std::string&, const std::string&, MoonrakerDeviceInfo& info, std::string&) const override + { + ++fetch_count; + info.dev_name = "Test printer"; + return true; + } +}; + +} // namespace + +TEST_CASE("Moonraker discovery waits for a complete device identity", "[MoonrakerDiscovery]") +{ + DiscoveryAgent agent(""); + unsigned announcements = 0; + nlohmann::json announcement; + agent.set_on_ssdp_msg_fn([&](std::string payload) { + ++announcements; + announcement = nlohmann::json::parse(payload); + }); + + agent.start_discovery(true, false); + REQUIRE(announcements == 0); + REQUIRE(agent.fetch_count == 0); + + agent.device_info.dev_id = "127.0.0.1:7125"; + agent.device_info.dev_ip = agent.device_info.dev_id; + agent.device_info.base_url = "http://127.0.0.1:7125"; + agent.start_discovery(true, false); + REQUIRE(announcements == 1); + REQUIRE(agent.fetch_count == 1); + REQUIRE(announcement.at("dev_id") == agent.device_info.dev_id); + REQUIRE(announcement.at("dev_ip") == agent.device_info.dev_ip); + REQUIRE(announcement.at("dev_name") == "Test printer"); +} diff --git a/tests/slic3rutils/test_network_discovery.cpp b/tests/slic3rutils/test_network_discovery.cpp new file mode 100644 index 0000000000..30463ecdb0 --- /dev/null +++ b/tests/slic3rutils/test_network_discovery.cpp @@ -0,0 +1,111 @@ +#include + +#include +#include +#include +#include + +#include "slic3r/Utils/bambu_networking.hpp" +#include "slic3r/Utils/MoonrakerPrinterAgent.hpp" +#include "slic3r/Utils/NetworkAgent.hpp" + +using namespace Slic3r; + +namespace { + +class DiscoverySource : public MoonrakerPrinterAgent +{ +public: + DiscoverySource() : MoonrakerPrinterAgent("") {} + + OnMsgArrivedFn discovery_callback; + + int set_on_ssdp_msg_fn(OnMsgArrivedFn fn) override + { + discovery_callback = std::move(fn); + return 0; + } +}; + +} // namespace + +TEST_CASE("Queued discovery cannot cross printer agent sessions", "[NetworkDiscovery]") +{ + auto source = std::make_shared(); + NetworkAgent network(nullptr, source); + std::vector> pending; + std::vector delivered; + network.set_queue_on_main_fn([&](std::function fn) { pending.push_back(std::move(fn)); }); + network.set_on_ssdp_msg_fn([&](std::string payload) { delivered.push_back(std::move(payload)); }); + + auto outgoing_callback = source->discovery_callback; + outgoing_callback("unsaved-device"); + outgoing_callback("saved-moonraker-device"); + REQUIRE(delivered.empty()); + REQUIRE(pending.size() == 2); + + bool expect_active = true; + SECTION("Switch to another agent") + { + network.set_printer_agent(std::make_shared()); + } + SECTION("Clear the live agent") + { + network.set_printer_agent(nullptr); + expect_active = false; + } + SECTION("Switch away and back to the cached agent") + { + network.set_printer_agent(std::make_shared()); + network.set_printer_agent(source); + } + SECTION("Unregister discovery") + { + network.set_on_ssdp_msg_fn(nullptr); + expect_active = false; + } + SECTION("Replace the discovery listener") + { + network.set_on_ssdp_msg_fn([&](std::string payload) { delivered.push_back(std::move(payload)); }); + } + + for (auto& fn : pending) + fn(); + pending.clear(); + REQUIRE(delivered.empty()); + + outgoing_callback("late-outgoing-device"); + REQUIRE(pending.empty()); + REQUIRE(delivered.empty()); + + if (expect_active) { + auto active = std::dynamic_pointer_cast(network.get_printer_agent()); + REQUIRE(active); + REQUIRE(active->discovery_callback); + active->discovery_callback("active-device"); + REQUIRE(delivered.empty()); + REQUIRE(pending.size() == 1); + pending.front()(); + REQUIRE(delivered == std::vector{"active-device"}); + } +} + +TEST_CASE("Discovery callbacks expire when networking is destroyed", "[NetworkDiscovery]") +{ + auto source = std::make_shared(); + std::vector> pending; + unsigned delivered = 0; + { + NetworkAgent network(nullptr, source); + network.set_queue_on_main_fn([&](std::function fn) { pending.push_back(std::move(fn)); }); + network.set_on_ssdp_msg_fn([&](std::string) { ++delivered; }); + source->discovery_callback("queued-device"); + } + + REQUIRE(pending.size() == 1); + pending.front()(); + pending.clear(); + source->discovery_callback("late-device"); + REQUIRE(pending.empty()); + REQUIRE(delivered == 0); +}