homectl: apply all --member-of= groups from a comma-separated list

Commit 0e1ede4b4b6d1ce6b5b6cda5f803e4f1b5aa4a03 introduced a bug where
we'd always fetch the "original" (empty) list of groups when processing
a comma-separated list of groups from the --member-of= option, so only
the last group from the list would get applied. This bug was then later
(in 316e9887f2a48bd1c4efa3e31b4bfbaeb22de3a3) refactored into a separate
function.

Follow-up for 0e1ede4b4b6d1ce6b5b6cda5f803e4f1b5aa4a03.
Resolves: #41286

diff --git a/src/home/homectl.c b/src/home/homectl.c
index db4e6639d5d9a50ed73ff91236e2050e8c7f9082..9dc135fd70508fbedb2eaa0dd0cfc6b2ec58ac07 100644 (file)
--- a/src/home/homectl.c
+++ b/src/home/homectl.c
@@ -3691,7 +3691,6 @@ static int parse_language_field(char ***languages, const char *arg) {
 }
 
 static int parse_group_field(
-                sd_json_variant *source_identity,
                 sd_json_variant **identity,
                 const char *field,
                 const char *arg) {
@@ -3717,7 +3716,7 @@ static int parse_group_field(
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid group name %s.", word);
 
                 _cleanup_(sd_json_variant_unrefp) sd_json_variant *mo =
-                        sd_json_variant_ref(sd_json_variant_by_key(source_identity, field));
+                        sd_json_variant_ref(sd_json_variant_by_key(*identity, field));
 
                 r = sd_json_variant_strv(mo, &list);
                 if (r < 0)
@@ -4383,7 +4382,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_ALIAS:
-                        r = parse_group_field(arg_identity_extra, &arg_identity_extra, "aliases", optarg);
+                        r = parse_group_field(&arg_identity_extra, "aliases", optarg);
                         if (r < 0)
                                 return r;
                         break;
@@ -4692,9 +4691,7 @@ static int parse_argv(int argc, char *argv[]) {
                 }
 
                 case 'G':
-                        r = parse_group_field(arg_identity_extra,
-                                              match_identity ?: &arg_identity_extra,
-                                              "memberOf", optarg);
+                        r = parse_group_field(match_identity ?: &arg_identity_extra, "memberOf", optarg);
                         if (r < 0)
                                 return r;
                         break;

diff --git a/test/units/TEST-46-HOMED.sh b/test/units/TEST-46-HOMED.sh
index 46abca9bace726ecc38e402980263af6a833c400..4b81799ef3deada02a966d26a4d59b06a653cee4 100755 (executable)
--- a/test/units/TEST-46-HOMED.sh
+++ b/test/units/TEST-46-HOMED.sh
@@ -87,6 +87,29 @@ testcase_basic() {
     PASSWORD=xEhErW0ndafV4s homectl update test-user --real-name="Inline test"
     inspect test-user
 
+    # --member-of=
+    systemd-sysusers --inline "g test-group1" "g test-group2"
+    # Single group
+    PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of="test-group1"
+    [[ "$(homectl inspect -j test-user | jq -c .memberOf)" == '["test-group1"]' ]]
+    # Multiple groups
+    PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of="test-group1,test-group2"
+    [[ "$(homectl inspect -j test-user | jq -c .memberOf)" == '["test-group1","test-group2"]' ]]
+    # Empty argument
+    PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of=
+    [[ "$(homectl inspect -j test-user | jq -c .memberOf)" == 'null' ]]
+    # Argument shenanigans
+    #   - only separators
+    (! PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of=",,,,,,,,,,,,,,,,,,")
+    #   - invalid group
+    (! PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of="test-group1,inv@lid.group?")
+    #   - separators & valid groups
+    PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of=",,,,,test-group1,,,,,,,,,,,,,,test-group2,"
+    [[ "$(homectl inspect -j test-user | jq -c .memberOf)" == '["test-group1","test-group2"]' ]]
+    #   - duplicate groups
+    PASSWORD=xEhErW0ndafV4s homectl update test-user --member-of="test-group2,test-group1,test-group1,test-group2"
+    [[ "$(homectl inspect -j test-user | jq -c .memberOf)" == '["test-group1","test-group2"]' ]]
+
     homectl deactivate test-user
     inspect test-user