Re: [PATCH 0/8] [RFC] Enhance credential helper protocol to include auth headers

[Derrick Stolee <derrickstolee@github.com>]

On 9/13/2022 3:25 PM, Matthew John Cheetham via GitGitGadget wrote:
> Hello! I have an RFC to update the existing credential helper design in
> order to allow for some new scenarios, and future evolution of auth methods
> that Git hosts may wish to provide. I outline the background, summary of
> changes and some challenges below. I also attach a series of patches to
> illustrate the design proposal.

It's unfortunate that we didn't get to talk about this during the
contributor summit, but it is super-technical and worth looking closely
at all the details. 

> One missing element from the patches are extensive tests of the new
> behaviour. It appears existing tests focus either on the credential helper
> protocol/format, or rely on testing basic authentication only via an Apache
> webserver. In order to have a full end to end test coverage of these new
> features it make be that we need a more comprehensive test bed to mock these
> more nuanced authentication methods. I lean on the experts on the list for
> advice here.

The microsoft/git fork has a feature (the GVFS Protocol) that requires a
custom HTTP server as a test helper. We might need a similar test helper
to return these WWW-Authenticate headers and check the full request list
from Git matches the spec. Doing that while also executing the proper Git
commands to serve the HTTP bodies is hopefully not too large. It might be
nice to adapt such a helper to replace the need for a full Apache install
in our test suite, but that's an independent concern from this RFC.

> Limitations
> ===========
> 
> Because this credential model was built mostly for password based
> authentication systems, it's somewhat limited. In particular:
> 
>  1. To generate valid credentials, additional information about the request
>     (or indeed the requestee and their device) may be required. For example,
>     OAuth is based around scopes. A scope, like "git.read", might be
>     required to read data from the remote. However, the remote cannot tell
>     the credential helper what scope is required for this request.
> 
>  2. This system is not fully extensible. Each time a new type of
>     authentication (like OAuth Bearer) is invented, Git needs updates before
>     credential helpers can take advantage of it (or leverage a new
>     capability in libcurl).
> 
> 
> Goals
> =====
> 
>  * As a user with multiple federated cloud identities:

I'm not sure if you mentioned it anywhere else, but this is specifically
for cases where a user might have multiple identities _on the same host
by DNS name_. The credential.useHttpPath config option might seem like it
could help here, but the credential helper might pick the wrong identity
that is the most-recent login. Either this workflow will require the user
to re-login with every new URL or the fetches/clones will fail when the
guess is wrong and the user would need to learn how to log into that other
identity.

Please correct me if I'm wrong about any of this, but the details of your
goals make it clear that the workflow will be greatly improved:

>    * Reach out to a remote and have my credential helper automatically
>      prompt me for the correct identity.
>    * Leverage existing authentication systems built-in to many operating
>      systems and devices to boost security and reduce reliance on passwords.
> 
>  * As a Git host and/or cloud identity provider:
>    
>    * Leverage newest identity standards, enhancements, and threat
>      mitigations - all without updating Git.
>    * Enforce security policies (like requiring two-factor authentication)
>      dynamically.
>    * Allow integration with third party standard based identity providers in
>      enterprises allowing customers to have a single plane of control for
>      critical identities with access to source code.

I had a question with this part of your proposal:

>     Because the extra information forms an ordered list, and the existing
>     credential helper I/O format only provides for simple key=value pairs,
>     we introduce a new convention for transmitting an ordered list of
>     values. Key names that are suffixed with a C-style array syntax should
>     have values considered to form an order list, i.e. key[n]=value, where n
>     is a zero based index of the values.
>     
>     For the WWW-Authenticate header values we opt to use the key wwwauth[n].
...
> Git sends over standard input:
> 
> protocol=https
> host=example.com
> wwwauth[0]=Bearer realm="login.example", scope="git.readwrite"
> wwwauth[1]=Basic realm="login.example"

The important part here is that we provide a way to specify a multi-valued
key as opposed to a "last one wins" key, right?

Using empty braces (wwwauth[]) would suffice to indicate this, right? That
allows us to not care about the values inside the braces. The biggest
issues I see with a value in the braces are:

1. What if it isn't an integer?
2. What if we are missing a value?
3. What if they come out of order?

Without a value inside, then the order in which they appear provides
implicit indices in their multi-valued list.

Other than that, I support this idea and will start looking at the code
now.

Thanks,
-Stolee