Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3ddba69
Remote Client: Finish support for OpenSSH client config file specific…
dalthviz May 2, 2025
8e0c598
Remote Client: Improve fields tips for config file based connection page
dalthviz May 2, 2025
ed3079f
Merge branch 'master' into fixes_issue_22464
dalthviz May 5, 2025
f8957be
Remote Client: Add optional fields for config file based connection
dalthviz May 9, 2025
83965c7
Apply suggestions from code review
dalthviz May 20, 2025
2e55e08
Merge branch 'master' into fixes_issue_22464
dalthviz May 20, 2025
4625411
Remote client: Add initial extra validations using OpenSSH client con…
dalthviz May 24, 2025
89b125f
Merge branch 'master' into fixes_issue_22464
dalthviz May 30, 2025
09ebedf
Merge branch 'master' into fixes_issue_22464
dalthviz Jun 1, 2025
e45a823
Merge branch 'master' into fixes_issue_22464
dalthviz Jun 17, 2025
59ba911
Remote Client: Add more default values to connect dialog for config f…
dalthviz Jun 17, 2025
8f11ebb
Merge branch 'master' into fixes_issue_22464
dalthviz Jul 9, 2025
c9c4097
Merge branch 'master' into fixes_issue_22464
dalthviz Aug 4, 2025
2dbeabb
Apply suggestions from code review
dalthviz Sep 22, 2025
08191f0
Merge branch 'master' into fixes_issue_22464
dalthviz Sep 22, 2025
9aae452
Merge branch 'master' into fixes_issue_22464
dalthviz Oct 23, 2025
f1cd227
Remote client: Revert height change and add advanced config section
dalthviz Oct 23, 2025
033cbc9
Merge branch 'master' into fixes_issue_22464
dalthviz Oct 27, 2025
2038673
Remote client: Remove config file as authentication method. Add confi…
dalthviz Oct 31, 2025
294e628
Merge branch 'fixes_issue_22464' of https://github.com/dalthviz/spyde…
dalthviz Oct 31, 2025
ad0fbe0
Merge branch 'master' into fixes_issue_22464
dalthviz Oct 31, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion spyder/plugins/remoteclient/api/manager/ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,11 @@ async def _create_new_connection(self) -> bool:
self._ssh_connection = await asyncssh.connect(
**connect_kwargs, client_factory=self.client_factory
)
except (OSError, asyncssh.Error) as e:
except (
OSError,
asyncssh.Error,
asyncssh.public_key.KeyImportError,
) as e:
self.logger.error(f"Failed to open ssh connection: {e}")
self._emit_connection_status(
ConnectionStatus.Error,
Expand Down
25 changes: 24 additions & 1 deletion spyder/plugins/remoteclient/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,30 @@ def _load_ssh_client_options(self, config_id):
):
options["passphrase"] = passphrase
elif config := self.get_conf(f"{config_id}/configfile"):
options["config"] = config
options["config"] = [config]

# Some validations to avoid passing empty values
if options["port"] == 0:
# Ignore value if 0 is set because it means the port we'll be
# read from the config file.
options.pop("port")

if not options["username"]:
# Ignore empty username
options.pop("username")

if password := self.get_conf(f"{config_id}/password", secure=True):
# Add password if available
options["password"] = password

if client_keys := self.get_conf(f"{config_id}/keyfile", ""):
# Add keyfile if available
options["client_keys"] = [client_keys]

if passphrase := self.get_conf(f"{config_id}/passphrase", secure=True):
# Add passphrase if available
options["passphrase"] = passphrase

else:
# Password is mandatory in this case
password = self.get_conf(f"{config_id}/password", secure=True)
Expand Down
196 changes: 189 additions & 7 deletions spyder/plugins/remoteclient/widgets/connectionpages.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
# Standard library imports
from __future__ import annotations
from collections.abc import Iterable
import logging
import re
from typing import TypedDict
import uuid

# Third party imports
import asyncssh
from qtpy.QtCore import Qt
from qtpy.QtWidgets import (
QButtonGroup,
Expand Down Expand Up @@ -57,6 +59,8 @@
except Exception:
ENV_MANAGER = False

logger = logging.getLogger(__name__)


# =============================================================================
# ---- Constants
Expand All @@ -65,6 +69,7 @@ class ValidationReasons(TypedDict):
repeated_name: bool | None
missing_info: bool | None
invalid_address: bool | None
invalid_host: bool | None
invalid_url: bool | None


Expand All @@ -80,7 +85,7 @@ class CreateEnvMethods:
class BaseConnectionPage(SpyderConfigPage, SpyderFontsMixin):
"""Base class to create connection pages."""

MIN_HEIGHT = 450
MIN_HEIGHT = 800
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this change introduces a scrollbar for all pages, which would require additional UI adjustments on all operating systems.

Furthermore, perhaps we should rethink having a single height for all pages to instead adjust it according to the one shown at the moment. That's because, also with this change, pages like the Connection status one end up with a lot of unnecessary blank space.

Since we're really close to the 6.1 final release, I think it's better to leave that for 6.1.1 so we can address it more carefully.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about a way to prevent this extra height as well as not always show fields that are not usable for simple use cases of a config file maybe adding an Advanced configuration collapsible section could help:

config_file_connection

What do you think @ccordoba12 ?

NEW_CONNECTION = False
CONF_SECTION = "remoteclient"

Expand Down Expand Up @@ -139,8 +144,33 @@ def validate_page(self):
validate_label.setVisible(False)

reasons: ValidationReasons = {}

if auth_method == AuthenticationMethod.ConfigFile:
host_widget = widgets[1]
host = host_widget.textbox.text()
config_filepath_widget = widgets[6]
config_filepath = config_filepath_widget.textbox.text()
config_filepath_widget.textbox._validate(config_filepath)

if not host:
reasons["missing_info"] = True
host_widget.status_action.setVisible(True)
host_widget.status_action.setToolTip(_("This field is empty"))
elif not self._validate_config_file(config_filepath):
reasons["invalid_host"] = True
host_widget.status_action.setVisible(True)
host_widget.status_action.setToolTip(_("Invalid host"))
else:
host_widget.status_action.setVisible(False)
config_filepath_widget.textbox.error_action.setVisible(False)

for widget in widgets:
if not widget.textbox.text():
if (
auth_method != AuthenticationMethod.ConfigFile
and not widget.textbox.text()
):
# Validate that the required fields are not empty when using
# a different authentication method than Config file
# Validate that the required fields are not empty
widget.status_action.setVisible(True)
widget.status_action.setToolTip(_("This field is empty"))
Expand All @@ -167,7 +197,7 @@ def validate_page(self):
if not self._validate_url(url):
reasons["invalid_url"] = True
widget.status_action.setVisible(True)
else:
elif auth_method != AuthenticationMethod.ConfigFile:
widget.status_action.setVisible(False)

if reasons:
Expand Down Expand Up @@ -268,12 +298,10 @@ def create_ssh_connection_info_widget(self):
intro_layout.addWidget(intro_tip)

# Authentication methods
# TODO: The config file method is not implemented yet, so we need to
# disable it for now.
methods = (
(_('Password'), AuthenticationMethod.Password),
(_('Key file'), AuthenticationMethod.KeyFile),
# (_('Configuration file'), AuthenticationMethod.ConfigFile),
(_('Configuration file'), AuthenticationMethod.ConfigFile),
)

self._auth_methods = self.create_combobox(
Expand Down Expand Up @@ -512,28 +540,136 @@ def _create_configfile_subpage(self):
configfile = self.create_browsefile(
text=_("Configuration file *"),
option=f"{self.host_id}/configfile",
tip=_("File with the OpenSSH client configuration to use"),
validate_callback=self._validate_config_file,
validate_reason=_("Invalid OpenSSH client configuration file"),
alignment=Qt.Vertical,
status_icon=ima.icon("error"),
)

host = self.create_lineedit(
text=_("Host *"),
option=f"{self.host_id}/{AuthenticationMethod.ConfigFile}/address",
default="",
tip=_(
"This is the host (i.e. <b>Host</b> configuration keyword) "
"that encapsulates the options to use for the connection"
),
status_icon=ima.icon("error"),
)

port = self.create_spinbox(
prefix=_("Port"),
suffix="",
option=f"{self.host_id}/{AuthenticationMethod.ConfigFile}/port",
default=0,
min_=0,
max_=65535,
tip=_(
"Introduce a port to use for this connection. Set <b>0</b> "
"to use the port specified in the configuration file"
),
)
port.spinbox.setStyleSheet("margin-left: 5px")

username = self.create_lineedit(
text=_("Username"),
option=f"{self.host_id}/{AuthenticationMethod.ConfigFile}/username",
default="",
status_icon=ima.icon("error"),
)

password = self.create_lineedit(
text=_("Password"),
option=f"{self.host_id}/password",
tip=(
_("Your password will be saved securely by Spyder")
if self.NEW_CONNECTION
else _("Your password is saved securely by Spyder")
),
status_icon=ima.icon("error"),
password=True
)

keyfile = self.create_browsefile(
text=_("Key file"),
option=f"{self.host_id}/keyfile",
alignment=Qt.Vertical,
status_icon=ima.icon("error"),
)

passphrase = self.create_lineedit(
text=_("Passphrase"),
option=f"{self.host_id}/passphrase",
tip=(
_("Your passphrase will be saved securely by Spyder")
if self.NEW_CONNECTION
else _("Your passphrase is saved securely by Spyder")
),
status_icon=ima.icon("error"),
password=True
)

validation_label = MessageLabel(self)

# Add widgets to their required dicts
self._name_widgets[AuthenticationMethod.ConfigFile] = name
self._widgets_for_validation[AuthenticationMethod.ConfigFile] = [
name,
host,
username,
password,
keyfile,
passphrase,
configfile,
]
self._validation_labels[
AuthenticationMethod.ConfigFile
] = validation_label

# Layout
# Hide container widgets for host and port since we only use their
# components
host.hide()
port.hide()

# Layout for the host label
host_label_layout = QHBoxLayout()
host_label_layout.setSpacing(0)
host_label_layout.addWidget(host.label)
host_label_layout.addWidget(host.help_label)
host_label_layout.addStretch()

# Layout for the port label
port_label_layout = QHBoxLayout()
port_label_layout.setSpacing(0)
port_label_layout.addWidget(port.plabel)
port_label_layout.addWidget(port.help_label)

# host + port layout
host_layout = QGridLayout()
host_layout.setContentsMargins(0, 0, 0, 0)

host_layout.addLayout(host_label_layout, 0, 0)
host_layout.addWidget(host.textbox, 1, 0)
host_layout.addLayout(port_label_layout, 0, 1)
host_layout.addWidget(port.spinbox, 1, 1)

configfile_layout = QVBoxLayout()
configfile_layout.setContentsMargins(0, 0, 0, 0)
configfile_layout.addWidget(name)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addLayout(host_layout)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addWidget(configfile)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addWidget(username)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addWidget(password)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addWidget(keyfile)
configfile_layout.addSpacing(5 * AppStyle.MarginSize)
configfile_layout.addWidget(passphrase)
configfile_layout.addSpacing(7 * AppStyle.MarginSize)
configfile_layout.addWidget(validation_label)
configfile_layout.addStretch()
Expand Down Expand Up @@ -655,6 +791,42 @@ def _validate_address(self, address):
address_re = re.compile(combined_pattern)
return True if address_re.match(address) else False

def _validate_config_file(self, config_filepath):
logger.info(config_filepath)
widgets = self._widgets_for_validation[AuthenticationMethod.ConfigFile]

host_textbox = widgets[1].textbox
host = host_textbox.text()
username_textbox = widgets[2].textbox
username = username_textbox.text() if username_textbox.text() else ()
keyfile_textbox = widgets[4].textbox
keyfile = keyfile_textbox.text()
config = asyncssh.config.SSHClientConfig.load(
None,
config_filepath,
True,
True,
False,
"local_user",
username,
host,
()
)
logger.info(config.get_options(False))

if not config.get_options(False):
username_textbox.setPlaceholderText("")
keyfile_textbox.setPlaceholderText("")
return False

if not username and config.get("User", None):
username_textbox.setPlaceholderText(config.get("User"))

if not keyfile and config.get("IdentityFile", None):
keyfile_textbox.setPlaceholderText(config.get("IdentityFile")[0])

return True

def _compose_failed_validation_text(self, reasons: ValidationReasons):
"""
Compose validation text from a dictionary of reasons for which it
Expand Down Expand Up @@ -685,6 +857,13 @@ def _compose_failed_validation_text(self, reasons: ValidationReasons):
+ suffix
)

if reasons.get("invalid_host"):
text += (
prefix
+ _("The host you provided is not a valid one.")
+ suffix
)

if reasons.get("invalid_url"):
text += (
prefix + _("The URL you provided is not valid.") + suffix
Expand Down Expand Up @@ -1124,9 +1303,12 @@ def remove_config_options(self):
"keyfile_login/port",
"keyfile_login/username",
"configfile_login/name",
"configfile_login/address",
"configfile_login/port",
"configfile_login/username",
"configfile",
"jupyterhub_login/name",
"keyfile",
"configfile",
"status",
"status_message",
"url",
Expand Down
6 changes: 5 additions & 1 deletion spyder/widgets/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,8 @@ def select_directory(self, edit):

def create_browsefile(self, text, option, default=NoDefault, section=None,
tip=None, filters=None, alignment=Qt.Horizontal,
status_icon=None):
status_icon=None, validate_callback=None,
validate_reason=None):
widget = self.create_lineedit(
text,
option,
Expand All @@ -793,6 +794,8 @@ def create_browsefile(self, text, option, default=NoDefault, section=None,
# vertical. If not, it'll be added below when setting the layout.
tip=tip if (tip and alignment == Qt.Vertical) else None,
status_icon=status_icon,
validate_callback=validate_callback,
validate_reason=validate_reason,
)

for edit in self.lineedits:
Expand Down Expand Up @@ -851,6 +854,7 @@ def select_file(self, edit, filters=None, **kwargs):
**kwargs)
if filename:
edit.setText(filename)
edit.setFocus()

def create_spinbox(self, prefix, suffix, option, default=NoDefault,
min_=None, max_=None, step=None, tip=None,
Expand Down
Loading