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

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(a)redhat.com&gt; > --- > 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'"