On Wed, Oct 01, 2014 at 04:41:28PM +0200, Fabiano Fidêncio wrote: > When connecting with SASL for authentication, some authentication > mechanisms need a username (the plain text and md5 ones, for example). > --- > For testing the patch, please, apply: > http://lists.freedesktop.org/archives/spice-devel/2014-October/017505.html > --- > src/virt-viewer-session-spice.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c > index 885399c..41a9a49 100644 > --- a/src/virt-viewer-session-spice.c > +++ b/src/virt-viewer-session-spice.c > @@ -483,6 +483,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED > VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session); > gchar *password = NULL, *user = NULL; > gboolean ret; > + static gboolean username_required = FALSE; > > g_return_if_fail(self != NULL); > > @@ -502,22 +503,52 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED > g_debug("main channel: switching host"); > break; > case SPICE_CHANNEL_ERROR_AUTH: > - g_debug("main channel: auth failure (wrong password?)"); > + g_debug("main channel: auth failure (wrong username/password?)"); > +#if SPICE_GTK_CHECK_VERSION(0, 23, 21) You could change that to the version when we return an error for missing username, but this one is fine too as this is when spice_channel_get_error() was introduced. > + { > + const GError *error = spice_channel_get_error(channel); > + if (error != NULL) { > + /* When the password is invalid, SPICE_CHANNEL_ERROR_AUTH is > + * returned with no GError associated. It means we just want > + * to change the 'user_required' according to the first try > + * to connect to the server and where a GError will be set to > + * indicate if the authentication needs password and username > + * (SALS case when using authentication mechanisms like "SASL" not "SALS" > + * md5-digest or plain-text) or if the authencation needs the "authentication" > + * password only. */ I'm not sure I can read this comment correctly :( The first sentence is fine, but I'm not sure the rest of the comment adds much to the g_error_matches() call just below. Or is there some gotcha here that I'm missing and that this comment explains? > + username_required = g_error_matches(error, > + SPICE_CHANNEL_ERROR, > + SPICE_CHANNEL_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME); > + } > + } > +#endif > > if (self->priv->pass_try > 0) > g_signal_emit_by_name(session, "session-auth-failed", > _("invalid password")); Maybe this 'invalid password' needs to be changed? > + I don't think we need to add a blank line here > self->priv->pass_try++; > > + /* A username is *only* pre-filled in case where some authentication > + * error happened. Unfortunately, we don't have a clear way to > + * differantiate bewteen invalid username and invalid password. 'differentiate between' > + * So, in both cases the username entry will be pre-filled with the > + * username used in the previous attempt. */ > + if (username_required) > + g_object_get(self->priv->session, "username", &user, NULL); > + > ret = virt_viewer_auth_collect_credentials(self->priv->main_window, > "SPICE", > NULL, > - NULL, &password); > + username_required ? &user : NULL, > + &password); > if (!ret) { > g_signal_emit_by_name(session, "session-cancelled"); > } else { > gboolean openfd; > > + if (username_required) > + g_object_set(self->priv->session, "username", user, NULL); I guess we could always set 'username' ? This would set it to NULL in caes the session is reused (dunno if this can happen). > g_object_set(self->priv->session, "password", password, NULL); > g_object_get(self->priv->session, "client-sockets", &openfd, NULL); > > -- > 1.9.3 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list
Attachment:
pgpUnxS023Ptp.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
