Re: [Libvir] PATCH: 5/10: public auth callback API

[Daniel Veillard <veillard redhat com>]

On Thu, Nov 29, 2007 at 05:19:31PM +0000, Daniel P. Berrange wrote:
> This patch introduces the public & driver APIs for collecting auth
> credentials via a callback. This allows username/password based auth
> schemes to be used.
> 
> This basically introduces a 3rd variant of the virConnectOpen call
> which takes a set of authentication parameters, and some flags.
> 
>  virConnectPtr           virConnectOpenAuth      (const char *name,
>  						 virConnectAuthPtr auth,
>  						 int flags);
> 
> 
> The flags parameter allows VIR_CONNECT_RO, so there is no need for a
> separate ReadOnly API. The 'auth' parameter is a small struct containing
> a list of credentials that the calling application knows how to collect,
> a function pointer for the callback, and an opaque data blob.
> 
>  struct _virConnectAuth {
>      int *credtype; /* List of supported virConnectCredentialType values */
>      unsigned int ncredtype;
> 
>      virConnectAuthCallbackPtr cb; /* Callback used to collect credentials */
>      void *cbdata;
>  };
> 
> At the very least apps should support the VIR_CRED_AUTHNAME and the
> VIR_CRED_PASSPHRASE credential types for collecting username+password
> respectively. There are a bunch of other credential type, but they're
> for fairly niche use cases.
> 
> When the callback is invoked, it will be passed a list of virConnectCredentialPtr
> structs which contain details on all the credentials that the authentication
> mechanism needs to collect. They are passed all at once to make it easy to
> construct a big form in UI:
> 
>  typedef int (*virConnectAuthCallbackPtr)(virConnectCredentialPtr cred,
>  					 unsigned int ncred,
>  					 void *cbdata);

  okay, I have just two remarks about the API: that the callback seems to
get passed an array of virConnectCredential as first argument, not a
list of virConnectCredentialPtr. Also I hope we won't end up with too many 
virConnectOpenAuth flags, maybe using a long to be sure we can fit at least
64 options might be a good safe thing to do. We have not been very consistent
so far in libvirt for the flags, sometime using int/unsigned int/unsigned long
maybe unsigned long is safer and cleaner, i could see how various
authentications options may end up growing that set over time.

> The virConnectCredentialPtr struct contains a prompt which can be displayed
> in the UI. There may optionally be a challenge if doing a challenge/response
> type authentication. There may also be a default result. The application
> should collect a credential from the user & fill it into the 'result' field,
> or use the default result. If the callback returns 0, authentication will
> continue. If it returns -1, it will assume the user wants to cancel the
> auth process.

  Yup looks fine to me :-)

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/