Commit 18293385d00f4aba3c580022b183e0fe25caa741

Edward Thomson 2021-08-29T21:40:16

Merge pull request #5395 from josharian/http-use-eauth Use error code GIT_EAUTH for authentication failures

diff --git a/docs/changelog.md b/docs/changelog.md
index edd2c3f..1bbbe60 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -509,6 +509,9 @@ with v0.28.0.
   The breaking change is that the `username` member of the underlying struct
   is now hidden, and a new `git_cred_get_username` function has been provided.
 
+* Some errors of class `GIT_ERROR_NET` now have class `GIT_ERROR_HTTP`.
+  Most authentication failures now have error code `GIT_EAUTH` instead of `GIT_ERROR`.
+
 ### Breaking CMake configuration changes
 
 * The CMake option to use a system http-parser library, instead of the
diff --git a/include/git2/errors.h b/include/git2/errors.h
index 8887b32..de51582 100644
--- a/include/git2/errors.h
+++ b/include/git2/errors.h
@@ -42,14 +42,14 @@ typedef enum {
 	GIT_ECONFLICT       = -13,	/**< Checkout conflicts prevented operation */
 	GIT_ELOCKED         = -14,	/**< Lock file prevented operation */
 	GIT_EMODIFIED       = -15,	/**< Reference value does not match expected */
-	GIT_EAUTH           = -16,      /**< Authentication error */
-	GIT_ECERTIFICATE    = -17,      /**< Server certificate is invalid */
+	GIT_EAUTH           = -16,	/**< Authentication error */
+	GIT_ECERTIFICATE    = -17,	/**< Server certificate is invalid */
 	GIT_EAPPLIED        = -18,	/**< Patch/merge has already been applied */
-	GIT_EPEEL           = -19,      /**< The requested peel operation is not possible */
-	GIT_EEOF            = -20,      /**< Unexpected EOF */
-	GIT_EINVALID        = -21,      /**< Invalid operation or input */
+	GIT_EPEEL           = -19,	/**< The requested peel operation is not possible */
+	GIT_EEOF            = -20,	/**< Unexpected EOF */
+	GIT_EINVALID        = -21,	/**< Invalid operation or input */
 	GIT_EUNCOMMITTED    = -22,	/**< Uncommitted changes in index prevented operation */
-	GIT_EDIRECTORY      = -23,      /**< The operation is not valid for a directory */
+	GIT_EDIRECTORY      = -23,	/**< The operation is not valid for a directory */
 	GIT_EMERGECONFLICT  = -24,	/**< A merge conflict exists and cannot continue */
 
 	GIT_PASSTHROUGH     = -30,	/**< A user-configured callback refused to act */
diff --git a/src/transports/auth.c b/src/transports/auth.c
index 4aa3df0..51763e3 100644
--- a/src/transports/auth.c
+++ b/src/transports/auth.c
@@ -18,7 +18,7 @@ static int basic_next_token(
 {
 	git_credential_userpass_plaintext *cred;
 	git_buf raw = GIT_BUF_INIT;
-	int error = -1;
+	int error = GIT_EAUTH;
 
 	GIT_UNUSED(ctx);
 
diff --git a/src/transports/auth_negotiate.c b/src/transports/auth_negotiate.c
index c538dbb..3146993 100644
--- a/src/transports/auth_negotiate.c
+++ b/src/transports/auth_negotiate.c
@@ -267,7 +267,7 @@ static int negotiate_init_context(
 
 	if (!ctx->oid) {
 		git_error_set(GIT_ERROR_NET, "negotiate authentication is not supported");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	git_buf_puts(&ctx->target, "HTTP@");
diff --git a/src/transports/auth_ntlm.c b/src/transports/auth_ntlm.c
index b511061..742db75 100644
--- a/src/transports/auth_ntlm.c
+++ b/src/transports/auth_ntlm.c
@@ -85,7 +85,7 @@ static int ntlm_next_token(
 	git_buf input_buf = GIT_BUF_INIT;
 	const unsigned char *msg;
 	size_t challenge_len, msg_len;
-	int error = -1;
+	int error = GIT_EAUTH;
 
 	GIT_ASSERT_ARG(buf);
 	GIT_ASSERT_ARG(ctx);
diff --git a/src/transports/http.c b/src/transports/http.c
index 9871be5..691bceb 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -162,7 +162,7 @@ static int handle_auth(
 
 	if (error > 0) {
 		git_error_set(GIT_ERROR_HTTP, "%s authentication required but no callback set", server_type);
-		error = -1;
+		error = GIT_EAUTH;
 	}
 
 	if (!error)
@@ -179,7 +179,7 @@ GIT_INLINE(int) handle_remote_auth(
 
 	if (response->server_auth_credtypes == 0) {
 		git_error_set(GIT_ERROR_HTTP, "server requires authentication that we do not support");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	/* Otherwise, prompt for credentials. */
@@ -201,7 +201,7 @@ GIT_INLINE(int) handle_proxy_auth(
 
 	if (response->proxy_auth_credtypes == 0) {
 		git_error_set(GIT_ERROR_HTTP, "proxy requires authentication that we do not support");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	/* Otherwise, prompt for credentials. */
@@ -259,7 +259,7 @@ static int handle_response(
 	} else if (response->status == GIT_HTTP_STATUS_UNAUTHORIZED ||
 	           response->status == GIT_HTTP_STATUS_PROXY_AUTHENTICATION_REQUIRED) {
 		git_error_set(GIT_ERROR_HTTP, "unexpected authentication failure");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	if (response->status != GIT_HTTP_STATUS_OK) {
@@ -416,7 +416,7 @@ static int http_stream_read(
 
 	if (stream->state == HTTP_STATE_SENDING_REQUEST) {
 		git_error_set(GIT_ERROR_HTTP, "too many redirects or authentication replays");
-		error = -1;
+		error = GIT_ERROR; /* not GIT_EAUTH, because the exact cause is unclear */
 		goto done;
 	}
 
@@ -554,7 +554,7 @@ static int http_stream_write(
 	if (stream->state == HTTP_STATE_NONE) {
 		git_error_set(GIT_ERROR_HTTP,
 		              "too many redirects or authentication replays");
-		error = -1;
+		error = GIT_ERROR; /* not GIT_EAUTH because the exact cause is unclear */
 		goto done;
 	}
 
diff --git a/src/transports/httpclient.c b/src/transports/httpclient.c
index ba86184..4612b43 100644
--- a/src/transports/httpclient.c
+++ b/src/transports/httpclient.c
@@ -597,6 +597,7 @@ static int apply_credentials(
 			free_auth_context(server);
 	} else if (!token.size) {
 		git_error_set(GIT_ERROR_HTTP, "failed to respond to authentication challenge");
+		error = GIT_EAUTH;
 		error = -1;
 		goto done;
 	}
diff --git a/src/transports/ssh.c b/src/transports/ssh.c
index 71c37e7..efa77a7 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -461,13 +461,13 @@ static int request_creds(git_credential **out, ssh_subtransport *t, const char *
 
 	if (no_callback) {
 		git_error_set(GIT_ERROR_SSH, "authentication required but no callback set");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	if (!(cred->credtype & auth_methods)) {
 		cred->free(cred);
-		git_error_set(GIT_ERROR_SSH, "callback returned unsupported credentials type");
-		return -1;
+		git_error_set(GIT_ERROR_SSH, "authentication callback returned unsupported credentials type");
+		return GIT_EAUTH;
 	}
 
 	*out = cred;
@@ -840,7 +840,7 @@ static int list_auth_methods(int *out, LIBSSH2_SESSION *session, const char *use
 	/* either error, or the remote accepts NONE auth, which is bizarre, let's punt */
 	if (list == NULL && !libssh2_userauth_authenticated(session)) {
 		ssh_error(session, "Failed to retrieve list of SSH authentication methods");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	ptr = list;
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index d21c506..ea2195a 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -154,7 +154,7 @@ static int apply_userpass_credentials(HINTERNET request, DWORD target, int mecha
 		native_scheme = WINHTTP_AUTH_SCHEME_BASIC;
 	} else {
 		git_error_set(GIT_ERROR_HTTP, "invalid authentication scheme");
-		error = -1;
+		error = GIT_EAUTH;
 		goto done;
 	}
 
@@ -193,7 +193,7 @@ static int apply_default_credentials(HINTERNET request, DWORD target, int mechan
 		native_scheme = WINHTTP_AUTH_SCHEME_NTLM;
 	} else {
 		git_error_set(GIT_ERROR_HTTP, "invalid authentication scheme");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	/*
@@ -616,7 +616,7 @@ static int parse_unauthorized_response(
 	 */
 	if (!WinHttpQueryAuthSchemes(request, &supported, &first, &target)) {
 		git_error_set(GIT_ERROR_OS, "failed to parse supported auth schemes");
-		return -1;
+		return GIT_EAUTH;
 	}
 
 	if (WINHTTP_AUTH_SCHEME_NTLM & supported) {
@@ -1040,7 +1040,7 @@ replay:
 	/* Enforce a reasonable cap on the number of replays */
 	if (replay_count++ >= GIT_HTTP_REPLAY_MAX) {
 		git_error_set(GIT_ERROR_HTTP, "too many redirects or authentication replays");
-		return -1;
+		return GIT_ERROR; /* not GIT_EAUTH because the exact cause is not clear */
 	}
 
 	/* Connect if necessary */