Merge pull request #1138 from YesWiki/fix_442_groups_in_groups

Fix issue #442
YesWiki · Mar 8, 2024 · a32e4bf · a32e4bf
2 parents 9628d56 + 8e3da2f
commit a32e4bf
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 12 deletions.
diff --git a/includes/YesWiki.php b/includes/YesWiki.php
@@ -964,17 +964,19 @@ public function GetGroupACL($group)
      *            The name of the group
      * @param string $acl
      *            The new acl for that group
-     * @return boolean True iff the new acl defines the group recursively
+     * @return boolean True if the new acl defines the group recursively
      */
     public function MakesGroupRecursive($gname, $acl, $origin = null, $checked = array())
     {
-        $gname = strtolower($gname);
+        $gname = strtolower(trim($gname));
         if ($origin === null) {
             $origin = $gname;
         } elseif ($gname === $origin) {
             return true;
         }
+        $acl = str_replace(["\r\n","\r"], "\n", $acl);
         foreach (explode("\n", $acl) as $line) {
+            $line = trim($line);
             if (! $line) {
                 continue;
             }
@@ -1017,6 +1019,11 @@ public function SetGroupACL($gname, $acl)
             return 1001;
         }
         $old = $this->GetGroupACL($gname);
+        // we get rid of lost spaces before saving to db
+        $acl = str_replace(["\r\n","\r"], "\n", $acl);
+        $acls = explode("\n", $acl);
+        $acls = array_map('trim', $acls);
+        $acl = implode("\n", $acls);
         if ($this->MakesGroupRecursive($gname, $acl)) {
             return 1000;
         }

diff --git a/includes/services/AclService.php b/includes/services/AclService.php
@@ -213,9 +213,11 @@ public function hasAccess($privilege, $tag = '', $user = '')
      *            Mode for cases when $acl contains '%'
      *            Default '', standard case. $mode = 'creation', the test returns true
      *            even if the user is connected
+     * @param array $formerGroups
+     * 		 to avoid loops we keep track of former calls
      * @return bool True if the $user satisfies the $acl, false otherwise
      */
-    public function check($acl, $user = null, $adminCheck = true, $tag = '', $mode = '')
+    public function check($acl, $user = null, $adminCheck = true, $tag = '', $mode = '', $formerGroups = [])
     {
         if (!$user) {
             $user = $this->authController->getLoggedUser();
@@ -270,12 +272,18 @@ public function check($acl, $user = null, $adminCheck = true, $tag = '', $mode =
                     case '@': // groups
                         $gname = substr($line, 1);
                         // paranoiac: avoid line = '@'
-                        if ($gname) {
-                            if (!empty($username) && $this->userManager->isInGroup($gname, $username, false/* we have allready checked if user was an admin */)) {
-                                $result = $std_response ;
-                            } else {
-                                $result = ! $std_response ;
-                            }
+			if ($gname) {
+	                   if (in_array($gname, $formerGroups)) {
+				   $this->wiki->setMessage('Error group '.$gname.' inside same groups, inception was a bad movie');
+				   $result = false;
+			   } else {
+				   $formerGroups[] = $gname;
+				   if (!empty($username) && $this->userManager->isInGroup($gname, $username, false/* we have allready checked if user was an admin */, $formerGroups)) {
+					$result = $std_response ;
+				   } else {
+					$result = ! $std_response ;
+				   }
+			   }
                         } else {
                             $result = false ; // line '@'
                         }

diff --git a/includes/services/UserManager.php b/includes/services/UserManager.php
@@ -281,13 +281,14 @@ public function groupsWhereIsMember(User $user, bool $adminCheck = true)
      * @param string $groupName The name of the group for which we are testing membership
      * @param string|null $username if null check current user
      * @param bool $admincheck
+     * @param array $formerGroups former groups list to avoid loops
      *
      * @return boolean True if the $user is member of $groupName, false otherwise
     */
-    public function isInGroup(string $groupName, ?string $username = null, bool $admincheck = true)
+    public function isInGroup(string $groupName, ?string $username = null, bool $admincheck = true, array $formerGroups = [])
     {
-        // aclService could  not be loaded in __construct because AclService already loads UserManager
-        return $this->wiki->services->get(AclService::class)->check($this->wiki->GetGroupACL($groupName), $username, $admincheck);
+	// aclService could  not be loaded in __construct because AclService already loads UserManager
+        return $this->wiki->services->get(AclService::class)->check($this->wiki->GetGroupACL($groupName), $username, $admincheck, '', '', $formerGroups);
     }
 
     /* ~~~~~~~~~~~~~~~~~~ implements  PasswordUpgraderInterface ~~~~~~~~~~~~~~~~~~ */