bug#63082: [PATCH v3 10/16] services: mpd: Let Shepherd effect the user/group change.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Relates to <https://issues.guix.gnu.org/63082>.

Quoting a MPD developer, regarding MPD's feature to switch user itself:
"that's legacy for the dark ages when proper service managers did not exist"
:-).

* gnu/services/audio.scm (mpd-serialize-user-account)
(mpd-serialize-user-group): Delete procedures.
* gnu/services/audio.scm (mpd-configuration) [user]: Do not serialize.
[group]: Likewise.
(mpd-shepherd-service): Provide the #:user, #:group and #:supplementary-groups
arguments.
(mympd-shepherd-service): Likewise, and remove the '--user' argument.
* doc/guix.texi (Audio Services): Update doc.
(mympd-configuration) [port]: Change default value to 8080.
[ssl-port]: Change default value to 443.
* gnu/tests/audio.scm (run-mympd-test): Adjust accordingly.
---
 doc/guix.texi          | 12 +++++-----
 gnu/services/audio.scm | 52 +++++++++++++++++++++++++-----------------
 gnu/tests/audio.scm    |  4 ++--
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 253b8f113b..cdc1f4dedc 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33598,7 +33598,7 @@ Audio Services
 The MPD package.
 
 @item @code{user} (type: user-account)
-The user to run mpd as.
+The user to run @command{mpd} as.
 
 @item @code{group} (default: @code{#f}) (type: boolean)
 Obsolete.  Do not use.
@@ -33642,7 +33642,7 @@ Audio Services
 The location of the sticker database.
 
 @item @code{default-port} (default: @code{6600}) (type: maybe-port)
-The default port to run mpd on.
+The default port to run @command{mpd} on.
 
 @item @code{endpoints} (type: maybe-list-of-strings)
 The addresses that mpd will bind to.  A port different from
@@ -33827,13 +33827,13 @@ Audio Services
 @uref{https://jcorporation.github.io/myMPD/, myMPD} is a web server
 frontend for MPD that provides a mobile friendly web client for MPD.
 
-The following example shows a myMPD instance listening on port 80,
+The following example shows a myMPD instance listening on port 8080,
 with album cover caching disabled.
 
 @lisp
 (service mympd-service-type
          (mympd-configuration
-          (port 80)
+          (port 8080)
           (covercache-ttl 0)))
 @end lisp
 
@@ -33877,7 +33877,7 @@ Audio Services
 @item @code{host} (default: @code{"[::]"}) (type: string)
 Host name to listen on.
 
-@item @code{port} (default: @code{80}) (type: maybe-port)
+@item @code{port} (default: @code{8080}) (type: maybe-port)
 HTTP port to listen on.
 
 @item @code{log-level} (default: @code{5}) (type: integer)
@@ -33903,7 +33903,7 @@ Audio Services
 @item @code{ssl?} (default: @code{#f}) (type: boolean)
 SSL/TLS support.
 
-@item @code{ssl-port} (default: @code{443}) (type: maybe-port)
+@item @code{ssl-port} (default: @code{4443}) (type: maybe-port)
 Port to listen for HTTPS.
 
 @item @code{ssl-cert} (type: maybe-string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 7fb4b8ccf7..f470ca20e0 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2022⁠–⁠2023 Bruno Victal <mirai@makinata.eu>
+;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -165,9 +166,6 @@ (define mpd-serialize-boolean mpd-serialize-field)
 (define (mpd-serialize-list-of-strings field-name value)
   #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
 
-(define (mpd-serialize-user-account field-name value)
-  (mpd-serialize-string field-name (user-account-name value)))
-
 (define-maybe string (prefix mpd-))
 (define-maybe list-of-strings (prefix mpd-))
 (define-maybe boolean (prefix mpd-))
@@ -390,10 +388,14 @@ (define-configuration mpd-configuration
    "The MPD package."
    empty-serializer)
 
+  ;; Note: The user and its group are not serialized, otherwise MPD would
+  ;; attempt to switch the user/group itself.  The task of switching the
+  ;; user/group is left to Shepherd instead.
   (user
    (user-account %mpd-user)
-   "The user to run mpd as."
-   (sanitizer mpd-user-sanitizer))
+   "The user to run @command{mpd} as."
+   (sanitizer mpd-user-sanitizer)
+   empty-serializer)
 
   (group
    (boolean #f)
@@ -458,7 +460,7 @@ (define-configuration mpd-configuration
 
   (default-port
    (maybe-port 6600)
-   "The default port to run mpd on.")
+   "The default port to run @command{mpd} on.")
 
   (endpoints
    maybe-list-of-strings
@@ -611,7 +613,11 @@ (define (mpd-shepherd-service config)
               (make-forkexec-constructor
                (list #$(file-append package "/bin/mpd") "--no-daemon"
                      #$config-file)
-               #:environment-variables '#$environment-variables))))
+               #:environment-variables '#$environment-variables
+               #:user #$username
+               #:group #$(user-account-group user)
+               #:supplementary-groups
+               '#$(user-account-supplementary-groups user)))))
        (stop  #~(make-kill-destructor))
        (actions
         (list (shepherd-configuration-action config-file)
@@ -654,7 +660,7 @@ (define mpd-service-type
           (service-extension account-service-type
                              mpd-accounts)
           (service-extension rottlog-service-type
-                             (compose list mpd-log-rotation))))
+                             mpd-log-rotation)))
    (default-value (mpd-configuration))))
 
 \f
@@ -770,7 +776,7 @@ (define-configuration/no-serialization mympd-configuration
    "Host name to listen on.")
 
   (port
-   (maybe-port 80)
+   (maybe-port 8080)
    "HTTP port to listen on.")
 
   (log-level
@@ -805,7 +811,7 @@ (define-configuration/no-serialization mympd-configuration
    "SSL/TLS support.")
 
   (ssl-port
-   (maybe-port 443)
+   (maybe-port 4443)
    "Port to listen for HTTPS.")
 
   (ssl-cert
@@ -901,17 +907,21 @@ (define (mympd-shepherd-service config)
                           '#$(map dirname (filter-map maybe-value
                                                       (list log-to
                                                             work-directory
-                                                            cache-directory)))))
-
-              (make-forkexec-constructor
-               `(#$(file-append package "/bin/mympd")
-                 "--user" #$username
-                 #$@(if (eq? log-to 'syslog) '("--syslog") '())
-                 "--workdir" #$work-directory
-                 "--cachedir" #$cache-directory)
-               #:environment-variables
-               (list #$(format #f "MYMPD_LOGLEVEL=~a" log-level))
-               #:log-file #$(maybe-value log-to)))))))))
+                                                            cache-directory))))
+
+                (make-forkexec-constructor
+                 `(#$(file-append package "/bin/mympd")
+                   "--user" #$username
+                   #$@(if (eq? log-to 'syslog) '("--syslog") '())
+                   "--workdir" #$work-directory
+                   "--cachedir" #$cache-directory)
+                 #:environment-variables
+                 (list #$(format #f "MYMPD_LOGLEVEL=~a" log-level))
+                 #:log-file #$(maybe-value log-to)
+                 #:user #$username
+                 #:group #$(user-account-group user)
+                 #:supplementary-groups
+                 '#$(user-account-supplementary-groups user))))))))))
 
 (define (mympd-accounts config)
   (match-record config <mympd-configuration>
diff --git a/gnu/tests/audio.scm b/gnu/tests/audio.scm
index acb91293e8..efa07b5ba9 100644
--- a/gnu/tests/audio.scm
+++ b/gnu/tests/audio.scm
@@ -89,7 +89,7 @@ (define (run-mympd-test)
   (define vm
     (virtual-machine
      (operating-system os)
-     (port-forwardings '((8080 . 80)))))
+     (port-forwardings '((8080 . 8080)))))
 
   (define test
     (with-imported-modules '((gnu build marionette))
@@ -113,7 +113,7 @@ (define (run-mympd-test)
                              marionette))
 
           (test-assert "HTTP port ready"
-            (wait-for-tcp-port 80 marionette))
+            (wait-for-tcp-port 8080 marionette))
 
           (test-equal "http-head"
             200
-- 
2.39.2