Re: [PATCH v3 00/11] Enhance credential helper protocol to include auth headers

[Glen Choo <chooglen@google.com>]

Hi Matthew!

We covered this series in Review Club. As usual, participants will send
their own feedback on this thread, but you may also find the meeting
notes handy:

  https://docs.google.com/document/d/14L8BAumGTpsXpjDY8VzZ4rRtpAjuGrFSRqn3stCuS_w/edit?pli=1#

"Matthew John Cheetham via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Background
> ==========
>
> [...]
>
> Limitations
> ===========
>
> [...]
>
> Goals
> =====
>
> [...]
>
> Design Principles
> =================
>
> [...]

Thanks for the well-written cover letter! I suspect that not many folks
are familiar with the history and workings of credential helpers, the
current state of auth and how credential helper limitations create
challenges for auth.

I've learned a lot reading this, and it makes the motivations of this
series clear :)

> Proposed Changes
> ================
>
>  1. Teach Git to read HTTP response headers, specifically the standard
>     WWW-Authenticate (RFC 7235 Section 4.1) headers.
>
>  2. Teach Git to include extra information about HTTP responses that require
>     authentication when calling credential helpers. Specifically the
>     WWW-Authenticate header information.
>     
>     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[]=value, where
>     the order of the key=value pairs in the stream specifies the order.
>     
>     For the WWW-Authenticate header values we opt to use the key wwwauth[].
>
>  3. Teach Git to specify authentication schemes other than Basic in
>     subsequent HTTP requests based on credential helper responses.
>

From a reading of this section + the subject line, it's not immediately
obvious that 3. also requires extending the credential helper protocol
to include the "authtype" field. IMO it's significant enough to warrant
an explicit call-out.