Bug 16947 – [GTK] Missing HTTP Auth challenge

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Luca Bruno]

Hello,
the GTK+ port is currently missing the HTTP Auth implementation, take for example the webkit trac itself requiring it for logging in.

[Luca Bruno]

I researched a bit trough the win and mac port but i'm not able to realize how they work.
This is what i do:
- Check for 401 response
- Call ResourceHandle::client()->didReceiveAuthenticationChallenge(AuthenticationChallenge())
- FrameLoaderClient::dispatchReceiveAuthenticationChallenge gets called
- A dialog is opened for username and password input

Once this is done i'm stuck. Now i've to do a new request for opening the page again with the given credentials but how do i specify credentials to the ResourceHandle? Or maybe this is a bad approach? I'm not able to realize how other ports do it, they somehow reach to specify credentials to their ResourceHandle and also whether to continue loading without credentials.
Any hints?

[Luca Bruno]

Created attachment 18585 [details]
proof

This is still a PoC, but working pretty well. No problems in the cURL side as i can see (Wow!) and everything seems much linear. What's missing:
- Other AUTH schemes
- Accessors in the WebKitAuthenticationChallenge
- Get the right informations from the server

[Luca Bruno]

Created attachment 19161 [details]
basic authentication

The patch should be complete now to handle basic authentication.
Now it's asynchronous and the dialog code is more clean than before.

[Luca Bruno]

Created attachment 19162 [details]
basic authentication

Fixed showing widgets as suggested by kalikiana, and set password visibility.

[Alp Toker]

Comment on attachment 19162 [details]
basic authentication

This is conceptually fine. There are a few style issues and bugs to be resolved before getting it in:

>Index: ChangeLog
>===================================================================
>--- ChangeLog	(revision 30341)
>+++ ChangeLog	(working copy)
>@@ -1,3 +1,12 @@
>[phone]  Luca Bruno  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        http://bugs.webkit.org/show_bug.cgi?id=16947
>+        Bug 16947: [GTK] Missing HTTP Auth challenge
>+
>+        * GNUmakefile.am: add webkitauthenticationchallenge.cpp and webkitauthenticationchallenge.h
>+
> 2008-02-15  Alp Toker  <[e-mail]>
> 
>         Reviewed by Holger.
>Index: GNUmakefile.am
>===================================================================
>--- GNUmakefile.am	(revision 30340)
>+++ GNUmakefile.am	(working copy)
>@@ -249,6 +249,7 @@ webkitgtk_cppflags += \
> 
> webkitgtk_h_api += \
> 	WebKit/gtk/webkit/webkit.h \
>+	WebKit/gtk/webkit/webkitauthenticationchallenge.h \
> 	WebKit/gtk/webkit/webkitdefines.h \
> 	WebKit/gtk/webkit/webkitnetworkrequest.h \
> 	WebKit/gtk/webkit/webkitwebbackforwardlist.h \
>@@ -272,6 +273,7 @@ webkitgtk_headers += \
> 	WebKit/gtk/WebCoreSupport/PasteboardHelperGtk.h
> 
> webkitgtk_sources += \
>+	WebKit/gtk/webkit/webkitauthenticationchallenge.cpp \
> 	WebKit/gtk/webkit/webkitnetworkrequest.cpp \
> 	WebKit/gtk/webkit/webkitprivate.cpp \
> 	WebKit/gtk/webkit/webkitwebbackforwardlist.cpp \
>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 30341)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>[phone]  Luca Bruno  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WARNING: NO TEST CASES ADDED OR CHANGED
>+
>+        http://bugs.webkit.org/show_bug.cgi?id=16947
>+        Bug 16947: [GTK] Missing HTTP Auth challenge
>+
>+        * WebCore.pro: add webkitauthenticationchallenge.cpp and webkitauthenticationchallenge.h
>+        * platform/network/ResourceHandle.h:
>+        * platform/network/ResourceHandleInternal.h:
>+        (WebCore::ResourceHandleInternal::ResourceHandleInternal):
>+        * platform/network/curl/ResourceHandleCurl.cpp:
>+        (WebCore::ResourceHandle::didReceiveAuthenticationChallenge): added, called by cURL backend
>+        (WebCore::ResourceHandle::receivedCredential): added
>+        (WebCore::ResourceHandle::receivedRequestToContinueWithoutCredential): added
>+        (WebCore::ResourceHandle::receivedCancellation): added
>+        * platform/network/curl/ResourceHandleManager.cpp:
>+        (WebCore::headerCallback): check for authentication
>+        (WebCore::ResourceHandleManager::startJob): use the authentication
>+
> 2008-02-16  Kevin Ollivier  <[e-mail]>
> 
>         wx build fix.
>Index: WebCore/WebCore.pro
>===================================================================
>--- WebCore/WebCore.pro	(revision 30340)
>+++ WebCore/WebCore.pro	(working copy)
>@@ -993,6 +993,7 @@ gtk-port {
>         ../WebCore/platform/gtk/ClipboardGtk.h \
>         ../WebCore/platform/gtk/PasteboardHelper.h \
>         ../WebKit/gtk/webkit/webkit.h \
>+        ../WebKit/gtk/webkit/webkitauthenticationchallenge.h \
>         ../WebKit/gtk/webkit/webkitdefines.h \
>         ../WebKit/gtk/webkit/webkitnetworkrequest.h \
>         ../WebKit/gtk/webkit/webkitprivate.h \
>@@ -1074,6 +1075,7 @@ gtk-port {
>         platform/image-decoders/bmp/BMPImageDecoder.cpp \
>         platform/image-decoders/ico/ICOImageDecoder.cpp \
>         platform/image-decoders/xbm/XBMImageDecoder.cpp \
>+        ../WebKit/gtk/webkit/webkitauthenticationchallenge.cpp \
>         ../WebKit/gtk/webkit/webkitnetworkrequest.cpp \
>         ../WebKit/gtk/webkit/webkitprivate.cpp \
>         ../WebKit/gtk/webkit/webkitwebbackforwardlist.cpp \
>Index: WebCore/platform/network/ResourceHandle.h
>===================================================================
>--- WebCore/platform/network/ResourceHandle.h	(revision 30340)
>+++ WebCore/platform/network/ResourceHandle.h	(working copy)
>@@ -116,6 +116,12 @@ public:
>     static void setHostAllowsAnyHTTPSCertificate(const String&);
>     static void setClientCertificate(const String& host, CFDataRef);
> #endif
>+#if PLATFORM(GTK)
>+    void didReceiveAuthenticationChallenge(const AuthenticationChallenge&);
>+    void receivedCredential(const AuthenticationChallenge&, const Credential&);
>+    void receivedRequestToContinueWithoutCredential(const AuthenticationChallenge&);
>+    void receivedCancellation(const AuthenticationChallenge&);
>+#endif

^ Don't copy this block, just add GTK to the existing block eg.

#if PLATFORM(MAC) || USE(CFNETWORK) || PLATFORM(GTK)



>     PassRefPtr<SharedBuffer> bufferedData();
>     static bool supportsBufferedData();
>     
>Index: WebCore/platform/network/ResourceHandleInternal.h
>===================================================================
>--- WebCore/platform/network/ResourceHandleInternal.h	(revision 30340)
>+++ WebCore/platform/network/ResourceHandleInternal.h	(working copy)
>@@ -101,6 +101,8 @@ namespace WebCore {
>             , m_file(0)
>             , m_formDataElementIndex(0)
>             , m_formDataElementDataOffset(0)
>+            , m_webChallenge()
>+            , m_continueWithoutCredential(false)
> #endif
> #if PLATFORM(QT)
>             , m_job(0)
>@@ -159,6 +161,9 @@ namespace WebCore {
>         size_t m_formDataElementIndex;
>         size_t m_formDataElementDataOffset;
>         Vector<char> m_postBytes;
>+
>+        AuthenticationChallenge m_webChallenge;
>+        bool m_continueWithoutCredential;
> #endif
> #if PLATFORM(QT)
> #if QT_VERSION < 0x040400
>Index: WebCore/platform/network/curl/ResourceHandleCurl.cpp
>===================================================================
>--- WebCore/platform/network/curl/ResourceHandleCurl.cpp	(revision 30340)
>+++ WebCore/platform/network/curl/ResourceHandleCurl.cpp	(working copy)
>@@ -95,4 +95,69 @@ void ResourceHandle::loadResourceSynchro
>     notImplemented();
> }
> 
>+void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChallenge& challenge)
>+{
>+    /* FIXME: add support for FTP, HTTPS, and FTPS authentication,
>+     * get the right port, server and authentication scheme
>+     */
>+    ResourceHandleInternal* d = getInternal();
>+    if (d->m_continueWithoutCredential)
>+        return;
>+    cancel();
>+
>+    ProtectionSpace protectionSpace;
>+    unsigned previousFailureCount = 0;
>+    if (challenge.isNull()) {
>+        protectionSpace = ProtectionSpace(d->m_response.httpHeaderField("Host"),
>+                                          80,
>+                                          ProtectionSpaceServerHTTP,
>+                                          d->m_response.httpHeaderField("Realm"),
>+                                          ProtectionSpaceAuthenticationSchemeDefault);
>+    } else {
>+        protectionSpace = d->m_webChallenge.protectionSpace();
>+        previousFailureCount = d->m_webChallenge.previousFailureCount()+1;
>+    }
>+    
>+    d->m_webChallenge = AuthenticationChallenge(protectionSpace,
>+                                                challenge.proposedCredential(),
>+                                                previousFailureCount,
>+                                                d->m_response,
>+                                                challenge.error());
>+
>+    if (d->client())
>+        d->client()->didReceiveAuthenticationChallenge(this, d->m_webChallenge);
>+}
>+
>+void ResourceHandle::receivedCredential(const AuthenticationChallenge& challenge, const Credential& credential)
>+{
>+    ASSERT(!challenge.isNull());   
>+    ResourceHandleInternal* d = getInternal();
>+    if (challenge != d->m_webChallenge)
>+        return;
>+
>+    d->m_webChallenge = AuthenticationChallenge(challenge.protectionSpace(),
>+                                                credential,
>+                                                challenge.previousFailureCount(),
>+                                                challenge.failureResponse(),
>+                                                challenge.error());
>+
>+    ResourceHandleManager::sharedInstance()->add(this);
>+}
>+
>+void ResourceHandle::receivedRequestToContinueWithoutCredential(const AuthenticationChallenge& challenge)
>+{
>+    ASSERT(!challenge.isNull());   
>+    ResourceHandleInternal* d = getInternal();
>+    if (challenge != d->m_webChallenge)
>+        return;
>+
>+    d->m_continueWithoutCredential = true;
>+    ResourceHandleManager::sharedInstance()->add(this);
>+}
>+
>+void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challenge)
>+{
>+    notImplemented();
>+}
>+
> } // namespace WebCore
>Index: WebCore/platform/network/curl/ResourceHandleManager.cpp
>===================================================================
>--- WebCore/platform/network/curl/ResourceHandleManager.cpp	(revision 30340)
>+++ WebCore/platform/network/curl/ResourceHandleManager.cpp	(working copy)
>@@ -168,6 +168,10 @@ static size_t headerCallback(char* ptr, 
>         d->m_response.setTextEncodingName(extractCharsetFromMediaType(d->m_response.httpHeaderField("Content-Type")));
>         d->m_response.setSuggestedFilename(filenameFromHTTPContentDisposition(d->m_response.httpHeaderField("Content-Disposition")));
> 
>+        // HTTP authentication
>+        if (httpCode == 401 && d->m_response.isHTTP())
>+            job->didReceiveAuthenticationChallenge(d->m_webChallenge);
>+
>         // HTTP redirection
>         if (httpCode >= 300 && httpCode < 400) {
>             String location = d->m_response.httpHeaderField("location");
>@@ -546,6 +550,9 @@ void ResourceHandleManager::startJob(Res
>     }
> 
>     d->m_handle = curl_easy_init();
>+    // HTTP Authentication cancels the job so be sure the new job is not cancelled
>+    d->m_cancelled = false;  
>+
> #ifndef NDEBUG
>     if (getenv("DEBUG_CURL"))
>         curl_easy_setopt(d->m_handle, CURLOPT_VERBOSE, 1);
>@@ -575,6 +582,23 @@ void ResourceHandleManager::startJob(Res
>         curl_easy_setopt(d->m_handle, CURLOPT_COOKIEJAR, m_cookieJarFileName);
>     }
> 
>+    if (!d->m_webChallenge.isNull() && !d->m_continueWithoutCredential) {
>+        long curlAuth;
>+        switch(d->m_webChallenge.protectionSpace().authenticationScheme()) {          
>+        case ProtectionSpaceAuthenticationSchemeHTTPBasic:
>+        case ProtectionSpaceAuthenticationSchemeDefault:
>+        default:
>+            curlAuth = CURLAUTH_BASIC;
>+            break;
>+        }
>+
>+        Credential credential = d->m_webChallenge.proposedCredential();
>+        String userpwd = credential.user() + ":" + credential.password();
>+        
>+        curl_easy_setopt(d->m_handle, CURLOPT_HTTPAUTH, curlAuth);
>+        curl_easy_setopt(d->m_handle, CURLOPT_USERPWD, strdup(userpwd.utf8().data()));
>+    }
>+
>     struct curl_slist* headers = 0;
>     if (job->request().httpHeaderFields().size() > 0) {
>         HTTPHeaderMap customHeaders = job->request().httpHeaderFields();
>Index: WebKit/gtk/ChangeLog
>===================================================================
>--- WebKit/gtk/ChangeLog	(revision 30341)
>+++ WebKit/gtk/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>[phone]  Luca Bruno  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        http://bugs.webkit.org/show_bug.cgi?id=16947
>+        Bug 16947: [GTK] Missing HTTP Auth challenge
>+
>+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
>+        (WebKit::FrameLoaderClient::dispatchDidReceiveAuthenticationChallenge): implemented
>+        * webkit/webkitdefines.h:
>+        * webkit/webkitprivate.h:
>+        * webkit/webkitwebview.cpp: add authentication-requested signal and create auth dialog
>+        * webkit/webkitwebview.h:
>+        * webkit/webkitauthenticationchallenge.cpp: added
>+        * webkit/webkitauthenticationchallenge.h: added
>+
> 2008-02-15  Alp Toker  <[e-mail]>
> 
>         Fix the GTK+ build following breakage introduced in r30243.
>Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
>===================================================================
>--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(revision 30340)
>+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(working copy)
>@@ -30,6 +30,7 @@
> #include "HTMLFrameOwnerElement.h"
> #include "HTMLNames.h"
> #include "Language.h"
>+#include "MainResourceLoader.h"
> #include "MIMETypeRegistry.h"
> #include "NotImplemented.h"
> #include "PlatformString.h"
>@@ -39,6 +40,7 @@
> #include "kjs_binding.h"
> #include "kjs_proxy.h"
> #include "kjs_window.h"
>+#include "webkitauthenticationchallenge.h"
> #include "webkitwebview.h"
> #include "webkitwebframe.h"
> #include "webkitprivate.h"
>@@ -165,9 +167,24 @@ void FrameLoaderClient::committedLoad(Do
>     fl->addData(data, length);
> }
> 
>-void FrameLoaderClient::dispatchDidReceiveAuthenticationChallenge(DocumentLoader*, unsigned long  identifier, const AuthenticationChallenge&)
>+void FrameLoaderClient::dispatchDidReceiveAuthenticationChallenge(DocumentLoader* documentLoader, unsigned long identifier, const AuthenticationChallenge& webChallenge)
> {
>-    notImplemented();
>+    ResourceHandle* handle = documentLoader->mainResourceLoader()->handle();
>+    ASSERT(handle);
>+    if (!handle)
>+        return;
>+    
>+    WebKitAuthenticationChallenge* challenge = webkit_authentication_challenge_new();
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+    *challengePrivate->webChallenge = webChallenge;
>+    challengePrivate->webHandle = handle;
>+    
>+    Credential credential = webChallenge.proposedCredential();
>+    webkit_authentication_challenge_set_user(challenge, credential.user().utf8().data());
>+    webkit_authentication_challenge_set_password(challenge, credential.password().utf8().data());
>+    
>+    WebKitWebView* webView = getViewFromFrame(m_frame);
>+    g_signal_emit_by_name(webView, "authentication-requested", m_frame, challenge, NULL);
> }
> 
> void FrameLoaderClient::dispatchDidCancelAuthenticationChallenge(DocumentLoader*, unsigned long  identifier, const AuthenticationChallenge&)
>Index: WebKit/gtk/webkit/webkitauthenticationchallenge.cpp
>===================================================================
>--- WebKit/gtk/webkit/webkitauthenticationchallenge.cpp	(revision 0)
>+++ WebKit/gtk/webkit/webkitauthenticationchallenge.cpp	(revision 0)
>@@ -0,0 +1,138 @@
>+/*
>+ * Copyright (C) 2008 Luca Bruno
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public License
>+ * along with this library; see the file COPYING.LIB.  If not, write to
>+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>+ * Boston, MA 02110-1301, USA.
>+ */
>+
>+#include "config.h"
>+
>+#include "webkitauthenticationchallenge.h"
>+#include "webkitprivate.h"
>+
>+#include "Credential.h"
>+#include "ResourceHandle.h"
>+
>+using namespace WebKit;
>+using namespace WebCore;
>+
>+extern "C" {
>+
>+G_DEFINE_TYPE(WebKitAuthenticationChallenge, webkit_authentication_challenge, G_TYPE_OBJECT);
>+
>+static void webkit_authentication_challenge_finalize(GObject* object)
>+{
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(object);
>+
>+    delete challengePrivate->webChallenge;
>+    g_free(challengePrivate->user);
>+    g_free(challengePrivate->password);
>+
>+    G_OBJECT_CLASS(webkit_authentication_challenge_parent_class)->finalize(object);
>+}
>+
>+static void webkit_authentication_challenge_class_init(WebKitAuthenticationChallengeClass* challengeClass)
>+{
>+    g_type_class_add_private(challengeClass, sizeof(WebKitAuthenticationChallengePrivate));
>+
>+    G_OBJECT_CLASS(challengeClass)->finalize = webkit_authentication_challenge_finalize;
>+}
>+
>+static void webkit_authentication_challenge_init(WebKitAuthenticationChallenge* challenge)

^ A better name (by our conventions so far) for this would be WebKitWebAuthChallenge. You'll need to change the filenames etc. to match this.



>+{
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+
>+    challengePrivate->webChallenge = new AuthenticationChallenge();
>+    challengePrivate->webHandle = NULL;
>+    challengePrivate->user = NULL;
>+    challengePrivate->password = NULL;
>+}
>+
>+WebKitAuthenticationChallenge* webkit_authentication_challenge_new()
>+{
>+    WebKitAuthenticationChallenge* challenge = WEBKIT_AUTHENTICATION_CHALLENGE(g_object_new(WEBKIT_TYPE_AUTHENTICATION_CHALLENGE, NULL));
>+
>+    return challenge;
>+}
>+
>+void webkit_authentication_challenge_set_user(WebKitAuthenticationChallenge* challenge, const gchar* user)
>+{
>+    g_return_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge));
>+    g_return_if_fail(user);
>+    
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+
>+    g_free(challengePrivate->user);
>+    challengePrivate->user = g_strdup(user);
>+}
>+
>+const gchar* webkit_authentication_challenge_get_user(WebKitAuthenticationChallenge* challenge)
>+{
>+    g_return_val_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge), NULL);
>+
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+
>+    return challengePrivate->user;
>+}
>+
>+void webkit_authentication_challenge_set_password(WebKitAuthenticationChallenge* challenge, const gchar* password)
>+{
>+    g_return_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge));
>+    g_return_if_fail(password);
>+    
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+
>+    g_free(challengePrivate->password);
>+    challengePrivate->password = g_strdup(password);
>+}
>+
>+const gchar* webkit_authentication_challenge_get_password(WebKitAuthenticationChallenge* challenge)
>+{
>+    g_return_val_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge), NULL);
>+
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);

^ We changed coding style when you were gone :-) We now use a direct struct reference to ->priv. You'll need to take a look at the other classes to see how it's done and update this patch.


>+
>+    return challengePrivate->password;
>+}
>+
>+void webkit_authentication_challenge_use_credential(WebKitAuthenticationChallenge* challenge)
>+{
>+    g_return_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge));
>+
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+    Credential credential = Credential(challengePrivate->user,
>+                                       challengePrivate->password,
>+                                       CredentialPersistenceForSession);

^ Careful, you need to use String::fromUTF8() here and for every other string conversion in this patch. I filed Bug #17432 to get this harmful implicit conversion removed, but till then you need to be vigilant.



>+    challengePrivate->webHandle->receivedCredential(*challengePrivate->webChallenge, credential);
>+}
>+
>+void webkit_authentication_challenge_continue_without_credential(WebKitAuthenticationChallenge* challenge)
>+{
>+    g_return_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge));
>+
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+    challengePrivate->webHandle->receivedRequestToContinueWithoutCredential(*challengePrivate->webChallenge);
>+}
>+
>+void webkit_authentication_challenge_cancel(WebKitAuthenticationChallenge* challenge)
>+{
>+    g_return_if_fail(WEBKIT_IS_AUTHENTICATION_CHALLENGE(challenge));
>+
>+    WebKitAuthenticationChallengePrivate* challengePrivate = WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(challenge);
>+
>+    challengePrivate->webHandle->receivedCancellation(*challengePrivate->webChallenge);
>+}
>+
>+}
>Index: WebKit/gtk/webkit/webkitauthenticationchallenge.h
>===================================================================
>--- WebKit/gtk/webkit/webkitauthenticationchallenge.h	(revision 0)
>+++ WebKit/gtk/webkit/webkitauthenticationchallenge.h	(revision 0)
>@@ -0,0 +1,73 @@
>+/*
>+ * Copyright (C) 2008 Luca Bruno
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public License
>+ * along with this library; see the file COPYING.LIB.  If not, write to
>+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>+ * Boston, MA 02110-1301, USA.
>+ */
>+
>+#ifndef WEBKIT_AUTHENTICATION_CHALLENGE_H
>+#define WEBKIT_AUTHENTICATION_CHALLENGE_H
>+
>+#include <glib-object.h>
>+
>+#include "webkitdefines.h"
>+
>+G_BEGIN_DECLS
>+
>+#define WEBKIT_TYPE_AUTHENTICATION_CHALLENGE            (webkit_authentication_challenge_get_type())
>+#define WEBKIT_AUTHENTICATION_CHALLENGE(obj)             (G_TYPE_CHECK_INSTANCE_CAST((obj), WEBKIT_TYPE_AUTHENTICATION_CHALLENGE, WebKitAuthenticationChallenge))
>+#define WEBKIT_AUTHENTICATION_CHALLENGE_CLASS(klass)     (G_TYPE_CHECK_CLASS_CAST((klass),  WEBKIT_TYPE_AUTHENTICATION_CHALLENGE, WebKitAuthenticationChallengeClass))
>+#define WEBKIT_IS_AUTHENTICATION_CHALLENGE(obj)         (G_TYPE_CHECK_INSTANCE_TYPE((obj), WEBKIT_TYPE_AUTHENTICATION_CHALLENGE))
>+#define WEBKIT_IS_AUTHENTICATION_CHALLENGE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass),  WEBKIT_TYPE_AUTHENTICATION_CHALLENGE))
>+#define WEBKIT_AUTHENTICATION_CHALLENGE_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS((obj),  WEBKIT_TYPE_AUTHENTICATION_CHALLENGE, WebKitAuthenticationChallengeClass))
>+
>+struct _WebKitAuthenticationChallenge {
>+    GObject parent;
>+};
>+
>+struct _WebKitAuthenticationChallengeClass {
>+    GObjectClass parent;
>+};
>+
>+WEBKIT_API GType
>+webkit_authentication_challenge_get_type (void);
>+
>+WEBKIT_API WebKitAuthenticationChallenge*
>+webkit_authentication_challenge_new ();
>+
>+WEBKIT_API void
>+webkit_authentication_challenge_set_user (WebKitAuthenticationChallenge* challenge, const gchar* user);
>+
>+WEBKIT_API const gchar*
>+webkit_authentication_challenge_get_user (WebKitAuthenticationChallenge* challenge);
>+
>+WEBKIT_API void
>+webkit_authentication_challenge_set_password (WebKitAuthenticationChallenge* challenge, const gchar* password);
>+
>+WEBKIT_API const gchar*
>+webkit_authentication_challenge_get_password (WebKitAuthenticationChallenge* challenge);
>+
>+WEBKIT_API void
>+webkit_authentication_challenge_use_credential (WebKitAuthenticationChallenge* challenge);
>+
>+WEBKIT_API void
>+webkit_authentication_challenge_continue_without_credential (WebKitAuthenticationChallenge* challenge);
>+
>+WEBKIT_API void
>+webkit_authentication_challenge_cancel (WebKitAuthenticationChallenge* challenge);
>+
>+G_END_DECLS
>+
>+#endif
>Index: WebKit/gtk/webkit/webkitdefines.h
>===================================================================
>--- WebKit/gtk/webkit/webkitdefines.h	(revision 30340)
>+++ WebKit/gtk/webkit/webkitdefines.h	(working copy)
>@@ -40,6 +40,9 @@
> 
> G_BEGIN_DECLS
> 
>+typedef struct _WebKitAuthenticationChallenge WebKitAuthenticationChallenge;
>+typedef struct _WebKitAuthenticationChallengeClass WebKitAuthenticationChallengeClass;
>+
> typedef struct _WebKitNetworkRequest WebKitNetworkRequest;
> typedef struct _WebKitNetworkRequestClass WebKitNetworkRequestClass;
> 
>Index: WebKit/gtk/webkit/webkitprivate.h
>===================================================================
>--- WebKit/gtk/webkit/webkitprivate.h	(revision 30340)
>+++ WebKit/gtk/webkit/webkitprivate.h	(working copy)
>@@ -26,6 +26,7 @@
>  * They are using WebCore which musn't be exposed to the outer world.
>  */
> 
>+#include <webkit/webkitauthenticationchallenge.h>
> #include <webkit/webkitdefines.h>
> #include <webkit/webkitwebview.h>
> #include <webkit/webkitwebframe.h>
>@@ -33,6 +34,7 @@
> #include <webkit/webkitnetworkrequest.h>
> #include <webkit/webkitwebbackforwardlist.h>
> 
>+#include "AuthenticationChallenge.h"
> #include "BackForwardList.h"
> #include "HistoryItem.h"
> #include "Settings.h"
>@@ -103,6 +105,15 @@ extern "C" {
>     struct _WebKitNetworkRequestPrivate {
>         gchar* uri;
>     };
>+ 
>+    #define WEBKIT_AUTHENTICATION_CHALLENGE_GET_PRIVATE(obj)    (G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_AUTHENTICATION_CHALLENGE, WebKitAuthenticationChallengePrivate))
>+    typedef struct _WebKitAuthenticationChallengePrivate WebKitAuthenticationChallengePrivate;
>+    struct _WebKitAuthenticationChallengePrivate {
>+        WebCore::AuthenticationChallenge* webChallenge;
>+        WebCore::ResourceHandle* webHandle;
>+        gchar* user;
>+        gchar* password;
>+    };
> 
>     WebKitWebFrame*
>     webkit_web_frame_init_with_web_view(WebKitWebView*, WebCore::HTMLFrameOwnerElement*);
>Index: WebKit/gtk/webkit/webkitwebview.cpp
>===================================================================
>--- WebKit/gtk/webkit/webkitwebview.cpp	(revision 30340)
>+++ WebKit/gtk/webkit/webkitwebview.cpp	(working copy)
>@@ -85,6 +85,7 @@ enum {
>     COPY_CLIPBOARD,
>     PASTE_CLIPBOARD,
>     CUT_CLIPBOARD,
>+    AUTHENTICATION_REQUESTED,
>     LAST_SIGNAL
> };
> 
>@@ -379,7 +380,7 @@ static gboolean webkit_web_view_focus_in
>     GtkWidget* toplevel = gtk_widget_get_toplevel(widget);
>     if (GTK_WIDGET_TOPLEVEL(toplevel) && gtk_window_has_toplevel_focus(GTK_WINDOW(toplevel))) {
>         WebKitWebView* webView = WEBKIT_WEB_VIEW(widget);
>-
>+        
>         Frame* frame = core(webView)->mainFrame();
>         core(webView)->focusController()->setActive(frame);
>     }
>@@ -605,6 +606,82 @@ static void webkit_web_view_real_paste_c
>     frame->editor()->command("Paste").execute();
> }
> 
>+typedef struct
>+{
>+    GtkWidget* user_entry;
>+    GtkWidget* password_entry;
>+    WebKitAuthenticationChallenge* challenge;
>+} AuthenticationDialogData;
>+
>+static void webkit_web_view_authentication_dialog_response_cb(GtkWidget* dialog, gint response, gpointer user_data)
>+{
>+    AuthenticationDialogData* data = reinterpret_cast<AuthenticationDialogData*>(user_data);
>+
>+    switch (response) {
>+    case GTK_RESPONSE_NONE:
>+    case GTK_RESPONSE_DELETE_EVENT:
>+    case GTK_RESPONSE_REJECT:
>+        webkit_authentication_challenge_continue_without_credential(data->challenge);
>+        break;
>+    case GTK_RESPONSE_ACCEPT:
>+        webkit_authentication_challenge_set_user(data->challenge,
>+                                                 gtk_entry_get_text(GTK_ENTRY(data->user_entry)));
>+        webkit_authentication_challenge_set_password(data->challenge,
>+                                                     gtk_entry_get_text(GTK_ENTRY(data->password_entry)));
>+        webkit_authentication_challenge_use_credential(data->challenge);
>+        break;
>+    default:
>+        g_assert_not_reached ();
>+        break;
>+    }
>+
>+    gtk_widget_destroy(dialog);
>+    g_object_unref(data->challenge);
>+}
>+
>+static void webkit_web_view_real_authentication_requested(WebKitWebView* webView, WebKitWebFrame* webFrame,
>+                                                          WebKitAuthenticationChallenge* challenge)
>+{
>+    GtkWidget* window;
>+    GtkWidget* dialog;
>+    GtkWidget* table;
>+    GtkWidget* label;
>+    AuthenticationDialogData* data;
>+
>+    window = gtk_widget_get_toplevel(GTK_WIDGET(webView));
>+    gchar* title = g_strconcat("Authentication - ", webkit_web_frame_get_uri(webFrame), NULL);
>+    dialog = gtk_dialog_new_with_buttons(title, GTK_WIDGET_TOPLEVEL(window) ? GTK_WINDOW(window) : 0,
>+                                         (GtkDialogFlags)(GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT),
>+                                         GTK_STOCK_OK, GTK_RESPONSE_ACCEPT,
>+                                         GTK_STOCK_CANCEL, GTK_RESPONSE_REJECT,
>+                                         NULL);
>+    g_free(title);
>+    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_REJECT);
>+
>+    data = g_new(AuthenticationDialogData, 1);
>+    data->user_entry = gtk_entry_new();
>+    data->password_entry = gtk_entry_new();
>+    gtk_entry_set_visibility(GTK_ENTRY(data->password_entry), FALSE);
>+    data->challenge = challenge;
>+    g_signal_connect(dialog, "response", G_CALLBACK(webkit_web_view_authentication_dialog_response_cb), data);
>+
>+    table = gtk_table_new(2, 2, FALSE);
>+    gtk_table_set_col_spacings(GTK_TABLE(table), 3);
>+    gtk_table_set_row_spacings(GTK_TABLE(table), 3);
>+
>+    label = gtk_label_new("Username:");
>+    gtk_table_attach_defaults(GTK_TABLE(table), label, 0, 1, 0, 1);
>+    gtk_table_attach_defaults(GTK_TABLE(table), data->user_entry, 1, 2, 0, 1);
>+
>+    label = gtk_label_new("Password:");
>+    gtk_table_attach_defaults(GTK_TABLE(table), label, 0, 1, 1, 2);
>+    gtk_table_attach_defaults(GTK_TABLE(table), data->password_entry, 1, 2, 1, 2);
>+
>+    gtk_container_add(GTK_CONTAINER(GTK_DIALOG(dialog)->vbox), table);
>+    gtk_widget_show_all(table);
>+    gtk_widget_show(dialog);
>+}
>+
> static void webkit_web_view_finalize(GObject* object)
> {
>     WebKitWebView* webView = WEBKIT_WEB_VIEW(object);
>@@ -971,6 +1048,25 @@ static void webkit_web_view_class_init(W
>             g_cclosure_marshal_VOID__VOID,
>             G_TYPE_NONE, 0);
> 
>+    /**
>+     * WebKitWebView::authentication-requested:
>+     * @web_view: the object which received the signal
>+     *
>+     * The ::authentication-requested signal is emitted when an authentication is
>+     * requested from the server to access a resource.
>+     *
>+     * The default behavior for this signal is creating an input dialog to get a suitable credential.
>+     */
>+    webkit_web_view_signals[AUTHENTICATION_REQUESTED] = g_signal_new("authentication-requested",
>+            G_TYPE_FROM_CLASS(webViewClass),
>+            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+            G_STRUCT_OFFSET(WebKitWebViewClass, authentication_requested),
>+            NULL, NULL,
>+            webkit_marshal_VOID__OBJECT_OBJECT,
>+            G_TYPE_NONE, 2,
>+            WEBKIT_TYPE_WEB_FRAME,
>+            WEBKIT_TYPE_AUTHENTICATION_CHALLENGE);
>+
>     /*
>      * implementations of virtual methods
>      */
>@@ -986,6 +1082,7 @@ static void webkit_web_view_class_init(W
>     webViewClass->cut_clipboard = webkit_web_view_real_cut_clipboard;
>     webViewClass->copy_clipboard = webkit_web_view_real_copy_clipboard;
>     webViewClass->paste_clipboard = webkit_web_view_real_paste_clipboard;
>+    webViewClass->authentication_requested = webkit_web_view_real_authentication_requested;
> 
>     GObjectClass* objectClass = G_OBJECT_CLASS(webViewClass);
>     objectClass->finalize = webkit_web_view_finalize;
>Index: WebKit/gtk/webkit/webkitwebview.h
>===================================================================
>--- WebKit/gtk/webkit/webkitwebview.h	(revision 30340)
>+++ WebKit/gtk/webkit/webkitwebview.h	(working copy)
>@@ -83,6 +83,7 @@ struct _WebKitWebViewClass {
>     void (*cut_clipboard) (WebKitWebView* web_view);
>     void (*copy_clipboard) (WebKitWebView* web_view);
>     void (*paste_clipboard) (WebKitWebView* web_view);
>+    void (*authentication_requested) (WebKitWebView* web_view, WebKitWebFrame* web_frame, WebKitAuthenticationChallenge* authentication_challenge);
> 
>     /*
>      * internal




I think apart from these issues the patch is good to go. Looking forward to seeing the revised patch.

[Luca Bruno]

Created attachment 19213 [details]
fix style issues

Fixed style issues hopefully well according to your suggestions.

[Jan Alonzo]

(In reply to comment #6)
> Created an attachment (id=19213) [edit]
> fix style issues
> 
> Fixed style issues hopefully well according to your suggestions.
> 

Hi Luca

Is it possible to have some API documentation before this gets committed?

cheers

[Luca Bruno]

(In reply to comment #7)
> Hi Luca
> 
> Is it possible to have some API documentation before this gets committed?
> 
> cheers

Ops, of course ;) i'll also add some gobject properties. 

[Luca Bruno]

Created attachment 19279 [details]
add comments

I commented the API, more comments needed?
Also added gobject properties for user and password.

[Luca Bruno]

Created attachment 19305 [details]
fail on response fired

When the response has been fired the application shouldn't be able to fire it again for networking matters. So i added a priv->response_fired variable to be checked. With this latest change this patch should hopefully be complete.

[Jan Alonzo]

(In reply to comment #10)
> Created an attachment (id=19305) [edit]
> fail on response fired
> 
> When the response has been fired the application shouldn't be able to fire it
> again for networking matters. So i added a priv->response_fired variable to be
> checked. With this latest change this patch should hopefully be complete.

Hi Lethalman

Some issues:

1. There are still instances where the dialog pops up at least twice (e.g, trying to log to my router pops up the dialog twice).

2. the table container shouldn't expand when the window resizes. Ditto with the username/password labels.

3. The OK/Cancel button should be Cancel/Ok for consistency with gtk/gnome apps.

4. The dialog window title says "Authentication - http://www.google.com" even though the domain i'm logging to is not google's? Maybe a different bug but fyi.

5. Also, should we add a title header with more info about the site we're logging into (like FF for example)?

That's all. Good work so far!

Cheers

[Luca Bruno]

> 1. There are still instances where the dialog pops up at least twice (e.g,
> trying to log to my router pops up the dialog twice).

Is it an HTTP Basic Auth? Can you give me more informations?

[Luca Bruno]

Created attachment 19324 [details]
keep the source handle

A slightly better patch which keeps track of the source handle directly in the AuthenticationChallenge.

The problem with the authentication that's requested twice is the lack of a cache for the credentials.
What we need is to specify the credential for each web resource.
But the credential is specified by the application. Should we keep the credential for each host or this is an end-application job?

[Tommi Komulainen]

Created attachment 19735 [details]
CURL: Basic Authentication support

The previous patch didn't apply to trunk so I updated it to what I think matches the current state. Quickly testing it seems to work.

[Tommi Komulainen]

Created attachment 19737 [details]
Soup: HTTP authentication

This is pretty much a direct copy of the CURL implementation, posting the patch in hopes someone finds it useful.

I'm not happy with the way SoupAuth* object is being stored. IMO it should be kept along with AuthenticationChallenge, but I ended up getting one g_object_unref() too many when trying to keep a ref'd pointer.

[Luca Ferretti]

There are other GNOME/GTK+ HIG "violations"

>+    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_REJECT);

Some missing calls:
  gtk_dialog_set_has_separator(GTK_DIALOG(dialog), FALSE);
  gtk_container_set_border_width (GTK_CONTAINER (dialog), 5);
  gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog)->vbox), 2);

those (plus the other _set_border_width below) are needed to ensure 12 pixels spacing between inner widgets and dialog border and between the button area

>+    table = gtk_table_new(2, 2, FALSE);
>+    gtk_table_set_col_spacings(GTK_TABLE(table), 3);
>+    gtk_table_set_row_spacings(GTK_TABLE(table), 3);

col_spacings should be 12 pixels, row_spacings should be 6 pixels.

Plus add
  gtk_container_set_border_width (GTK_CONTAINER(table), 5);

>+    label = gtk_label_new("Username:");
<snip>
>+    label = gtk_label_new("Password:");

missing accelerators ("_Username:" and "_Password:") and missing l10n/i18n

[Szabolcs Frühwald]

Hi all,


 first of all, thank you for your work. We need this function working asap, but this patch is really outdated. Sources under WebCore/* are really changed in current trunk. I just ask you Luca or anybody else to update this patch to make it working with current trunk.

It would be nice if this time the patch would get reviewed and committed!!

Thanks, i hope you guys can resurrect this FIX!

[Gustavo Noronha (kov)]

(In reply to comment #15)
> Created an attachment (id=19737) [review]
> Soup: HTTP authentication
> 
> This is pretty much a direct copy of the CURL implementation, posting the patch
> in hopes someone finds it useful.
> 
> I'm not happy with the way SoupAuth* object is being stored. IMO it should be
> kept along with AuthenticationChallenge, but I ended up getting one
> g_object_unref() too many when trying to keep a ref'd pointer.
> 

It seems to me like you forgot to add the AuthenticationChallengeSoup.cpp file.

[Christian Dywan]

Do we actually need this feature in this form anymore now that the plan is to expose libSoup?

Maybe it would actually be more useful at this point to put a dialogue and login storage in a libSoupGtk package or something similar. That way any HTTP client based on Gtk could use the same interface.

[Xan Lopez]

Created attachment 27591 [details]
keyring.patch

Since we are going to only support soup in the GTK+ backend, this patchs adds HTTP auth the simple way: connect to the 'authenticate' signal in the session, and use gnome keyring for the credentials storage. It's optionally enabled with --enable-gnomekeyring for those who don't want to use it for some reason.

[Xan Lopez]

Forgot to say, you *need* gnome-keyring trunk, because was a bug that prevented from using it with any C++ compiler. So we should depend on 2.26 when it's out...

[Gustavo Noronha (kov)]

Comment on attachment 19737 [details]
Soup: HTTP authentication

obsoleted by xan's patch

[Gustavo Noronha (kov)]

(In reply to comment #20)
> Created an attachment (id=27591) [review]
> keyring.patch
> 
> Since we are going to only support soup in the GTK+ backend, this patchs adds
> HTTP auth the simple way: connect to the 'authenticate' signal in the session,
> and use gnome keyring for the credentials storage. It's optionally enabled with
> --enable-gnomekeyring for those who don't want to use it for some reason.
> 

So that we can remember the things I pointed out: we probably want to take advantage of Frame being passed to ResourceHandle::start and store the frame, or even the WebKitWebView in ResourceHandleInternal, so that we can get the toplevel for the dialog. Qt seems to deal with this in a similar way. Notice that we will have a frameless jobs when Download lands. I wonder if we should provide some kind of API to WebKitDownload, to specify the parent window, for that case.

[Xan Lopez]

Created attachment 27698 [details]
authdialog.patch

Create a WebKitSoupAuthDialog soup session feature with the UI and, optionally, keyring storage of credentiales. If keyring is not enabled the feature is still usable, but obviously nothing will be remembered across sessions. Also, as this is a feature, it will be possible to completely disable it by users when the get_session patch lands.

[Gustavo Noronha (kov)]

I'm playing with the patch here, and I like it. Some nitpicks:

>  8 files changed, 391 insertions(+), 2 deletions(-)
>  create mode 100644 WebCore/platform/network/soup/webkit-soup-auth-dialog.c
>  create mode 100644 WebCore/platform/network/soup/webkit-soup-auth-dialog.h

I believe you will want to rename these so that they match WebCore's naming convention.

> +    gtk_window_set_default_size(GTK_WINDOW(dialog), 100, 50);
> +    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_ACCEPT);
> +                                         

This last line, and some more in this file have unwanted trailing whitespaces. Since I know you're an Emacs user and enjoys learning new tricks I'm gonna say set show-trailing-whitespace to t and/or M-x delete-trailing-whitespace =)

[Dan Winship]

> +static void session_authenticate(SoupSession* session, SoupMessage* msg, SoupAuth* auth, gboolean retrying)

you're missing ", gpointer user_data" at the end

[Gustavo Noronha (kov)]

Created attachment 27704 [details]
add parent window information to the auth dialog

I believe we could do something like this to pass the parent window to the dialog.

[Xan Lopez]

(In reply to comment #25)
> I'm playing with the patch here, and I like it. Some nitpicks:
> 
> >  8 files changed, 391 insertions(+), 2 deletions(-)
> >  create mode 100644 WebCore/platform/network/soup/webkit-soup-auth-dialog.c
> >  create mode 100644 WebCore/platform/network/soup/webkit-soup-auth-dialog.h
> 
> I believe you will want to rename these so that they match WebCore's naming
> convention.

Well, I've done this on purpose, because they are pure C files (and hence .c and not .cpp), but I don't really care either way.

> 
> > +    gtk_window_set_default_size(GTK_WINDOW(dialog), 100, 50);
> > +    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_ACCEPT);
> > +                                         
> 
> This last line, and some more in this file have unwanted trailing whitespaces.
> Since I know you're an Emacs user and enjoys learning new tricks I'm gonna say
> set show-trailing-whitespace to t and/or M-x delete-trailing-whitespace =)
> 

Ah, didn't know about the former, good one. Should also configure git to refuse to commit with trailing white-space, that way you can't miss it :)

[Xan Lopez]

(In reply to comment #26)
> > +static void session_authenticate(SoupSession* session, SoupMessage* msg, SoupAuth* auth, gboolean retrying)
> 
> you're missing ", gpointer user_data" at the end
> 

Yes, although leaving the user_data parameter out when it's unused is kind of common I think (because of G_CALLBACK probably).

[Xan Lopez]

(In reply to comment #27)
> Created an attachment (id=27704) [review]
> add parent window information to the auth dialog
> 
> I believe we could do something like this to pass the parent window to the
> dialog.
> 

I wonder if we should just go the extra mile and provide a way for the feature to ask for the current toplevel each time 'authenticate' is fired. That way we can keep it pure C and completely independent from WebKit.

[Gustavo Noronha (kov)]

(In reply to comment #30)
> I wonder if we should just go the extra mile and provide a way for the feature
> to ask for the current toplevel each time 'authenticate' is fired. That way we
> can keep it pure C and completely independent from WebKit.

I don't know how that would be done, but I would rather keep this simple. The current code plus that change can already be easily be copied to be used elsewhere with minimal change. And we don't really need to make simple things like this too generic, IMO.

[Xan Lopez]

(In reply to comment #31)
> (In reply to comment #30)
> > I wonder if we should just go the extra mile and provide a way for the feature
> > to ask for the current toplevel each time 'authenticate' is fired. That way we
> > can keep it pure C and completely independent from WebKit.
> 
> I don't know how that would be done, but I would rather keep this simple. The
> current code plus that change can already be easily be copied to be used
> elsewhere with minimal change. And we don't really need to make simple things
> like this too generic, IMO.
> 

We just need a signal to connect from ResourceHandleSoup.cpp so we can pass the current toplevel each time it's needed. I don't think it's much more complex and it keeps the code modular. I wouldn't care too much in principle, but once we have gone this far I think it's silly to break the copy&paste-ability at the lat minute.

[Xan Lopez]

Created attachment 27721 [details]
marshal.patch

[Xan Lopez]

Created attachment 27722 [details]
keyring.patch

[Xan Lopez]

Ok, I've added a signal 'current-toplevel' so that the auth feature can ask webkit about that without messing it with internal details about WebCore/WebKit. That keeps it plain C and completely reusable.

One problem is that I needed to add a marshaller from WebCore, and the current system of generating them grepping through the source files in WebKit/gtk/ wasn't enough anymore. Instead of adding many more sources to grep (would slow down compiles a lot) or hack up some smart system to just go through the actually changed files (would slow down compiles somewhat, and needs to be implemented) I've just reverted to the standard behavior of maintaining a manual list of marshallers to generate. Adding one is a really rare situation, and it takes 5 seconds at best, so I think adding extra-compile time for this is the wrong kind of optimization.

[Gustavo Noronha (kov)]

Comment on attachment 27704 [details]
add parent window information to the auth dialog

Xan's patch now feature this functionality in a more modularized way.

[Xan Lopez]

Created attachment 27730 [details]
keyring.patch

Take UI code from GTK+ gtkmountoperation.c, follows the HIG better and the way the layout is structured is more sane.

[Xan Lopez]

Created attachment 27731 [details]
keyring.patch

Set both entries as activating the default action.

[Christian Dywan]

(In reply to comment #38)
> Created an attachment (id=27731) [review]
> keyring.patch
> 
> Set both entries as activating the default action.

The construction of the dialogue looks a little complicated after the "HIG improvements", but I trust it's worth it. Anyway I like the patch, very nice.

And I'm fine with going back to a standard marshallers file, automating that was a goodie anyway and build performance is much more important.

[Holger Freyther]

Removing curl keyword, curl is not for gtk+.

[Holger Freyther]

Comment on attachment 27721 [details]
marshal.patch

okay, looks like the file is complete...

[Holger Freyther]

Comment on attachment 27731 [details]
keyring.patch


> +        Frame* m_frame;

To use RefPtr or not to use it. Can the Frame be removed during a request? This might be a source of a crash, but we can go for wait and see.

>  
> +    // Used to set the authentication dialog toplevel; may be NULL
> +    d->m_frame = frame;
> +

maybe move the comment to the header file?


*skipped widget foo*

[Holger Freyther]

Oh, and there are some minor style issues in the auth dialog, the '*' is placed next to the variable and not to the type. this can be fixed afterwards though.

[Xan Lopez]

Created attachment 28025 [details]
keyringv2.patch

Update SoupAuthDialog to pass the current SoupMessage as a parameter in the current-toplevel signal, so we can take the toplevel from the ResourceHandle we set on each message in startHttp.

[Xan Lopez]

Created attachment 28027 [details]
keyringv3.patch

Fix some style issues.

[Holger Freyther]

Comment on attachment 28027 [details]
keyringv3.patch


> +    GtkWidget* toplevel;
> +    GtkWidget *widget;
> +    GtkDialog *dialog;
> +    GtkWindow *window;
> +    GtkWidget *entryContainer;
> +    GtkWidget *hbox, *mainVBox, *vbox, *icon;
> +    GtkWidget *table;
> +    GtkWidget *messageLabel;
> +    char* message;
> +    SoupURI* uri;
> +#if USE(GNOMEKEYRING)
> +    GtkWidget* rememberBox;
> +    GtkWidget* checkButton;
> +#endif
>

minor inconsistency... pick one, stay with it here :)

[Gustavo Noronha (kov)]

Landed last patch as r41270.

[Paanky]

(In reply to comment #14)
> Created an attachment (id=19735) [review]
> CURL: Basic Authentication support
> 
> The previous patch didn't apply to trunk so I updated it to what I think
> matches the current state. Quickly testing it seems to work.
> 

I applied the patch to r40777, with CURL. But it did not work for me. I am giving the correct username and password. But intended page is not loading. Luca, Can you please guide me? Authorization fails. I am using the latest version of CURL.