[PATCH] gnu: greetd-service-type: Add greeter-extra-groups config field.

  • Done
  • quality assurance status badge
Details
4 participants
  • Liliana Marie Prikler
  • Liliana Marie Prikler
  • muradm
  • (
Owner
unassigned
Submitted by
muradm
Severity
normal
M
M
muradm wrote on 22 Jul 2022 13:45
(address . guix-patches@gnu.org)
20220722114501.5273-1-mail@muradm.net
* gnu/services/base.scm (greetd-service-type): Added configurable groups.
[extensions]: Switching accounts-service-type from const to function.
(<greetd-configuration>): Added greeter-extra-groups field of type list.
(greetd-accounts-service): New variable, function returning list necessary
accounts for accounts-service-type, including the greeter-extra-groups.
(%greetd-accounts): Removed.
* doc/guix.texi: Mention greeter-extra-groups field with example.
---
doc/guix.texi | 8 ++++++++
gnu/services/base.scm | 28 +++++++++++++++-------------
2 files changed, 23 insertions(+), 13 deletions(-)

Toggle diff (74 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 3c5864ec1a..51678b7f19 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18493,6 +18493,14 @@ the 'root' account has just been created.
@item @code{terminals} (default: @code{'()})
List of @code{greetd-terminal-configuration} per terminal for which
@code{greetd} should be started.
+
+@item @code{greeter-extra-groups} (default: @code{'()})
+List of groups which should be added to @code{greeter} user. For instance:
+@lisp
+(greeter-extra-groups '("seat"))
+@end lisp
+Note that, however it will fail if @code{seatd-service-type} is not present,
+or to be more specific, @code{seat} group is not present.
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 27eae75c46..94c8dcac2a 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2918,17 +2918,6 @@ (define (make-greetd-terminal-configuration-file config)
"user = " default-session-user "\n"
"command = " default-session-command "\n")))
-(define %greetd-accounts
- (list (user-account
- (name "greeter")
- (group "greeter")
- ;; video group is required for graphical greeters.
- (supplementary-groups '("video"))
- (system? #t))
- (user-group
- (name "greeter")
- (system? #t))))
-
(define %greetd-file-systems
(list (file-system
(device "none")
@@ -2956,7 +2945,20 @@ (define-record-type* <greetd-configuration>
greetd-configuration?
(motd greetd-motd (default %default-motd))
(allow-empty-passwords? greetd-allow-empty-passwords? (default #t))
- (terminals greetd-terminals (default '())))
+ (terminals greetd-terminals (default '()))
+ (greeter-extra-groups greetd-greeter-extra-groups (default '())))
+
+(define (greetd-accounts-service config)
+ (list (user-group (name "greeter") (system? #t))
+ (user-account
+ (name "greeter")
+ (group "greeter")
+ ;; video group is required for graphical greeters.
+ (supplementary-groups
+ (append
+ '("video")
+ (greetd-greeter-extra-groups config)))
+ (system? #t))))
(define (make-greetd-pam-mount-conf-file config)
(computed-file
@@ -3033,7 +3035,7 @@ (define greetd-service-type
login manager daemon.")
(extensions
(list
- (service-extension account-service-type (const %greetd-accounts))
+ (service-extension account-service-type greetd-accounts-service)
(service-extension file-system-service-type (const %greetd-file-systems))
(service-extension etc-service-type greetd-etc-service)
(service-extension pam-root-service-type greetd-pam-service)
--
2.36.1
(
CLO13EG52QG0.F152G8L34MRH@guix-aspire
On Fri Jul 22, 2022 at 12:45 PM BST, muradm wrote:
Toggle quote (5 lines)
> + ;; video group is required for graphical greeters.
> + (supplementary-groups
> + (append
> + '("video")
> + (greetd-greeter-extra-groups config)))
Change to (cons "video" (greetd-greeter-extra-groups config)) or maybe
use cons* if you think there's a possibility that more groups will later
need to be added.

Otherwise SGTM :)

-- (
L
L
Liliana Marie Prikler wrote on 4 Aug 2022 13:08
Re: greeter user permissions are not enough to talk with seatd
(address . control@debbugs.gnu.org)
b5687a1a3eebc0cce2564634bc4e191cf7abd931.camel@ist.tugraz.at
block 56971 by 56690 56699
thanks

Hi muradm,

Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:
Toggle quote (9 lines)
> [...] greeter (e.g. gtkgreet) requiring communication
> with seatd is failing to start, causing "black screen"
> behavior on active terminal (switching to the other non seatd
> related terminal is possible, for manual permissions
> adjustment as workaround).
>
> To address this issue, we need more flexible control over
> seatd user/group, which creates seatd.sock, and greeter user
> which connects to seatd.sock.
Okay.

Toggle quote (2 lines)
> However, not all greeters require that, so I decided to make
> more flexible.
Flexibility for its own sake is not always the right solution. On the
other hand, looking at the two patches, it appears they are to be used
in combination?

Toggle quote (7 lines)
> Propsed solutions consists of:
>
> * 56690 - gnu: seatd-service-type: Should use seat group.
> With this change, if seatd-service-type is present in the
> system configuration, "seat" group will be added, and seatd
> will run as root/seat. Group is configurable, but default is
> "seat".
Why just the group and no user? Is it not possible to launch seatd as
non-root?

Toggle quote (5 lines)
> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups
>   config field.
> With this change, if user wants to use seatd-service-type with
> greeter requiring seatd.sock, he can add "seat" group to
> greeter-extra-groups field.
Note that you still have a TODO on that patch.

Cheers
M
M
muradm wrote on 5 Aug 2022 08:44
Re: [bug#56699] [PATCH] gnu: greetd-service-type: Add greeter-extra-groups config field.
(name . ()(address . paren@disroot.org)
87v8r71909.fsf@muradm.net
supplimentary-groups receiving a list, so I find it more
informative
when adding apples with apples. And yes with high enough chance
more groups could be added to the list of defaults.

Is there any preference on using cons* in favour of more readable
append?

thanks in advance,
muradm

"(" <paren@disroot.org> writes:

Toggle quote (15 lines)
> On Fri Jul 22, 2022 at 12:45 PM BST, muradm wrote:
>> + ;; video group is required for graphical greeters.
>> + (supplementary-groups
>> + (append
>> + '("video")
>> + (greetd-greeter-extra-groups config)))
> Change to (cons "video" (greetd-greeter-extra-groups config)) or
> maybe
> use cons* if you think there's a possibility that more groups
> will later
> need to be added.
>
> Otherwise SGTM :)
>
> -- (
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEESPY5lma9A9l5HGLP6M7O0mLOBeIFAmLsvSYACgkQ6M7O0mLO
BeJeyBAAmowok7Um3s7D+Deg8pGTAW5s4WzJi/9grZM891wtzed8KOJJT/9/85Zf
9Aso9b36GGLkQDh+D/XznV7y/VTpu7bVIqqR9tC+Uiw584ixsbJcycy1sTeujGoO
uxyqinu0Es9X6dWsiFG9ss5OvYkbDzdfO87jXttfOExd8aEeoh96yNun7uj9zWbB
Onvo9KEny47tR2hMVmKPMcQ+jPWI3xH7wS2Am9+y+MqI2vI3dwirMmSCR78rS4/8
genDLCn5PTQZ8BmqFw42YBe1ByjDBNADEpJwSwUUdLBaDUEdTGpexg4cAVFM7xKa
y/uqaabDpLTS2RgE3V+OFs5CBfTPNXg5rQ9+zhWt7qjg5/zrq9+sMA70tzTOxRCJ
tEFuzfDHmMji6AWZeo1kgWy9EbnxE0uOn+Plh2aAQCYpCn9yTZ7wJtoWd4cE+LzK
GtkqXJhHfO7aMvbNnuN1N7BZtuAipE8kt0pqU1JKLNzM788lCsyIz6N4NJto1rxZ
+RHBFBtDupOTlGR+iCSujcl9wwUvbK0UuERGJmDk9LBtm2RV3HjhEI2btLAu3/S7
gog0NmeVjuOr9qkjVNnqEAp2K5K7UJbhy9gSFN79Xc1/EZuJOStjEx8i5JAbL9s8
qxmukgk21JSix19XPkS0v0XFYTfSH+Jg8lROCQSmPph+iQCtH2U=
=zZ0L
-----END PGP SIGNATURE-----

L
L
Liliana Marie Prikler wrote on 5 Aug 2022 09:54
(address . 56699@debbugs.gnu.org)
cf6a4c2413045877b6c6143a6c35b8178e0dc77b.camel@ist.tugraz.at
Am Freitag, dem 05.08.2022 um 09:44 +0300 schrieb muradm:
Toggle quote (3 lines)
> supplimentary-groups receiving a list, so I find it more
> informative when adding apples with apples. And yes with high enough
> chance more groups could be added to the list of defaults.
What are "the defaults" here, though? A sequence of hard-coded values
known at compile time. There is no need to make that sequence a list.

Toggle quote (2 lines)
> Is there any preference on using cons* in favour of more readable
> append?
Use of cons or cons* signals that you are only expecting user-supplied
groups to be a list and everything else known values. Use of append on
the other hand signals that you are expecting multiple lists, which
even if you did are not currently present.

Cheers
M
M
muradm wrote on 7 Aug 2022 23:42
Re: [bug#56699] [PATCH v2] gnu: greetd-service-type: Add greeter-extra-groups config field.
(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)
874jyn20et.fsf@muradm.net
Correct, no defaults are given. Attached is v2. Defaults
non existent, append/cons* irrelevant.



thanks in advance,
muradm

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (22 lines)
> Am Freitag, dem 05.08.2022 um 09:44 +0300 schrieb muradm:
>> supplimentary-groups receiving a list, so I find it more
>> informative when adding apples with apples. And yes with high
>> enough
>> chance more groups could be added to the list of defaults.
> What are "the defaults" here, though? A sequence of hard-coded
> values
> known at compile time. There is no need to make that sequence a
> list.
>
>> Is there any preference on using cons* in favour of more
>> readable
>> append?
> Use of cons or cons* signals that you are only expecting
> user-supplied
> groups to be a list and everything else known values. Use of
> append on
> the other hand signals that you are expecting multiple lists,
> which
> even if you did are not currently present.
>
> Cheers
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEESPY5lma9A9l5HGLP6M7O0mLOBeIFAmLwMmsACgkQ6M7O0mLO
BeISvQ/+OnYE07rD+0gZcBs7XA2u02k8E31ywGhXeO4Ja8BHfHf9kmSDecmj0kD7
+k5Zwl8IrrADU8GP7MqJwcrc2grwpgMYbwqHTUPN6mHpINjgTyWGtKYsiuHbZ3Tf
+e0HfzVZ968G34xtZy9fiDX5XjGE3LNAj3x/rT1pWdEeLJYZPugTPqaDRZv/yUBv
W+bBKOIYO9X1MWDmsqAcNN1JiibLisgCKC7zIdvFgB4vYLdpni8I7z25ban1loqT
CBjT/3Lf1vUO1Kszf3reUWWVtXZ/a1znDutsvrFapdN9L+0PcLvKAc6wBtCde2sm
v5NcNGn2gse2wCSnEvJS24xouunZ1jiAusPk1jXH03sNLUAvTcC5aEpI2Wbfp2a6
WvOHpUTqojm0MByHgoPSaZwaT9rUKRK9HV1iRwVn8BaoKIkyM53pc98EV2syMPcW
fOVkBEFQCPUectmV5kryYnFvWUQdh7Wb3TR4heSuSTiKVwQFxWXqTG61hh3C2q9S
JkLcmF1HbuyARLwsAWZIlUfMUNdSjJZmp8urdJ+qPgcSCzcDFLQOjFwOKXZuj1Wt
cAvVcctO2CzMSl+blsWRELazYC9Jdfy0SxSXgQ8QPOzxun82ZwVESXzVVLJJgGCE
OWLrX4PWS4B9+QDlsVGq+cekxck8A0PHC0+RpJjGe6iFTFfZQoU=
=rpws
-----END PGP SIGNATURE-----

M
M
muradm wrote on 7 Aug 2022 23:48
[PATCH v2] gnu: greetd-service-type: Add greeter-extra-groups config field.
20220807214804.22323-1-mail@muradm.net
* gnu/services/base.scm (greetd-service-type): Added configurable groups.
[extensions]: Switching accounts-service-type from const to function.
(<greetd-configuration>): Added greeter-groups field of type list.
(greetd-accounts-service): New variable, function returning list necessary
accounts for accounts-service-type, including the greeter-extra-groups.
(%greetd-accounts): Removed.
* gnu/tests/desktop.scm (%minimal-services): Add test for greeter-groups.
* doc/guix.texi: Mention greeter-extra-groups field with example.
---
doc/guix.texi | 8 ++++++++
gnu/services/base.scm | 24 +++++++++++-------------
gnu/tests/desktop.scm | 7 +++++++
3 files changed, 26 insertions(+), 13 deletions(-)

Toggle diff (95 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 21cee4e369..2b09bea3b0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18509,6 +18509,14 @@ the 'root' account has just been created.
@item @code{terminals} (default: @code{'()})
List of @code{greetd-terminal-configuration} per terminal for which
@code{greetd} should be started.
+
+@item @code{greeter-groups} (default: @code{'()})
+List of groups which should be added to @code{greeter} user. For instance:
+@lisp
+(greeter-groups '("seat" "video"))
+@end lisp
+Note that, however it will fail if @code{seatd-service-type} is not present,
+or to be more specific, @code{seat} group is not present.
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 27eae75c46..85de6decfe 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2918,17 +2918,6 @@ (define (make-greetd-terminal-configuration-file config)
"user = " default-session-user "\n"
"command = " default-session-command "\n")))
-(define %greetd-accounts
- (list (user-account
- (name "greeter")
- (group "greeter")
- ;; video group is required for graphical greeters.
- (supplementary-groups '("video"))
- (system? #t))
- (user-group
- (name "greeter")
- (system? #t))))
-
(define %greetd-file-systems
(list (file-system
(device "none")
@@ -2956,7 +2945,16 @@ (define-record-type* <greetd-configuration>
greetd-configuration?
(motd greetd-motd (default %default-motd))
(allow-empty-passwords? greetd-allow-empty-passwords? (default #t))
- (terminals greetd-terminals (default '())))
+ (terminals greetd-terminals (default '()))
+ (greeter-groups greetd-greeter-groups (default '())))
+
+(define (greetd-accounts-service config)
+ (list (user-group (name "greeter") (system? #t))
+ (user-account
+ (name "greeter")
+ (group "greeter")
+ (supplementary-groups (greetd-greeter-groups config))
+ (system? #t))))
(define (make-greetd-pam-mount-conf-file config)
(computed-file
@@ -3033,7 +3031,7 @@ (define greetd-service-type
login manager daemon.")
(extensions
(list
- (service-extension account-service-type (const %greetd-accounts))
+ (service-extension account-service-type greetd-accounts-service)
(service-extension file-system-service-type (const %greetd-file-systems))
(service-extension etc-service-type greetd-etc-service)
(service-extension pam-root-service-type greetd-pam-service)
diff --git a/gnu/tests/desktop.scm b/gnu/tests/desktop.scm
index 25971f9225..ef4a7e0ec9 100644
--- a/gnu/tests/desktop.scm
+++ b/gnu/tests/desktop.scm
@@ -122,6 +122,7 @@ (define %minimal-services
(service seatd-service-type)
(service greetd-service-type
(greetd-configuration
+ (greeter-groups '("input" "video"))
(terminals
(list
;; we can make any terminal active by default
@@ -286,6 +287,12 @@ (define (greetd-pid-to-sock pid)
(marionette-type "echo alice > /run/user/1000/test\n" marionette)
(file-get-all-strings "/run/user/1000/test")))
+ (test-equal "check greeter user has correct groups"
+ "greeter input video\n"
+ (begin
+ (marionette-type "id -Gn greeter > /run/user/1000/greeter-groups\n" marionette)
+ (file-get-all-strings "/run/user/1000/greeter-groups")))
+
(test-assert "screendump"
(begin
(marionette-control (string-append "screendump " #$output
--
2.37.1
L
L
Liliana Marie Prikler wrote on 8 Aug 2022 07:41
(name . ()(address . paren@disroot.org)
1d138ba85a305947acc267d995179effd6be0edd.camel@ist.tugraz.at
Am Montag, dem 08.08.2022 um 00:48 +0300 schrieb muradm:
Toggle quote (24 lines)
> ---
>  doc/guix.texi         |  8 ++++++++
>  gnu/services/base.scm | 24 +++++++++++-------------
>  gnu/tests/desktop.scm |  7 +++++++
>  3 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 21cee4e369..2b09bea3b0 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -18509,6 +18509,14 @@ the 'root' account has just been created.
>  @item @code{terminals} (default: @code{'()})
>  List of @code{greetd-terminal-configuration} per terminal for which
>  @code{greetd} should be started.
> +
> +@item @code{greeter-groups} (default: @code{'()})
> +List of groups which should be added to @code{greeter} user. For
> instance:
> +@lisp
> +(greeter-groups '("seat" "video"))
> +@end lisp
> +Note that, however it will fail if @code{seatd-service-type} is not
> present,
> +or to be more specific, @code{seat} group is not present.
Note that this example will fail if the @code{seat} group does not
exist.

Toggle quote (1 lines)
> +  (greeter-groups greetd-greeter-groups (default '())))
I think, we can err a little on the side of verbosity here and make
clear that it's greeter-supplementary-groups.

Other than that LGTM
M
M
muradm wrote on 8 Aug 2022 21:27
(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)
87edxqzg95.fsf@muradm.net
From c8ba263cd3323c06cd3044243347e76a85cb9628 Mon Sep 17 00:00:00 2001
From: muradm <mail@muradm.net>
Date: Fri, 22 Jul 2022 14:28:57 +0300
Subject: [PATCH v3] gnu: greetd-service-type: Add greeter-extra-groups config
field.
To: 56699@debbugs.gnu.org

* gnu/services/base.scm (greetd-service-type): Added configurable
supplementary groups.
[extensions]: Switching accounts-service-type from const to function.
(<greetd-configuration>): Added greeter-supplementary-groups field.
(greetd-accounts-service): New variable, function returning list
necessary accounts for accounts-service-type, including the
greeter-supplementary-groups.
(%greetd-accounts): Removed.
* gnu/tests/desktop.scm (%minimal-services): Add test for
greeter-supplementary-groups.
* doc/guix.texi: Mention greeter-supplementary-groups field with example.
---
doc/guix.texi | 8 ++++++++
gnu/services/base.scm | 24 +++++++++++-------------
gnu/tests/desktop.scm | 7 +++++++
3 files changed, 26 insertions(+), 13 deletions(-)

Toggle diff (95 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 9a6a5c307d..8eda5bb2c0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18541,6 +18541,14 @@ the 'root' account has just been created.
@item @code{terminals} (default: @code{'()})
List of @code{greetd-terminal-configuration} per terminal for which
@code{greetd} should be started.
+
+@item @code{greeter-supplementary-groups} (default: @code{'()})
+List of groups which should be added to @code{greeter} user. For instance:
+@lisp
+(greeter-supplementary-groups '("seat" "video"))
+@end lisp
+Note that, however it will fail if @code{seatd-service-type} is not present,
+or to be more specific, @code{seat} group is not present.
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 27eae75c46..251196b108 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2918,17 +2918,6 @@ (define (make-greetd-terminal-configuration-file config)
"user = " default-session-user "\n"
"command = " default-session-command "\n")))
-(define %greetd-accounts
- (list (user-account
- (name "greeter")
- (group "greeter")
- ;; video group is required for graphical greeters.
- (supplementary-groups '("video"))
- (system? #t))
- (user-group
- (name "greeter")
- (system? #t))))
-
(define %greetd-file-systems
(list (file-system
(device "none")
@@ -2956,7 +2945,16 @@ (define-record-type* <greetd-configuration>
greetd-configuration?
(motd greetd-motd (default %default-motd))
(allow-empty-passwords? greetd-allow-empty-passwords? (default #t))
- (terminals greetd-terminals (default '())))
+ (terminals greetd-terminals (default '()))
+ (greeter-supplementary-groups greetd-greeter-supplementary-groups (default '())))
+
+(define (greetd-accounts-service config)
+ (list (user-group (name "greeter") (system? #t))
+ (user-account
+ (name "greeter")
+ (group "greeter")
+ (supplementary-groups (greetd-greeter-supplementary-groups config))
+ (system? #t))))
(define (make-greetd-pam-mount-conf-file config)
(computed-file
@@ -3033,7 +3031,7 @@ (define greetd-service-type
login manager daemon.")
(extensions
(list
- (service-extension account-service-type (const %greetd-accounts))
+ (service-extension account-service-type greetd-accounts-service)
(service-extension file-system-service-type (const %greetd-file-systems))
(service-extension etc-service-type greetd-etc-service)
(service-extension pam-root-service-type greetd-pam-service)
diff --git a/gnu/tests/desktop.scm b/gnu/tests/desktop.scm
index 25971f9225..f20423f0aa 100644
--- a/gnu/tests/desktop.scm
+++ b/gnu/tests/desktop.scm
@@ -122,6 +122,7 @@ (define %minimal-services
(service seatd-service-type)
(service greetd-service-type
(greetd-configuration
+ (greeter-supplementary-groups '("input" "video"))
(terminals
(list
;; we can make any terminal active by default
@@ -286,6 +287,12 @@ (define (greetd-pid-to-sock pid)
(marionette-type "echo alice > /run/user/1000/test\n" marionette)
(file-get-all-strings "/run/user/1000/test")))
+ (test-equal "check greeter user has correct groups"
+ "greeter input video\n"
+ (begin
+ (marionette-type "id -Gn greeter > /run/user/1000/greeter-groups\n" marionette)
+ (file-get-all-strings "/run/user/1000/greeter-groups")))
+
(test-assert "screendump"
(begin
(marionette-control (string-append "screendump " #$output
--
2.37.1
Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (32 lines)
> Am Montag, dem 08.08.2022 um 00:48 +0300 schrieb muradm:
>> ---
>>  doc/guix.texi         |  8 ++++++++
>>  gnu/services/base.scm | 24 +++++++++++-------------
>>  gnu/tests/desktop.scm |  7 +++++++
>>  3 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index 21cee4e369..2b09bea3b0 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -18509,6 +18509,14 @@ the 'root' account has just been
>> created.
>>  @item @code{terminals} (default: @code{'()})
>>  List of @code{greetd-terminal-configuration} per terminal for
>> which
>>  @code{greetd} should be started.
>> +
>> +@item @code{greeter-groups} (default: @code{'()})
>> +List of groups which should be added to @code{greeter} user.
>> For
>> instance:
>> +@lisp
>> +(greeter-groups '("seat" "video"))
>> +@end lisp
>> +Note that, however it will fail if @code{seatd-service-type}
>> is not
>> present,
>> +or to be more specific, @code{seat} group is not present.
> Note that this example will fail if the @code{seat} group does
> not
> exist.
Which is stated right on the next line.

Toggle quote (4 lines)
>> +  (greeter-groups greetd-greeter-groups (default '())))
> I think, we can err a little on the side of verbosity here and
> make
> clear that it's greeter-supplementary-groups.
done

Toggle quote (1 lines)
> Other than that LGTM
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEESPY5lma9A9l5HGLP6M7O0mLOBeIFAmLxY/YACgkQ6M7O0mLO
BeJEpRAAkwJTDFqHu7oYZzuZdzFj4l0IfaEuKrJ5CYYEjuRkNbSo2bN0+26BeKVQ
JGCcMelNAtBMblrDd5DCDq16NQIXwnkiHyl/6bjLdDHSA4oZOR0WTopYOaRcyagP
dJ3kNnaNX2RZeZzm5OMkC5A4NlHGsBV6QlY8IcCz9+/UfO6CMhwX28k5XvPiMiSw
i3/5i0HpRnsutQAK/ZCuwcafqg70EOwaasab78fJVnsoDjFiJb8Aju8IIzPJCbgM
lOF4+CjsXE9BcqJgUCTESuVuNFESiX9JdpOQBlwRVhN+cDsLtXsnSOg1rGux421j
cJ/sHDesL7tKJXTEPY9B8p/3CfT+t/swwFAT8+9UWYTMnlsA9EsCqwv/ZcdmHVWV
JWu32Axu6xVnXycB8BgNcJOU0WBduo41xgpj5cGT4zB5gGDmRoncVbsCpA/urOfq
g573iHT/B7PzVbCbYjQ1qb2l34CYht4PRZMa9HcPikepFAFPX5u6vLfFaMv0un7f
qlREWw/+14a2GCKt6h213oBBPUGylIqIA/0RNb4+QdOCCjJUmpBXz+75y+dTQrGK
HLDxppK1aZQkRRcLoQyzAi+ujoKRRBt+EvY+Tww+9GIcc81Z6YIXB3exYECPJTx4
x+aaGLv+K+qTZAsOeB3RU8+hF5QHB5MghCBhzjfIWYCODUGyFqQ=
=Qib/
-----END PGP SIGNATURE-----

L
L
Liliana Marie Prikler wrote on 9 Aug 2022 08:25
(name . muradm)(address . mail@muradm.net)
eb2684ed616a896916fbb63f42bbd3bc2a07171e.camel@ist.tugraz.at
Am Montag, dem 08.08.2022 um 22:27 +0300 schrieb muradm:
Toggle quote (35 lines)
>
> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
>
> > Am Montag, dem 08.08.2022 um 00:48 +0300 schrieb muradm:
> > > ---
> > >  doc/guix.texi         |  8 ++++++++
> > >  gnu/services/base.scm | 24 +++++++++++-------------
> > >  gnu/tests/desktop.scm |  7 +++++++
> > >  3 files changed, 26 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/doc/guix.texi b/doc/guix.texi
> > > index 21cee4e369..2b09bea3b0 100644
> > > --- a/doc/guix.texi
> > > +++ b/doc/guix.texi
> > > @@ -18509,6 +18509,14 @@ the 'root' account has just been
> > > created.
> > >  @item @code{terminals} (default: @code{'()})
> > >  List of @code{greetd-terminal-configuration} per terminal for
> > > which
> > >  @code{greetd} should be started.
> > > +
> > > +@item @code{greeter-groups} (default: @code{'()})
> > > +List of groups which should be added to @code{greeter} user.
> > > For
> > > instance:
> > > +@lisp
> > > +(greeter-groups '("seat" "video"))
> > > +@end lisp
> > > +Note that, however it will fail if @code{seatd-service-type}
> > > is not
> > > present,
> > > +or to be more specific, @code{seat} group is not present.
> > Note that this example will fail if the @code{seat} group does
> > not exist.
> Which is stated right on the next line.
In a convoluted way. "Note that this example will fail if the
@code{seat} group does not exist." is imho easier on the reader.

Cheers
M
M
muradm wrote on 9 Aug 2022 21:40
Re: [PATCH v4] gnu: greetd-service-type: Add greeter-extra-groups config field.
(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)
8735e5yziu.fsf@muradm.net
fixed
From d92efe5c5f26645513911ac11ec8876681768b4b Mon Sep 17 00:00:00 2001
From: muradm <mail@muradm.net>
Date: Fri, 22 Jul 2022 14:28:57 +0300
Subject: [PATCH v4] gnu: greetd-service-type: Add greeter-extra-groups config
field.
To: 56699@debbugs.gnu.org

* gnu/services/base.scm (greetd-service-type): Added configurable
supplementary groups.
[extensions]: Switching accounts-service-type from const to function.
(<greetd-configuration>): Added greeter-supplementary-groups field.
(greetd-accounts-service): New variable, function returning list
necessary accounts for accounts-service-type, including the
greeter-supplementary-groups.
(%greetd-accounts): Removed.
* gnu/tests/desktop.scm (%minimal-services): Add test for
greeter-supplementary-groups.
* doc/guix.texi: Mention greeter-supplementary-groups field with example.
---
doc/guix.texi | 7 +++++++
gnu/services/base.scm | 24 +++++++++++-------------
gnu/tests/desktop.scm | 7 +++++++
3 files changed, 25 insertions(+), 13 deletions(-)

Toggle diff (94 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 896c830aeb..3f04a32f2d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18554,6 +18554,13 @@ the 'root' account has just been created.
@item @code{terminals} (default: @code{'()})
List of @code{greetd-terminal-configuration} per terminal for which
@code{greetd} should be started.
+
+@item @code{greeter-supplementary-groups} (default: @code{'()})
+List of groups which should be added to @code{greeter} user. For instance:
+@lisp
+(greeter-supplementary-groups '("seat" "video"))
+@end lisp
+Note that, this example will fail if @code{seat} group does not exist.
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 27eae75c46..251196b108 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2918,17 +2918,6 @@ (define (make-greetd-terminal-configuration-file config)
"user = " default-session-user "\n"
"command = " default-session-command "\n")))
-(define %greetd-accounts
- (list (user-account
- (name "greeter")
- (group "greeter")
- ;; video group is required for graphical greeters.
- (supplementary-groups '("video"))
- (system? #t))
- (user-group
- (name "greeter")
- (system? #t))))
-
(define %greetd-file-systems
(list (file-system
(device "none")
@@ -2956,7 +2945,16 @@ (define-record-type* <greetd-configuration>
greetd-configuration?
(motd greetd-motd (default %default-motd))
(allow-empty-passwords? greetd-allow-empty-passwords? (default #t))
- (terminals greetd-terminals (default '())))
+ (terminals greetd-terminals (default '()))
+ (greeter-supplementary-groups greetd-greeter-supplementary-groups (default '())))
+
+(define (greetd-accounts-service config)
+ (list (user-group (name "greeter") (system? #t))
+ (user-account
+ (name "greeter")
+ (group "greeter")
+ (supplementary-groups (greetd-greeter-supplementary-groups config))
+ (system? #t))))
(define (make-greetd-pam-mount-conf-file config)
(computed-file
@@ -3033,7 +3031,7 @@ (define greetd-service-type
login manager daemon.")
(extensions
(list
- (service-extension account-service-type (const %greetd-accounts))
+ (service-extension account-service-type greetd-accounts-service)
(service-extension file-system-service-type (const %greetd-file-systems))
(service-extension etc-service-type greetd-etc-service)
(service-extension pam-root-service-type greetd-pam-service)
diff --git a/gnu/tests/desktop.scm b/gnu/tests/desktop.scm
index 25971f9225..f20423f0aa 100644
--- a/gnu/tests/desktop.scm
+++ b/gnu/tests/desktop.scm
@@ -122,6 +122,7 @@ (define %minimal-services
(service seatd-service-type)
(service greetd-service-type
(greetd-configuration
+ (greeter-supplementary-groups '("input" "video"))
(terminals
(list
;; we can make any terminal active by default
@@ -286,6 +287,12 @@ (define (greetd-pid-to-sock pid)
(marionette-type "echo alice > /run/user/1000/test\n" marionette)
(file-get-all-strings "/run/user/1000/test")))
+ (test-equal "check greeter user has correct groups"
+ "greeter input video\n"
+ (begin
+ (marionette-type "id -Gn greeter > /run/user/1000/greeter-groups\n" marionette)
+ (file-get-all-strings "/run/user/1000/greeter-groups")))
+
(test-assert "screendump"
(begin
(marionette-control (string-append "screendump " #$output
--
2.37.1
Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (44 lines)
> Am Montag, dem 08.08.2022 um 22:27 +0300 schrieb muradm:
>>
>> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
>>
>> > Am Montag, dem 08.08.2022 um 00:48 +0300 schrieb muradm:
>> > > ---
>> > >  doc/guix.texi         |  8 ++++++++
>> > >  gnu/services/base.scm | 24 +++++++++++-------------
>> > >  gnu/tests/desktop.scm |  7 +++++++
>> > >  3 files changed, 26 insertions(+), 13 deletions(-)
>> > >
>> > > diff --git a/doc/guix.texi b/doc/guix.texi
>> > > index 21cee4e369..2b09bea3b0 100644
>> > > --- a/doc/guix.texi
>> > > +++ b/doc/guix.texi
>> > > @@ -18509,6 +18509,14 @@ the 'root' account has just been
>> > > created.
>> > >  @item @code{terminals} (default: @code{'()})
>> > >  List of @code{greetd-terminal-configuration} per terminal
>> > > for
>> > > which
>> > >  @code{greetd} should be started.
>> > > +
>> > > +@item @code{greeter-groups} (default: @code{'()})
>> > > +List of groups which should be added to @code{greeter}
>> > > user.
>> > > For
>> > > instance:
>> > > +@lisp
>> > > +(greeter-groups '("seat" "video"))
>> > > +@end lisp
>> > > +Note that, however it will fail if
>> > > @code{seatd-service-type}
>> > > is not
>> > > present,
>> > > +or to be more specific, @code{seat} group is not present.
>> > Note that this example will fail if the @code{seat} group
>> > does
>> > not exist.
>> Which is stated right on the next line.
> In a convoluted way. "Note that this example will fail if the
> @code{seat} group does not exist." is imho easier on the reader.
>
> Cheers
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEESPY5lma9A9l5HGLP6M7O0mLOBeIFAmLyuKkACgkQ6M7O0mLO
BeLNphAAuMlTmTNEr7TMRg4H8IjukSoamQAzk0/UZQiPkWNrPlVIH4gijCKEwbcQ
MYxYR/YBZyRnrNlwhkha4wBYkVBPBnMEWdWcc089lNpMMAUcjiXa7QbsPrQ2htoa
XgR0iE5PuFN7S82hW42bgu4lEV1F5ii2gy0mixt8bcYWtiNCJHg67/oQFSeB8fbM
D5Ysyb3U/p0kWzFDnTxgAdnYw9ihAzfoMvbvEpCuOt7HKU/+fBkUAcfDAeH0sA/B
SwmWlcwx/oKCbxv+YYv8BZPMC63dbDTvRLYDX6KBAAArrxm/zSmmsy/R7aMBqPeU
syKi0SIjNE7flvpncCoXmTRUTaDsEMbTys2oD+p0D6D1jnlyNOKSvoIZh0QpLEii
/mEylYFASD63Quml0Ck2rnxgKv0LN/Vnc9qix3SgvK/4nwY27vjDRjNFNJkFbfMs
xlFKryu7gQSisnO3RsSH7gXl/LVWcUXLeZeD03Dh0bZfYrnfHNSY++CONm+x+BRt
kOx8xKstq/4I8m4Q2j7n14E6KYkgBgEx8bWCWfeoMYuihEd3dCKlmF4POaISo1+t
DVdUPpxl0+znqdQBcApyrYit5wK65ZbhPrcKFmvpTdVYAKBxSgauxqIw7WnzEgwZ
/JWYEMHE78dLTD2OYMJQhA9YKuXddYwKxFCutgv5PNcetu/ddSg=
=sQB+
-----END PGP SIGNATURE-----

L
L
Liliana Marie Prikler wrote on 10 Aug 2022 09:49
(name . muradm)(address . mail@muradm.net)
7dfb270fe1a914b02dbd62baa94c7197a7ba541b.camel@ist.tugraz.at
Am Dienstag, dem 09.08.2022 um 22:40 +0300 schrieb muradm:
Toggle quote (1 lines)
> fixed
v4 LGTM
L
L
Liliana Marie Prikler wrote on 26 Aug 2022 19:06
Re: greeter user permissions are not enough to talk with seatd
(name . muradm)(address . mail@muradm.net)
400cf1fed0d340398da6e2e0e32bebdb8fd842ef.camel@gmail.com
Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:
Toggle quote (5 lines)
> * 56690 - gnu: seatd-service-type: Should use seat group.
> With this change, if seatd-service-type is present in the
> system configuration, "seat" group will be added, and seatd
> will run as root/seat. Group is configurable, but default is
> "seat".
I made it so that by default the sanitizer is used to turn the string
"seat" into a group and used (ice-9 match), reducing some needless
redundancy. I also reworded the manual to the best of my ability
following our conversations and adapted the commit message.

Toggle quote (5 lines)
> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups
>   config field.
> With this change, if user wants to use seatd-service-type with
> greeter requiring seatd.sock, he can add "seat" group to
> greeter-extra-groups field.
I fixed some minor issue in the manual and reindented the marionette-
type in the tests, also reworded the commit message.

I didn't get the chance to run the system tests – some timeout causes
the marionette build to fail on my machine – but I verified
independently that at least the seatd socket has the right permissions.
I hope this will be enough for you to get gtkgreet running.

Cheers
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 56699@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 56699
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch