Re: [PATCH 03/13] introduce credentials API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 29, 2011 at 09:34:54AM -0800, Junio C Hamano wrote:

> >> > +`credential_fill`::
> >> > +
> >> > +	Attempt to fill the username and password fields of the passed
> >> > +	credential struct, first consulting storage helpers, then asking
> >> > +	the user. Guarantees that the username and password fields will
> >> > +	be filled afterwards (or die() will be called).

> Immensely, at least to me. From the perspective of a user of the API, a
> call to credential_fill() is to "fill in the credential" in the sense that
> "call the function to fill in the credential" but I find it easier to
> understand if it were explained to me as "ask the API to fill in the
> credential, which may involve helpers to interact with the user--the point
> of the API is that the caller does not care how it is done".  Same for the
> reject/accept calls---the example makes it clear that they are to tell the
> decision to reject/accept made by the application to the credential API,
> and it is up to the API layer what it does using that decision (like
> removing the cached and now stale password).

Ahh, I see your confusion now. It is not that the description is
necessarily lacking any content, but that the tense of the sentences is
misleading. I'll fix that.

> The above example is a bit too simplistic and misleading, though. You
> would call reject only on authentication failure (do not trash stored and
> good password upon network being unreachable temporarily or the server
> being overloaded).

Good point. I'll fix that and add the example to the documentation.

> > So one possible rule would be:
> >
> >   1. If it starts with "!", clip off the "!" and hand it to the shell.
> >
> >   2. Otherwise, if is_absolute_path(), hand it to the shell directly.
> >
> >   3. Otherwise, prepend "git credential-" and hand it to the shell.
> >
> > I think that is slightly less confusing than the "first word is alnum"
> > thing.
> 
> Simpler and easier to explain. Good ;-)

OK, I'll implement that, then.

> > How do you feel about the "values cannot contain a newline" requirement?
> 
> In the context of asking username, password, or passphrase, I think "LF is
> the end of the line and you cannot have that byte in your response" is
> perfectly reasonable. I've yet to find a way to use LF in a passphrase to
> unlock my Gnome keychain ;-).

The potential issue is that other values get that, too. So if you have a
URL with "\n" in the path, it cannot be transmitted verbatim. We can
url-encode, of course, but I didn't want the helpers to have to deal
with quoting issues.

> >> Two style nits.
> >
> > I'm supposed to guess? ;P
> 
> Sorry, but you guessed right.

OK, will fix.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]