[webkit-gtk] Authentication dialog API

Carlos Garcia Campos cgarcia at igalia.com
Thu Jul 25 09:08:13 PDT 2013


El jue, 25-07-2013 a las 16:17 +0100, Brian Holt escribió:
> Hi, 
> 
> There still remains an open question about the ownership of the WebKitCredential object that could do with some discussion, but before we consider the arguments, here is some pseudocode of how I would envisage the API being used for a custom dialog:
> 
> gboolean runAuthenticateCallback(WebKitWebView*, WebKitAuthenticationRequest* request, gpointer) <- the signal handler for authenticate 
> {
> 	WebKitCredential* credential = webkit_authentication_request_get_credential(request);
> 	// use the proposed credential to populate the dialog
> }
> 
> void okButtonClicked(GtkWidget* widget, gpointer)
> {
> 	// perhaps check whether the credentials are the same as the proposed credentials
> 	WebKitAuthenticationRequest* request = priv->request;	
> 
> 	// if not the same as stored

I think you always have to create a new credential.

> 	WebKitCredential* credential = webkit_credential_new(button.username, button.password, persistence);
> 	
> 	// do the authentication
> 	webkit_authentication_request_authenticate(request, credential)
> } 
> 
> The question to discuss now is how to best design the API regarding the ownership of the WebKitCredential. I see 2 main options:
> 
> 1) The WebKitAuthenticationRequest caches the WebKitCredential and
> returns a const* in webkit_authentication_request_get_credential()
> (transfer none). The credential parameter in
> webkit_authentication_request_authenticate() is (transfer full).

For a method returning an object we can cache it and return transfer
none, but for a parameter I prefer if it's always transfer none and the
caller, who created the credential, frees it.

>  Memory is freed in (a) the request's destructor, (b) in
> authenticate() if a previous credential is present

I don't think the proposal credential object and the one provided by the
user is the same thing. and authenticate is not supposed to be called
twice, so the cached credential would only be freed in the destructor.

>  and (c) in the caller *if* the caller creates a WebKitCredential but
> doesn't pass it to authenticate(). 

This is one of the reason why I think the caller should take the
ownership of the credential provided to authenticate.

>   This case is easy from the point of view of the client, you can
> create the credential, use it to authenticate and forget about it.
> 
> 2) Do not cache the WebKitCredential.  The WebKitCredential is created
> and returned each time webkit_authentication_request_get_credential()
> (transfer full) is called and
> webkit_authentication_request_authenticate() (transfer none) does not
> free the memory.  It remains the responsibility of the client to
> always free using webkit_credential_free(). 
> 
> What do you think? 

Even if 1) looks more convenient for the user, given that this methods
are expected to be called once and the crendential object is mostly read
only and single use only too (that's why I proposed to not make it
refcounted), I think option 2) makes things consistent:

gboolean authenticateCallback()
{
    WebKitCredential *credential = webkit_authentication_request_get_credential (request);
    if (credential) {
        // Fill the user/pass using the credential.
    }
    webkit_credential_free (credential);
}

void okButtonClicked ()
{
    WebKitCredential *credential = webkit_credential_new (user, pass, persistence);
    webkit_authentication_request_authenticate (request, credential);
    webkit_credential_free (credential);
}


> Regards
> Brian
> 
> -----Original Message-----
> From: webkit-gtk-bounces at lists.webkit.org [mailto:webkit-gtk-bounces at lists.webkit.org] On Behalf Of Brian Holt
> Sent: 22 July 2013 17:08
> To: webkit-gtk at lists.webkit.org
> Subject: Re: [webkit-gtk] Authentication dialog API
> 
> Hi all, 
> 
> After some feedback and comments here is a new proposal for the API:
> 
> The request object that will be emitted along with the "authenticate" signal is a WebKitAuthenticationRequest with this public interface
> 
>     WEBKIT_API GType
>     webkit_authentication_request_get_type      (void);
> 
>     WEBKIT_API gboolean
>     webkit_authentication_request_can_save_credential  (WebKitAuthenticationRequest *request);
> 
>     WEBKIT_API WebKitCredential*
>     webkit_authentication_request_get_credential       (WebKitAuthenticationRequest *request);
> 
>     WEBKIT_API void
>     webkit_authentication_request_authenticate         (WebKitAuthenticationRequest *request,
>                                                         WebKitCredential *credential);
> 
>     WEBKIT_API void
>     webkit_authentication_request_cancel               (WebKitAuthenticationRequest *request);
> 
> webkit_authentication_request_can_save_credential() will return FALSE if private browsing is enabled or if there is no libsecret support.
> 
> 
> A new class WebKitCredential is provided to encapsulate the credential, which simplifies the API will also allow us to add support for client certificate authentication using the same API in the future.  The user can pass back the existing credential or can create a new one prior to authentication. The public API for this object would look as follows:
> 
>     WEBKIT_API GType
>     webkit_credential_get_type           (void);
> 
>     WEBKIT_API WebKitCredential *
>     webkit_credential_ref                  (WebKitCredential *credential);
> 
>     WEBKIT_API void
>     webkit_credential_unref                (WebKitCredential *credential);
> 
>     WEBKIT_API WebKitCredential *
>     webkit_credential_new                  (const gchar *username, const gchar *password, WebKitCredentialPersistence persistence);
> 
>     WEBKIT_API const gchar *
>     webkit_credential_get_username         (WebKitCredential *credential);
> 
>     WEBKIT_API const gchar *
>     webkit_credential_get_password         (WebKitCredential *credential);
> 
>     WEBKIT_API WebKitCredentialPersistence
>     webkit_credential_get_persistence      (WebKitCredential *credential);
> 
> 
> Does this seem like a reasonable approach? 
> 
> Regards
> Brian
>  
> 
> -----Original Message-----
> From: webkit-gtk-bounces at lists.webkit.org [mailto:webkit-gtk-bounces at lists.webkit.org] On Behalf Of Brian Holt
> Sent: 19 July 2013 18:03
> To: 'Carlos Garcia Campos'; webkit-gtk at lists.webkit.org
> Subject: Re: [webkit-gtk] Authentication dialog API
> 
> Hi Carlos, 
> 
> Thanks for your comments and suggestions.  I've just uploaded a second patch, renaming the signal to "authenticate" and the request class  WebKitWebViewAuthenticationRequest now has the API you suggested.
> 
> Any other feedback and comments welcome.
> 
> Regards
> Brian
> 
> -----Original Message-----
> From: webkit-gtk-bounces at lists.webkit.org [mailto:webkit-gtk-bounces at lists.webkit.org] On Behalf Of Carlos Garcia Campos
> Sent: 19 July 2013 15:49
> To: webkit-gtk at lists.webkit.org
> Subject: Re: [webkit-gtk] Authentication dialog API
> 
> El vie, 19-07-2013 a las 14:21 +0000, Brian Holt escribió:
> > Hi WebKitGtk+,
> > 
> > I'm creating the API for the Authentication Dialog (see
> > https://trac.webkit.org/wiki/WebKitGTK/WebKit2Roadmap) tracked under 
> > https://bugs.webkit.org/show_bug.cgi?id=99352.  The basic idea is that 
> > some application authors might want to bring up their own 
> > authentication dialog for http authentication, and so this work is to 
> > enable them to do so by connecting to a new signal and interacting 
> > with a new API.
> 
> Not necessarily a dialog, API users could use any auth mechanism, as long as they provide a user/pass. So I would avoid the word dialog, WebKitWebView::authenticate and WebKitWebViewAuthenticationRequest sound good to me.
> 
> > A prototype is working, now I just need to check that I've got the 
> > names and architecture right.
> > 
> > When  WebLoaderClient::didReceiveAuthenticationChallengeInFrame() is 
> > called I propose to create a new WebkitAuthenticationDialogRequest 
> > object. This class will be the public interface for the authentication 
> > request and I propose that it have the following public API:
> > 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_set_username                        (WebKitAuthenticationDialogRequest *request,
> >                                                                                                                                          
> > const gchar *username);
> > 
> > WEBKIT_API const gchar *
> > webkit_authentication_dialog_request_get_username                        (WebKitAuthenticationDialogRequest *request);
> > 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_set_password                        (WebKitAuthenticationDialogRequest *request,
> >                                                                                                                                         
> > const gchar* password);
> > 
> > WEBKIT_API const gchar *
> > webkit_authentication_dialog_request_get_password                        (WebKitAuthenticationDialogRequest *request);
> 
> Instead of having get and set for the user/pass I think the request should be created with the user/pass of the challenge in case it was stored in the keyring, and new user/pass should be passed in the authenticate method, together with the storage option.
> 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_set_remember_password               (WebKitAuthenticationDialogRequest *request,
> >                                                                                                                                                        
> > gboolean remember);
> > 
> > WEBKIT_API gboolean
> > webkit_authentication_dialog_request_get_remember_password               (WebKitAuthenticationDialogRequest *request);
> 
> Instead of a boolean this should probably be an enum that would allow us to extend it in the future to support other persistence modes like permanent, session and none, for example.
> 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_set_private_browsing                (WebKitAuthenticationDialogRequest *request,
> >                                                                                                                                                 
> > gboolean privateBrowsingEnabled);
> > 
> > WEBKIT_API gboolean
> > webkit_authentication_dialog_request_get_private_browsing                (WebKitAuthenticationDialogRequest *request);
> 
> We have a setting for this, so I think it should be handled internally, probably ignoring the saving mode when private browsing is enabled.
> 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_authenticate                        (WebKitAuthenticationDialogRequest *request);
> > 
> > WEBKIT_API void
> > webkit_authentication_dialog_request_cancel                              (WebKitAuthenticationDialogRequest *request);
> > 
> > To support the existing webkitAuthenticationDialog  custom widget 
> > without much change I propose to also support a private pair of 
> > functions to access the AuthenticationChallengeProxy*
> > 
> > void
> > webkit_authentication_dialog_request_set_authentication_challenge
> > (WebKitAuthenticationDialogRequest *request,
> >                                                                                                                                                    
> > WebKit::AuthenticationChallengeProxy * authenticationChallenge);
> > 
> > WebKit::AuthenticationChallengeProxy*
> > webkit_authentication_dialog_request_get_authentication_challenge
> > (WebKitAuthenticationDialogRequest *request);
> 
> Remember private functions should use the WebKit coding style.
> 
> > From WebLoaderClient::didReceiveAuthenticationChallengeInFrame(), the
> > WebkitAuthenticationDialogRequest* is passed into the existing
> > webkitWebViewHandleAuthenticationChallenge(...) (parameters now
> > changed) which emits a new signal.
> > 
> > I propose the signal name be "run-authentication-dialog", since in 
> > principle it is quite similar to "run-file-chooser" in that it brings 
> > up a dialog for the user to interact with.
> > 
> > The default handler for the signal does what was previously done, i.e.
> > call webkitWebViewRunAuthenticationDialog() which creates the custom 
> > webkitAuthenticationDialog.
> > 
> > A WIP patch has been uploaded to
> > https://bugs.webkit.org/show_bug.cgi?id=99352 if you want more detail.
> 
> Thanks, I'll review it.
> 
> > What do you think?
> > 
> > Regards
> > Brian
> > 
> > Brian Holt
> > Senior Software Engineer
> > 
> > Samsung Electronics (UK) Limited
> > Registered number:  03086621
> > Registered address: Samsung House, 1000 Hillswood Drive, Chertsey,
> >                     Surrey KT16 0PS, England
> > 
> > South Street        Email:  brian.holt at samsung.com<mailto:brian.holt at samsung.com>
> > Staines             Fax:    +44 (0)1784 428620
> > MIDDLESEX           Office: +44 (0)1784 428600 (ext 380)
> > TW18 4QE
> > England
> 
> 
> --
> Carlos Garcia Campos
> http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
> 
> 
> _______________________________________________
> webkit-gtk mailing list
> webkit-gtk at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-gtk
> 
> 
> _______________________________________________
> webkit-gtk mailing list
> webkit-gtk at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-gtk
> 
> 
> _______________________________________________
> webkit-gtk mailing list
> webkit-gtk at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-gtk

-- 
Carlos Garcia Campos
http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.webkit.org/pipermail/webkit-gtk/attachments/20130725/fb68b6f6/attachment-0001.sig>


More information about the webkit-gtk mailing list