Subject: RE: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

From: <[email-masked]>
Date: Fri, 10 Jan 2025 20:01:06 -0500

On January 10, 2025 7:45 PM, Junio C Hamano wrote:
>"brian m. carlson" <[email-masked]> writes:
>
>>> When the former is true and the latter is false, it is an indication
>>> that for that site with the username-or-token, there wasn't anything
>>> necessary to authenticate and the access was authorized.  Which is
>>> what the original poster wants us to warn against.
>>
>> No, I don't think that will work in the general case.  Here's why.  If
>> I do `git push
>> https://[email-masked]/git/bmc/xyzzy.git`,
>> that uses Kerberos (Negotiate).  There's a username there to make
>> libcurl enable auth, but it's never used and a credential helper is
>> never invoked, so case 1 is true and case 2 is false.
>>
>> Now, we _could_ do that only for Basic auth, which would catch the
>> GitHub case.  However, it's _also_ possible to use TLS client
>> certificate auth (and I think Bitbucket does support that) and use the
>> username only for choosing the account (because, say, your work
>> account uses a client certificate and your personal account uses
>> something else).  There might be Basic auth sent (say, if you'd set
>> `http.proactiveAuth`), but the server would ignore it since you were
>> already authenticated via the TLS cert.  That would also make case 1
>> be true and case 2 be false.

If the use case is like mine, where someone wants to use something similar
to a PAT for creating a Pull Request, then GitHub, BitBucket, GitLab, and
AzureGit all behave in a very similar way. The PAT structure is different,
but
there is conceptual equivalence.

--Randall

>>
>> Perhaps that latter case is not worth worrying about, but it is a
>> possibility and I'm sure some people will hit it.  Maybe with a config
>> option for the advice that's okay, though.
>
>Thanks, so in short, if we really want to do this "warning", it has to look
at the
>"username" string and "guess" that it is something the user does not want
to have
>as a value to remote.*.url field X-<.
>
>So "should warn" in the title is not really #leftoverbits (i.e. we clearly
know it is a
>good idea, we just didn't have bandwidth to do it, anybody is welcome to
fill the
>void) at this point.
>Thanks.