[libvirt] [PATCH v2 1/3] polkit: Adjust message when action-id isn't found

[Daniel P. Berrange]

On Thu, Feb 11, 2016 at 06:38:12PM -0500, John Ferlan wrote:
> When there isn't a ssh -X type session running and a user has not
> been added to the libvirt group, attempts to run 'virsh -c qemu:///system'
> commands from an otherwise unprivileged user will fail with rather
> generic or opaque error message:
> 
>     "error: authentication failed: no agent is available to authenticate"
> 
> This patch will adjust the error message to help reflect not only
> that it was a polkit agent, but which 'action-id' was not found. It
> does so carefully, by adding to the text rather than adjusting it
> since virpolkittest is looking for a specific string. The result on
> a failure then becomes:
> 
>     "error: authentication failed: no agent is available to authenticate -
>      no polkit agent for action 'org.libvirt.unix.manage'"
> 
> A bit more history on this - at one time a failure generated the
> following type message when running the 'pkcheck' as a subprocess:
> 
> "error: authentication failed: polkit\56retains_authorization_after_challenge=1
> Authorization requires authentication but no agent is available."
> 
> but, a patch was generated to adjust the error message to help provide
> more details about what failed. This was pushed as commit id '96a108c99'.
> That patch prepended a "polkit: " to the output. It really didn't solve
> the problem, but gave a hint.
> 
> After some time it was deemed using DBus API calls directly was a
> better way to go (since pkcheck calls them anyway). So, commit id
> '1b854c76' (more or less) copied the code from remoteDispatchAuthPolkit
> and adjusted it. Then commit id 'c7542573' adjusted the remote.c
> code to call the new API (virPolkitCheckAuth). Finally, commit id
> '308c0c5a' altered the code to call DBus APIs directly. In doing
> so, it reverted the failing error message to the generic message
> that would have been received from DBus anyway.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/util/virpolkit.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
> index 8da91f2..d837a14 100644
> --- a/src/util/virpolkit.c
> +++ b/src/util/virpolkit.c
> @@ -1,7 +1,7 @@
>  /*
>   * virpolkit.c: helpers for using polkit APIs
>   *
> - * Copyright (C) 2013, 2014 Red Hat, Inc.
> + * Copyright (C) 2013, 2014, 2016 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -121,8 +121,10 @@ int virPolkitCheckAuth(const char *actionid,
>              virReportError(VIR_ERR_AUTH_CANCELLED, "%s",
>                             _("user cancelled authentication process"));
>          else if (is_challenge)
> -            virReportError(VIR_ERR_AUTH_FAILED, "%s",
> -                           _("no agent is available to authenticate"));
> +            virReportError(VIR_ERR_AUTH_FAILED,
> +                           _("no agent is available to authenticate - "
> +                             "no polkit agent for action '%s'"),

This is a littled repetitive still. ACK if you make it say

   "No polkit agent available to authenticate action '%s'"


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|