Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

AuthMethod in http/common.go needs to export setAuth #350

Closed
jdoklovic opened this issue Apr 19, 2017 · 3 comments
Closed

AuthMethod in http/common.go needs to export setAuth #350

jdoklovic opened this issue Apr 19, 2017 · 3 comments
Labels

Comments

@jdoklovic
Copy link

right now the setAuth is not exported. It should be SetAuth. Otherwise, there's no way for a consumer to apply basic auth (e.g. ask for user/pass) when it's needed and instead they need to do it when the Auth is set on the Config which means you may ask for a user/pass before you know if an endpoint actually reuires auth.

Making the SetAuth exported is a quick fix so consumers can just implement the interface instead of using the built-in BasicAuth.

A better route would probably be to add a callback for grabbing the user/pass.

@smola
Copy link
Collaborator

smola commented May 5, 2017

@jdoklovic Thanks for reporting this. I would be inclined to implement a new AuthMethod with a callback.

@jdoklovic
Copy link
Author

@smola I believe that's what I essentially did in my PR.
I had to make it par of the basic auth since at the end of the day it has to set the creds on the http request. I'm open to suggestions on how to implement it better though. I really just need to get something working and would rather not fork this lib just for this

@smola
Copy link
Collaborator

smola commented May 9, 2017

@jdoklovic I discussed this with the team and the preferred method for implementing this is retrying if authentication is required. Clone and other operations return transport.ErrAuthenticationRequired if authentication is required for the given repository. If such error is returned, you can set credentials with whatever logic you want and then retry.

Something like this:

opts := opts.CloneOpts{}
err := git.Clone(opts)
if err == transport.ErrAuthRequired {
    user, pass := ask for password
    opts.Auth := NewBasicAuth(user,pass)
    git.Clone(opts)
}

I'm closing this, since this behavior is already supported with this mechanism.

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

No branches or pull requests

2 participants