Skip to content
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

Merged
merged 3 commits into from
Aug 29, 2021
Merged

Added fix for auth_fail #583

merged 3 commits into from
Aug 29, 2021

Conversation

TonioGela
Copy link
Member

@TonioGela TonioGela commented Jun 10, 2021

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 implementation SshdSessionFactory "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 using SshAgentSessionFactory 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 override getDefaultKnownHostsFiles.

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

sbt publishLocal
cs bootstrap org.foundweekends.giter8:giter8-launcher_2.12:0.13.1-SNAPSHOT --main giter8.LauncherMain -f -o ~/Desktop/g8-test
./g8-test <ssh:// or git@ URL of a repo>

If it works it should:

and probably #176 and #548 too

@TonioGela
Copy link
Member Author

TonioGela commented Jun 11, 2021

@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.

@eed3si9n
Copy link
Member

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.

@TonioGela
Copy link
Member Author

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.
P.s. ofc with my company's GitLab instance works.
Thanks Eugene

@TonioGela TonioGela removed the request for review from xuwei-k June 15, 2021 09:56
@TonioGela
Copy link
Member Author

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.
P.s. ofc with my company's GitLab instance works.
Thanks Eugene

@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?

@eed3si9n
Copy link
Member

Sorry I just noticed the mention here. I'm happy to move forward if you think it's ready.

@TonioGela
Copy link
Member Author

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 personally think that it's ready, but that is my opinion, after my personal tests. Probably it's worth merging. Eventual bugs can be addressed later if any. What do you think?

@eed3si9n eed3si9n marked this pull request as ready for review August 29, 2021 20:14
@eed3si9n eed3si9n merged commit 095e960 into foundweekends:develop Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants