fix #10909 unnecessary ssh password/passphrase prompts#141
fix #10909 unnecessary ssh password/passphrase prompts#141m-wrzr wants to merge 4 commits intotreeverse:mainfrom
Conversation
|
|
|
I can reproduce the issue and would highly appreciate if this fix would be merged. |
| if port := config.get("port"): | ||
| login_info["port"] = port | ||
|
|
||
| # in case preferred_auth is empty, fallback is publickey |
There was a problem hiding this comment.
can we double check:
it probably means "do not constrain the method list", not fallback
There was a problem hiding this comment.
you're right, I've tried to clarify the comment.
| ask_password=True, | ||
| ) | ||
|
|
||
| mock_getpass = mocker.patch( |
There was a problem hiding this comment.
should it be patched before we construct the file system above?
There was a problem hiding this comment.
patching here is fine, since the f.fs call is initialising/making the actual connection
|
|
||
|
|
||
| def test_password_set(ssh_server, mocker): | ||
| f = SSHFileSystem( |
There was a problem hiding this comment.
it's checking that we're not running into a password or ssh key password prompt, which would timeout the test. I've added a comment that makes this a bit clearer
this test fails with the current dvc-ssh version
| login_info[option] = config.get(option) | ||
|
|
||
| if login_info[option] or config.get(f"ask_{option}"): | ||
| login_info["preferred_auth"].append( |
There was a problem hiding this comment.
how about keyboard-interactive method - let's check if we also need to pass it? and apply a fix like I was trying to do here https://github.com/treeverse/dvc-ssh/pull/143/changes
There was a problem hiding this comment.
I'm not to familiar with keyboard-interactive auth tbh, it's already part of this PR? #144
|
|
||
| if login_info[option] or config.get(f"ask_{option}"): | ||
| login_info["preferred_auth"].append( | ||
| { |
There was a problem hiding this comment.
suggestion / minor: can we please refactor if a bit: move this dict as a var outside with some meaningful name, so otherwise it is written in way that it seems we append both each time tbh
4 participants
Hey, this PR addresses this issue in the main
dvcrepository treeverse/dvc#10909The main issue was, that no matter what ssh configuration you set, the passphrase for the key is always prompted, even if you use
ask_passwordand password based auth.This is due to the
InteractiveSSHClientstill trying to use thepublickeyauth if we don't set thepreferred_auth, i.e. we will always callpublic_key_auth_requested.I've tried to replicate this behavior by expanding the docker based ssh tests, please let me know if you need some additional details/tests/etc