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

[Steven Allen]

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.

>From 62ea28d5d3b34ddd83bdcf5357ebfa0b24d8688e Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 4 Jul 2024 20:45:07 +0200
Subject: [PATCH] Support interactive D-Bus authorization

When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authorize t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

* src/dbusbind.c (dbus-message-internal): Allow interactive
authorization by passing :authorize t.
* lisp/net/dbus.el (dbus-call-method-asynchronously): Document the new
parameter.
* doc/misc/dbus.texi (Synchronous Methods, Asynchronous Methods):
Document the new parameter.
* configure.ac (HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION):
Set a new variable if
`dbus_message_set_allow_interactive_authorization' is available.
---
 configure.ac       |  5 ++++-
 doc/misc/dbus.texi | 12 ++++++++++--
 etc/NEWS           |  6 ++++++
 lisp/net/dbus.el   |  8 ++++++++
 src/dbusbind.c     | 37 +++++++++++++++++++++++++++++++------
 5 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 909f5786c9a..ee2ef1c60fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3943,6 +3943,8 @@ AC_DEFUN
      dnl dbus_watch_get_unix_fd has been introduced in D-Bus 1.1.1.
      dnl dbus_type_is_valid and dbus_validate_* have been introduced in
      dnl D-Bus 1.5.12.
+     dnl dbus_message_set_allow_interactive_authorization was introduced
+     dnl in D-Bus 1.8.10.
      OLD_LIBS=$LIBS
      LIBS="$LIBS $DBUS_LIBS"
      AC_CHECK_FUNCS([dbus_watch_get_unix_fd \
@@ -3950,7 +3952,8 @@ AC_DEFUN
                    dbus_validate_bus_name \
                     dbus_validate_path \
                    dbus_validate_interface \
-                   dbus_validate_member])
+                   dbus_validate_member \
+                    dbus_message_set_allow_interactive_authorization])
      LIBS=$OLD_LIBS
      DBUS_OBJ=dbusbind.o
    fi
diff --git a/doc/misc/dbus.texi b/doc/misc/dbus.texi
index e5d867acd40..46a666084bb 100644
--- a/doc/misc/dbus.texi
+++ b/doc/misc/dbus.texi
@@ -1208,7 +1208,7 @@ Synchronous Methods
 be called, and a reply message returning the resulting output
 parameters from the object.
 
-@defun dbus-call-method bus service path interface method &optional :timeout 
timeout &rest args
+@defun dbus-call-method bus service path interface method &optional :timeout 
timeout :authorize auth &rest args
 @anchor{dbus-call-method}
 This function calls @var{method} on the D-Bus @var{bus}.  @var{bus} is
 either the keyword @code{:system} or the keyword @code{:session}.
@@ -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-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
@@ -1302,7 +1306,7 @@ Asynchronous Methods
 @cindex method calls, asynchronous
 @cindex asynchronous method calls
 
-@defun dbus-call-method-asynchronously bus service path interface method 
handler &optional :timeout timeout &rest args
+@defun dbus-call-method-asynchronously bus service path interface method 
handler &optional :timeout timeout :authorize auth &rest args
 This function calls @var{method} on the D-Bus @var{bus}
 asynchronously.  @var{bus} is either the keyword @code{:system} or the
 keyword @code{:session}.
@@ -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-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
diff --git a/etc/NEWS b/etc/NEWS
index 3d2b86cfb6a..4705b28238c 100644
--- 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.
+A new ':authorize t' parameter has been added to 'dbus-call-method'
+and 'dbus-call-method-asynchronously' to allow the user to interactively
+authorize the invoked D-Bus method (e.g., via polkit).
+
 
 * Changes in Emacs 31.1 on Non-Free Operating Systems
 
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index dd5f0e88859..d526423e089 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -297,6 +297,10 @@ dbus-call-method
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
@@ -427,6 +431,10 @@ dbus-call-method-asynchronously
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 35ce03c7911..67a1a30dc55 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1314,7 +1314,7 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, 
Sdbus_message_internal,
 `dbus-call-method', `dbus-call-method-asynchronously':
   (dbus-message-internal
     dbus-message-type-method-call BUS SERVICE PATH INTERFACE METHOD HANDLER
-    &optional :timeout TIMEOUT &rest ARGS)
+    &optional :timeout TIMEOUT :authorize AUTH &rest ARGS)
 
 `dbus-send-signal':
   (dbus-message-internal
@@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, 
Sdbus_message_internal,
        XD_SIGNAL1 (build_string ("Unable to create an error message"));
     }
 
-  /* Check for timeout parameter.  */
-  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
+  while ((count + 2 <= nargs))
     {
-      CHECK_FIXNAT (args[count+1]);
-      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
-      count = count+2;
+      /* Check for timeout parameter.  */
+      if (EQ (args[count], QCtimeout))
+        {
+         if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+           XD_SIGNAL1 (build_string (":timeout is only supported on method 
calls"));
+
+          CHECK_FIXNAT (args[count+1]);
+          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
+          count = count+2;
+       }
+      /* Check for authorize parameter.  */
+      else if (EQ (args[count], QCauthorize))
+        {
+         if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+           XD_SIGNAL1 (build_string (":authorize is only supported on method 
calls"));
+
+         /* 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
+
+          count = count+2;
+       }
+      else break;
+
     }
 
   /* Initialize parameter list of message.  */
@@ -1895,6 +1917,9 @@ syms_of_dbusbind (void)
   /* Lisp symbol for method call timeout.  */
   DEFSYM (QCtimeout, ":timeout");
 
+  /* Lisp symbol for method interactive authorization.  */
+  DEFSYM (QCauthorize, ":authorize");
+
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCbyte, ":byte");
   DEFSYM (QCboolean, ":boolean");
-- 
2.45.2