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++;
>
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
