GNOME Bugzilla – Bug 489187
users-admin will incorrectly remove a user's privileges in some cases
Last modified: 2010-02-03 11:52:36 UTC
Please describe the problem: users-admin has a bug in the way it handles setting the users/groups configuration, that can lead in group membership being incorrectly added or removed. This is triggered by some specific (but standard) series of tasks. Steps to reproduce: 1. launch users-admin 2. create a test1 user 3. create a test2 user (with admin rights) 4. create a test3 user 5. delete the test1 user 6. delete the test3 user Actual results: the test2 user lost its admin rights, he is no longer it the admin group. Expected results: the test2 user should keep its group membership. Does this happen every time? yes for the given scenario. Other information: I made this bug critical as this can lead in some user loosing its admin rights and not being able to perform any administrative tasks, and there may be more scenario to trigger this kind of behaviour. (the ubuntu bug is good candidate: https://bugs.launchpad.net/ubuntu/+source/gnome-system-tools/+bug/26338 )
Some explanations for this bug. This happens because of the way system-tools-backends works. When you retrieve the user list from s-t-b, each entry is given an id (which is the line number), when you set back the configuration after some modifications, to know what to do, s-t-b will get the actual configuration and compare the id with the given configuration. For example, if you have 3 users in /etc/passwd: test1, test2, test3 and you rename test2 in test4, he is what happens: users-admin got, you set s-t-b compare with 1 test1 1 test1 1 test1 2 test2 2 test4 2 test2 3 test3 3 test3 3 test3 s-t-b notices the entry 2 changed name and launch the usermod command. So why is this a problem ? Let's follow step 5. You delete the test1 user, ie the entry 1. users-admin got, you set s-t-b compare with 1 test1 1 test1 2 test2 2 test2 2 test2 3 test3 3 test3 3 test3 s-t-b notices the entry 1 doesn't exist anymore and then does a "userdel test1" Everything is fine until now, but it's going wrong with step 6. users-admin got, you set s-t-b compare with 1 test2 2 test2 2 test2 2 test3 3 test3 s-t-b notices the entry 1 doesn't exist anymore, so ... it deletes user2 !! Then it notices the entry 2 changed, and hence rename test3 user in test2. So the user test2 seems to still be here but it has the groups membership of test3 ! I think s-t-b uses the line id (instead of the uid or login name) in order to be able to change the uid of the login. So maybe the fix to this bug is to have users-admin re-getting the users/groups configuration after each modification instead of getting only at launch time. The same problem happens with group although I was not able to produce a case.
Thanks a lot! you've been the first one to provide the necessary steps to reproduce the bug, And you were right about your assumptions. In gnome-2-20 I've proceeded as suggested, and just reload the configuration after any change to it to avoid any mismatch. For trunk, I've chosen to just remove IDs altogether and compare users by login name in system-tools-backends. With this change s-t-b will see any modification to an user login as an user addition+removal, I think this feature is very rarely used, and this change makes the whole thing quite more reliable. So, I think this bug can be closed. Thanks again!
I'm afraid this needs to be reopened, since the current patch only fixes situations where gnome-system-tools is used exclusively. Try this: 1. launch users-admin 2. create test1 user in users-admin 3. "sudo useradd test2" in terminal 4. create test3 user in users-admin 5. "id test2" will show that user test2 has been nuked. Thus g-s-t needs to update the world model *before* it does any operation, too. http://svn.gnome.org/viewvc/gnome-system-tools?view=revision&revision=4017 is thus not sufficient. Thank you!
Hi this bug has already been reported: http://bugzilla.gnome.org/show_bug.cgi?id=488859 This race condition can't be completly avoided because of the way s-t-b works but the risk can be reduced if users-admin refresh its internal state before doing any modification.
Martin, that won't be a problem in trunk, regarding the solution in 2.20, it's clearly stated that it's a workaround, refreshing internal state before doing any change may be a bit cumbersome, and it's probably too complex to do what you suggest to include it in a stable branch without risking to break other things. (e.g: what if the modified underlying data and the g-s-t modification can't be merged?) g-s-t *never* had the feature to listen for configuration changes while it's running, such feature is planned for 2.22 (hence fixing #488859), but while that's cool to have, I think it's quite a side case for an usually short-lived app like users-admin to backport to a stable branch in upstream. Nonetheless, I accept patches, if you find an easy way to update config and merge g-s-t changes in it in a safe way, I'm all for it :) Or maybe you want to backport related changes from s-t-b 2.5.2 and g-s-t 2.21.2, but that means a behavior change unlikely for upstream... (that's why I did the workaround)
Carlos, thanks for your reply. I see that fixing it properly would be too intrusive for a stable update, so let's go with the current patch. Thank you!
AFAIK this has been properly fixed in the stb 2.6 (listening to changes in config files).