[Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately

[Christophe Fergeau]

On Sun, Feb 19, 2017 at 04:47:17PM +0200, Snir Sheriber 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"));

I don't understand what you are aiming for with this series. I thought
the goal was to get better error messages on autentication failures, but
this change merges 3 distinct error messages
_("Authentication failed: password and username are required"));
_("Authentication failed: username is required"));
_("Authentication failed: password is required"));

to just

_("SASL authentication failed"));

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170221/7e5203e9/attachment.sig>