From bdccd5d747d90182853865594c2135e1550b1eb2 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Wed, 31 Jul 2024 18:02:58 +0300 Subject: [PATCH] WIP - show more warnings when connecting to remote machines --- pkg/shell/base_index.js | 10 ++ pkg/shell/hosts.jsx | 100 +++++++++++++-- pkg/shell/hosts_dialog.jsx | 166 ++++++++++++++++++------- pkg/shell/indexes.jsx | 50 +++----- test/common/testlib.py | 4 +- test/verify/check-shell-host-switching | 3 + 6 files changed, 249 insertions(+), 84 deletions(-) diff --git a/pkg/shell/base_index.js b/pkg/shell/base_index.js index 3271f9ce5904..9aef9f70f1fa 100644 --- a/pkg/shell/base_index.js +++ b/pkg/shell/base_index.js @@ -147,6 +147,16 @@ function Frames(index, setupIdleResetTimers) { /* Need to create a new frame */ if (!frame) { + /* Never create new frames for machines that are not + connected yet. That would open a channel to them (for + loading the URL), which woould trigger the bridge to + attempt a log in. We want all logins to happen in a + single place (in hosts.jsx) so that we can get the + options right, and show a warning dialog. + */ + if (host != "localhost" && machine.state !== "connected") + return null; + new_frame = true; frame = document.createElement("iframe"); frame.setAttribute("class", "container-frame"); diff --git a/pkg/shell/hosts.jsx b/pkg/shell/hosts.jsx index d8b23cb788b5..7dae9ecf9fef 100644 --- a/pkg/shell/hosts.jsx +++ b/pkg/shell/hosts.jsx @@ -15,7 +15,7 @@ import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip"; import 'polyfills'; import { CockpitNav, CockpitNavItem } from "./nav.jsx"; -import { HostModal } from "./hosts_dialog.jsx"; +import { HostModal, try2Connect, codes } from "./hosts_dialog.jsx"; import { useLoggedInUser } from "hooks"; const _ = cockpit.gettext; @@ -75,6 +75,10 @@ export class CockpitHosts extends React.Component { current_key: props.machine.key, show_modal: false, edit_machine: null, + switch_machine: null, + error_options: null, + problem: null, + just_connect: false, }; this.toggleMenu = this.toggleMenu.bind(this); @@ -82,6 +86,7 @@ export class CockpitHosts extends React.Component { this.onAddNewHost = this.onAddNewHost.bind(this); this.onEditHosts = this.onEditHosts.bind(this); this.onHostEdit = this.onHostEdit.bind(this); + this.onHostSwitch = this.onHostSwitch.bind(this); this.onRemove = this.onRemove.bind(this); } @@ -89,6 +94,16 @@ export class CockpitHosts extends React.Component { cockpit.user().then(user => { this.setState({ current_user: user.name || "" }); }).catch(exc => console.log(exc)); + + window.trigger_connection_flow = machine => { + if (!this.state.show_modal) + this.onHostSwitch(machine, true); + }; + this.props.index.navigate(null, true); + } + + componentWillUnmount() { + window.trigger_connection_flow = null; } static getDerivedStateFromProps(nextProps, prevState) { @@ -124,6 +139,45 @@ export class CockpitHosts extends React.Component { this.setState({ show_modal: true, edit_machine: machine }); } + onHostSwitch(machine, just_connect) { + if (machine.state == "connected" || machine.address == "localhost") { + if (!just_connect) { + const addr = this.props.hostAddr({ host: machine.address }, true); + this.props.jump(addr); + } + } else if (machine.state != "connecting") { + if (machine.problem && codes[machine.problem]) { + this.setState({ + show_modal: true, + switch_machine: machine, + problem: machine.problem, + just_connect, + }); + } else if (!window.sessionStorage.getItem("connection-warning-shown")) + this.setState({ show_modal: true, switch_machine: machine, just_connect }); + else { + try2Connect(this.props.machines, machine.connection_string) + .then(() => { + const parts = this.props.machines.split_connection_string(machine.connection_string); + const addr = this.props.hostAddr({ host: parts.address }, true); + this.props.loader.connect(parts.address); + if (just_connect) + this.props.index.navigate(); + else + this.props.jump(addr); + }) + .catch(err => { + this.setState({ show_modal: true, + switch_machine: machine, + error_options: err, + problem: err.problem, + just_connect + }); + }); + } + } + } + onEditHosts() { this.setState(s => { return { editing: !s.editing } }); } @@ -180,7 +234,7 @@ export class CockpitHosts extends React.Component { header={(m.user ? m.user : this.state.current_user) + " @"} status={m.state === "failed" ? { type: "error", title: _("Connection error") } : null} className={m.state} - jump={this.props.jump} + jump={() => this.onHostSwitch(m)} actions={<> @@ -242,18 +296,40 @@ export class CockpitHosts extends React.Component { {this.state.show_modal && this.setState({ show_modal: false, edit_machine: null })} - address={this.state.edit_machine ? this.state.edit_machine.address : null} - caller_callback={this.state.edit_machine - ? (new_connection_string) => { - const parts = this.props.machines.split_connection_string(new_connection_string); - if (this.state.edit_machine == this.props.machine && parts.address != this.state.edit_machine.address) { - const addr = this.props.hostAddr({ host: parts.address }, true); + onClose={() => this.setState({ + show_modal: false, + edit_machine: null, + switch_machine: null, + error_options: null, + problem: null, + })} + address={this.state.edit_machine?.address || this.state.switch_machine?.address} + template={this.state.switch_machine + ? (this.state.problem + ? codes[this.state.problem] || "change-port" + : "connect") + : null} + error_options={this.state.error_options} + caller_callback={(new_connection_string) => { + const parts = this.props.machines.split_connection_string(new_connection_string); + const addr = this.props.hostAddr({ host: parts.address }, true); + if (this.state.edit_machine) { + if (this.state.edit_machine == this.props.machine && + parts.address != this.state.edit_machine.address) { + this.props.jump(addr); + } + } else if (this.state.switch_machine) { + this.props.loader.connect(parts.address); + this.props.index.frames.remove(this.state.switch_machine); + if (this.state.just_connect) { + this.props.index.navigate(); + } else { this.props.jump(addr); } - return Promise.resolve(); } - : null } /> + return Promise.resolve(); + }} + /> } ); @@ -263,6 +339,8 @@ export class CockpitHosts extends React.Component { CockpitHosts.propTypes = { machine: PropTypes.object.isRequired, machines: PropTypes.object.isRequired, + index: PropTypes.object.isRequired, + loader: PropTypes.object.isRequired, selector: PropTypes.string.isRequired, hostAddr: PropTypes.func.isRequired, jump: PropTypes.func.isRequired, diff --git a/pkg/shell/hosts_dialog.jsx b/pkg/shell/hosts_dialog.jsx index 45d3941e1f6e..d1c869eb4d2a 100644 --- a/pkg/shell/hosts_dialog.jsx +++ b/pkg/shell/hosts_dialog.jsx @@ -38,14 +38,17 @@ import { Popover } from "@patternfly/react-core/dist/esm/components/Popover/inde import { Radio } from "@patternfly/react-core/dist/esm/components/Radio/index.js"; import { Stack } from "@patternfly/react-core/dist/esm/layouts/Stack/index.js"; import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput/index.js"; -import { OutlinedQuestionCircleIcon } from "@patternfly/react-icons"; +import { OutlinedQuestionCircleIcon, ExternalLinkAltIcon } from "@patternfly/react-icons"; +import { HelperText, HelperTextItem } from "@patternfly/react-core/dist/esm/components/HelperText/index.js"; import { FormHelper } from "cockpit-components-form-helper"; import { ModalError } from "cockpit-components-inline-notification.jsx"; +import { fmt_to_fragments } from "utils.js"; const _ = cockpit.gettext; export const codes = { + danger: "connect", "no-cockpit": "not-supported", "not-supported": "not-supported", "protocol-error": "not-supported", @@ -101,6 +104,74 @@ class NotSupported extends React.Component { } } +export const CrossMachineWarning = () => { + return ; +}; + +class Connect extends React.Component { + constructor(props) { + super(props); + + this.state = { + inProgress: false, + }; + } + + onConnect() { + window.sessionStorage.setItem("connection-warning-shown", true); + this.setState({ inProgress: true }); + this.props.run(this.props.try2Connect(this.props.full_address), ex => { + if (ex.problem === "no-host") { + let host_id_port = this.props.full_address; + let port = "22"; + const port_index = host_id_port.lastIndexOf(":"); + if (port_index === -1) + host_id_port = this.props.full_address + ":22"; + else + port = host_id_port.substr(port_index + 1); + + ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port); + ex.problem = "not-found"; + } + this.setState({ inProgress: false }); + this.props.setError(ex); + }); + } + + render() { + return ( + {this.props.host})} + titleIconVariant="warning" + footer={<> + + {_("You will be reminded once per session.")} + + + + } + > +

{_("Remote hosts have the ability to run JavaScript on all connected hosts. Only connect to machines that you trust.")}

+

+ + {_("Read more")} + +

+ + ); + } +} + class AddMachine extends React.Component { constructor(props) { super(props); @@ -124,6 +195,8 @@ class AddMachine extends React.Component { old_machine = props.machines_ins.lookup(props.old_address); if (old_machine) color = this.rgb2Hex(old_machine.color); + if (old_machine && !old_machine.visible) + old_machine = null; this.state = { user: host_user || "", @@ -222,22 +295,26 @@ class AddMachine extends React.Component { }); }); - this.props.run(this.props.try2Connect(address), ex => { - if (ex.problem === "no-host") { - let host_id_port = address; - let port = "22"; - const port_index = host_id_port.lastIndexOf(":"); - if (port_index === -1) - host_id_port = address + ":22"; - else - port = host_id_port.substr(port_index + 1); - - ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port); - ex.problem = "not-found"; - } - this.setState({ inProgress: false }); - this.props.setError(ex); - }); + if (!window.sessionStorage.getItem("connection-warning-shown")) { + this.props.setError({ problem: "danger", command: "close" }); + } else { + this.props.run(this.props.try2Connect(address), ex => { + if (ex.problem === "no-host") { + let host_id_port = address; + let port = "22"; + const port_index = host_id_port.lastIndexOf(":"); + if (port_index === -1) + host_id_port = address + ":22"; + else + port = host_id_port.substr(port_index + 1); + + ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port); + ex.problem = "not-found"; + } + this.setState({ inProgress: false }); + this.props.setError(ex); + }); + } } render() { @@ -495,7 +572,6 @@ class HostKey extends React.Component {
{_("The fingerprint should match:")} {fingerprint_help}
{fp} - ; } @@ -902,6 +978,31 @@ class ChangeAuth extends React.Component { } } +export function try2Connect(machines_ins, address, options) { + return new Promise((resolve, reject) => { + const conn_options = { ...options, payload: "echo", host: address }; + + conn_options["init-superuser"] = get_init_superuser_for_options(conn_options); + + const machine = machines_ins.lookup(address); + if (machine && machine.host_key && !machine.on_disk) { + conn_options['temp-session'] = false; // Compatibility option + conn_options.session = 'shared'; + conn_options['host-key'] = machine.host_key; + } + + const client = cockpit.channel(conn_options); + client.send("x"); + client.addEventListener("message", () => { + resolve(); + client.close(); + }); + client.addEventListener("close", (event, options) => { + reject(options); + }); + }); +} + export class HostModal extends React.Component { constructor(props) { super(props); @@ -910,7 +1011,7 @@ export class HostModal extends React.Component { current_template: this.props.template || "add-machine", address: full_address(props.machines_ins, props.address), old_address: full_address(props.machines_ins, props.address), - error_options: null, + error_options: this.props.error_options, dialogError: "", // Error to be shown in the modal }; @@ -940,28 +1041,7 @@ export class HostModal extends React.Component { } try2Connect(address, options) { - return new Promise((resolve, reject) => { - const conn_options = { ...options, payload: "echo", host: address }; - - conn_options["init-superuser"] = get_init_superuser_for_options(conn_options); - - const machine = this.props.machines_ins.lookup(address); - if (machine && machine.host_key && !machine.on_disk) { - conn_options['temp-session'] = false; // Compatibility option - conn_options.session = 'shared'; - conn_options['host-key'] = machine.host_key; - } - - const client = cockpit.channel(conn_options); - client.send("x"); - client.addEventListener("message", () => { - resolve(); - client.close(); - }); - client.addEventListener("close", (event, options) => { - reject(options); - }); - }); + return try2Connect(this.props.machines_ins, address, options); } complete() { @@ -1050,7 +1130,9 @@ export class HostModal extends React.Component { complete: this.complete, }; - if (template === "add-machine") + if (template === "connect") + return ; + else if (template === "add-machine") return ; else if (template === "unknown-hostkey" || template === "unknown-host" || template === "invalid-hostkey") return ; diff --git a/pkg/shell/indexes.jsx b/pkg/shell/indexes.jsx index 3ef8aa1fd1bd..bb8dd216da81 100644 --- a/pkg/shell/indexes.jsx +++ b/pkg/shell/indexes.jsx @@ -25,7 +25,7 @@ import { createRoot } from "react-dom/client"; import { CockpitNav, CockpitNavItem, SidebarToggle } from "./nav.jsx"; import { TopNav } from ".//topnav.jsx"; import { CockpitHosts, CockpitCurrentHost } from "./hosts.jsx"; -import { codes, HostModal } from "./hosts_dialog.jsx"; +import { codes } from "./hosts_dialog.jsx"; import { EarlyFailure, EarlyFailureReady } from './failures.jsx'; import { WithDialogs } from "dialogs.jsx"; @@ -80,9 +80,6 @@ function MachinesIndex(index_options, machines, loader) { update_topbar(); }); - /* Is troubleshooting dialog open */ - let troubleshooting_opened = false; - sidebar_toggle_root.render(); // Focus with skiplinks @@ -170,11 +167,18 @@ function MachinesIndex(index_options, machines, loader) { paragraph={cockpit.message(watchdog_problem)} />); } + function trigger_connection_flow(machine) { + if (window.trigger_connection_flow) { + if (machine.state != "connected" && machine.state != "connecting") { + index.frames.remove(machine); + window.trigger_connection_flow(machine); + } + } + } + /* Handles navigation */ function navigate(state, reconnect) { - /* If this is a watchdog problem or we are troubleshooting - * let the dialog handle it */ - if (watchdog_problem || troubleshooting_opened) + if (watchdog_problem) return; if (!state) @@ -204,7 +208,11 @@ function MachinesIndex(index_options, machines, loader) { machine.state = "failed"; machine.problem = "not-found"; } else if (reconnect) { - loader.connect(state.host); + if (state.host == "localhost") { + loader.connect(state.host); + } else { + trigger_connection_flow(machine); + } } const compiled = compile(machine); @@ -394,6 +402,8 @@ function MachinesIndex(index_options, machines, loader) { React.createElement(CockpitHosts, { machine: machine || {}, machines, + index, + loader, selector: "nav-hosts", hostAddr: index.href, jump: index.jump, @@ -443,27 +453,7 @@ function MachinesIndex(index_options, machines, loader) { return component; } - let troubleshoot_dialog_root = null; - function update_frame(machine, state, compiled) { - function render_troubleshoot() { - troubleshooting_opened = true; - const template = codes[machine.problem] || "change-port"; - if (!troubleshoot_dialog_root) - troubleshoot_dialog_root = root('troubleshoot-dialog'); - troubleshoot_dialog_root.render(React.createElement(HostModal, { - template, - address: machine.address, - machines_ins: machines, - onClose: () => { - troubleshoot_dialog_root.unmount(); - troubleshoot_dialog_root = null; - troubleshooting_opened = false; - navigate(null, true); - } - })); - } - let current_frame = index.current_frame(); if (machine.state != "connected") { @@ -508,9 +498,9 @@ function MachinesIndex(index_options, machines, loader) { title={title} reconnect={reconnect} troubleshoot={troubleshooting} - onTroubleshoot={render_troubleshoot} + onTroubleshoot={() => trigger_connection_flow(machine)} watchdog_problem={watchdog_problem} - navigate={navigate} + navigate={() => trigger_connection_flow(machine)} paragraph={message} />); update_title(null, machine); diff --git a/test/common/testlib.py b/test/common/testlib.py index 643a277b3293..2eb1ed4c4733 100644 --- a/test/common/testlib.py +++ b/test/common/testlib.py @@ -1133,8 +1133,10 @@ def start_machine_troubleshoot( known_host: bool = False, password: str | None = None, expect_closed_dialog: bool = True, + need_click: bool = True ) -> None: - self.click('#machine-troubleshoot') + if need_click: + self.click('#machine-troubleshoot') self.wait_visible('#hosts_setup_server_dialog') if new: diff --git a/test/verify/check-shell-host-switching b/test/verify/check-shell-host-switching index 3a01051a36d2..9c3bb9b44bb9 100755 --- a/test/verify/check-shell-host-switching +++ b/test/verify/check-shell-host-switching @@ -85,6 +85,9 @@ class HostSwitcherHelpers: def connect_and_wait(self, b, address, expected_user=None): b.click(f"a[href='/@{address}']") + # wait for host switcher to close after connecting + b.wait_not_present("#nav-hosts.interact") + # open it again b.click("#hosts-sel button") b.wait_visible(f".connected a[href='/@{address}']") if expected_user: