Commit 216165ecfa6bdea6a000a1b69617b54f48203f87

Josh Bleecher Snyder 2020-02-07T10:06:28

transports: use GIT_EAUTH for authentication failures When the failure is clearly an auth failure (as opposed to possibly an auth failure), use the error code GIT_EAUTH instead of GIT_ERROR. While we're here, fix a typo and improve an error message. Fixes #5389.

diff --git a/docs/changelog.md b/docs/changelog.md
index 72973b3..af80d48 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -67,6 +67,7 @@ v0.28 + 1
   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
 
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 8a614b8..a04efce 100644
--- a/src/transports/auth_negotiate.c
+++ b/src/transports/auth_negotiate.c
@@ -260,7 +260,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 02a861f..645aa34 100644
--- a/src/transports/auth_ntlm.c
+++ b/src/transports/auth_ntlm.c
@@ -84,7 +84,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;
 
 	assert(buf && ctx && ctx->ntlm);
 
diff --git a/src/transports/http.c b/src/transports/http.c
index 66731b0..e83d6d7 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -159,7 +159,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)
@@ -176,7 +176,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. */
@@ -198,7 +198,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. */
@@ -256,7 +256,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) {
@@ -413,7 +413,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;
 	}
 
@@ -551,7 +551,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 f5c2ce5..d8d7227 100644
--- a/src/transports/httpclient.c
+++ b/src/transports/httpclient.c
@@ -585,8 +585,8 @@ static int apply_credentials(
 		if (auth->connection_affinity)
 			free_auth_context(server);
 	} else if (!token.size) {
-		git_error_set(GIT_ERROR_HTTP, "failed to respond to authentication challange");
-		error = -1;
+		git_error_set(GIT_ERROR_HTTP, "failed to respond to authentication challenge");
+		error = GIT_EAUTH;
 		goto done;
 	}
 
diff --git a/src/transports/ssh.c b/src/transports/ssh.c
index 68b3cbe..a605aec 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;
@@ -805,7 +805,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 ee8cb15..ce1eb39 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -146,7 +146,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;
 	}
 
@@ -185,7 +185,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;
 	}
 
 	/*
@@ -602,7 +602,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) {
@@ -972,7 +972,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 */