[Spice-devel] [vdagent-linux v3 4/4] console-kit: implement check for session type

[Jonathon Jongsma]

On Thu, 2016-04-28 at 15:37 +0200, Victor Toso wrote:
> Implementing the function that was introduced in previous commit:
> session_info_is_user()
> 
> In console-kit we can gather the session type with GetSessionType api
> from Session interface. This function is not well documented but seems
> reliable enough as GDM was using it [0]
> 
> [0] session_is_login_window on gui/simple-greeter/gdm-user-manager.c
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323640
> ---
>  src/console-kit.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> -
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/src/console-kit.c b/src/console-kit.c
> index 036234e..182e886 100644
> --- a/src/console-kit.c
> +++ b/src/console-kit.c
> @@ -503,8 +503,72 @@ gboolean session_info_session_is_locked(struct
> session_info *info)
>      return (info->session_is_locked || info->session_idle_hint);
>  }
>  
> +/* This function should only be called after session_info_get_active_session
> + * in order to verify if active session belongs to user (non greeter) */
>  gboolean session_info_is_user(struct session_info *info)
>  {
> -    /* TODO */
> -    return TRUE;
> +    DBusError error;
> +    DBusMessage *message;
> +    DBusMessage *reply;

would prefer to initialize message and reply to NULL just in case at some point
the code changes and there is a 'goto exit' call before one of these variables
gets assigned.

> +    gchar *session_type = NULL;
> +    gboolean ret = TRUE;
> +
> +    if (info == NULL || info->connection == NULL || info->active_session ==
> NULL)
> +        return TRUE;

Here again, we're defaulting to permissive. As in the previous reviews, should
we default to restrictive? Maybe not, but I just wanted to mention it.

> +
> +    message = dbus_message_new_method_call(INTERFACE_CONSOLE_KIT,
> +                                           info->active_session,
> +                                           INTERFACE_CONSOLE_KIT_SESSION,
> +                                           "GetSessionType");
> +    if (message == NULL) {
> +        syslog(LOG_ERR,
> +               "(console-kit) Unable to create dbus message for
> GetSessionType");
> +        return TRUE;
> +    }
> +
> +    dbus_error_init(&error);
> +    reply = dbus_connection_send_with_reply_and_block(info->connection,
> +                                                      message,
> +                                                      -1,
> +                                                      &error);
> +    if (reply == NULL || dbus_error_is_set(&error)) {
> +        if (dbus_error_is_set(&error)) {
> +            syslog(LOG_ERR, "GetSessionType failed: %s", error.message);
> +            dbus_error_free(&error);
> +        } else
> +            syslog(LOG_ERR, "GetSessionType failed");
> +        goto exit;
> +    }
> +
> +    dbus_error_init(&error);
> +    if (!dbus_message_get_args(reply,
> +                               &error,
> +                               DBUS_TYPE_STRING, &session_type,
> +                               DBUS_TYPE_INVALID)) {
> +        if (dbus_error_is_set(&error)) {
> +            syslog(LOG_ERR,
> +                   "(console-kit) fail to get session-type from reply: %s",
> +                   error.message);
> +            dbus_error_free(&error);
> +        } else {
> +            syslog(LOG_ERR, "(console-kit) fail to get session-type from
> reply");
> +        }
> +        session_type = NULL;
> +        goto exit;
> +    }
> +
> +    /* Empty session_type means user */
> +    if (info->verbose)
> +        syslog(LOG_DEBUG, "(console-kit) session-type is '%s'",
> session_type);
> +
> +    ret = (g_strcmp0 (session_type, "LoginWindow") != 0);
> +
> +exit:
> +    if (reply != NULL) {
> +        dbus_message_unref(reply);
> +    }
> +    if (message != NULL) {
> +        dbus_message_unref(message);
> +    }
> +    return ret;
>  }


Otherwise it looks ok.

Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>