[libsoup] SoupAuthNTLM: fix to do a "retrying" authenticate

[Dan Winship <danw src gnome org>]

commit 080103de0bc8c70d37125fe60a8886371ca35cfc
Author: Dan Winship <danw gnome org>
Date:   Thu Feb 7 15:03:41 2013 -0500

    SoupAuthNTLM: fix to do a "retrying" authenticate
    
    If the first attempt at NTLM auth fails, let the auth manager emit a
    "retrying" authenticate as well, just like normal auths do.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=693222

 libsoup/soup-auth-ntlm.c |   79 ++++++++++++++++++++++++++++++++++-------
 tests/ntlm-test.c        |   88 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 14 deletions(-)
---
diff --git a/libsoup/soup-auth-ntlm.c b/libsoup/soup-auth-ntlm.c
index 367a3ca..7b11a43 100644
--- a/libsoup/soup-auth-ntlm.c
+++ b/libsoup/soup-auth-ntlm.c
@@ -48,10 +48,17 @@ typedef struct {
 	char *response_header;
 } SoupNTLMConnectionState;
 
+typedef enum {
+	SOUP_NTLM_PASSWORD_NONE,
+	SOUP_NTLM_PASSWORD_PROVIDED,
+	SOUP_NTLM_PASSWORD_ACCEPTED,
+	SOUP_NTLM_PASSWORD_REJECTED
+} SoupNTLMPasswordState;
+
 typedef struct {
 	char *username, *domain;
 	guchar nt_hash[21], lm_hash[21];
-	gboolean authenticated;
+	SoupNTLMPasswordState password_state;
 
 #ifdef USE_NTLM_AUTH
 	/* Use Samba's 'winbind' daemon to support NTLM single-sign-on,
@@ -282,23 +289,37 @@ soup_auth_ntlm_update_connection (SoupConnectionAuth *auth, SoupMessage *msg,
 	SoupNTLMConnectionState *conn = state;
 	gboolean success = TRUE;
 
+	/* Note that we only return FALSE if some sort of parsing error
+	 * occurs. Otherwise, the SoupAuth is still reusable (though it may
+	 * no longer be _ready or _authenticated).
+	 */
+
+	if (!g_str_has_prefix (auth_header, "NTLM"))
+		return FALSE;
+
 	if (conn->state > SOUP_NTLM_SENT_REQUEST) {
-		/* We already authenticated, but then got another 401.
-		 * That means "permission denied", so don't try to
-		 * authenticate again.
-		 */
 		conn->state = SOUP_NTLM_FAILED;
 
-		/* FIXME: we should only do this if the password never worked */
-		priv->authenticated = FALSE;
+		if (priv->password_state == SOUP_NTLM_PASSWORD_ACCEPTED) {
+			/* We know our password is correct, so a 401
+			 * means "permission denied". Since the conn
+			 * state is now FAILED, the auth is no longer
+			 * is_ready() for this message, so this will
+			 * cause a "retrying" authenticate signal.
+			 */
+			return TRUE;
+		}
 
-		return FALSE;
+		/* Otherwise, we just have a bad password. */
+		priv->password_state = SOUP_NTLM_PASSWORD_REJECTED;
+		return TRUE;
 	}
 
-	if (!g_str_has_prefix (auth_header, "NTLM "))
-		return FALSE;
+	if (conn->state == SOUP_NTLM_NEW && !auth_header[4])
+		return TRUE;
 
-	if (!soup_ntlm_parse_challenge (auth_header + 5, &conn->nonce, &priv->domain)) {
+	if (!soup_ntlm_parse_challenge (auth_header + 5, &conn->nonce,
+					priv->domain ? NULL : &priv->domain)) {
 		conn->state = SOUP_NTLM_FAILED;
 		return FALSE;
 	}
@@ -327,7 +348,8 @@ soup_auth_ntlm_update_connection (SoupConnectionAuth *auth, SoupMessage *msg,
 			g_free (response);
 		} else {
 			conn->response_header = response;
-			priv->authenticated = TRUE;
+			if (priv->password_state != SOUP_NTLM_PASSWORD_ACCEPTED)
+				priv->password_state = SOUP_NTLM_PASSWORD_PROVIDED;
 		}
 	}
  out:
@@ -385,7 +407,7 @@ soup_auth_ntlm_authenticate (SoupAuth *auth, const char *username,
 	soup_ntlm_nt_hash (password, priv->nt_hash);
 	soup_ntlm_lanmanager_hash (password, priv->lm_hash);
 
-	priv->authenticated = TRUE;
+	priv->password_state = SOUP_NTLM_PASSWORD_PROVIDED;
 }
 
 static gboolean
@@ -393,7 +415,8 @@ soup_auth_ntlm_is_authenticated (SoupAuth *auth)
 {
 	SoupAuthNTLMPrivate *priv = SOUP_AUTH_NTLM_GET_PRIVATE (auth);
 
-	return priv->authenticated;
+	return (priv->password_state != SOUP_NTLM_PASSWORD_NONE &&
+		priv->password_state != SOUP_NTLM_PASSWORD_REJECTED);
 }
 
 static gboolean
@@ -401,11 +424,32 @@ soup_auth_ntlm_is_connection_ready (SoupConnectionAuth *auth,
 				    SoupMessage        *msg,
 				    gpointer            state)
 {
+	SoupAuthNTLMPrivate *priv = SOUP_AUTH_NTLM_GET_PRIVATE (auth);
 	SoupNTLMConnectionState *conn = state;
 
+	if (priv->password_state == SOUP_NTLM_PASSWORD_REJECTED)
+		return FALSE;
+
+	if (priv->password_state == SOUP_NTLM_PASSWORD_PROVIDED)
+		return TRUE;
+
 	return conn->state != SOUP_NTLM_FAILED && conn->state != SOUP_NTLM_SSO_FAILED;
 }
 
+static void
+got_final_auth_result (SoupMessage *msg, gpointer data)
+{
+	SoupAuth *auth = data;
+
+	g_signal_handlers_disconnect_by_func (msg, G_CALLBACK (got_final_auth_result), auth);
+
+	if (auth != soup_message_get_auth (msg))
+		return;
+
+	if (msg->status_code != SOUP_STATUS_UNAUTHORIZED)
+		SOUP_AUTH_NTLM_GET_PRIVATE (auth)->password_state = SOUP_NTLM_PASSWORD_ACCEPTED;
+}
+
 static char *
 soup_auth_ntlm_get_connection_authorization (SoupConnectionAuth *auth,
 					     SoupMessage        *msg,
@@ -454,6 +498,13 @@ soup_auth_ntlm_get_connection_authorization (SoupConnectionAuth *auth,
 		}
 		g_clear_pointer (&conn->nonce, g_free);
 		conn->state = SOUP_NTLM_SENT_RESPONSE;
+
+		if (priv->password_state != SOUP_NTLM_PASSWORD_ACCEPTED) {
+			/* We need to know if this worked */
+			g_signal_connect (msg, "got-headers",
+					  G_CALLBACK (got_final_auth_result),
+					  auth);
+		}
 		break;
 #ifdef USE_NTLM_AUTH
 	case SOUP_NTLM_SSO_FAILED:
diff --git a/tests/ntlm-test.c b/tests/ntlm-test.c
index fa31280..3588c55 100644
--- a/tests/ntlm-test.c
+++ b/tests/ntlm-test.c
@@ -463,6 +463,89 @@ do_ntlm_tests (SoupURI *base_uri, gboolean use_builtin_ntlm)
 	do_ntlm_round (base_uri, FALSE, "alice", use_builtin_ntlm);
 }
 
+static void
+retry_test_authenticate (SoupSession *session, SoupMessage *msg,
+			 SoupAuth *auth, gboolean retrying,
+			 gpointer user_data)
+{
+	gboolean *retried = user_data;
+
+	if (!retrying) {
+		/* server_callback doesn't actually verify the password,
+		 * only the username. So we pass an incorrect username
+		 * rather than an incorrect password.
+		 */
+		soup_auth_authenticate (auth, "wrong", "password");
+	} else if (!*retried) {
+		soup_auth_authenticate (auth, "alice", "password");
+		*retried = TRUE;
+	}
+}
+
+static void
+do_retrying_test (SoupURI *base_uri)
+{
+	SoupSession *session;
+	SoupMessage *msg;
+	SoupURI *uri;
+	gboolean retried = FALSE;
+
+	debug_printf (1, "  /alice\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION,
+					 SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_AUTH_NTLM,
+					 NULL);
+	g_signal_connect (session, "authenticate",
+			  G_CALLBACK (retry_test_authenticate), &retried);
+
+	uri = soup_uri_new_with_base (base_uri, "/alice");
+	msg = soup_message_new_from_uri ("GET", uri);
+	soup_uri_free (uri);
+
+	soup_session_send_message (session, msg);
+
+	if (!retried) {
+		debug_printf (1, "    Didn't retry!\n");
+		errors++;
+	}
+	if (msg->status_code != SOUP_STATUS_OK) {
+		debug_printf (1, "    Unexpected final status %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	g_object_unref (msg);
+
+	soup_test_session_abort_unref (session);
+
+	debug_printf (1, "  /bob\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION,
+					 SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_AUTH_NTLM,
+					 NULL);
+	g_signal_connect (session, "authenticate",
+			  G_CALLBACK (retry_test_authenticate), &retried);
+	retried = FALSE;
+
+	uri = soup_uri_new_with_base (base_uri, "/bob");
+	msg = soup_message_new_from_uri ("GET", uri);
+	soup_uri_free (uri);
+
+	soup_session_send_message (session, msg);
+
+	if (!retried) {
+		debug_printf (1, "    Didn't retry!\n");
+		errors++;
+	}
+	if (msg->status_code != SOUP_STATUS_UNAUTHORIZED) {
+		debug_printf (1, "    Unexpected final status %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	g_object_unref (msg);
+
+	soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -503,6 +586,11 @@ main (int argc, char **argv)
 	debug_printf (1, "\nExternal -> fallback support\n");
 	do_ntlm_tests (uri, TRUE);
 
+	/* Other tests */
+	g_setenv ("SOUP_NTLM_AUTH_DEBUG", "", TRUE);
+	debug_printf (1, "\nRetrying on failed password\n");
+	do_retrying_test (uri);
+
 	soup_uri_free (uri);
 
 	soup_test_server_quit_unref (server);