Re: [PATCH v2] Add a credential-helper for KDE

From: Jeff King <peff@peff.net>
Date: Sep 18, 2011, 18:49 UTC
Message-ID: <20110918184954.GB31176@sigill.intra.peff.net>
In-Reply-To: <4E7605CA.7020204@gmail.com>

On Sun, Sep 18, 2011 at 04:52:58PM +0200, Lukas Sandström wrote:

> Right. Multiple usernames per "unique" context is supported in this version.
> I looked at the git-credential-storage helper when I wrote the first patch,
> which didn't have obvious support for multiple usernames per unique context.

Yeah, sorry about that. The -cache helper is much more fully fleshed out (though I have improved the -store helper in the past few days to handle multiple usernames better).

> Keeping the username outside the token is probably a good thing, but perhaps it
> should be clarified in the api-docs that multiple usernames has to be supported.

OK, I'll try to write up a clarification.

> Also; what about rejecting credentials. This code currently deletes just a 
> username/password pair if a username is specified, and all credentials associated
> with the token if only --unique and --reject is specified. Is this correct/expected
> behavior?

Yes, that's what I think should happen, and what both of my helpers do. In practice, I don't think it will be called that way by git, which will always be rejecting a username we just tried. But I wanted to leave things flexible in case a user wants to manually remove a credential from a store.

Show 5 quoted lines

> When I first wrote the helper I tried to immediately ask for a new password if a
> credential was rejected, but this didn't work with the HTTP auth code, since it
> doesn't retry the auth with the new credentials after a reject. I think it would
> be better if we asked for a new password instead of just saying "auth failed" and 
> having the user retry the fetch/pull when the stored credentials are incorrect.

Yeah, I had a patch early on to retry authentication a few times before exiting, but I wondered how helpful it was. It's usually pretty easy to retry your command again via shell history, and sometimes looping on asking for authentication can be annoying (because things like askpass will actually grab the keyboard focus).

So I dunno what is best. I don't consider it a big deal, but maybe others do.

Even if we did do the retry from git, the helper shouldn't ask immediately for the new credential inside a --reject. It should wait to be invoked again asking for the password. I know this may be an extra fork/exec/startup cycle, but it keeps the interface to the helper simple and flexible.

-Peff