Could you please explain why we want this?

Isn't this another way of having an empty GROUPS list? Why not have an empty GROUPS list instead? (I'm not sure I liked the GROUPS list in the first place.)

Could you please explain why we want this?

Isn't this another way of having an empty GROUPS list? Why not have an empty GROUPS list instead? (I'm not sure I liked the GROUPS list in the first place.)

GROUPS allows maintainers to set a list of additional (supplementary) default groups, for example users, dialout, audio, serial and so on.

Or, for example, only users, which is then expanded through the libnss-role, i.e. these groups (dialout, audio, serial) are added through libnss-role to those users who are in the users group.

But at this moment, it works so that when creating a system user via useradd --system or useradd -r, these groups are also added to it, which is most often undesirable (more precisely, I do not even know the case when it would be necessary).

Therefore, this new option was created, which allows maintainers to disable auto-addition of additional groups for system users.

But at this moment, it works so that when creating a system user via useradd --system or useradd -r, these groups are also added to it, which is most often undesirable (more precisely, I do not even know the case when it would be necessary).

Since you are the original author of this functionality and you don't know the case, why don't you filter the group membership addition for non-system users instead of creating a new option? I think that would be the most sensible approach.

But at this moment, it works so that when creating a system user via useradd --system or useradd -r, these groups are also added to it, which is most often undesirable (more precisely, I do not even know the case when it would be necessary).

Since you are the original author of this functionality and you don't know the case, why don't you filter the group membership addition for non-system users instead of creating a new option? I think that would be the most sensible approach.

Because I am not original author of this functionality.

So, that's why I suggest this option, which will keep the possibility of the original behavior (when these groups automatically adds to system users).

P.S But in general, with the option, it seems to me a more flexible.

I mean, the GROUPS feature itself with SYS_USER_AUTO_GROUPS_ENAB enabled will be like a complete alternative to specifying default supplementary groups using -G.

Because I am not original author of this functionality.

Ups sorry, I have mixed up the names.

@AZaugg any opinion on the following topic?

But at this moment, it works so that when creating a system user via useradd --system or useradd -r, these groups are also added to it, which is most often undesirable (more precisely, I do not even know the case when it would be necessary).

Since you are the original author of this functionality and you don't know the case, why don't you filter the group membership addition for non-system users instead of creating a new option? I think that would be the most sensible approach.

@ikerexxe Hi! I can redo this PR as you suggested (filter the group membership addition for non-system users instead of creating a new option) - since the author @AZaugg of the original change has not expressed any objections for quite a long time.

@SgAkErRu yes, go ahead

@ikerexxe

@SgAkErRu yes, go ahead

Done

The current implementation contains a flaw. The grp_add() function also sets do_grp_update = true in line 1950. That function is called when useradd is called with -U flag. That means that if a user is created with -U and without -G flags, then the logic you implemented will be executed. That's an undesired execution and it will leave some users without their default supplementary groups.

But the new code check do_grp_update and sets do_grp_update = false inside the process_flags() function, which is called in main() BEFORE calling the grp_add() when the -U flag is set.

So, the new code check do_grp_update, which was changed to true inside the get_defaults function for the GROUPS variable case, and based on the order in which commands are executed, it should not conflict with this logic with -U flag.

Or am I wrong?

Finally, one of the commits mentions useradd: fix test 69_useradd_default_GROUPS_name, what is it exactly you are fixing? I'm assuming you are adapting to the new GROUPS behaviour you are trying to implement, but I'd make it more obvious in the commit message.

No, the behavior of the test itself does not change in any way and does not adapt to the current PR, because the current PR does not change the behavior for the regular user, only for the system user. And a new test number 70 has been created for the system user.

The corrections are visible in the commit itself:

1. Missing files have been added for the test to work: gshadow, passwd, shadow. Without them, the foo user was created with a different UID and thus the test failed.

2. Added an empty line at the end of the file in config/group

3. The extra space at the beginning of the file in data/group was removed and an empty line was added at the end of the file. That extra space also led to a test failure due to a mismatch between the initial line of the group file in the data and config subfolders.

4. Fixed a typo in the log message in useradd.test

If necessary, I can add this information to the description of the commit.

Or am I wrong?

No, you are right. I must have misunderstood something but checking it again it behaves as you specified.

1. Missing files have been added for the test to work: `gshadow`, `passwd`, `shadow`. Without them, the foo user was created with a different UID and thus the test failed.

2. Added an empty line at the end of the file in `config/group`

3. The extra space at the beginning of the file in `data/group` was removed and an empty line was added at the end of the file. That extra space also led to a test failure due to a mismatch between the initial line of the `group` file in the data and config subfolders.

4. Fixed a typo in the log message in `useradd.test`

If necessary, I can add this information to the description of the commit.

Please mention 1 and you can merge 2, 3 and 4 with minor improvements or something like similar.