Re: [PATCH] credential: do not store credentials received from helpers

Jeff King [off-list ref] writes:

On Fri, Apr 06, 2012 at 09:56:18PM -0700, Junio C Hamano wrote:

quoted

I am afraid that "do not trigger the cache helper" might be throwing the
baby with bathwater to solve the real problem the patch tries to address,
which is:

Peff>   2. If you use a time-based storage helper like
Peff>      "git-credential-cache", every time you run a git
Peff>      command which uses the credential, it will also
Peff>      re-insert the credential after use, freshening the
Peff>      cache timestamp. So the cache will eventually expire N
Peff>      time units after the last _use_, not after the time the
Peff>      user actually typed the password. This is probably not

Actually, that was not the real problem. The real problem I had was the
leakage between helpers. I just noticed this one while thinking about
it. So the very thing that is useful to Shawn is also potentially
dangerous to people who are doing something less clever.

quoted

Shouldn't the memory cache based helper already have enough clue to tell
when a new entry is first inserted vs when the existing entry it supplied
came back from the network layer after use?  If there is not enough clue
with the current network-layer-to-helper protocol, then wouldn't it be a
better approach to add that, so that the memory cache helper can make more
intelligent management of its timer?

You can approximate it. The daemon sees that somebody is inserting the
same thing that it already there, and can guess that it probably came
from the daemon in the first place. There are some corner cases with
expiration boundaries (we get the credential, the daemon expires it,
then the http request succeeds, and we tell the daemon "hey, store
this").

quoted

Once that is fixed, I would imagine that you can tell your users to use 
two helpers (yours and generic caching one) and configure them so that (1)
the caching one is asked first and then fall back to ask yours, and (2)
the expiration time of the caching one is set close to $X.

Yeah, in my other response to Shawn, I mentioned that we could add a
flag to do the "leaking" behavior if that's what the user wants. But it
would have the side effect of refreshing his timestamp on each use, so
his $X would not expire (although that is also the case now, and he
hasn't complained).

So I actually do think he would be better to implement the caching
inside his helper, even if it is by calling out to git-credential-cache.

While I'm for keeping the interface simple, at the same time, "I have this
credential obtained and it is valid for $X time duration" sounds like a
very common thing, and it is somewhat a shame for the API to force its
users (i.e. helpers) to reimplement the caching logic over and over.

The network layer (i.e. the one that gets the authentication material from
helpers and uses it to talk with a remote git service on the other end)
could be thought of as a proxy server that gets information from its true
source (i.e. helpers) and relays it to the browser (i.e. the remote git
service). In that context, it would be natural for the API to allow the
source of the truth (i.e. the helpers) to pass its information to the
proxy with validity duration, so that the proxy can handle the expiration
in a way hidden from the information sources and the browser.  That proxy
(i.e. the network layer) could use git-credential-cache as a non-volatile
memory but that will become an implementation detail hidden from both the
remote service and the helpers.

So,... I dunno.