Bug #14363
closed
"All" user group overwritten after assigning an existing user to a group
100%
Description
Tested on 23.01.
- Create a new user; assign a password, no group memberships, and save.
- Edit the user; assign the
adminsgroup and save.
The user groups tab now shows twoadminsgroups.
Config diff:
--- /conf/backup/config-1683597400.xml 2023-05-09 01:56:51.585652000 +0000
+++ /conf/config.xml 2023-05-09 01:56:51.586333000 +0000
@@ -8,10 +8,13 @@
<domain>home.arpa</domain>
<dnsallowoverride></dnsallowoverride>
<group>
- <name>all</name>
- <description><![CDATA[All Users]]></description>
+ <name>admins</name>
+ <description><![CDATA[System Administrators]]></description>
<scope>system</scope>
- <gid>1998</gid>
+ <gid>1999</gid>
+ <member>0</member>
+ <member>2000</member>
+ <priv>page-all</priv>
</group>
<group>
<name>admins</name>
@@ -19,6 +22,7 @@
<scope>system</scope>
<gid>1999</gid>
<member>0</member>
+ <member>2000</member>
<priv>page-all</priv>
</group>
<user>
@@ -247,8 +251,8 @@
<qinqs></qinqs>
<laggs></laggs>
<revision>
- <time>1683597400</time>
- <description><![CDATA[admin@10.0.5.50 (Local Database): Successfully created user test]]></description>
+ <time>1683597411</time>
+ <description><![CDATA[admin@10.0.5.50 (Local Database): Successfully edited user test]]></description>
<username><![CDATA[admin@10.0.5.50 (Local Database)]]></username>
</revision>
<gateways></gateways>
Updated by Jim Pingle over 1 year ago
- Status changed from Confirmed to In Progress
- Assignee set to Jim Pingle
- Target version set to 2.7.0
- Plus Target Version set to 23.09
Looks like more PHP weirdness. There are several loops in local_user_set_groups() and each uses a variable $group and in some cases assigns a value to it by reference. Though those variables are declared/used inside different foreach() loops, the contents/reference remain after the loop. It's all OK until it hits the loop that calls local_group_set() at which point it ends up mangling the groups in the way shown in the description here.
Changing each loop to use a different name instead of $group every time ensures this reference behavior doesn't affect the other loops.
Though it also works to do unset($group); after (not inside) each foreach() loop, that alone seems like it would be prone to future errors.
Updated by Jim Pingle over 1 year ago
- Status changed from In Progress to Feedback
- % Done changed from 0 to 100
Applied in changeset a2a2e8a8bee55d5b0c393d2c2d311a2fc8903bce.
Updated by Marcos M over 1 year ago
- Subject changed from User groups are overridden after assigning a group membership to a user. to User groups are overridden after assigning a group membership to a user
Updated by Jim Pingle over 1 year ago
- Subject changed from User groups are overridden after assigning a group membership to a user to "All" user group overwritten after assigning an existing user to a group
Updated by Jim Pingle over 1 year ago
- Plus Target Version changed from 23.09 to 23.05
Picked back to 23.05 since there are potential security implications.
Updated by Jim Pingle over 1 year ago
Re-tested on the latest 23.05 snapshot and it's working as expected. Only the intended group is modified.
Unfortunately there is no way easy/automated way to repair the damage to a corrupted "all" group. Affected users will need to either restore an old backup or remove the second "admins" group and splice in the "all" group from an old backup into a current configuration.