Subject: Re: [bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

From: Maya <[email-masked]>
Date: Wed, 27 Jul 2022 20:26:32 +0000

>This can be simplified to
>
>    (let ((fprintd-module (file-append (fprintd-configuration-fprintd
>config) "/lib/security/pam_fprintd.so")))

Yes, thank you, I am not yet that great with my guix-fu.

> > +                               #:login-uid? #t))

> What's this line for?  I'm not finding 'login-uid?' anywhere in the
> manual, a comment would be in order.

I've got this from the unix-pam-service and from gdm-service-type. The code this refers to in gnu/system/pam.scm:

,@(if login-uid?
     (list (pam-entry       ;to fill in /proc/self/loginuid
                (control "required")
                (module "pam_loginuid.so")))
     '())

gdm-service-type uses it in all 3 of it's pam modules. So I figured it ought to be there. I can investigate further, but it seems like I should not touch it.

> Documentation is missing (in the manual), so as-is, this new feature is
> hard to find.

Oh? I didn't know that. Doesn't define-configuration generate documentation automatically? If it does not, I will hapilly add it, but I have never written any, so it will be a learning process.

> Also, the manual required giving every top-level procedure a docstring
> IIRC,

There is that requirement, yes. But there weren't any around this method so I thought the configuration sufficed, but if it is a requirement, I will do that.

> >   gnu/services/authentication.scm | 49 +++++++++++++++++++++++++++++++--
> >   1 file changed, 46 insertions(+), 3 deletions(-)
> >
> > diff --git a/gnu/services/authentication.scm b/gnu/services/authentication.scm
> > index f7becdfafb..5737c15f4c 100644
> > --- a/gnu/services/authentication.scm
> > +++ b/gnu/services/authentication.scm
> > @@ -44,9 +44,50 @@ (define-module (gnu services authentication)
> >               nslcd-configuration?
> >               nslcd-service-type))
> >
> > -(define-configuration fprintd-configuration
> > +(define-configuration/no-serialization fprintd-configuration
> >     (fprintd      (file-like fprintd)
> > -                "The fprintd package"))
> > +                "The fprintd package")
> > +  (unlock-gdm?
> > +   (boolean #t)
> > +   "Generate PAM configuration that unlocks gdm with fprintd.")
> > +  (unlock-other
> > +   (list '("polkit-1" "sddm")) ;; polkit-1 is the name of a PAM module for GNOME polkit
> > +   "List of other PAM modules that can be unlocked with fprintd.
> > +
> > +This depends on your desktop configuration. If you for example want GNOME prompts to be unlocked by fingerprint, you add @code{polkit-1} to this list. (This is enabled by default.)
> +"))

> This documentation is unclear -- does this field need to be set to the
> _name_ of the module, or to the _file name_ of the _shared library_ (as
> a file-like, not a direct file name, because of staging), or ...?  Also,
> the 'list' check can be more precise, IIRC there was some method for not
> just using list? but doing things like list-of-strings?.

The name of the pam module, not a shared library. So the file in /etc/pam.d. It is a direct name, since it is not inside the store, pam modules have static path.

As for the configuration options, it's my first time using them and I didn't really understand the define-syntax definition, so I really just skimmed through the guix repository for some uses.

> Anyway, I don't really know PAM, but I've written some comments on the
> patch, hopefully they are useful.

They are a lot! Thank you very much. I hope those comments will be less needed in the future, as I become better as a contributor.

With all the best for tomorrow and all the days to come,
Maya.