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

Provide authentication error reporting to cred_acquire_cb #3761

Closed
wildart opened this issue Apr 27, 2016 · 15 comments
Closed

Provide authentication error reporting to cred_acquire_cb #3761

wildart opened this issue Apr 27, 2016 · 15 comments

Comments

@wildart
Copy link
Contributor

wildart commented Apr 27, 2016

I'm trying to implement a credential caching mechanism in git_cred_acquire_cb function. However, SSH session authentication does not report any error for libssh2 calls that return LIBSSH2_ERROR_PASSWORD_EXPIRED or LIBSSH2_ERROR_AUTHENTICATION_FAILED error codes (see _git_ssh_authenticate_session). The only exception is the ssh-agent authentication that sets error when failed. Failure to report auth errors makes impossible to identify if cached credentials where correct or not.

@wildart
Copy link
Contributor Author

wildart commented Apr 27, 2016

Same thing happens with http transport - no auth error is reported.

@ethomson
Copy link
Member

I'm not sure what you're asking for; do you want more information about the previous error in your git_cred_acquire_cb?

@wildart
Copy link
Contributor Author

wildart commented Apr 27, 2016

Yes, I want to know if provided credentials were accepted or not.

@ethomson
Copy link
Member

If you're getting called a second time, then no, they were not.

In the HTTP case you call out, there's really not much more information that we could provide, is there?

@spraints
Copy link
Contributor

spraints commented Apr 27, 2016

I've seen some cases where credentials get requested more than once. I assume it's from a Connection: close, but I haven't investigated to see if that's what's actually leading to >1 request for creds. It would be super-useful if the credentials callback got a reason argument, or something, so that it could tell if this is the first request for creds of if creds from the last request were bad. Here's some code that we're using to avoid infinite loops from credentials (using rugged):

repository = Rugged::Repository.new(".")
remote = repository.remotes.create_anonymous("https://github.com/libgit2/libgit2")
puts remote.ls(:credentials => CredHelper.new(Rugged::Credentials::UserPassword.new(:username => "spraints", :password => "lol"))).to_a

class CredHelper
  def initialize(real_creds, ttl: 5)
    @real_creds = real_creds
    @ttl = ttl
  end

  def call(*args)
    raise "auth error" if @ttl < 1
    @ttl -= 1
    @realcreds.call(*args)
  end
end

Obviously, in this example the creds won't get used because libgit2/libgit2 is public. I initially had set ttl = 1, but that led to some spurious errors. ttl=5 seems to be working well. The ttl (which is really just a backstop) could go away if call said that it was calling because of an auth error or not.

@wildart
Copy link
Contributor Author

wildart commented Apr 27, 2016

You could count number of requests for creds for a one repository but if the same credentials are used for multiple repositories this strategy does not seem viable.

@ethomson Can error be set every time 401 code is returned? Same for SSH when LIBSSH2_ERROR_AUTHENTICATION_FAILED is reported?

@wildart
Copy link
Contributor Author

wildart commented Apr 29, 2016

Found a way to identify invalid credentials without error status.

@wildart wildart closed this as completed Apr 29, 2016
@ethomson
Copy link
Member

Neat - @wildart would you mind sharing your solution? I think that there's still an opportunity to make this easier for callers but I'm curious about what you did. Thanks!

@wildart
Copy link
Contributor Author

wildart commented Apr 29, 2016

It the same solution that @spraints suggested - count number of times when the credential callback is called. I found a way in my project to reset the call counter between fetch calls. I'm still using error class to determine if git_cred_ssh_key_from_agent credentials failed, and then fall back to git_cred_ssh_key_new.

@wildart wildart changed the title No SSH auth error reporting Provide authentication error reporting to credential callback Jul 17, 2016
@wildart
Copy link
Contributor Author

wildart commented Jul 17, 2016

I reopen this issue because the solution that I used for caching credentials is a half-measure after all.

Even though counting auth. fails is working, but only for one credential provider. I would like to switch to different credential provide if one that was called before fails. An example would be a use of sh-agent and manual ssh key credentials. If ssh-agent credential fail for some reason, it would be nice to fallback back manually provided credentials rather then terminate operation.

@wildart wildart reopened this Jul 17, 2016
@wildart wildart changed the title Provide authentication error reporting to credential callback Provide authentication error reporting to cred_acquire_cb Jul 17, 2016
@Keno
Copy link

Keno commented Jul 29, 2016

Relatedly, it would also be useful to get a callback when authentication succeeded (for example to tell a credential store to cache the given credentials for future use, which you ideally don't want to do if the callback failed).

@jjlorenzo
Copy link

I came into a similar problem, I consider my solution a hack that solve basically my requirements. I want to be able to fetch from several
private repositories asking for credentials only one time if there are corrects, if not I ask again until it is correct.
I use the GitRepository object id to allows the reuse of the credentials for different objects for the same repository, but this doesn't
cover the same object calling the fetch several times which it is probably expected. Probably I will expand this hack to cover this case later. I think that after the GitRepository.fetch instead of returning immediately, I can mark the credentials as valid for the url(without the r_oid) and cover the case.

class GitRemoteCallbacks(pygit2.RemoteCallbacks):
  """
  A hack for reusing valid credentials and for asking again if it is invalid.
  """

  def __init__(self):
    super(GitRemoteCallbacks, self).__init__()
    self._cred = None
    self._last = None

  def for_repo(self, r_oid):
    self._r_oid = r_oid
    return self

  def credentials(self, url, username_from_url, allowed_types):
    current = '{oid}-{url}'.format(oid=self._r_oid, url=url)
    if not self._last or self._last == current:
      username = click.prompt("Username for '{url}'".format(url=url), type=str)
      password = click.prompt("Password for '{url}'".format(url=url), hide_input=True, type=str)
      self._cred = pygit2.UserPass(username, password)
    self._last = current
    return self._cred


# a singleton
gitRemoteCallbacks = GitRemoteCallbacks()


class GitRepository(object):

  def __init__(self, path):
    self.repository = pygit2.Repository(path)
    # we store a reference for the singleton  here
    self.callbacks = gitRemoteCallbacks
    ....

  def fetch(self, credentials=None, remote='origin'):
    # with `self.callbacks.for_repo(id(self))` I prepare the singleton to identify 
    # furthers calls to the `credentials` callback if this is the case I ask for new credentials, if not, I reuse.
    return self.repository.remotes[remote].fetch(
      callbacks=self.callbacks.for_repo(id(self))
    )

@ed11e
Copy link

ed11e commented Jan 9, 2017

I have the same issue.

@jjlorenzo
Copy link

In the end I add support for ssh keys that way not face the problem of enter bad password cause a typo, but the solution I posted worked for my requirements although is quite hacky.

@ethomson
Copy link
Member

We've significantly refactored the network code and w/r/t authentication; you should now only get a second request for credentials if the first ones were actually refused. (Credentials persist through redirects and other replays.) Closing as obsolete.

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

6 participants