bug#63082: [PATCH 11/17] services: mpd: Warn when the MPD user is not in the "audio" group.

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

Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Freitag, dem 28.04.2023 um 10:27 -0400 schrieb Maxim Cournoyer:
>> Relates to <https://issues.guix.gnu.org>.
>> 
>> * gnu/services/audio.scm (%mpd-user) [group]: Add comment.
>> (mpd-user-sanitizer): Warn if the MPD user is not in the audio group.
>> ---
>>  gnu/services/audio.scm | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>> 
>> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
>> index 550ccc542c..9579432ea3 100644
>> --- a/gnu/services/audio.scm
>> +++ b/gnu/services/audio.scm
>> @@ -30,6 +30,7 @@ (define-module (gnu services audio)
>>    #:use-module (gnu services configuration)
>>    #:use-module (gnu services shepherd)
>>    #:use-module (gnu services admin)
>> +  #:use-module (gnu system accounts)
>>    #:use-module (gnu system shadow)
>>    #:use-module (gnu packages admin)
>>    #:use-module (gnu packages mpd)
>> @@ -172,6 +173,8 @@ (define-maybe boolean (prefix mpd-))
>>  (define %mpd-user
>>    (user-account
>>     (name "mpd")
>> +   ;; Being in the audio group ensures that PulseAudio can access
>> sound
>> +   ;; devices.
>>     (group "audio")
>>     (system? #t)
>>     (comment "Music Player Daemon (MPD) user")
>> @@ -208,10 +211,17 @@ (define (mpd-serialize-port field-name value)
>>  
>>  (define-maybe port (prefix mpd-))
>>  
>> -;;; Procedures for unsupported value types, to be removed.
>> -
>> +;;; Sanitizer procedures.
>>  (define (mpd-user-sanitizer value)
>> -  (cond ((user-account? value) value)
>> +  (cond ((user-account? value)
>> +         (match-record value <user-account>
>> +           (group supplementary-groups)
>> +           (unless (or (string=? "audio" group)
>> +                       (member "audio" supplementary-groups))
>> +             ;; Being in the "audio" group is necessary for access
>> to the
>> +             ;; sound devices.
>> +             (warning (G_ "mpd user not member of \"audio\"
>> group~%"))))
>> +         value)
>>          ((string? value)
>>           (warning (G_ "string value for 'user' is deprecated, use \
>>  user-account instead~%"))
> I think this check is "only" required when using alsa/pulseaudio for
> outputs and should be a hard error then.  When configured to write to
> httpd or null outputs, other checks are needed.

I agree, but then the check couldn't be made in a sanitizer and would
need to happen much later (in the start slot to ensure it runs at the
right time?).

I figured the current behavior, while not perfect, is better than the
later.

-- 
Thanks,
Maxim