vfs/sftpfs: Fix keyboard-interactive auth

[tuffnatty]

This is a replacement for #5160.

Proposed changes

• Don't mix password auth and keyboard-interactive auth.

• Show all prompts verbatim to user.

• Don't save responses, as we can't differentiate between permanent passwords and one-time codes.

• Resolves: ftp /sftp does not work #4585.

Checklist

• I have referenced the issue(s) resolved by this PR (if any)
• I have signed-off my contribution with git commit --amend -s
• Lint and unit tests pass locally with my changes (make indent && make check)
• I have added tests that prove my fix is effective or that my feature works
• I have added the necessary documentation (if appropriate)

[ossilator]

cool!

to reflect the true scope of the patch, it'd suggest rewriting the commit message along the lines of

Ticket #5162: Rewrite keyboard-interactive auth in vfs/sftpfs

The existing implementation imposed the following limitations:
- ...

Therefore, make the process server-driven instead. ...

Also, would it be possible to pull ahead the removal of response caching, so the actual rewrite commit becomes smaller?

[tuffnatty]

to reflect the true scope of the patch, it'd suggest rewriting the commit message along the lines of

Ticket #5162: Rewrite keyboard-interactive auth in vfs/sftpfs

The existing implementation imposed the following limitations:
- ...

Therefore, make the process server-driven instead. ...

You seem to know well how the commit message should look, could you please contribute your version in full? Thanks.

Also, would it be possible to pull ahead the removal of response caching, so the actual rewrite commit becomes smaller?

I am not sure what you're talking about.

[ossilator]

You seem to know well how the commit message should look, could you please contribute your version in full? Thanks.

i just gave you the skeleton. as the author, you are (or at least should be) best suited to actually fill it in.

note that i'm not making any outlandish demands here. this is accepted best practice by any project that takes "commit hygiene" seriously. see for example git's own policy. it's a worthwhile investment to learn how to do it properly.

Also, would it be possible to pull ahead the removal of response caching,

I am not sure what you're talking about.

the point "Don't save responses, as we can't differentiate between permanent passwords and one-time codes" sounds like a code removal. this is usually a good "prelude" to refactoring/rewriting code, so it makes sense to try to prepend a separate commit that does only that.

[tuffnatty]

note that i'm not making any outlandish demands here. this is accepted best practice by any project that takes "commit hygiene" seriously. see for example git's own policy. it's a worthwhile investment to learn how to do it properly.

Could you point me to a well-documented commit of yours in this repo?

I did the commit messages the way you describe long ago, before Git, when I had to also maintain a detailed ChangeLog. I don't have time for this at the moment. I also don't believe these are outlandish demands, but I believe you're asking too much from me. I have already contributed my time to fix the bugs for the benefit of all MC users. I don't desperately need the project to merge my code - the project and its users need it. I am not sure how much they need the commit messages, though.

the point "Don't save responses, as we can't differentiate between permanent passwords and one-time codes" sounds like a code removal. this is usually a good "prelude" to refactoring/rewriting code, so it makes sense to try to prepend a separate commit that does only that.

Nope. I'm separating the kbd-interactive auth support from password auth support, and keep permanent password saving in the password auth part, but don't duplicate it in the kbd-interactive part. I think it's evident from the patch.

[ossilator]

I'm separating the kbd-interactive auth support from password auth support, and keep permanent password saving in the password auth part, but don't duplicate it in the kbd-interactive part.

right. it would make sense to emphasize this in the commit message, something like "unlike the pre-existing password-auth path, the new kbd-interactive path doesn't ... because ...". alternatively, a proper code comment could highlight the difference.

[zyv]

I tried to have a look at this while I'm waiting on Andrew's feedback on the error handling. I hope this makes sense...

[tuffnatty]

@zyv thanks! I've updated the code with your suggestions.