Re: [PATCH] docs: clarify that credential discards unrecognised attributes

From: Jeff King <peff@peff.net>
Date: Nov 14, 2022, 22:40 UTC
Message-ID: <Y3LD1Ilq8kLPlJMO@coredump.intra.peff.net>
In-Reply-To: <CAGJzqsnd3UbCuj6xfsAgVEFz5OaDBhnCTNeAvOVPK9NWBDnDRA@mail.gmail.com>

On Sat, Nov 12, 2022 at 07:08:42PM +0000, M Hickford wrote:

Show 22 quoted lines

> On Sat, 12 Nov 2022 at 16:47, Jeff King <peff@peff.net> wrote:
> > > > We did discuss patches a long time ago that would let Git carry
> > > > arbitrary keys between helpers, even if Git itself didn't understand it.
> > > > One of the intended uses was to let helpers talk to each other about
> > > > TTLs. So if you had say:
> > > >
> > > >   [credential]
> > > >   helper = generate-some-token
> > > >   helper = cache
> > > >
> > > > where the first helper generates a token, and the second caches it, the
> > > > first one could shove a "ttl" or "expiration" key into the protocol,
> > > > which the cache could then learn to respect.
> > >
> >
> > What you're doing works fine with the code as-is; you just can't carry
> > extra data (like a ttl) between the two.
> 
> FWIW I have a draft patch that adds password_expiry_utc and
> oauth_refresh_token attributes to credential
> https://github.com/gitgitgadget/git/pull/1394 introducing expiry logic
> in the credential layer. I'll share a RFC sometime in future.

Neat.

I'm not _totally_ opposed to introducing these as something Git understands, but I think it makes more sense to just teach Git to relay unknown entries between helpers.

The oauth thing is going to be very helper specific, and not something I think Git would ever do anything with itself.

In theory Git might care about expiration, but in practice I think it doesn't. It's very unlikely for a token to expire in the course of Git using it. It's only much later, when we ask for it back, that a helper will notice it's expired. Git could save the helper some work by noticing this on read, but since the helper has to learn to store and report the expiration in the first place, not much is gained.

And in the case of something like credential-cache, we want to do more than just store; we'd actually drop the credential entirely (and maybe even cause the daemon to exit) if it expires before the usual timeout.

-Peff