From c2f658a43ebd9edbc5ea6336d574158cf23c0436 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Fri, 6 Sep 2024 09:56:39 +0300 Subject: [PATCH] shell: Require confirmation before connecting to remote machines --- doc/guide/Makefile-guide.am | 1 + doc/guide/cockpit-guide.xml | 1 + doc/guide/multi-host.xml | 54 +++++++ doc/man/cockpit.conf.xml | 12 ++ pkg/shell/base_index.js | 10 ++ pkg/shell/hosts.jsx | 123 ++++++++++++--- pkg/shell/hosts_dialog.jsx | 184 ++++++++++++++++------ pkg/shell/indexes.jsx | 71 +++++---- test/common/testlib.py | 20 ++- test/verify/check-shell-host-switching | 59 +++++-- test/verify/check-shell-multi-machine | 66 ++++---- test/verify/check-shell-multi-machine-key | 8 + test/verify/check-shell-multi-os | 2 +- test/verify/check-superuser | 5 +- test/verify/check-system-realms | 4 + test/verify/check-system-shutdown-restart | 2 +- 16 files changed, 478 insertions(+), 144 deletions(-) create mode 100644 doc/guide/multi-host.xml diff --git a/doc/guide/Makefile-guide.am b/doc/guide/Makefile-guide.am index 66c425cbf27..ad55f1efa84 100644 --- a/doc/guide/Makefile-guide.am +++ b/doc/guide/Makefile-guide.am @@ -21,6 +21,7 @@ GUIDE_INCLUDES = \ doc/guide/cockpit-session.xml \ doc/guide/cockpit-spawn.xml \ doc/guide/cockpit-util.xml \ + doc/guide/multi-host.xml \ doc/guide/authentication.xml \ doc/guide/embedding.xml \ doc/guide/feature-firewall.xml \ diff --git a/doc/guide/cockpit-guide.xml b/doc/guide/cockpit-guide.xml index 77e4cf923c9..13fc80fa98c 100644 --- a/doc/guide/cockpit-guide.xml +++ b/doc/guide/cockpit-guide.xml @@ -27,6 +27,7 @@ + diff --git a/doc/guide/multi-host.xml b/doc/guide/multi-host.xml new file mode 100644 index 00000000000..c0034ed12cf --- /dev/null +++ b/doc/guide/multi-host.xml @@ -0,0 +1,54 @@ + + + + + Managing multiple hosts at the same time + + + + Cockpit allows you to access multiple hosts in a single session, + by establishing SSH connections to other hosts. This is quite + similar to logging into these other hosts using the "ssh" command + on the command line, with one very important difference: + + + Code from the local host and all the remote hosts run at the same + time, in the same browser context. They are not sufficiently + isolated from each other in the browser. All code effectively has + the same privileges as the primary session on the local host. + + + Thus, you should only only connect to remote hosts that + you trust. You must be sure that none of the hosts that + you connect to will cause Cockpit to load malicious JavaScript + code into your browser. + + + Going forward, Cockpit will try to provide sufficient isolation to + make it safe to manage multiple hosts in a single Cockpit + session. But until we get there, Cockpit will at least warn you + before connecting to more than one host. It is also possible to + disable multiple hosts entirely, and some operating systems do + this already by default. + + + You can prevent loading of JavaScript, HTML, etc from more than + one host by adding this to cockpit.conf: + + + [WebService] + AllowMultiHost=false + + + When you allow multiple hosts in a single Cockpit session by + setting AllowMultiHost to true, then the user will be + warned once per session, before connecting to the second host. If + that is still too much, you can switch it off completely by adding + the following to cockpit.conf: + + + [Session] + WarnBeforeConnecting=false + + diff --git a/doc/man/cockpit.conf.xml b/doc/man/cockpit.conf.xml index 275988b347c..94da85c0f11 100644 --- a/doc/man/cockpit.conf.xml +++ b/doc/man/cockpit.conf.xml @@ -271,6 +271,18 @@ IdleTimeout=15 When not specified, there is no idle timeout by default. + + + + Whether to warn before connecting to remote hosts from the Shell. Defaults to true + + +[Session] +WarnBeforeConnecting=false + + + + diff --git a/pkg/shell/base_index.js b/pkg/shell/base_index.js index 3271f9ce590..9aef9f70f1f 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 d8b23cb788b..07ecb73f7bc 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; @@ -73,8 +73,8 @@ export class CockpitHosts extends React.Component { editing: false, current_user: "", current_key: props.machine.key, - show_modal: false, - edit_machine: null, + modal_properties: null, + modal_callback: null, }; this.toggleMenu = this.toggleMenu.bind(this); @@ -89,6 +89,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.modal_properties) + this.connectHost(machine); + }; + this.props.index.navigate(null, true); + } + + componentWillUnmount() { + window.trigger_connection_flow = null; } static getDerivedStateFromProps(nextProps, prevState) { @@ -116,12 +126,86 @@ export class CockpitHosts extends React.Component { }); } - onAddNewHost() { - this.setState({ show_modal: true }); + showModal(properties) { + return new Promise((resolve, reject) => { + this.setState({ modal_properties: properties, + modal_callback: result => { resolve(result); return Promise.resolve() }, + }); + }); } - onHostEdit(event, machine) { - this.setState({ show_modal: true, edit_machine: machine }); + async onAddNewHost() { + const connection_string = await this.showModal({ }); + if (connection_string) { + const parts = this.props.machines.split_connection_string(connection_string); + const addr = this.props.hostAddr({ host: parts.address }, true); + this.props.loader.connect(parts.address); + this.props.jump(addr); + } + } + + async onHostEdit(event, machine) { + const connection_string = await this.showModal({ address: machine.address }); + if (connection_string) { + const parts = this.props.machines.split_connection_string(connection_string); + const addr = this.props.hostAddr({ host: parts.address }, true); + if (machine == this.props.machine && parts.address != machine.address) { + this.props.loader.connect(parts.address); + this.props.jump(addr); + } + } + } + + async connectHost(machine) { + if (machine.address == "localhost" || machine.state == "connected" || machine.state == "connecting") + return machine.connection_string; + + let connection_string = null; + + if (machine.problem && codes[machine.problem]) { + // trouble shooting + connection_string = await this.showModal({ + address: machine.address, + template: codes[machine.problem], + }); + } else if (!window.sessionStorage.getItem("connection-warning-shown")) { + // connect by launching into the "Connection warning" dialog. + connection_string = await this.showModal({ + address: machine.address, + template: "connect" + }); + } else { + // Try to connect without any dialog + try { + await try2Connect(this.props.machines, machine.connection_string); + connection_string = machine.connection_string; + } catch (err) { + // continue with troubleshooting in the dialog + connection_string = await this.showModal({ + address: machine.address, + template: codes[err.problem] || "change-port", + error_options: err, + }); + } + } + + if (connection_string) { + // make the rest of the shell aware that the machine is now connected + const parts = this.props.machines.split_connection_string(connection_string); + this.props.loader.connect(parts.address); + this.props.index.navigate(); + } + + return connection_string; + } + + async onHostSwitch(machine) { + const connection_string = await this.connectHost(machine); + if (connection_string) { + const parts = this.props.machines.split_connection_string(connection_string); + const addr = this.props.hostAddr({ host: parts.address }, true); + this.props.jump(addr); + } } onEditHosts() { @@ -180,7 +264,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={<> @@ -240,20 +324,13 @@ 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); - this.props.jump(addr); - } - return Promise.resolve(); - } - : null } /> + {this.state.modal_properties && + this.setState({ modal_properties: null })} + {...this.state.modal_properties} + caller_callback={this.state.modal_callback} + caller_cancelled={() => this.state.modal_callback(null)} + /> } ); @@ -263,6 +340,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 45d3941e1f6..d9e48a482d0 100644 --- a/pkg/shell/hosts_dialog.jsx +++ b/pkg/shell/hosts_dialog.jsx @@ -38,14 +38,18 @@ 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 { Text, TextContent, TextVariants } from "@patternfly/react-core/dist/esm/components/Text" 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 +105,74 @@ class NotSupported extends React.Component { } } +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 => { + let keep_message = false; + 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"; + keep_message = true; + } + this.setState({ inProgress: false }); + this.props.setError(ex, keep_message); + }); + } + + 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 +196,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 +296,27 @@ 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); + 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); - }); + 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 +574,6 @@ class HostKey extends React.Component {
{_("The fingerprint should match:")} {fingerprint_help}
{fp} - ; } @@ -902,6 +980,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 +1013,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 }; @@ -934,34 +1037,17 @@ export class HostModal extends React.Component { return host; } - changeContent(template, error_options) { + changeContent(template, error_options, with_error_message) { if (this.state.current_template !== template) - this.setState({ current_template: template, error_options }); + this.setState({ + current_template: template, + error_options, + dialogError: with_error_message ? cockpit.message(error_options) : null, + }); } 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() { @@ -975,7 +1061,7 @@ export class HostModal extends React.Component { this.promise_callback = callback; } - setError(error) { + setError(error, keep_message_on_change) { if (error === null) return this.setState({ dialogError: null }); @@ -984,7 +1070,7 @@ export class HostModal extends React.Component { template = codes[error.problem]; if (template && this.state.current_template !== template) - this.changeContent(template, error); + this.changeContent(template, error, keep_message_on_change); else this.setState({ error_options: error, dialogError: cockpit.message(error) }); } @@ -1041,7 +1127,11 @@ export class HostModal extends React.Component { error_options: this.state.error_options, dialogError: this.state.dialogError, machines_ins: this.props.machines_ins, - onClose: this.props.onClose, + onClose: () => { + if (this.props.caller_cancelled) + this.props.caller_cancelled(); + this.props.onClose(); + }, run: this.run, setGoal: this.setGoal, setError: this.setError, @@ -1050,7 +1140,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 3ef8aa1fd1b..3c9441d2d1b 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 @@ -105,11 +102,31 @@ function MachinesIndex(index_options, machines, loader) { if (meta_multihost instanceof HTMLMetaElement && meta_multihost.content == "yes") host_switcher_enabled = true; + /* Should show warning before connecting? */ + let config_ready = false; + cockpit.dbus(null, { bus: "internal" }).call("/config", "cockpit.Config", "GetString", + ["Session", "WarnBeforeConnecting"], []) + .then(([result]) => { + if (result == "false" || result == "no") { + window.sessionStorage.setItem("connection-warning-shown", "yes"); + } + }) + .catch(e => { + if (e.name != "cockpit.Config.KeyError") + console.warn("Error reading WarnBeforeConnecting configuration:", e.message); + }) + .finally(() => { + config_ready = true; + on_ready(); + }); + /* Navigation */ let ready = false; function on_ready() { - ready = true; - index.ready(); + if (machines.ready && config_ready) { + ready = true; + index.ready(); + } } function preload_frames () { @@ -170,11 +187,15 @@ function MachinesIndex(index_options, machines, loader) { paragraph={cockpit.message(watchdog_problem)} />); } + function trigger_connection_flow(machine) { + if (window.trigger_connection_flow) { + 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 +225,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 +419,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 +470,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 +515,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 97099c35237..a7f4d6c0423 100644 --- a/test/common/testlib.py +++ b/test/common/testlib.py @@ -1134,13 +1134,23 @@ def start_machine_troubleshoot( known_host: bool = False, password: str | None = None, expect_closed_dialog: bool = True, + expect_warning: bool = True, + need_click: bool = True ) -> None: - self.click('#machine-troubleshoot') + if need_click: + self.click('#machine-troubleshoot') + + if not new and expect_warning: + self.wait_visible('#hosts_connect_server_dialog') + self.click("#hosts_connect_server_dialog button.pf-m-warning") self.wait_visible('#hosts_setup_server_dialog') if new: self.wait_text("#hosts_setup_server_dialog button.pf-m-primary", "Add") self.click("#hosts_setup_server_dialog button.pf-m-primary") + if expect_warning: + self.wait_visible('#hosts_connect_server_dialog') + self.click("#hosts_connect_server_dialog button.pf-m-warning") if not known_host: self.wait_in_text('#hosts_setup_server_dialog', "You are connecting to") self.wait_in_text('#hosts_setup_server_dialog', "for the first time.") @@ -1154,10 +1164,14 @@ def start_machine_troubleshoot( if expect_closed_dialog: self.wait_not_present('#hosts_setup_server_dialog') - def add_machine(self, address: str, known_host: bool = False, password: str = "foobar") -> None: + def add_machine(self, address: str, known_host: bool = False, password: str = "foobar", + expect_warning: bool = True) -> None: self.switch_to_top() self.go(f"/@{address}") - self.start_machine_troubleshoot(new=True, known_host=known_host, password=password) + self.start_machine_troubleshoot(new=True, + known_host=known_host, + password=password, + expect_warning=expect_warning) self.enter_page("/system", host=address) def grant_permissions(self, *args: str) -> None: diff --git a/test/verify/check-shell-host-switching b/test/verify/check-shell-host-switching index 3a01051a36d..197ce570773 100755 --- a/test/verify/check-shell-host-switching +++ b/test/verify/check-shell-host-switching @@ -63,7 +63,8 @@ class HostSwitcherHelpers: # HACK: Dropping the machine does not terminate SSH connection; https://github.com/cockpit-project/cockpit/issues/19672 self.machine.execute(f"pkill -f [s]sh.*{address}; while pgrep -f [s]sh.*{address}; do sleep 1; done") - def add_new_machine(self, b, address, known_host=False, pixel_label=None, user=None, expect_password_auth=False): + def add_new_machine(self, b, address, known_host=False, pixel_label=None, user=None, expect_password_auth=False, + expect_warning=False): b.click("button:contains('Add new host')") b.wait_visible('#hosts_setup_server_dialog') b.set_input_text('#add-machine-address', address) @@ -72,6 +73,9 @@ class HostSwitcherHelpers: if pixel_label: b.assert_pixels("#hosts_setup_server_dialog", pixel_label) b.click('#hosts_setup_server_dialog .pf-m-primary:contains("Add")') + if expect_warning: + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') if not known_host: b.wait_in_text('#hosts_setup_server_dialog', f"You are connecting to {address.removeprefix('ssh://')} for the first time") @@ -83,8 +87,14 @@ class HostSwitcherHelpers: with b.wait_timeout(30): b.wait_not_present('#hosts_setup_server_dialog') - def connect_and_wait(self, b, address, expected_user=None): + def connect_and_wait(self, b, address, expected_user=None, expect_warning=False): b.click(f"a[href='/@{address}']") + if expect_warning: + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') + # 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: @@ -173,7 +183,30 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): b.wait_in_text("#nav-hosts .nav-item a", "mydhcpname") m1.execute("hostnamectl set-hostname 'localhost'") - self.add_new_machine(b, "10.111.113.2", pixel_label="host-add-dialog") + # Add a host with a couple of mistakes on the way + b.click("button:contains('Add new host')") + b.wait_visible('#hosts_setup_server_dialog') + b.set_input_text('#add-machine-address', "10.111.113.2:1234") + b.click('#hosts_setup_server_dialog .pf-m-primary:contains("Add")') + # Port is wrong but we first have to confirm that we really want to connect + b.wait_visible('#hosts_connect_server_dialog') + b.assert_pixels("#hosts_connect_server_dialog", "host-connect-dialog") + b.click('#hosts_connect_server_dialog button.pf-m-warning') + # Now we are back in the "Add host" dialog with the error + b.wait_in_text('#hosts_setup_server_dialog', "Unable to contact the given host 10.111.113.2:1234. Make sure it has ssh running on port 1234, or specify another port in the address.") + # Give another wrong address + b.set_input_text('#add-machine-address', "10.111.113.2:4321") + b.click('#hosts_setup_server_dialog .pf-m-primary:contains("Add")') + # Error happens immediatly now. + b.wait_in_text('#hosts_setup_server_dialog', "Unable to contact the given host 10.111.113.2:4321. Make sure it has ssh running on port 4321, or specify another port in the address.") + # Now do it right + b.set_input_text('#add-machine-address', "10.111.113.2") + b.assert_pixels("#hosts_setup_server_dialog", "host-add-dialog") + b.click('#hosts_setup_server_dialog .pf-m-primary:contains("Add")') + b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time") + b.click('#hosts_setup_server_dialog .pf-m-primary') + b.wait_not_present('#hosts_setup_server_dialog') + self.wait_host_addresses(b, ["localhost", "10.111.113.2"]) # defaults to current host user name "admin" self.connect_and_wait(b, "10.111.113.2", "admin") @@ -296,12 +329,18 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): self.connect_and_wait(b, "10.111.113.2", "someone") self.machine_remove(b, "10.111.113.2", second_to_last=True) + # switch off warnings for the rest of this test (nneds the + # relogin below to take effect) + m1.write("/etc/cockpit/cockpit.conf", + '[Session]\nWarnBeforeConnecting=false\n', + append=True) + # reset session store to forget previous user/host connections b.relogin() b.click("#hosts-sel button") - # ssh:// prefix and implied user - self.add_new_machine(b, "ssh://10.111.113.2", known_host=True) + # ssh:// prefix and implied user, no warning because we switched it off above + self.add_new_machine(b, "ssh://10.111.113.2", known_host=True, expect_warning=False) self.wait_host_addresses(b, ["localhost", "10.111.113.2"]) self.connect_and_wait(b, "10.111.113.2", "admin") self.machine_remove(b, "10.111.113.2", second_to_last=True) @@ -365,11 +404,11 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): b2.click("#hosts-sel button") self.wait_host_addresses(b2, ["localhost"]) - self.add_new_machine(b, "10.111.113.2") + self.add_new_machine(b, "10.111.113.2", expect_warning=True) self.wait_host_addresses(b, ["localhost", "10.111.113.2"]) self.wait_host_addresses(b2, ["localhost", "10.111.113.2"]) self.connect_and_wait(b, "10.111.113.2") - self.connect_and_wait(b2, "10.111.113.2") + self.connect_and_wait(b2, "10.111.113.2", expect_warning=True) # Main host should have both buttons disabled, the second both enabled b.click("button:contains('Edit hosts')") @@ -445,7 +484,7 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): self.login_and_go(superuser=False) b.click("#hosts-sel button") - self.add_new_machine(b, "10.111.113.3") + self.add_new_machine(b, "10.111.113.3", expect_warning=True) self.wait_host_addresses(b, ["localhost", "10.111.113.3"]) self.connect_and_wait(b, "10.111.113.3") @@ -526,9 +565,9 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): self.enable_multihost(self.machine) self.login_and_go(None) - # And and connect to a second machine + # Add and connect to a second machine b.click("#hosts-sel button") - self.add_new_machine(b, "10.111.113.2") + self.add_new_machine(b, "10.111.113.2", expect_warning=True) b.click("a[href='/@10.111.113.2']") b.wait_visible("iframe.container-frame[name='cockpit1:10.111.113.2/system']") self.assertIn("admin", m2.execute("loginctl")) diff --git a/test/verify/check-shell-multi-machine b/test/verify/check-shell-multi-machine index 9b49b1ee610..f544850e647 100755 --- a/test/verify/check-shell-multi-machine +++ b/test/verify/check-shell-multi-machine @@ -148,7 +148,7 @@ class TestMultiMachineAdd(testlib.MachineCase): self.login_and_go(None) b.add_machine("10.111.113.2", password=None) - b.add_machine(m3_host, password=None) + b.add_machine(m3_host, password=None, expect_warning=False) b.switch_to_top() b.click("#hosts-sel button") @@ -225,7 +225,7 @@ class TestMultiMachineAdd(testlib.MachineCase): self.login_and_go(None) b.add_machine("m2", password=None) - b.add_machine("m3", password=None) + b.add_machine("m3", password=None, expect_warning=False) b.switch_to_top() b.click("#hosts-sel button") @@ -506,8 +506,11 @@ class TestMultiMachine(testlib.MachineCase): # navigating there again will fail b.go(m2_path) with b.wait_timeout(30): - b.wait_text(".curtains-ct h1", "Not connected to host") - b.wait_text("#machine-troubleshoot", "Log in") + b.wait_visible("#hosts_setup_server_dialog") + b.start_machine_troubleshoot(need_click=False, password="foobar", + expect_closed_dialog=False, expect_warning=False) + b.wait_in_text("#hosts_setup_server_dialog .pf-v5-c-alert", "Login failed") + b.click("#hosts_setup_server_dialog button:contains(Cancel)") # wait for system to load b.go("/system") @@ -522,8 +525,8 @@ class TestMultiMachine(testlib.MachineCase): b.reload() b.go(m2_path) with b.wait_timeout(30): - b.wait_text(".curtains-ct h1", "Not connected to host") - b.start_machine_troubleshoot(password="foobar") + b.wait_visible("#hosts_setup_server_dialog") + b.start_machine_troubleshoot(need_click=False, password="foobar", expect_warning=False) b.enter_page("/playground/test", "10.111.113.2", reconnect=True) # image is back because it page was reloaded after disconnection @@ -622,21 +625,21 @@ class TestMultiMachine(testlib.MachineCase): m1.execute("mkdir -p /home/admin/.ssh/") break_hostkey(m1, "10.111.113.2") - b.start_machine_troubleshoot(new=True, known_host=True, expect_closed_dialog=False) + b.start_machine_troubleshoot(new=True, known_host=True, expect_closed_dialog=False, expect_warning=False) b.wait_in_text('#hosts_setup_server_dialog', "10.111.113.2 key changed") b.click("#hosts_setup_server_dialog button:contains(Cancel)") fix_hostkey(m1) # Bad cockpit break_bridge(m2) - b.start_machine_troubleshoot(new=True, password="foobar", expect_closed_dialog=False) + b.start_machine_troubleshoot(new=True, password="foobar", expect_closed_dialog=False, expect_warning=False) check_failed_state(b, "Cockpit is not installed") fix_bridge(m2) # Troubleshoot existing # Properly add machine fix_hostkey(m1) - b.add_machine("10.111.113.2") + b.add_machine("10.111.113.2", expect_warning=False) b.logout() b.wait_visible("#login") @@ -644,16 +647,17 @@ class TestMultiMachine(testlib.MachineCase): break_bridge(m2) self.login_and_go(None) b.go(machine_path) - with b.wait_timeout(240): - b.start_machine_troubleshoot(password="foobar", expect_closed_dialog=False) + with b.wait_timeout(20): + b.start_machine_troubleshoot(need_click=False, password="foobar", expect_closed_dialog=False) check_failed_state(b, "Cockpit is not installed") - b.wait_visible("#machine-troubleshoot") + b.wait_visible("#machine-reconnect") fix_bridge(m2) # Clear host key fix_hostkey(m1) - b.start_machine_troubleshoot(expect_closed_dialog=False) + b.click("#machine-reconnect") + b.start_machine_troubleshoot(need_click=False, expect_closed_dialog=False, expect_warning=False) b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time.") # show fingerprint validation @@ -687,13 +691,10 @@ class TestMultiMachine(testlib.MachineCase): self.login_and_go(None) b.go(machine_path) - with b.wait_timeout(120): - b.wait_visible("#machine-troubleshoot") - b.start_machine_troubleshoot(expect_closed_dialog=False) + with b.wait_timeout(20): + b.wait_visible("#hosts_connect_server_dialog") + b.start_machine_troubleshoot(need_click=False, expect_closed_dialog=False) b.wait_in_text('#hosts_setup_server_dialog', "Unable to log in") - b.set_input_text('#login-custom-password', "") - fail_login(b) - b.set_input_text("#login-custom-password", "bad") fail_login(b) b.set_input_text("#login-custom-password", "alt-password") @@ -712,11 +713,11 @@ class TestMultiMachine(testlib.MachineCase): self.login_and_go(None) b.go(machine_path) - with b.wait_timeout(120): - b.wait_visible("#machine-troubleshoot") - b.start_machine_troubleshoot(expect_closed_dialog=False) - b.wait_in_text('#hosts_setup_server_dialog h1', "Could not contact") - b.set_input_text("#edit-machine-port", "2222") + with b.wait_timeout(20): + b.wait_visible("#hosts_connect_server_dialog") + b.start_machine_troubleshoot(need_click=False, expect_closed_dialog=False) + b.wait_in_text('#hosts_setup_server_dialog', "Unable to contact") + b.set_input_text("#add-machine-address", "10.111.113.2:2222") b.click(f'#hosts_setup_server_dialog {self.primary_btn_class}') # ssh(1) tracks known hosts by name/IP, not by port; so not expecting a host key prompt here b.wait_in_text('#hosts_setup_server_dialog h1', "Log in to") @@ -756,12 +757,13 @@ class TestMultiMachine(testlib.MachineCase): self.login_and_go(None) b.go("/@10.111.113.2") - b.start_machine_troubleshoot(expect_closed_dialog=False) b.click('#machine-troubleshoot') b.wait_visible('#hosts_setup_server_dialog') b.wait_in_text('#hosts_setup_server_dialog', "new host") b.set_input_text('#add-machine-user', "fred") b.click('#hosts_setup_server_dialog button:contains(Add)') + b.wait_visible('#hosts_connect_server_dialog') + b.click("#hosts_connect_server_dialog button.pf-m-warning") b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time.") b.click("#hosts_setup_server_dialog button:contains('Trust and add host')") b.wait_in_text('#hosts_setup_server_dialog', "Unable to log in") @@ -787,14 +789,17 @@ class TestMultiMachine(testlib.MachineCase): self.assertEqual(m1.execute("cat /home/admin/.ssh/id_rsa.pub"), m2.execute("cat /home/fred/.ssh/authorized_keys")) - # Relogin. This should now work seamlessly. + # Relogin. This should now work seamlessly (except for the warning). b.relogin(None, wait_remote_session_machine=m1) + b.wait_visible('#hosts_connect_server_dialog') + b.click("#hosts_connect_server_dialog button.pf-m-warning") b.enter_page("/system", host="fred@10.111.113.2") # De-authorize key and relogin, then re-authorize. m2.execute("rm /home/fred/.ssh/authorized_keys") b.relogin(None, wait_remote_session_machine=m1) - b.click('#machine-troubleshoot') + b.wait_visible('#hosts_connect_server_dialog') + b.click("#hosts_connect_server_dialog button.pf-m-warning") b.wait_visible('#hosts_setup_server_dialog') b.wait_in_text('#hosts_setup_server_dialog', "Unable to log in") b.wait_in_text("#hosts_setup_server_dialog", "Authorize SSH key") @@ -810,7 +815,8 @@ class TestMultiMachine(testlib.MachineCase): # change the passphrase back to the login password m1.execute("ssh-keygen -q -f /home/admin/.ssh/id_rsa -p -P foobar -N foobarfoo") b.relogin(None, wait_remote_session_machine=m1) - b.click('#machine-troubleshoot') + b.wait_visible('#hosts_connect_server_dialog') + b.click("#hosts_connect_server_dialog button.pf-m-warning") b.wait_visible('#hosts_setup_server_dialog') b.wait_in_text('#hosts_setup_server_dialog', "The SSH key for logging in") b.set_checked('#hosts_setup_server_dialog input[value=key]', val=True) @@ -826,6 +832,8 @@ class TestMultiMachine(testlib.MachineCase): # which don't have pam-ssh-add in its PAM stack.) if not m1.ostree_image: b.relogin(None, wait_remote_session_machine=m1) + b.wait_visible('#hosts_connect_server_dialog') + b.click("#hosts_connect_server_dialog button.pf-m-warning") b.enter_page("/system", host="fred@10.111.113.2") # The authorized_keys files should still only have a single key @@ -859,6 +867,8 @@ class TestMultiMachine(testlib.MachineCase): b.click('#machine-troubleshoot') b.wait_visible('#hosts_setup_server_dialog') b.click('#hosts_setup_server_dialog button:contains(Add)') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button:contains(Connect)') b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time.") b.click("#hosts_setup_server_dialog button:contains('Trust and add host')") b.wait_in_text('#hosts_setup_server_dialog', "The SSH key") diff --git a/test/verify/check-shell-multi-machine-key b/test/verify/check-shell-multi-machine-key index 73da4ef77e5..d822647ee2a 100755 --- a/test/verify/check-shell-multi-machine-key +++ b/test/verify/check-shell-multi-machine-key @@ -156,6 +156,8 @@ class TestMultiMachineKeyAuth(testlib.MachineCase): b.wait_text(f'#hosts_setup_server_dialog {self.primary_btn_class}', "Add") b.click(f'#hosts_setup_server_dialog {self.primary_btn_class}') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time.") b.click(f'#hosts_setup_server_dialog {self.primary_btn_class}') b.wait_in_text('#hosts_setup_server_dialog h1', "Log in to") @@ -185,6 +187,8 @@ class TestMultiMachineKeyAuth(testlib.MachineCase): self.load_key('id_rsa', 'foobar') b.go("/@10.111.113.2") + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_visible("iframe.container-frame[name='cockpit1:10.111.113.2/system']") # Change user @@ -253,6 +257,8 @@ Host 10.111.113.2 b.wait_visible('#hosts_setup_server_dialog') b.set_input_text('#add-machine-address', "10.111.113.2") b.click("#hosts_setup_server_dialog .pf-m-primary") + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_in_text("#hosts_setup_server_dialog", "You are connecting to 10.111.113.2 for the first time.") b.click("#hosts_setup_server_dialog .pf-m-primary") b.wait_in_text("#hosts_setup_server_dialog", "/home/admin/.ssh/id_ed25519") @@ -288,6 +294,8 @@ Host 10.111.113.2 b.wait_visible('#hosts_setup_server_dialog') b.set_input_text("#add-machine-address", "10.111.113.2") b.click("#hosts_setup_server_dialog .pf-m-primary") + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_in_text("#hosts_setup_server_dialog", "You are connecting to 10.111.113.2 for the first time.") b.click("#hosts_setup_server_dialog .pf-m-primary") b.wait_in_text("#hosts_setup_server_dialog", "/home/admin/.ssh/id_rsa") diff --git a/test/verify/check-shell-multi-os b/test/verify/check-shell-multi-os index a0ab399a3a0..6e380bd70b5 100755 --- a/test/verify/check-shell-multi-os +++ b/test/verify/check-shell-multi-os @@ -79,7 +79,7 @@ class TestRHEL8(testlib.MachineCase): # stock → dev: via shell Add host stock_b.login_and_go() - stock_b.add_machine("10.111.113.1") + stock_b.add_machine("10.111.113.1", expect_warning=False) stock_b.wait_in_text(".ct-overview-header-hostname", dev_hostname) diff --git a/test/verify/check-superuser b/test/verify/check-superuser index fb7b149a286..2c577632505 100755 --- a/test/verify/check-superuser +++ b/test/verify/check-superuser @@ -415,6 +415,8 @@ class TestSuperuserDashboard(testlib.MachineCase): b.wait_visible('#hosts_setup_server_dialog') b.wait_visible('#hosts_setup_server_dialog button:contains("Add")') b.click('#hosts_setup_server_dialog button:contains("Add")') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_in_text('#hosts_setup_server_dialog', "You are connecting to 10.111.113.2 for the first time.") b.click('#hosts_setup_server_dialog button.pf-m-primary') b.wait_in_text('#hosts_setup_server_dialog', "Unable to log in") @@ -444,7 +446,8 @@ class TestSuperuserDashboard(testlib.MachineCase): # superuser on m2 (once we have logged in there). self.allow_restart_journal_messages() b.relogin() - b.click('#machine-troubleshoot') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.wait_visible('#hosts_setup_server_dialog') b.set_input_text("#login-custom-password", "foobar") b.click('#hosts_setup_server_dialog button:contains("Log in")') diff --git a/test/verify/check-system-realms b/test/verify/check-system-realms index 92609aec54b..519eadad52e 100755 --- a/test/verify/check-system-realms +++ b/test/verify/check-system-realms @@ -813,6 +813,8 @@ ipa-advise enable-admins-sudo | sh -ex b.wait_visible('#hosts_setup_server_dialog') b.click('#hosts_setup_server_dialog button:contains(Add)') b.wait_not_present('#hosts_setup_server_dialog') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') # Getting root privs through sudo with kerberos in the remote SSH session does not currently work. # ssh -K is supposed to forward the credentials cache, but doesn't; klist in the ssh session is empty @@ -1071,6 +1073,8 @@ class TestKerberos(testlib.MachineCase): b.wait_visible('#hosts_setup_server_dialog') b.click('#hosts_setup_server_dialog button:contains(Add)') b.wait_not_present('#hosts_setup_server_dialog') + b.wait_visible('#hosts_connect_server_dialog') + b.click('#hosts_connect_server_dialog button.pf-m-warning') b.enter_page("/system/terminal", host="x0.cockpit.lan") b.wait_visible(".terminal") diff --git a/test/verify/check-system-shutdown-restart b/test/verify/check-system-shutdown-restart index b338338f171..a56ce2e8ab2 100755 --- a/test/verify/check-system-shutdown-restart +++ b/test/verify/check-system-shutdown-restart @@ -92,7 +92,7 @@ class TestShutdownRestart(testlib.MachineCase): with b2.wait_timeout(30): b2.wait_visible("#machine-troubleshoot") - b2.start_machine_troubleshoot(password="foobar") + b2.start_machine_troubleshoot(password="foobar", expect_warning=False) b2.enter_page("/system", host="10.111.113.1", reconnect=False)