Subject: Re: bug#71969: [PATCH] Support interactive D-Bus authentication

From: Michael Albinus <[email-masked]>
Date: Mon, 08 Jul 2024 14:29:55 +0200

Steven Allen <[email-masked]> writes:

Hi Steven,

>>> Remaining questions:
>>>
>>> 1. I'm not sure if :authorize is quite correct either. Really, the key
>>> part is that it allows /interactive/ authorization. I wonder if
>>> :interactive-authorization or :interactive might be better (although
>>> they're kind of long).
>>
>> I believe :authorize is OK. In the docstrings as well as in the D-Bus
>> manual, interactive authorization is mentioned, so a user shall know
>> what's about.
>
> Hm, it's still bugging me. We're _not_ authorizing the request, we're
> telling D-Bus that it's ok to ask the user if they want to authorize it.
> I'm hoping the example below will make this clearer.

What about :authorizable? I don't like the alternative
:interactive-authorize; it's too long to type, and it's also not obvious
w/o knowing the context.

>> Furthermore, you haven't given an example. I really would like to see
>> how it works in practice.
>
> Sorry about that. To restart the bluetooth service, execute:
>
>     (dbus-call-method
>      :system
>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>      :authorize t
>      "bluetooth.service" "replace")
>
> Assuming you have a polkit agent running (most DEs will run one by
> default, but agents like mate-polkit work pretty well standalone),
> you'll be prompted to authorize the operation and the bluetooth service
> will be restarted.

Nice. I get an authorization prompt.

However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
matter, whether I use ':authorize t', ':authorize nil', or none of
them. Is interactive authorization enabled by default, and we don't need
to care about?

>>> +If the parameter @code{:authorize} is given and the following
>>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>>
>> non-@code{nil}

> Done and done (the info manuals are pretty inconsistent in this regard...).

If you see it somewhere else in the manuals, it is an error. The rule is
to use @code{nil}, non-@code{nil}, and @code{t}. Feel free to correct this.

>>> +	  /* Ignore this keyword if unsupported. */
>>> +	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
>>> +	  dbus_message_set_allow_interactive_authorization
>>> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
>>> +	  #endif
>>
>> #ifdef end #endif shall start in column 1. Futhermore, we need an #else
>> clause. There shall be an error or a warning, that :authorize is not supported.
>
> I'm going to disagree on this last point. The flag is specifying whether
> or not the D-Bus is _allowed_ to ask the user to ask the user to
> authorize requests which can fail for multiple reasons anyways (e.g., if
> no polkit agent is running, the user rejects the interactive
> authorization, etc.).
>
> If authorization is required and wasn't possible for some reason,
> D-Bus will return an error to the user anyways. So the user will get
> their warning either way _if_ something actually goes wrong.

Good point. However, we shall support developers if they run into this
case. What about a debug message like

--8<---------------cut here---------------start------------->8---
#ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
	  dbus_message_set_allow_interactive_authorization
	    (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
#else
	  XD_DEBUG_MESSAGE (":authorize not supported");
#endif
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.