Bug 101972 - Add support for a list of extra groups for admin users
Summary: Add support for a list of extra groups for admin users
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-30 13:50 UTC by Cosimo Cecchi
Modified: 2018-05-10 17:04 UTC (History)
6 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Add support for a list of extra groups for admin users (4.22 KB, patch)
2017-07-30 13:50 UTC, Cosimo Cecchi
Details | Splinter Review
Fix useradd failures caused by invalid read (2.53 KB, patch)
2018-05-03 07:42 UTC, Ondrej Holy
Details | Splinter Review

Description Cosimo Cecchi 2017-07-30 13:50:15 UTC
This patch upstreams a functionality that Debian/Ubuntu carry downstream patches for.
Comment 1 Cosimo Cecchi 2017-07-30 13:50:17 UTC
Created attachment 133135 [details] [review]
Add support for a list of extra groups for admin users

It's common for distributions to want to add administrator users to
additional groups (e.g. lpadmin or systemd-journal); Debian and Ubuntu
have patches that add this kind of functionality already.

This commit adds a configure option to specify a comma-separated list of
extra groups for admin users and adds support for it when both adding a
new admin user and promoting an user to admin.
Comment 2 Robert Ancell 2017-12-19 02:28:47 UTC
Comment on attachment 133135 [details] [review]
Add support for a list of extra groups for admin users

Review of attachment 133135 [details] [review]:
-----------------------------------------------------------------

LGTM
Comment 3 Cosimo Cecchi 2017-12-19 02:55:38 UTC
Thanks for the review, Robert! Is there any chance you can push this for me? I don't think I have write access to this repo.
Comment 4 Robert Ancell 2017-12-19 20:33:52 UTC
Pushed, I just wanted to check with Ray he was OK with this change first. Thanks for the patch!
Comment 5 Ondrej Holy 2018-05-03 07:42:56 UTC
Created attachment 139293 [details] [review]
Fix useradd failures caused by invalid read

Commit 93e2a85 added support for extra admin groups, but causes useradd
failures, when adding admin accounts. It is because admin_groups are
released before they are actually used, which causes that garbage is
used instead of the group and useradd fails consequently with invalid
group name. Do not release the memory before it is used and also fix
one corresponding memory leak.
Comment 6 Ondrej Holy 2018-05-03 07:43:18 UTC
See:
https://bugzilla.gnome.org/show_bug.cgi?id=795640
Comment 7 Cosimo Cecchi 2018-05-03 15:01:55 UTC
Whoops, good catch... the patch looks good to me.
Comment 8 Felipe 2018-05-08 23:17:20 UTC
Hello, when will this error be fixed?
Comment 9 Ray Strode [halfline] 2018-05-10 14:23:39 UTC
hey i missed this go by in my mail and did two similar fixes yesterday:

https://cgit.freedesktop.org/accountsservice/commit/?h=wip/fix-useradd&id=25437b69da82bd1dccc220f8dff516006a99471a
https://cgit.freedesktop.org/accountsservice/commit/?h=wip/fix-useradd&id=c17b57e1ffc3d6086cb4bbe17ff40a091662d1be

(but didn't push them to master yet)

Anyway, seems good to me, please push !
Comment 10 Ray Strode [halfline] 2018-05-10 14:37:22 UTC
(downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=1575780 )
Comment 11 Ray Strode [halfline] 2018-05-10 17:04:48 UTC
9d14729..1557846  master -> master


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.