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

[Patrick Steinhardt <ps@pks.im>]

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

On Thu, Apr 04, 2024 at 12:39:58AM +0000, brian m. carlson wrote:
> 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.

Great. Thanks a lot for your explanations!

Patrick

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