[virt-tools-list] [PATCH virt-viewer] remote-viewer: allow username in ovirt URIs

[Fabiano Fidêncio]

Howdy!

On Fri, Jul 18, 2014 at 11:51 PM, Jonathon Jongsma <jjongsma at redhat.com>
wrote:

> When the user launches remote-viewer with an ovirt URI of the form
>
>         ovirt://user@host/vmname
>
> Pre-populate the authentication dialog with the specified username. We
> don't support specifying the password on the commandline, since that is
> a potential security risk.
>
> rhbz#1061826
> ---
>  src/remote-viewer.c    | 20 ++++++++++++++++----
>  src/virt-viewer-auth.c | 10 +++++++++-
>  2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 85a794b..1c1b981 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -626,7 +626,7 @@ remote_viewer_window_added(VirtViewerApp *app
> G_GNUC_UNUSED,
>
>  #ifdef HAVE_OVIRT
>  static gboolean
> -parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name)
> +parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name,
> char** username)
>

char **username, here.


>  {
>      char *vm_name = NULL;
>      char *rel_path;
> @@ -664,6 +664,9 @@ parse_ovirt_uri(const gchar *uri_str, char **rest_uri,
> char **name)
>      vm_name = path_elements[element_count-1];
>      path_elements[element_count-1] = NULL;
>
> +    if (username && uri->user)
> +        *username = g_strdup(uri->user);
> +
>      /* build final URI */
>      rel_path = g_strjoinv("/", path_elements);
>      /* FIXME: how to decide between http and https? */
> @@ -683,10 +686,14 @@ static gboolean
>  authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED RestProxyAuth *auth,
>                  G_GNUC_UNUSED gboolean retrying, gpointer user_data)
>  {
> -    gchar *username;
> -    gchar *password;
> +    gchar *username = NULL;
> +    gchar *password = NULL;
>      VirtViewerWindow *window;
>
> +    g_object_get(proxy,
> +                 "username", &username,
> +                 NULL);
> +
>      window = virt_viewer_app_get_main_window(VIRT_VIEWER_APP(user_data));
>      int ret =
> virt_viewer_auth_collect_credentials(virt_viewer_window_get_window(window),
>                                                     "oVirt",
> @@ -718,6 +725,7 @@ create_ovirt_session(VirtViewerApp *app, const char
> *uri)
>      GError *error = NULL;
>      char *rest_uri = NULL;
>      char *vm_name = NULL;
> +    char* username = NULL;
>

char *username, here.


>      gboolean success = FALSE;
>      guint port;
>      guint secure_port;
> @@ -732,11 +740,15 @@ create_ovirt_session(VirtViewerApp *app, const char
> *uri)
>
>      g_return_val_if_fail(VIRT_VIEWER_IS_APP(app), FALSE);
>
> -    if (!parse_ovirt_uri(uri, &rest_uri, &vm_name))
> +    if (!parse_ovirt_uri(uri, &rest_uri, &vm_name, &username))


And if parse_ovirt_uri doesn't fail, username is a new allocated string,
right? ...


>          goto error;
>      proxy = ovirt_proxy_new(rest_uri);
>      if (proxy == NULL)
>          goto error;
>

... and you're leaking it here :-)
Please, free the username in the end of function.

+
> +    g_object_set(proxy,
> +                 "username", username,
> +                 NULL);
>      ovirt_set_proxy_options(proxy);
>      g_signal_connect(G_OBJECT(proxy), "authenticate",
>                       G_CALLBACK(authenticate_cb), app);
> diff --git a/src/virt-viewer-auth.c b/src/virt-viewer-auth.c
> index a5b6393..6745566 100644
> --- a/src/virt-viewer-auth.c
> +++ b/src/virt-viewer-auth.c
> @@ -32,6 +32,10 @@
>  #include "virt-viewer-auth.h"
>
>
> +/* NOTE: if username is provided, and *username is non-NULL, the user
> input
> + * field will be pre-filled with this value. The existing string will be
> freed
> + * before setting the output parameter to the user-entered value.
> + */
>  int
>  virt_viewer_auth_collect_credentials(GtkWindow *window,
>                                       const char *type,
> @@ -60,6 +64,8 @@ virt_viewer_auth_collect_credentials(GtkWindow *window,
>      promptPassword = GTK_WIDGET(gtk_builder_get_object(creds,
> "prompt-password"));
>
>      gtk_widget_set_sensitive(credUsername, username != NULL);
> +    if (username && *username)
> +        gtk_entry_set_text(GTK_ENTRY(credUsername), *username);
>      gtk_widget_set_sensitive(promptUsername, username != NULL);
>      gtk_widget_set_sensitive(credPassword, password != NULL);
>      gtk_widget_set_sensitive(promptPassword, password != NULL);
> @@ -82,8 +88,10 @@ virt_viewer_auth_collect_credentials(GtkWindow *window,
>      gtk_widget_hide(dialog);
>
>      if (response == GTK_RESPONSE_OK) {
> -        if (username)
> +        if (username) {
> +            g_free(*username);
>              *username =
> g_strdup(gtk_entry_get_text(GTK_ENTRY(credUsername)));
> +        }
>          if (password)
>              *password =
> g_strdup(gtk_entry_get_text(GTK_ENTRY(credPassword)));
>      }
> --
> 1.9.3
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>


Best Regards,
-- 
Fabiano Fidêncio
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20140721/2c99b106/attachment.htm>