public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Ashlesh Gawande <git@ashlesh•me>
To: Jeff King <peff@peff•net>, Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, sandals@crustytoothpaste•net
Subject: Re: [PATCH v3] t5550: add netrc tests for http 401/403
Date: Fri, 6 Feb 2026 21:23:18 +0530	[thread overview]
Message-ID: <7583bd2c-4f2f-4a43-a36f-7e0698da8a57@ashlesh.me> (raw)
In-Reply-To: <8ac465f8-6fda-43a1-8bfc-3e88f30d1ca5@ashlesh.me>


On 2/6/26 20:55, Ashlesh Gawande wrote:
>
> On 2/6/26 15:08, Jeff King wrote:
>> On Thu, Feb 05, 2026 at 09:05:51PM -0800, Junio C Hamano wrote:
>>
>>>>    - Third test case checks that the git clone fails when the 
>>>> .netrc file
>>>>      provides credentials that are valid but do not have permission 
>>>> for
>>>>      this user. For example one may have multiple tokens in GitHub
>>>>      and uses the one which was not authorized for cloning this repo.
>>>>      In such a case the HTTP server returns 403 Forbidden.
>>>>      For this test, the apache.conf is modified to return a 403
>>>>      on finding a forbidden-user. No prompt for username/password is
>>>>      expected after the 403 (unlike 401). This is because prompting 
>>>> may wipe
>>>>      out existing credentials or conflict with custom credential 
>>>> helpers.
>>> Nicely summarised.  So we say 401 when we do not know you, while we
>>> say 403 when we know you and do not want you to be accessing the
>>> resource.  We test for both.
>> I think it is fine to check the 403 handling, but note that this _isn't_
>> how GitHub would respond. If you try to fetch from a repository you
>> don't have access to, it will return a 401 first (so you try to log in)
>> and then a 404. The idea being to avoid revealing the existence of the
>> repository to unauthorized users.
> In the case of fine-grained access token such that the token has read 
> access to the repository
> but not write access GitHub does return a 403.
> (I think this is correct behavior as the token has read access so user 
> is authorized/knows about the repository).
So should I modify that test case to do a push instead for this specific 
scenario (and update the description)?
>>> Just out of curiosity, do we test for these codes with other
>>> credential helpers or is this only relevant for .netrc users?
>> The netrc support here should not involve credential helpers at all. It
>> is all being done internally by curl. So in this (third and final) test:
>>
>>>> +test_expect_success 'netrc authorized but forbidden credentials 
>>>> (fail on 403)' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    set_askpass wrong &&
>>>> +    set_netrc 127.0.0.1 forbidden-user@host pass@host &&
>>>> +    test_must_fail git clone "$HTTPD_URL/auth/dumb/repo.git" 
>>>> clone-auth-netrc-403 2>err &&
>>>> +    expect_askpass none &&
>>>> +    grep "The requested URL returned error: 403" err
>>>> +'
>> ...what is happening is roughly:
>>
>>    - curl sends the first request with no credentials, which gets a 401
>>
>>    - curl internally, without returning a response to Git, looks up the
>>      netrc value and repeats the request with an Authorization header
>>
>>    - curl returns the resulting 403 to Git
>>
>>    - Git calls this an error (just like it would a 404) and bails
>>
>> But from Git's perspective the use of netrc here is not really
>> interesting. We don't even know it happened! And if the server did
>> return a 401, we'd happily try to get credentials (from the user or from
>> a helper) in the usual way. And that's what happens in the second test:
>>
>>>> +test_expect_success 'netrc unauthorized credentials (prompt after 
>>>> 401)' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    set_askpass wrong &&
>>>> +    set_netrc 127.0.0.1 user@host pass@wrong &&
>>>> +    test_must_fail git clone "$HTTPD_URL/auth/dumb/repo.git" 
>>>> clone-auth-netrc-401 &&
>>>> +    expect_askpass both wrong
>>>> +'
>> Curl tries the credential under the hood, but we have no idea, and we
>> process a 401 in the usual way.
>>
>> And in the first one:
>>
>>>> +test_expect_success 'using credentials from netrc to clone 
>>>> successfully' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    set_askpass wrong &&
>>>> +    set_netrc 127.0.0.1 user@host pass@host &&
>>>> +    git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-netrc &&
>>>> +    expect_askpass none
>>>> +'
>> We do not ever even see the 401, and curl just magically handles it for
>> us. We see only the successful 200 code, just as if authentication was
>> not required in the first place.
>>
>>
>> So really, none of this is testing anything novel in Git at all that is
>> not covered elsewhere, except for the fact that we pass the flag to curl
>> that says "you may use netrc". And so there's some value in adding it in
>> that case. But trying to answer your question about other credential
>> helpers, no, they're not even entering the picture here.
>>
>> -Peff
>>
>

  reply	other threads:[~2026-02-06 15:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-06  9:34 [PATCH] t5550: add netrc tests for http 401/403 Ashlesh Gawande
2026-01-06 10:20 ` Junio C Hamano
2026-01-06 11:47   ` Ashlesh Gawande
2026-01-06 11:40 ` [PATCH v2] " Ashlesh Gawande
2026-01-07  0:32   ` Junio C Hamano
2026-01-07  7:47   ` [PATCH v3] " Ashlesh Gawande
2026-01-31 12:33     ` Ashlesh Gawande
2026-02-06  5:05     ` Junio C Hamano
2026-02-06  9:38       ` Jeff King
2026-02-06 15:25         ` Ashlesh Gawande
2026-02-06 15:53           ` Ashlesh Gawande [this message]
2026-02-06 20:44             ` Jeff King
2026-02-06 17:39         ` Junio C Hamano
2026-02-06 20:53           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7583bd2c-4f2f-4a43-a36f-7e0698da8a57@ashlesh.me \
    --to=git@ashlesh$(echo .)me \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=sandals@crustytoothpaste$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox