Re: [PATCH 05/13] credential: gate new fields on capability

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

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

On 2024-04-02 at 10:04:31, Patrick Steinhardt wrote:
> The intent of this is quite clear to me, but thanks for re-explaining
> the bigger picture :)

Sorry I misunderstood what you were getting it.

> I think you misunderstood my confusion. I didn't meant to say that there
> should be non-boolean capabilities. I was rather missing the picture of
> how exactly you can advertise multiple capabilities with the infra that
> currently exists, and why the infra supports per-phase capabilities.
> 
> Basically, what I would have expected is a protocol where both Git and
> the credential helper initially did a single "handshake" that also
> announces capabilities. So something like:
> 
>     HELPER: capability foobar
>     HELPER: capability barfoo
>        GIT: capability foobar
> 
> Git would only acknowledge capabilities that it both understands and
> that have been announced by the helper. So at the end of this both have
> agreed on a single capability "foobar".
> 
> This is roughly how the remote helper capability subsystem works. What
> this patch is introducing seems quite a bit more complicated than that
> though because we have "staged" capabilities. I assume there is good
> reason for this complexity, but I didn't yet manage to figure out the
> reasoning behind it.
> 
> To ask more specifically: why would one side ever announce a capability
> in phase 1, but not in phase 2? Is the reason that capabilities are in
> fact tied to credentials?

More that they're tied to the credential helper.  For example, say we
have helpers A and B, in that order.  B is incapable, but both A and Git
understand the new authtype capability.  If we announce the capability
as in this series, we can get a new credential using the authtype
capability if A is willing to provide something to us, but we can't if A
has no credentials for us and B wants to provide them for us.

This would also be true if we used your proposal of negotiation, but
because we have external callers (e.g., Git LFS) who may invoke `git
credential fill`, which would be a separate process from `git credential
capability`, we'd still have to have some way to tell `git credential
fill` what capabilities the external caller supported.

The per-phase capabilities are such that we don't request functionality
that our callers can't use.  For example, if our external caller (phase
1) doesn't support the `authtype` credential, then we don't pass it to
the helper (phase 2), since the external caller might not be able to use
the result if we do.  If the external caller (phase 1) _does_ support
it, but the helper does not (phase 2), then we won't return the
capability as the result of `git credential fill` (phase 3), so our
external caller will know that this isn't supported.  As a practical
matter, that doesn't provide a great deal of useful information to the
caller at the moment, but it definitely could in the future (say, if we
had a capability for a certain form of data encoding).

All of this is also true for internal (e.g., git-http-backend) callers,
except that phase 1 has all the capabilities we know about automatically
set, and phase 3 stores the data in the internal structure we'll use for
the `store` and `erase` calls.

I do, however, think some way to query capabilities more generically
would be helpful, so I'll see if I can add such changes into a v2.  I
think we still need the current approach to make the use case I
mentioned work, though.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

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