[virt-tools-list] [PATCH virt-viewer v2 3/7] Fix inconsistencies in session auth failures

[Pavel Grunt]

On Thu, 2015-06-18 at 15:15 -0500, Jonathon Jongsma wrote:
> The spice session implementation can retry authentication on its own,
> whereas the vnc session needs to tear down the session and re-connect in
> order to retry a failed authentication. This results in the following
> inconsistent behavior:
> 
> VNC session:
>  - emits a 'session-auth-failed' signal when the client does not support
>    a particular authentication type (i.e.: a non-recoverable error)
> Spice session:
> - emits a 'session-auth-failed' signal when user enters an incorrect
>   password, and immediately retries auth internally
> 
> VNC session:
>  - emits a 'session-auth-refused' error when user enters an invalid
>    password (i.e.: a recoverable error)
> Spice Session:
> - never emits a 'session-auth-refused' signal
> 
> Because of these differences, the VirtViewerApp code to handle authentication
> failures is a bit confusing and difficult to maintain. To fix this issue, make
> both the spice and VNC sessions emit the same signal when similar errors 
> occur.
> We use the new session API added in the last commit to determine whether the
> session supports automatic retries so we know how to handle the signal.
> ---
>  src/virt-viewer-app.c           | 41 ++++++++++++++++++++++++----------------
> -
>  src/virt-viewer-session-spice.c |  2 +-
>  2 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 2bf74f7..96afc78 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -1488,7 +1488,7 @@ static void virt_viewer_app_cancelled(VirtViewerSession 
> *session,
>  }
>  
>  
> -static void virt_viewer_app_auth_refused(VirtViewerSession *session 
> G_GNUC_UNUSED,
> +static void virt_viewer_app_auth_refused(VirtViewerSession *session,
>                                           const char *msg,
>                                           VirtViewerApp *self)
>  {
> @@ -1496,26 +1496,33 @@ static void 
> virt_viewer_app_auth_refused(VirtViewerSession *session G_GNUC_UNUSE
>      int ret;
>      VirtViewerAppPrivate *priv = self->priv;
>  
> -    dialog = gtk_message_dialog_new(virt_viewer_window_get_window(priv
> ->main_window),
> -                                    GTK_DIALOG_MODAL |
> -                                    GTK_DIALOG_DESTROY_WITH_PARENT,
> -                                    GTK_MESSAGE_ERROR,
> -                                    GTK_BUTTONS_YES_NO,
> -                                    _("Unable to authenticate with remote 
> desktop server at %s: %s\n"
> -                                      "Retry connection again?"),
> -                                    priv->pretty_address, msg);
> -
> -    ret = gtk_dialog_run(GTK_DIALOG(dialog));
> +    if (virt_viewer_session_can_retry_auth(session)) {
> +        virt_viewer_app_simple_message_dialog(self,
> +                                              _("Unable to authenticate with 
> remote desktop server: %s"),
> +                                              msg);
Why not keeping the format string "...remote desktop server at %s: %s" ? 

It is possible to return and save the indentation level, you are removing this
part in the PATCH 6/7 anyway.

Pavel

> +    } else {
> +        /* if the session implementation cannot retry auth automatically, the
> +         * VirtViewerApp needs to schedule a new connection to retry */
> +        dialog = gtk_message_dialog_new(virt_viewer_window_get_window(priv
> ->main_window),
> +                                        GTK_DIALOG_MODAL |
> +                                        GTK_DIALOG_DESTROY_WITH_PARENT,
> +                                        GTK_MESSAGE_ERROR,
> +                                        GTK_BUTTONS_YES_NO,
> +                                        _("Unable to authenticate with remote 
> desktop server at %s: %s\n"
> +                                          "Retry connection again?"),
> +                                        priv->pretty_address, msg);
> +
> +        ret = gtk_dialog_run(GTK_DIALOG(dialog));
>  
> -    gtk_widget_destroy(dialog);
> +        gtk_widget_destroy(dialog);
>  
> -    if (ret == GTK_RESPONSE_YES)
> -        priv->authretry = TRUE;
> -    else
> -        priv->authretry = FALSE;
> +        if (ret == GTK_RESPONSE_YES)
> +            priv->authretry = TRUE;
> +        else
> +            priv->authretry = FALSE;
> +    }
>  }
>  
> -
>  static void virt_viewer_app_auth_failed(VirtViewerSession *session 
> G_GNUC_UNUSED,
>                                          const char *msg,
>                                          VirtViewerApp *self)
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index ec6ffa5..9976291 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -617,7 +617,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel 
> *channel,
>          }
>  
>          if (self->priv->pass_try > 0)
> -            g_signal_emit_by_name(session, "session-auth-failed",
> +            g_signal_emit_by_name(session, "session-auth-refused",
>                                    error != NULL ? error->message : _("Invalid 
> password"));
>          self->priv->pass_try++;
>