[libsoup] SoupAuthManager: deal with "disappearing" auth headers

[Dan Winship <danw src gnome org>]

commit 162abf754b75238ed0f772563d602a964e8dc149
Author: Dan Winship <danw gnome org>
Date:   Wed Mar 6 12:17:19 2013 -0500

    SoupAuthManager: deal with "disappearing" auth headers
    
    Normally when sending a 401 response, a server re-sends the initial
    WWW-Authenticate challenge. However, it doesn't actually have to, and
    libsoup was getting confused if it didn't. Fix that.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=916224

 libsoup/soup-auth-manager.c |   38 +++++++++++---------
 tests/auth-test.c           |   78 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 98 insertions(+), 18 deletions(-)
---
diff --git a/libsoup/soup-auth-manager.c b/libsoup/soup-auth-manager.c
index e945dd7..b164791 100644
--- a/libsoup/soup-auth-manager.c
+++ b/libsoup/soup-auth-manager.c
@@ -83,7 +83,8 @@ typedef struct {
 
 static void soup_auth_host_free (SoupAuthHost *host);
 static SoupAuth *record_auth_for_uri (SoupAuthManagerPrivate *priv,
-                                     SoupURI *uri, SoupAuth *auth);
+                                     SoupURI *uri, SoupAuth *auth,
+                                     gboolean prior_auth_failed);
 
 static void
 soup_auth_manager_init (SoupAuthManager *manager)
@@ -378,19 +379,22 @@ create_auth (SoupAuthManagerPrivate *priv, SoupMessage *msg)
 static gboolean
 check_auth (SoupMessage *msg, SoupAuth *auth)
 {
-       const char *header;
-       char *challenge;
-       gboolean ok;
+       const char *header, *scheme;
+       char *challenge = NULL;
+       gboolean ok = TRUE;
 
-       header = auth_header_for_message (msg);
-       if (!header)
-               return FALSE;
+       scheme = soup_auth_get_scheme_name (auth);
 
-       challenge = soup_auth_manager_extract_challenge (header, soup_auth_get_scheme_name (auth));
-       if (!challenge)
-               return FALSE;
+       header = auth_header_for_message (msg);
+       if (header)
+               challenge = soup_auth_manager_extract_challenge (header, scheme);
+       if (!challenge) {
+               ok = FALSE;
+               challenge = g_strdup (scheme);
+       }
 
-       ok = soup_auth_update (auth, msg, challenge);
+       if (!soup_auth_update (auth, msg, challenge))
+               ok = FALSE;
        g_free (challenge);
        return ok;
 }
@@ -432,7 +436,7 @@ make_auto_ntlm_auth (SoupAuthManagerPrivate *priv, SoupAuthHost *host)
        auth = g_object_new (SOUP_TYPE_AUTH_NTLM,
                             SOUP_AUTH_HOST, host->uri->host,
                             NULL);
-       record_auth_for_uri (priv, host->uri, auth);
+       record_auth_for_uri (priv, host->uri, auth, FALSE);
        g_object_unref (auth);
        return TRUE;
 }
@@ -497,7 +501,7 @@ authenticate_auth (SoupAuthManager *manager, SoupAuth *auth,
 
 static SoupAuth *
 record_auth_for_uri (SoupAuthManagerPrivate *priv, SoupURI *uri,
-                    SoupAuth *auth)
+                    SoupAuth *auth, gboolean prior_auth_failed)
 {
        SoupAuthHost *host;
        SoupAuth *old_auth;
@@ -531,11 +535,11 @@ record_auth_for_uri (SoupAuthManagerPrivate *priv, SoupURI *uri,
        soup_auth_free_protection_space (auth, pspace);
 
        /* Now, make sure the auth is recorded. (If there's a
-        * pre-existing auth, we keep that rather than the new one,
+        * pre-existing good auth, we keep that rather than the new one,
         * since the old one might already be authenticated.)
         */
        old_auth = g_hash_table_lookup (host->auths, auth_info);
-       if (old_auth) {
+       if (old_auth && (old_auth != auth || !prior_auth_failed)) {
                g_free (auth_info);
                return old_auth;
        } else {
@@ -569,7 +573,7 @@ auth_got_headers (SoupMessage *msg, gpointer manager)
        }
 
        new_auth = record_auth_for_uri (priv, soup_message_get_uri (msg),
-                                       auth);
+                                       auth, prior_auth_failed);
        g_object_unref (auth);
 
        /* If we need to authenticate, try to do it. */
@@ -729,7 +733,7 @@ soup_auth_manager_use_auth (SoupAuthManager *manager,
        SoupAuthManagerPrivate *priv = manager->priv;
 
        g_mutex_lock (&priv->lock);
-       record_auth_for_uri (priv, uri, auth);
+       record_auth_for_uri (priv, uri, auth, FALSE);
        g_mutex_unlock (&priv->lock);
 }
 
diff --git a/tests/auth-test.c b/tests/auth-test.c
index fba6f1e..992e3d5 100644
--- a/tests/auth-test.c
+++ b/tests/auth-test.c
@@ -817,7 +817,7 @@ select_auth_test_one (SoupURI *uri,
        } else if (!second_headers && sad.round[1].headers) {
                debug_printf (1, "    Didn't expect a second round!\n");
                errors++;
-       } else if (second_headers) {
+       } else if (second_headers && second_response) {
                if (strcmp (sad.round[1].headers, second_headers) != 0) {
                        debug_printf (1, "    Second round header order wrong: expected %s, got %s\n",
                                      second_headers, sad.round[1].headers);
@@ -1124,6 +1124,81 @@ do_infinite_auth_test (const char *base_uri)
        g_object_unref (msg);
 }
 
+static void
+disappear_request_read (SoupServer *server, SoupMessage *msg,
+                       SoupClientContext *context, gpointer user_data)
+{
+       /* Remove the WWW-Authenticate header if this was a failed attempt */
+       if (soup_message_headers_get_one (msg->request_headers, "Authorization") &&
+           msg->status_code == SOUP_STATUS_UNAUTHORIZED)
+               soup_message_headers_remove (msg->response_headers, "WWW-Authenticate");
+}
+
+static void
+disappear_authenticate (SoupSession *session, SoupMessage *msg,
+                       SoupAuth *auth, gboolean retrying, gpointer data)
+{
+       int *counter = data;
+
+       (*counter)++;
+       if (!retrying)
+               soup_auth_authenticate (auth, "user", "bad");
+}
+
+static void
+do_disappearing_auth_test (void)
+{
+       SoupServer *server;
+       SoupAuthDomain *auth_domain;
+       SoupURI *uri;
+       SoupMessage *msg;
+       SoupSession *session;
+       int counter;
+
+       debug_printf (1, "\nTesting auth when server does not repeat challenge on failure:\n");
+
+       server = soup_test_server_new (FALSE);
+       soup_server_add_handler (server, NULL,
+                                server_callback, NULL, NULL);
+
+       uri = soup_uri_new ("http://127.0.0.1/";);
+       soup_uri_set_port (uri, soup_server_get_port (server));
+
+       auth_domain = soup_auth_domain_basic_new (
+                                                 SOUP_AUTH_DOMAIN_REALM, "auth-test",
+                                                 SOUP_AUTH_DOMAIN_ADD_PATH, "/",
+                                                 SOUP_AUTH_DOMAIN_BASIC_AUTH_CALLBACK, 
server_basic_auth_callback,
+                                                 NULL);
+       soup_server_add_auth_domain (server, auth_domain);
+       g_signal_connect (server, "request-read",
+                         G_CALLBACK (disappear_request_read), NULL);
+
+       session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+
+       counter = 0;
+       g_signal_connect (session, "authenticate",
+                         G_CALLBACK (disappear_authenticate), &counter);
+
+       msg = soup_message_new_from_uri ("GET", uri);
+       soup_session_send_message (session, msg);
+
+       if (counter > 2) {
+               debug_printf (1, "    FAILED: Got stuck in loop");
+               errors++;
+       } else if (msg->status_code != SOUP_STATUS_UNAUTHORIZED) {
+               debug_printf (1, "    Final status wrong: expected 401, got %u\n",
+                             msg->status_code);
+               errors++;
+       }
+
+       g_object_unref (msg);
+       soup_test_session_abort_unref (session);
+
+       g_object_unref (auth_domain);
+       soup_uri_free (uri);
+       soup_test_server_quit_unref (server);
+}
+
 static SoupAuthTest relogin_tests[] = {
        { "Auth provided via URL, should succeed",
          "Basic/realm12/", "1", TRUE, "01", SOUP_STATUS_OK },
@@ -1250,6 +1325,7 @@ main (int argc, char **argv)
        do_select_auth_test ();
        do_auth_close_test ();
        do_infinite_auth_test (base_uri);
+       do_disappearing_auth_test ();
 
        test_cleanup ();
        return errors != 0;