Merge pull request #16690 from poettering/userdb-group-desc

description field for group records

12 files changed, 60 insertions, 20 deletions

diff --git a/TODO b/TODO
index d6ae200a34..c0fbf8bc6a 100644
--- a/TODO
+++ b/TODO
@@ -372,7 +372,6 @@ Features:
   - in systemd's PAMName= logic: query passwords with ssh-askpassword, so that we can make "loginctl set-linger" mode work
   - fingerprint authentication, pattern authentication, …
   - make sure "classic" user records can also be managed by homed
-  - description field for groups
   - make size of $XDG_RUNTIME_DIR configurable in user record
   - reuse pwquality magic in firstboot
   - query password from kernel keyring first
diff --git a/docs/GROUP_RECORD.md b/docs/GROUP_RECORD.md
index 2ea0b73a36..01800494da 100644
--- a/docs/GROUP_RECORD.md
+++ b/docs/GROUP_RECORD.md
@@ -22,6 +22,10 @@ UNIX/glibc NSS `struct group`, or the shadow structure `struct sgrp`'s
 `realm` → The "realm" the group belongs to, conceptually identical to the same
 field of user records. A string in DNS domain name syntax.
 
+`description` → A descriptive string for the group. This is similar to the
+`realName` field of user records, and accepts arbitrary strings, as long as
+they follow the same GECOS syntax requirements as `realName`.
+
 `disposition` → The disposition of the group, conceptually identical to the
 same field of user records. A string.
 
diff --git a/docs/USER_RECORD.md b/docs/USER_RECORD.md
index f6d22c217b..960a82d894 100644
--- a/docs/USER_RECORD.md
+++ b/docs/USER_RECORD.md
@@ -221,12 +221,14 @@ optional, when unset the user should not be considered part of any realm. A
 user record with a realm set is never compatible (for the purpose of updates,
 see above) with a user record without one set, even if the `userName` field matches.
 
-`realName` → The real name of the user, a string. This should contain the user's
-real ("human") name, and corresponds loosely to the GECOS field of classic UNIX
-user records. When converting a `struct passwd` to a JSON user record this
-field is initialized from GECOS (i.e. the `pw_gecos` field), and vice versa
-when converting back. That said, unlike GECOS this field is supposed to contain
-only the real name and no other information.
+`realName` → The real name of the user, a string. This should contain the
+user's real ("human") name, and corresponds loosely to the GECOS field of
+classic UNIX user records. When converting a `struct passwd` to a JSON user
+record this field is initialized from GECOS (i.e. the `pw_gecos` field), and
+vice versa when converting back. That said, unlike GECOS this field is supposed
+to contain only the real name and no other information. This field must not
+contain control characters (such as `\n`) or colons (`:`), since those are used
+as record separators in classic `/etc/passwd` files and similar formats.
 
 `emailAddress` → The email address of the user, formatted as
 string. [`pam_systemd`](https://www.freedesktop.org/software/systemd/man/pam_systemd.html)
diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c
index eca27f4d7d..54f1cc7974 100644
--- a/src/core/core-varlink.c
+++ b/src/core/core-varlink.c
@@ -136,6 +136,7 @@ static int build_group_json(const char *group_name, gid_t gid, JsonVariant **ret
         return json_build(ret, JSON_BUILD_OBJECT(
                                    JSON_BUILD_PAIR("record", JSON_BUILD_OBJECT(
                                        JSON_BUILD_PAIR("groupName", JSON_BUILD_STRING(group_name)),
+                                       JSON_BUILD_PAIR("description", JSON_BUILD_STRING("Dynamic Group")),
                                        JSON_BUILD_PAIR("gid", JSON_BUILD_UNSIGNED(gid)),
                                        JSON_BUILD_PAIR("service", JSON_BUILD_STRING("io.systemd.DynamicUser")),
                                        JSON_BUILD_PAIR("disposition", JSON_BUILD_STRING("dynamic"))))));
diff --git a/src/home/user-record-util.c b/src/home/user-record-util.c
index a9f6f05e13..516ffaa8a6 100644
--- a/src/home/user-record-util.c
+++ b/src/home/user-record-util.c
@@ -104,7 +104,7 @@ int user_record_synthesize(
 }
 
 int group_record_synthesize(GroupRecord *g, UserRecord *h) {
-        _cleanup_free_ char *un = NULL, *rr = NULL, *group_name_and_realm = NULL;
+        _cleanup_free_ char *un = NULL, *rr = NULL, *group_name_and_realm = NULL, *description = NULL;
         char smid[SD_ID128_STRING_MAX];
         sd_id128_t mid;
         int r;
@@ -133,10 +133,15 @@ int group_record_synthesize(GroupRecord *g, UserRecord *h) {
                         return -ENOMEM;
         }
 
+        description = strjoin("Primary Group of User ", un);
+        if (!description)
+                return -ENOMEM;
+
         r = json_build(&g->json,
                        JSON_BUILD_OBJECT(
                                        JSON_BUILD_PAIR("groupName", JSON_BUILD_STRING(un)),
                                        JSON_BUILD_PAIR_CONDITION(!!rr, "realm", JSON_BUILD_STRING(rr)),
+                                       JSON_BUILD_PAIR("description", JSON_BUILD_STRING(description)),
                                        JSON_BUILD_PAIR("binding", JSON_BUILD_OBJECT(
                                                                        JSON_BUILD_PAIR(sd_id128_to_string(mid, smid), JSON_BUILD_OBJECT(
                                                                                                        JSON_BUILD_PAIR("gid", JSON_BUILD_UNSIGNED(user_record_gid(h))))))),
diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c
index 058ee5c8ea..de8cdeb87d 100644
--- a/src/machine/machined-varlink.c
+++ b/src/machine/machined-varlink.c
@@ -93,6 +93,8 @@ static int user_lookup_name(Manager *m, const char *name, uid_t *ret_uid, char *
         int r;
 
         assert(m);
+        assert(ret_uid);
+        assert(ret_real_name);
 
         if (!valid_user_group_name(name, 0))
                 return -ESRCH;
@@ -186,7 +188,7 @@ static int vl_method_get_user_record(Varlink *link, JsonVariant *parameters, Var
         return varlink_reply(link, v);
 }
 
-static int build_group_json(const char *group_name, gid_t gid, JsonVariant **ret) {
+static int build_group_json(const char *group_name, gid_t gid, const char *description, JsonVariant **ret) {
         assert(group_name);
         assert(gid_is_valid(gid));
         assert(ret);
@@ -195,6 +197,7 @@ static int build_group_json(const char *group_name, gid_t gid, JsonVariant **ret
                                    JSON_BUILD_PAIR("record", JSON_BUILD_OBJECT(
                                        JSON_BUILD_PAIR("groupName", JSON_BUILD_STRING(group_name)),
                                        JSON_BUILD_PAIR("gid", JSON_BUILD_UNSIGNED(gid)),
+                                       JSON_BUILD_PAIR_CONDITION(!isempty(description), "description", JSON_BUILD_STRING(description)),
                                        JSON_BUILD_PAIR("service", JSON_BUILD_STRING("io.systemd.Machine")),
                                        JSON_BUILD_PAIR("disposition", JSON_BUILD_STRING("container"))))));
     }
@@ -211,8 +214,8 @@ static bool group_match_lookup_parameters(LookupParameters *p, const char *name,
         return true;
 }
 
-static int group_lookup_gid(Manager *m, gid_t gid, char **ret_name) {
-        _cleanup_free_ char *n = NULL;
+static int group_lookup_gid(Manager *m, gid_t gid, char **ret_name, char **ret_description) {
+        _cleanup_free_ char *n = NULL, *d = NULL;
         gid_t converted_gid;
         Machine *machine;
         int r;
@@ -220,6 +223,7 @@ static int group_lookup_gid(Manager *m, gid_t gid, char **ret_name) {
         assert(m);
         assert(gid_is_valid(gid));
         assert(ret_name);
+        assert(ret_description);
 
         if (gid < 0x10000) /* Host GID range */
                 return -ESRCH;
@@ -236,18 +240,27 @@ static int group_lookup_gid(Manager *m, gid_t gid, char **ret_name) {
         if (!valid_user_group_name(n, 0))
                 return -ESRCH;
 
+        if (asprintf(&d, "GID " GID_FMT " of Container %s", converted_gid, machine->name) < 0)
+                return -ENOMEM;
+        if (!valid_gecos(d))
+                d = mfree(d);
+
         *ret_name = TAKE_PTR(n);
+        *ret_description = TAKE_PTR(d);
+
         return 0;
 }
 
-static int group_lookup_name(Manager *m, const char *name, gid_t *ret_gid) {
-        _cleanup_free_ char *mn = NULL;
+static int group_lookup_name(Manager *m, const char *name, gid_t *ret_gid, char **ret_description) {
+        _cleanup_free_ char *mn = NULL, *desc = NULL;
         gid_t gid, converted_gid;
         Machine *machine;
         const char *e, *d;
         int r;
 
         assert(m);
+        assert(ret_gid);
+        assert(ret_description);
 
         if (!valid_user_group_name(name, 0))
                 return -ESRCH;
@@ -278,7 +291,13 @@ static int group_lookup_name(Manager *m, const char *name, gid_t *ret_gid) {
         if (r < 0)
                 return r;
 
+        if (asprintf(&desc, "GID " GID_FMT " of Container %s", gid, machine->name) < 0)
+                return -ENOMEM;
+        if (!valid_gecos(desc))
+                desc = mfree(desc);
+
         *ret_gid = converted_gid;
+        *ret_description = desc;
         return 0;
 }
 
@@ -295,7 +314,7 @@ static int vl_method_get_group_record(Varlink *link, JsonVariant *parameters, Va
         LookupParameters p = {
                 .gid = GID_INVALID,
         };
-        _cleanup_free_ char *found_name = NULL;
+        _cleanup_free_ char *found_name = NULL, *found_description = NULL;
         uid_t found_gid = GID_INVALID, gid;
         Manager *m = userdata;
         const char *gn;
@@ -312,9 +331,9 @@ static int vl_method_get_group_record(Varlink *link, JsonVariant *parameters, Va
                 return varlink_error(link, "io.systemd.UserDatabase.BadService", NULL);
 
         if (gid_is_valid(p.gid))
-                r = group_lookup_gid(m, p.gid, &found_name);
+                r = group_lookup_gid(m, p.gid, &found_name, &found_description);
         else if (p.group_name)
-                r = group_lookup_name(m, p.group_name, (uid_t*) &found_gid);
+                r = group_lookup_name(m, p.group_name, (uid_t*) &found_gid, &found_description);
         else
                 return varlink_error(link, "io.systemd.UserDatabase.EnumerationNotSupported", NULL);
         if (r == -ESRCH)
@@ -328,7 +347,7 @@ static int vl_method_get_group_record(Varlink *link, JsonVariant *parameters, Va
         if (!group_match_lookup_parameters(&p, gn, gid))
                 return varlink_error(link, "io.systemd.UserDatabase.ConflictingRecordFound", NULL);
 
-        r = build_group_json(gn, gid, &v);
+        r = build_group_json(gn, gid, found_description, &v);
         if (r < 0)
                 return r;
 
diff --git a/src/shared/group-record-show.c b/src/shared/group-record-show.c
index d0300e483c..8b59f919fa 100644
--- a/src/shared/group-record-show.c
+++ b/src/shared/group-record-show.c
@@ -68,6 +68,9 @@ void group_record_show(GroupRecord *gr, bool show_full_user_info) {
                 }
         }
 
+        if (gr->description && !streq(gr->description, gr->group_name))
+                printf(" Description: %s\n", gr->description);
+
         if (!strv_isempty(gr->hashed_password))
                 printf("   Passwords: %zu\n", strv_length(gr->hashed_password));
 
diff --git a/src/shared/group-record.c b/src/shared/group-record.c
index 3c9520693d..d999ff95f8 100644
--- a/src/shared/group-record.c
+++ b/src/shared/group-record.c
@@ -28,6 +28,7 @@ static GroupRecord *group_record_free(GroupRecord *g) {
         free(g->group_name);
         free(g->realm);
         free(g->group_name_and_realm_auto);
+        free(g->description);
 
         strv_free(g->members);
         free(g->service);
@@ -192,6 +193,7 @@ int group_record_load(
         static const JsonDispatch group_dispatch_table[] = {
                 { "groupName",      JSON_VARIANT_STRING,   json_dispatch_user_group_name,  offsetof(GroupRecord, group_name),       JSON_RELAX},
                 { "realm",          JSON_VARIANT_STRING,   json_dispatch_realm,            offsetof(GroupRecord, realm),            0         },
+                { "description",    JSON_VARIANT_STRING,   json_dispatch_gecos,            offsetof(GroupRecord, description),      0         },
                 { "disposition",    JSON_VARIANT_STRING,   json_dispatch_user_disposition, offsetof(GroupRecord, disposition),      0         },
                 { "service",        JSON_VARIANT_STRING,   json_dispatch_string,           offsetof(GroupRecord, service),          JSON_SAFE },
                 { "lastChangeUSec", JSON_VARIANT_UNSIGNED, json_dispatch_uint64,           offsetof(GroupRecord, last_change_usec), 0         },
diff --git a/src/shared/group-record.h b/src/shared/group-record.h
index b72a43e50d..85c91eb1f5 100644
--- a/src/shared/group-record.h
+++ b/src/shared/group-record.h
@@ -13,6 +13,8 @@ typedef struct GroupRecord {
         char *realm;
         char *group_name_and_realm_auto;
 
+        char *description;
+
         UserDisposition disposition;
         uint64_t last_change_usec;
 
diff --git a/src/shared/user-record.c b/src/shared/user-record.c
index 2c0de383e0..678f04e537 100644
--- a/src/shared/user-record.c
+++ b/src/shared/user-record.c
@@ -203,7 +203,7 @@ int json_dispatch_realm(const char *name, JsonVariant *variant, JsonDispatchFlag
         return 0;
 }
 
-static int json_dispatch_gecos(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata) {
+int json_dispatch_gecos(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata) {
         char **s = userdata;
         const char *n;
 
diff --git a/src/shared/user-record.h b/src/shared/user-record.h
index 1bfd095d27..39580b6b76 100644
--- a/src/shared/user-record.h
+++ b/src/shared/user-record.h
@@ -388,6 +388,7 @@ int user_record_test_password_change_required(UserRecord *h);
 
 /* The following six are user by group-record.c, that's why we export them here */
 int json_dispatch_realm(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata);
+int json_dispatch_gecos(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata);
 int json_dispatch_user_group_list(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata);
 int json_dispatch_user_disposition(const char *name, JsonVariant *variant, JsonDispatchFlags flags, void *userdata);
 
diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c
index c973ee9c01..12c6943ebd 100644
--- a/src/userdb/userdbctl.c
+++ b/src/userdb/userdbctl.c
@@ -232,6 +232,7 @@ static int show_group(GroupRecord *gr, Table *table) {
                                 TABLE_STRING, gr->group_name,
                                 TABLE_STRING, user_disposition_to_string(group_record_disposition(gr)),
                                 TABLE_GID, gr->gid,
+                                TABLE_STRING, gr->description,
                                 TABLE_INT, (int) group_record_disposition(gr));
                 if (r < 0)
                         return table_log_add_error(r);
@@ -255,13 +256,14 @@ static int display_group(int argc, char *argv[], void *userdata) {
                 arg_output = argc > 1 ? OUTPUT_FRIENDLY : OUTPUT_TABLE;
 
         if (arg_output == OUTPUT_TABLE) {
-                table = table_new("name", "disposition", "gid", "disposition-numeric");
+                table = table_new("name", "disposition", "gid", "description", "disposition-numeric");
                 if (!table)
                         return log_oom();
 
                 (void) table_set_align_percent(table, table_get_cell(table, 0, 2), 100);
+                (void) table_set_empty_string(table, "-");
                 (void) table_set_sort(table, (size_t) 3, (size_t) 2, (size_t) -1);
-                (void) table_set_display(table, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) -1);
+                (void) table_set_display(table, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) 3, (size_t) -1);
         }
 
         if (argc > 1) {