Michael Albinus <michael.albinus@gmx.de> writes:
> Steven Allen <steven@stebalien.com> writes:
>
> Hi Steven,
>
>> I've attached a patch that addresses the feedback so far:
>>
>> 1. Defines HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION and
>>    uses it.
>> 2. Renames :authenticate to :authorize for consistency.
>> 3. Signals an error when either :timeout or :authorize are passed when
>>    not invoking a method.
>
> Thanks!
>
>> 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.

>> 2. Am I correctly signaling the error? I just copied that code from
>> other parts of debusbind.c.
>
> I guess the better call would be
>
> --8<---------------cut here---------------start------------->8---
> 	    XD_SIGNAL1 (build_string (":timeout is only supported on method calls"));
> --8<---------------cut here---------------end--------------->8---
>
> The bus argument isn't needed.
>
> 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.

> Some remarks:
>
>> --- a/doc/misc/dbus.texi
>> +++ b/doc/misc/dbus.texi
>> @@ -1223,6 +1223,10 @@ Synchronous Methods
>>  call doesn't return in time, a D-Bus error is raised (@pxref{Errors
>>  and Events}).
>>
>> +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}
>
>> @@ -1321,6 +1325,10 @@ Asynchronous Methods
>>  no reply message in time, a D-Bus error is raised (@pxref{Errors and
>>  Events}).
>>
>> +If the parameter @code{:authorize} is given and the following
>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>
> ditto
>

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

>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
>>  
>>  * Lisp Changes in Emacs 31.1
>>
>> ++++
>> +*** Support interactive D-Bus authorization
>
> Please add a trailing period.
>
>> +A new ':authorization t' parameter has been added to 'dbus-call-method'
>
> ':authorize t'
>

done and done.

>> --- a/src/dbusbind.c
>> +++ b/src/dbusbind.c
>> @@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
>> +	    XD_SIGNAL2 (build_string (":timeout is only supported on method calls"), bus);
>
> XD_SIGNAL1

Ah... I was wondering about the difference between the different
signals. I didn't even notice the bus argument... Thanks!

>> +	    XD_SIGNAL2 (build_string (":authorize is only supported on method calls"), bus);
>
> XD_SIGNAL1
>
>> +	  /* 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.