Skip to content

Commit

Permalink
Fix .ssh/config file not being closed (#513)
Browse files Browse the repository at this point in the history
* Fix .ssh/config file not being closed

The existing parsing logic was very brittle
and did not take into account a possible variantions
(e.g. capitalization, extra white space etc.)
So I switched to a proper parser from aiida-core,
that we already use in this file anyway.

I also discovered other possible bugs in the code,
left them as TODO for future PRs.

* review

* Add links to Github issues
  • Loading branch information
danielhollas committed Sep 25, 2023
1 parent 2c29a9e commit 6bcad02
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
18 changes: 15 additions & 3 deletions aiidalab_widgets_base/computational_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,12 +421,17 @@ def _can_login(self):
return ret == 0

def _is_in_config(self):
"""Check if the config file contains host information."""
"""Check if the SSH config file contains host information."""
fpath = Path.home() / ".ssh" / "config"
if not fpath.exists():
return False
cfglines = open(fpath).read().split("\n")
return "Host " + self.hostname.value in cfglines
sshcfg = parse_sshconfig(self.hostname.value)
# NOTE: parse_sshconfig returns a dict with a hostname
# even if it is not in the config file.
# We require at least the user to be specified.
if "user" not in sshcfg:
return False
return True

def _write_ssh_config(self, private_key_abs_fname=None):
"""Put host information into the config file."""
Expand Down Expand Up @@ -477,8 +482,15 @@ def _on_setup_ssh_button_pressed(self, _=None):

# Write private key in ~/.ssh/ and use the name of upload file,
# if exist, generate random string and append to filename then override current name.
# TODO(danielhollas): I don't think this works as intended. If there is an existing private key,
# the new filename is never propagated to the caller here.
# https://github.com/aiidalab/aiidalab-widgets-base/issues/516
self._add_private_key(private_key_abs_fname, private_key_content)

# TODO(danielhollas): I am not sure this is correct. What if the user wants
# to overwrite the private key? Or any other config? The configuration would never be written.
# And the user is not notified that we did not write anything.
# https://github.com/aiidalab/aiidalab-widgets-base/issues/516
if not self._is_in_config():
self._write_ssh_config(private_key_abs_fname=private_key_abs_fname)

Expand Down
3 changes: 2 additions & 1 deletion tests/test_computational_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ def test_ssh_computer_setup_widget():
widget.username.value = "aiida"

# Write the information to ~/.ssh/config and check that it is there.
assert widget._is_in_config() is False
widget._write_ssh_config()
assert widget._is_in_config()
assert widget._is_in_config() is True

# Check that ssh-keygen operation is successful.
widget._ssh_keygen()
Expand Down

0 comments on commit 6bcad02

Please sign in to comment.