Skip to content

Only use username from ssh config as default#470

Open
hexahurz wants to merge 1 commit into
MarketSquare:masterfrom
hexahurz:overwrite_username
Open

Only use username from ssh config as default#470
hexahurz wants to merge 1 commit into
MarketSquare:masterfrom
hexahurz:overwrite_username

Conversation

@hexahurz

Copy link
Copy Markdown

If a username is specified in the Login keyword, it should supersede the username from the ssh config.

@Noordsestern

Copy link
Copy Markdown
Member

Hi,

thank you for your contribution. Could you provide test cases for the fix, please?

@Noordsestern Noordsestern added this to the 4.1 milestone Mar 7, 2025
If a username is specified in the `Login` keyword, it should supersede
the username from the ssh config.
@hexahurz hexahurz force-pushed the overwrite_username branch from 38c2aaf to 8c455db Compare March 11, 2025 11:47
@hexahurz

Copy link
Copy Markdown
Author

Rebased and added tests.

@hexahurz

hexahurz commented Jun 5, 2025

Copy link
Copy Markdown
Author

Hi @Noordsestern,

Any feedback, comments, or thoughts? Or am I done and you will pick this up for the next release?

Just making sure you're not waiting for anything from me...

@Noordsestern Noordsestern added needs review acknowledge To be acknowledged in release notes labels Jul 8, 2026
@Noordsestern Noordsestern requested a review from Copilot July 8, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates SSHLibrary’s ssh-config username handling so that a username explicitly provided to Login is not overridden by ~/.ssh/config, while keeping ssh-config as a default source when no username is supplied.

Changes:

  • Adjust password-based _login to only apply the ssh-config username when the caller did not provide a username.
  • Add an acceptance test covering “explicit username overrides ssh-config user”.
  • Extend acceptance-test docs/resources to include a new test_user_hostname ssh-config host alias.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/SSHLibrary/client.py Changes ssh-config username application logic for password-based login to preserve explicitly provided usernames.
atest/resources/common.robot Adds ${TEST_USER_HOSTNAME} variable for the new acceptance test.
atest/README.rst Documents adding test_user_hostname to ~/.ssh/config for running acceptance tests.
atest/login.robot Adds a new acceptance test asserting explicit username supersedes ssh-config user.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/SSHLibrary/client.py
Comment on lines +887 to +890
self.config.host, username_from_config, self.config.port, proxy_cmd = \
self._read_login_ssh_config(hostname, username, self.config.port, proxy_cmd)
if not username or username == b'None':
username = username_from_config
Comment thread src/SSHLibrary/client.py
Comment on lines +889 to +890
if not username or username == b'None':
username = username_from_config
Comment thread atest/login.robot
Comment on lines +73 to +75
Login With Specified User Differing From Config File
[Setup] Open Connection ${TEST_USER_HOSTNAME} prompt=${PROMPT}
Login ${USERNAME} ${PASSWORD} read_config=True
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge To be acknowledged in release notes bug needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants