Project

General

Profile

Actions

Bug #14363

closed

"All" user group overwritten after assigning an existing user to a group

Added by Marcos M over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
User Manager / Privileges
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
23.05
Release Notes:
Default
Affected Version:
2.7.0
Affected Architecture:
All

Description

Tested on 23.01.

Steps to reproduce on a default configuration:
  1. Create a new user; assign a password, no group memberships, and save.
  2. Edit the user; assign the admins group and save.
    The user groups tab now shows two admins groups.

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>

Actions #1

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.

Actions #2

Updated by Jim Pingle over 1 year ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 100
Actions #3

Updated by Marcos M over 1 year ago

  • Status changed from Feedback to Resolved

Patch fixed the issue.

Actions #4

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
Actions #5

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
Actions #6

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.

Actions #7

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.

Actions #8

Updated by Jim Pingle over 1 year ago

  • Affected Version set to 2.7.0
Actions

Also available in: Atom PDF