-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added fix for auth_fail #583
Conversation
@eed3si9n whenever you have time, I'll be glad to know what do you think about it. If it seems okay to you, I'll ask privately to a bunch of people to test it. |
Generally I support the idea of moving away from JSch. Last two updates were from 2016 and 2018, and the project has been unresponsive to bug reports like https://sourceforge.net/p/jsch/bugs/129/ "JSchException: invalid privatekey", so it totally makes sense for JGit to migrate to Apache MINA implementation instead. |
Okay, fantastic. I'll begin asking some people to check it out to ensure that it works with most configurations. I'll keep you posted. |
@eed3si9n I had the chance to make just a couple of people test the issue, I haven't had so much traction. What do you suggest? Can we merge it o it's worth waiting? |
Sorry I just noticed the mention here. I'm happy to move forward if you think it's ready. |
I have to say that I'm using it for a lot of time and had no issue. Truth to be told, not a lot of persons had the chance to test it. I don't sincerely know which is the right thing to do. |
I think I found a working fix for the famous
auth fail
problem that occurs with private repositories or in the case of custom configurations like when you have a~/.ssh/config
I've tried to keep the philosophy introduced by @steinybot with this commit and the following
The main idea is that PROBABLY the
JschConfigSessionFactory
that was overridden before cannot read custom ssh configurations, identity files or keys, while the default implementationSshdSessionFactory
"mina-sshd" by apache seems to be able to.I spotted the working implementation simply commenting this line and printing
x
one the next one. Since not usingSshAgentSessionFactory
worked for me and my custom configuration I thought it could have been a good starting point.To keep the "custom known_host" configurability option I used the input
knownHosts: Option[String]
to overridegetDefaultKnownHostsFiles
.I'll keep this PR as a draft until a consistent number of people might be able to test it.
In order to test it, the easiest way is to checkout this PR and then
If it works it should:
and probably #176 and #548 too