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

Authentication not working #81

Closed
OlivierLemoine opened this issue Feb 24, 2022 · 14 comments
Closed

Authentication not working #81

OlivierLemoine opened this issue Feb 24, 2022 · 14 comments

Comments

@OlivierLemoine
Copy link

Expected Behavior

In a private remote Github repo, fetch / push the changes.

Actual Behavior

With a fetch / push, jj says:

  • ssh: Error: Unexpected git error when fetching: authentication required but no callback set; class=Ssh (23); code=Auth (-16)
  • https : Error: Unexpected git error when fetching: remote authentication required but no callback set; class=Http (34); code=Auth (-16)

Steps to Reproduce the Problem

  1. Clone a private repo from Github
  2. Try jj git fetch

Specifications

  • Version: jj 0.2.0
  • Platform: macOs Montery x86-64,
@ThomasSowders
Copy link

ThomasSowders commented Feb 24, 2022

This same error is also affecting me on Windows 10 with jj 0.2.0 and Git for Windows 2.32.0.windows.2

@martinvonz
Copy link
Owner

What kind of authentication do you use with your remotes? So far, the only supported types of authentication are by ssh-agent and by password-less SSH key in ~/.ssh/id_rsa. See [these hacks] (

fn create_remote_callbacks() -> RemoteCallbacks<'static> {
). This may also be a duplicate of #63.

@OlivierLemoine
Copy link
Author

I see.
Here :

let key_path = std::path::Path::new(&home_dir).join(".ssh").join("id_rsa");

jj is only looking for a key named id_rsa, which was not the case for me.
Thank you !

@tp-woven
Copy link
Collaborator

I was just "bitten" by this, too - GitHub's documentation no longer recommends RSA, and now directs users to use ed25519 instead. Given that, would it make sense to add support for other key locations / types?

@martinvonz
Copy link
Owner

Given that, would it make sense to add support for other key locations / types?

Definitely! #63 is probably the right way to do that, right? I know very little about ~/.ssh/config (I don't have one myself), but it seems like the key file can be configured there. I'd appreciate any help I can get on fixing that issue.

@tp-woven
Copy link
Collaborator

TBH, I don't actually know... I'm not sure what the connection between ~/.ssh/config and git is, or how it knows where to find keys. If I had to guess, I'd say maybe this should use ssh-agent (via git2::Cred::ssh_key_from_agent)?

@martinvonz
Copy link
Owner

ssh-agent is actually already supported, so if using ssh-agent is an option for you, I'd really recommend that (with jj and other tools). Let me know if you have ssh-agent set up and it's not working, of course.

I'm also happy to take a patch adding support for just the kind of key file and path you're using.

@tp-woven
Copy link
Collaborator

Hmm, then I'm not sure what the issue is - I had the same issue as the original reporter, even though I do have ssh-agent set up (and ssh-add -L does show the key used by our private GH repo)... Do I need to configure jj to use it somehow? 🤔

I'm also happy to take a patch adding support for just the kind of key file and path you're using.

Thanks! I'll try to find some time this/next week and add this.

@martinvonz
Copy link
Owner

I'm not sure what the connection between ~/.ssh/config and git is

I don't know much about how this works either :) I think ~/.ssh/config is specifically for OpenSSH, but jj uses libssh2 via libgit2 (as noted in #63 (comment)). So attempting to respecting ~/.ssh/config might actually be confusing because it would probably be very partial support for as long as libgit2 uses libssh2 at least. So I'm not sure what the best solution is :(

Do I need to configure jj to use it somehow?

You shouldn't have to configure anything specifically for jj. The code is here. Do you have $SSH_AUTH_SOCK and $SSH_AGENT_PID exported in your environment? I would think you do because that's how any other tools would be able to find ssh-agent, AFAIK.

I'll try to find some time this/next week and add this.

Thanks!

@tp-woven
Copy link
Collaborator

Do you have $SSH_AUTH_SOCK and $SSH_AGENT_PID exported in your environment?

Interesting. I only have $SSH_AUTH_SOCK set, but not $SSH_AGENT_PID. When running SSH_AGENT_PID=(pgrep ssh-agent) jj git clone ... the operation was successful, so it looks like that is the issue. Other tools do work without the PID, though, so I wonder if changing the test to in git.rs:439 to check for either environment variable would work? Or maybe just trying to call ssh_key_from_agent and falling back to the hard-coded file if it fails?

@martinvonz
Copy link
Owner

I wonder if changing the test to in git.rs:439 to check for either environment variable would work?

After some quick googling, it seems that $SSH_AUTH_SOCK is the important one (as I said, I don't know much about this stuff :)). Want to send a patch replacing the current check for $SSH_AGENT_PID by a check for that (whenever you have time)?

Or maybe just trying to call ssh_key_from_agent and falling back to the hard-coded file if it fails?

That might work and seems best if it does work. My concern is that the documentation for the function says "Create a new ssh key credential object used for querying an ssh-agent.", which made me wonder if it has already checked if it can connect to ssh-agent when it's called or not. Again, if the return value seems reliable, then please do use that method.

@martinvonz
Copy link
Owner

which made me wonder if it has already checked if it can connect to ssh-agent when it's called or not.

FYI, I did some quick tests and it seems that it returns Ok even if I unset $SSH_AUTH_SOCK (and $SSH_AGENT_PID) in my shell.

@tp-woven
Copy link
Collaborator

Hmm, yeah, I think this is the libgit2 code that does this, and it doesn't actually try to connect to the agent...

OK, so for now I'll just send a PR that changes the PID check to check for the socket instead?

@martinvonz
Copy link
Owner

OK, so for now I'll just send a PR that changes the PID check to check for the socket instead?

Please do. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants