Re: [PATCH v2] t5550: add netrc tests for http 401/403

[Junio C Hamano <gitster@pobox.com>]

Ashlesh Gawande <git@ashlesh.me> writes:

Here between the title and your sign-off is a space to explain why
it makes sense to add these new tests.  One way to do so may be to
explain that some cases were missing in the existing tests, and the
new ones are added to cover those cases, i.e., what the new tests
try to see under what situation, what behaviour do we expect out of
the system, and why do we expect that behaviour?

Thanks.

> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
> ---
> Range-diff against v1:
> 1:  27e112ea42 ! 1:  0b68f1d1af t5550: add netrc tests for http 401/403
>     @@ Commit message
>          Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
>      
>       ## t/lib-httpd.sh ##
>     -@@ t/lib-httpd.sh: set_askpass() {
>     +@@ t/lib-httpd.sh: setup_askpass_helper() {
>     + 	'
>     + }
>     + 
>     +-set_askpass() {
>     ++set_askpass () {
>     + 	>"$TRASH_DIRECTORY/askpass-query" &&
>     + 	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>       	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>       }
>       
>     -+set_netrc() {
>     +-expect_askpass() {
>     ++set_netrc () {
>      +	# $HOME=$TRASH_DIRECTORY
>     -+	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc
>     ++	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
>      +}
>      +
>     -+clear_netrc() {
>     -+	rm "$TRASH_DIRECTORY/.netrc"
>     ++clear_netrc () {
>     ++	rm -f "$TRASH_DIRECTORY/.netrc"
>      +}
>      +
>     - expect_askpass() {
>     ++expect_askpass () {
>       	dest=$HTTPD_DEST${3+/$3}
>       
>     + 	{
>      
>       ## t/lib-httpd/apache.conf ##
>      @@ t/lib-httpd/apache.conf: SSLEngine On
>     @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'cloning password-protected repo
>       '
>       
>      +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
>      +'
>     -+clear_netrc
>      +
>      +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
>      +'
>     -+clear_netrc
>      +
>      +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
>      +'
>     -+clear_netrc
>      +
>       test_expect_success 'http auth can use user/pass in URL' '
>       	set_askpass wrong &&
>
>  t/lib-httpd.sh             | 13 +++++++++++--
>  t/lib-httpd/apache.conf    |  4 ++++
>  t/lib-httpd/passwd         |  1 +
>  t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
>  4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 5091db949b..5f42c311c2 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -319,13 +319,22 @@ setup_askpass_helper() {
>  	'
>  }
>  
> -set_askpass() {
> +set_askpass () {
>  	>"$TRASH_DIRECTORY/askpass-query" &&
>  	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>  	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>  }
>  
> -expect_askpass() {
> +set_netrc () {
> +	# $HOME=$TRASH_DIRECTORY
> +	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
> +}
> +
> +clear_netrc () {
> +	rm -f "$TRASH_DIRECTORY/.netrc"
> +}
> +
> +expect_askpass () {
>  	dest=$HTTPD_DEST${3+/$3}
>  
>  	{
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index e631ab0eb5..6b8c50a51a 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -238,6 +238,10 @@ SSLEngine On
>  	AuthName "git-auth"
>  	AuthUserFile passwd
>  	Require valid-user
> +
> +	# return 403 for authenticated user: forbidden-user@host
> +	RewriteCond "%{REMOTE_USER}" "^forbidden-user@host"
> +	RewriteRule ^ - [F]
>  </Location>
>  
>  <LocationMatch "^/auth-push/.*/git-receive-pack$">
> diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd
> index d9c122f348..3bab7b6423 100644
> --- a/t/lib-httpd/passwd
> +++ b/t/lib-httpd/passwd
> @@ -1 +1,2 @@
>  user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
> +forbidden-user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> index ed0ad66fad..9530f01b9e 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -102,6 +102,31 @@ test_expect_success 'cloning password-protected repository can fail' '
>  	expect_askpass both wrong
>  '
>  
> +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
> +'
> +
> +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
> +'
> +
> +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
> +'
> +
>  test_expect_success 'http auth can use user/pass in URL' '
>  	set_askpass wrong &&
>  	git clone "$HTTPD_URL_USER_PASS/auth/dumb/repo.git" clone-auth-none &&