> On 19 Feb 2017, at 15:47, Snir Sheriber <ssheribe@xxxxxxxxxx> wrote: > > Remove handling with failures in the SASL authentication > process to separate function > --- > src/spice-channel.c | 44 +++++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 17 deletions(-) > > diff --git a/src/spice-channel.c b/src/spice-channel.c > index af67931..cbf1291 100644 > --- a/src/spice-channel.c > +++ b/src/spice-channel.c > @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel *channel, void *data, size_t length) > return length; > } > > +#if HAVE_SASL > /* coroutine context */ > -static void spice_channel_failed_authentication(SpiceChannel *channel, > - gboolean invalidPassword) > +static void spice_channel_failed_sasl_authentication(SpiceChannel *channel) > { > SpiceChannelPrivate *c = channel->priv; > + gint err_code; /* Affects the authentication window activated fileds */ > > if (c->auth_needs_username && c->auth_needs_password) > - g_set_error_literal(&c->error, > - SPICE_CLIENT_ERROR, > - SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME, > - _("Authentication failed: password and username are required")); > + err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME; > else if (c->auth_needs_username) > - g_set_error_literal(&c->error, > - SPICE_CLIENT_ERROR, > - SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME, > - _("Authentication failed: username is required")); > - else if (c->auth_needs_password) > - g_set_error_literal(&c->error, > - SPICE_CLIENT_ERROR, > - SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD, > - _("Authentication failed: password is required")); > - else if (invalidPassword) > + err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME; > + else > + err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD; > + > + g_set_error_literal(&c->error, > + SPICE_CLIENT_ERROR, > + err_code, > + _("SASL authentication failed")); Per the recent discussion (Feb 14 with Christophe F), can’t we map common SASL errors to Spice messages? To me, it’s different if the problem is that I used a wrong password or if the server is down. The message as is seems quite terse. Errors that seem be reportable (although not all of them seem relevant to Spice): SASL_BADAUTH Authentication failure. SASL_NOAUTHZ Authorization failure. SASL_EXPIRED The passphrase expired and must be reset. SASL_DISABLED Account disabled. SASL_NOUSER User not found. SASL_BADVERS Version mismatch with plug-in. SASL_NOVERIFY The user exists, but there is no verifier for the user. SASL_WEAKPASS The passphrase is too weak for security policy. SASL_NOUSERPASS User supplied passwords are not permitted. Some that may need to be “translated” in Spicese if they ever get back to us: SASL_TOOWEAK The mechanism is too weak for this user. SASL_ENCRYPT Encryption is needed to use this mechanism. SASL_TRANS One time use of a plaintext password will enable requested mechanism for user. Others should probably collected into a “default” in a switch statement, something like “Unexpected SASL error code <blah>”. > + > + c->event = SPICE_CHANNEL_ERROR_AUTH; > + > + c->has_error = TRUE; /* force disconnect */ > +} > +#endif > + > +/* coroutine context */ > +static void spice_channel_failed_authentication(SpiceChannel *channel, > + gboolean invalidPassword) > +{ > + SpiceChannelPrivate *c = channel->priv; > + > + if (invalidPassword) > g_set_error_literal(&c->error, > SPICE_CLIENT_ERROR, > SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD, > @@ -1808,7 +1818,7 @@ error: > if (saslconn) > sasl_dispose(&saslconn); > > - spice_channel_failed_authentication(channel, FALSE); > + spice_channel_failed_sasl_authentication(channel); > ret = FALSE; > > cleanup: > -- > 2.9.3 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel