Re: [PATCH v2] credential-cache: respect request capabilities

>
> On 2025-01-06 at 19:52:11, M Hickford via GitGitGadget wrote:
> > From: M Hickford <mirth.hickford@gmail.com>
> >
> > Previously, credential-cache responded with capability[]=authtype
> > regardless of request.
>
> That's the correct behaviour.
>
> > The capabilities in a credential helper response should be a subset of
> > the capabilities in the request.
>
> No, it should not.  Otherwise, it's impossible for Git to know whether
> the helper does or does not support the capability.  We rely on that
> information to correctly pass data back when saving data.
>
> > diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
> > index bc22f5c6d24..692216cf83c 100644
> > --- a/builtin/credential-cache--daemon.c
> > +++ b/builtin/credential-cache--daemon.c
> > @@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)
> >       else if (!strcmp(action.buf, "get")) {
> >               struct credential_cache_entry *e = lookup_credential(&c);
> >               if (e) {
> > -                     e->item.capa_authtype.request_initial = 1;
> > -                     e->item.capa_authtype.request_helper = 1;
> > -
> > -                     fprintf(out, "capability[]=authtype\n");
> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {
> > +                             fprintf(out, "capability[]=authtype\n");
> > +                     }
>
> This part is not correct.

>
> >                       if (e->item.username)
> >                               fprintf(out, "username=%s\n", e->item.username);
> >                       if (e->item.password)
> >                               fprintf(out, "password=%s\n", e->item.password);
> > -                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)
> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)
> >                               fprintf(out, "authtype=%s\n", e->item.authtype);
> > -                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)
> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)
>
> This part may very well be correct.
>
> >                               fprintf(out, "credential=%s\n", e->item.credential);
> >                       if (e->item.password_expiry_utc != TIME_MAX)
> >                               fprintf(out, "password_expiry_utc=%"PRItime"\n",
> > diff --git a/t/lib-credential.sh b/t/lib-credential.sh
> > index 58b9c740605..324ecc792d5 100644
> > --- a/t/lib-credential.sh
> > +++ b/t/lib-credential.sh
> > @@ -566,6 +566,21 @@ helper_test_authtype() {
> >               EOF
> >       '
> >
> > +     test_expect_success "helper ($HELPER) get authtype only if request has authtype capability" '
> > +             check fill $HELPER <<-\EOF
> > +             protocol=https
> > +             host=git.example.com
> > +             --
> > +             protocol=https
> > +             host=git.example.com
> > +             username=askpass-username
> > +             password=askpass-password
> > +             --
> > +             askpass: Username for '\''https://git.example.com'\'':
> > +             askpass: Password for '\''https://askpass-username@git.example.com'\'':
> > +             EOF
> > +     '
> > +
> >       test_expect_success "helper ($HELPER) stores authtype and credential with username" '
> >               check approve $HELPER <<-\EOF
> >               capability[]=authtype
> >
> > base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f
> > --
> > gitgitgadget
>
> --
> brian m. carlson (they/them or he/him)
> Toronto, Ontario, CA