Subject: Re: [PATCH v9 1/3] t5563: add tests for basic and anoymous HTTP access

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Feb 2023 14:15:07 -0800

"Matthew John Cheetham via GitGitGadget" <[email-masked]>
writes:

> +if test -n "$HTTP_AUTHORIZATION" && \
> +	grep -qsi "^${HTTP_AUTHORIZATION}\$" "$VALID_CREDS_FILE"

Do we require a regexp match (and worry about metacharacters in
HTTP_AUTHORIZATION variable), or would we want to use "grep -F -x"
here to force match with the entire line?

> +then
> +	# Note that although git-http-backend returns a status line, it
> +	# does so using a CGI 'Status' header. Because this script is an
> +	# No Parsed Headers (NPH) script, we must return a real HTTP
> +	# status line.
> +	# This is only a test script, so we don't bother to check for
> +	# the actual status from git-http-backend and always return 200.
> +	echo 'HTTP/1.1 200 OK'
> +	exec "$GIT_EXEC_PATH"/git-http-backend
> +fi

OK.  That's the successful auth case.  Otherwise ...

> +echo 'HTTP/1.1 401 Authorization Required'
> +if test -f "$CHALLENGE_FILE"
> +then
> +	cat "$CHALLENGE_FILE"
> +fi
> +echo

OK.  We'll just give a challenge.

> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> new file mode 100755
> index 00000000000..e0682039de7
> --- /dev/null
> +++ b/t/t5563-simple-http-auth.sh
> @@ -0,0 +1,81 @@
> +#!/bin/sh
> +
> +test_description='test http auth header and credential helper interop'
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-httpd.sh
> +
> +start_httpd
> +
> +test_expect_success 'setup_credential_helper' '
> +	mkdir "$TRASH_DIRECTORY/bin" &&
> +	PATH=$PATH:"$TRASH_DIRECTORY/bin" &&
> +	export PATH &&
> +
> +	CREDENTIAL_HELPER="$TRASH_DIRECTORY/bin/git-credential-test-helper" &&
> +	write_script "$CREDENTIAL_HELPER" <<-\EOF
> +	cmd=$1
> +	teefile=$cmd-query.cred
> +	catfile=$cmd-reply.cred
> +	sed -n -e "/^$/q" -e "p" >>$teefile
> +	if test "$cmd" = "get"
> +	then
> +		cat $catfile
> +	fi
> +	EOF
> +'
> +
> +set_credential_reply() {

Style. Have SP before "()" as well as after.

> +	cat >"$TRASH_DIRECTORY/$1-reply.cred"
> +}
> +
> +expect_credential_query() {

Style. Have SP before "()" as well as after.

> +	cat >"$TRASH_DIRECTORY/$1-expect.cred" &&
> +	test_cmp "$TRASH_DIRECTORY/$1-expect.cred" \
> +		 "$TRASH_DIRECTORY/$1-query.cred"
> +}
> +
> +per_test_cleanup () {
> +	rm -f *.cred &&
> +	rm -f "$HTTPD_ROOT_PATH"/custom-auth.*
> +}
> +
> +test_expect_success 'setup repository' '
> +	test_commit foo &&
> +	git init --bare "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +	git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
> +'

OK.

> +test_expect_success 'access using basic auth' '
> +	test_when_finished "per_test_cleanup" &&
> +
> +	set_credential_reply get <<-EOF &&
> +	username=alice
> +	password=secret-passwd
> +	EOF
> +
> +	cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
> +	Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
> +	EOF

Perhaps we want to note that this matches the "alice:secret-passwd"
we prepared earlier?

> +	cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
> +	WWW-Authenticate: Basic realm="example.com"
> +	EOF

OK.

> +	test_config_global credential.helper test-helper &&
> +	git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
> +
> +	expect_credential_query get <<-EOF &&
> +	protocol=http
> +	host=$HTTPD_DEST
> +	EOF
> +
> +	expect_credential_query store <<-EOF
> +	protocol=http
> +	host=$HTTPD_DEST
> +	username=alice
> +	password=secret-passwd
> +	EOF
> +'

OK.

> +test_done