Re: [PATCH 08/13] credential: add an argument to keep state

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 3500 bytes --]

On 2024-04-01 at 21:05:28, mirth hickford wrote:
> On Sun, Mar 24, 2024 at 1:13 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > Until now, our credential code has mostly deal with usernames and
> > passwords and we've let libcurl deal with the variant of authentication
> > to be used.  However, now that we have the credential value, the
> > credential helper can take control of the authentication, so the value
> > provided might be something that's generated, such as a Digest hash
> > value.
> >
> > In such a case, it would be helpful for a credential helper that gets an
> > erase or store command to be able to keep track of an identifier for the
> > original secret that went into the computation.  Furthermore, some types
> > of authentication, such as NTLM and Kerberos, actually need two round
> > trips to authenticate, which will require that the credential helper
> > keep some state.
> >
> > In order to allow for these use cases and others, allow storing state in
> > a field called "state[]".  This value is passed back to the credential
> > helper that created it, which avoids confusion caused by parsing values
> > from different helpers.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  Documentation/git-credential.txt | 29 ++++++++++++++++++-----------
> >  credential.c                     | 20 +++++++++++++++++---
> >  credential.h                     |  7 +++++++
> >  t/t0300-credentials.sh           | 29 +++++++++++++++++++++++++++++
> >  4 files changed, 71 insertions(+), 14 deletions(-)
> >
> > diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
> > index f3ed3a82fa..ef30c89c00 100644
> > --- a/Documentation/git-credential.txt
> > +++ b/Documentation/git-credential.txt
> > @@ -196,6 +196,15 @@ provided on input.
> >  This value should not be sent unless the appropriate capability (see below) is
> >  provided on input.
> >
> > +`state[]`::
> > +       This value provides an opaque state that will be passed back to this helper
> > +       if it is called again.  Each different credential helper may specify this
> > +       once.  The value should include a prefix unique to the credential helper and
> > +       should ignore values that don't match its prefix.
> 
> Does Git ever populate state[] in 'store' or 'erase' requests,  or
> only 'get' requests? It might be worthwhile to spell this out.

Yes, it's populated with whatever the last state value was from `get`.

> This seems somewhat different to other multi-valued attributes,
> particularly the "set at most one value" constraint. As an
> alternative, how about a single-valued attribute stored independently
> for each helper (vector length equal to the number of configured
> helpers)? Then in repeat requests send the "nth state to the nth
> helper". This would avoid the complexity of the prefix mechanism.

I originally tried that approach, but if you have external callers of
`git credential` (like Git LFS), that doesn't work, since you need to
make two separate calls: one (with `get`) to fetch the credentials that
returns multiple state values, and one (with `store` or `erase`) that
sends the data back to accept or reject the credentials.  Since there's
no internal state in Git between the two calls, it's not possible to
only send certain data to certain helpers.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]