Bug 57284

Created attachment 74817 [details] [review]
Use auth_admin* instead of auth_self* in examples.

From time to time, application developers just copy example configuration without examining it in details.  Because polkit is typically used to control access to system-level operations, the policy (and therefore the examples) should limit access to system administrators only.

In particular, examples should show auth_admin* instead of auth_self*.

Past instances of problems caused by incorrectly using auth_self*:
https://bugzilla.redhat.com/show_bug.cgi?id=878115
https://bugzilla.redhat.com/show_bug.cgi?id=878102
http://git.fedorahosted.org/cgit/system-config-users.git/commit/?id=8abd89064889723e9e6f33fdeea8e02e935500c9
and at least one other.

Comment on attachment 74817 [details] [review]
Use auth_admin* instead of auth_self* in examples.

Review of attachment 74817 [details] [review]:
-----------------------------------------------------------------

If it's that much of a trap, maybe the overview.xml should attempt to warn about this inline; something like:

Note that for <type>GtkLockButton</type> to work well, the
polkit action backing it should use <literal>auth_admin_keep</literal> (or
more rarely <literal>auth_self_keep</literal> for services which don't
affect other users).

Or maybe even better add a ulink to polkit.8 man page and explain a bit more in depth
why this is a bad idea there?

This doesn't block this patch going in, just suggestions for improvements.

Created attachment 78193 [details] [review]
0001-More-warnings-about-using-auth_self.patch

(In reply to comment #2)
> If it's that much of a trap, maybe the overview.xml should attempt to warn
> about this inline; something like:
> 
> Note that for <type>GtkLockButton</type> to work well, the
> polkit action backing it should use <literal>auth_admin_keep</literal> (or
> more rarely <literal>auth_self_keep</literal> for services which don't
> affect other users).
> 
> Or maybe even better add a ulink to polkit.8 man page and explain a bit more
> in depth
> why this is a bad idea there?

Thanks for the suggestions, the attached patch incorporates them (and adds even more warnings to the "writing polkit applications" section).

Comment on attachment 78193 [details] [review]
0001-More-warnings-about-using-auth_self.patch

Review of attachment 78193 [details] [review]:
-----------------------------------------------------------------

Looks good to me.

Thanks, applied.