From 2c18de9a5a3255792eada2f08a8ea03618c5851f Mon Sep 17 00:00:00 2001 From: albertlast Date: Sat, 5 Sep 2026 12:28:51 +0200 Subject: [PATCH 1/3] Hands back the permission sets that came from the cache With caching on, guests and members lost every permission they had. A guest asking for the stats page got the log-in prompt, the search box vanished from every page, and boards and topics rendered without the things a permission gates. Admins were unaffected, which is why this survived: their permissions are filled in before the cache is consulted and their group is dropped from the lookup, so anyone testing while logged in as an admin sees nothing wrong. GroupPermissionSet::load() collects the instances it constructs and returns them. loadGlobalPermissionData() and loadBoardPermissionData() then either fill those same instances in from the database, which the caller sees because it is holding them, or replace the entries in self::$loaded outright with the sets read from the cache, which the caller does not see because it is still holding the empty ones it was given. So a cache hit returned sets with no permissions in them while the correct ones sat in self::$loaded. Reading the return value back out of self::$loaded once the data is in covers both routes. UserPermissionSet::load() is the caller that builds User::$me's permissions, so this is the whole of what a member is allowed to do. In the same function, the test for whether every profile was found in the cache is written if ($hits = \count($profiles)) { which assigns instead of comparing and is therefore always true, dropping the group from the list of those still to look up whether or not anything was found for it. Board permissions were never queried for a group once the cache had been consulted at level 2 or above. Co-Authored-By: Claude Opus 5 Signed-off-by: albertlast --- Sources/Permissions/GroupPermissionSet.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sources/Permissions/GroupPermissionSet.php b/Sources/Permissions/GroupPermissionSet.php index 17126719b7d..7ff62ec3a4e 100644 --- a/Sources/Permissions/GroupPermissionSet.php +++ b/Sources/Permissions/GroupPermissionSet.php @@ -300,6 +300,19 @@ public static function load(array|int $profiles, array|int $groups, bool $refres // Board permissions. self::loadBoardPermissionData($profiles, $query_groups); + + // A set that came from the cache takes the place of the instance + // built above, so the sets to hand back are whichever ones ended up + // in self::$loaded rather than the ones collected on the way in. + $loaded = []; + + foreach ($profiles as $profile) { + foreach ($groups as $group) { + if (isset(self::$loaded[$profile][$group])) { + $loaded[] = self::$loaded[$profile][$group]; + } + } + } } return $loaded; @@ -421,7 +434,7 @@ protected static function loadBoardPermissionData(array $profiles, array $groups } } - if ($hits = \count($profiles)) { + if ($hits === \count($profiles)) { unset($groups[$g]); } } From e7dcb3c41e2852992c15051088dc9bbb1c220c35 Mon Sep 17 00:00:00 2001 From: albertlast Date: Sun, 6 Sep 2026 01:08:38 +0200 Subject: [PATCH 2/3] Update Sources/Permissions/GroupPermissionSet.php Co-authored-by: Jon Stovell --- Sources/Permissions/GroupPermissionSet.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Sources/Permissions/GroupPermissionSet.php b/Sources/Permissions/GroupPermissionSet.php index 7ff62ec3a4e..5210f5d89b7 100644 --- a/Sources/Permissions/GroupPermissionSet.php +++ b/Sources/Permissions/GroupPermissionSet.php @@ -301,13 +301,8 @@ public static function load(array|int $profiles, array|int $groups, bool $refres // Board permissions. self::loadBoardPermissionData($profiles, $query_groups); - // A set that came from the cache takes the place of the instance - // built above, so the sets to hand back are whichever ones ended up - // in self::$loaded rather than the ones collected on the way in. - $loaded = []; - foreach ($profiles as $profile) { - foreach ($groups as $group) { + foreach ($query_groups as $group) { if (isset(self::$loaded[$profile][$group])) { $loaded[] = self::$loaded[$profile][$group]; } From 75130310d35faefec5fb34b5d8e98cbc0d138b8a Mon Sep 17 00:00:00 2001 From: albertlast Date: Sun, 6 Sep 2026 01:49:16 +0200 Subject: [PATCH 3/3] Returns one set per group rather than two The instance the constructor builds is appended to the list, and the loop below appends whatever ended up in self::$loaded for the same group, so a group that had to be looked up came back twice. On a cache hit the two are different objects: the first is the empty instance the cached set replaced. Callers that read the list with current() take the first of the pair, and two of them mutate every set the list hands them and save it, so the pair turns one write into two against objects that do not agree. The constructor registers each instance in self::$loaded itself, so dropping the append leaves exactly one set per profile and group, whichever the load ended up with. Co-Authored-By: Claude Opus 5 Signed-off-by: albertlast --- Sources/Permissions/GroupPermissionSet.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/Permissions/GroupPermissionSet.php b/Sources/Permissions/GroupPermissionSet.php index 5210f5d89b7..462eeaa9a67 100644 --- a/Sources/Permissions/GroupPermissionSet.php +++ b/Sources/Permissions/GroupPermissionSet.php @@ -284,8 +284,9 @@ public static function load(array|int $profiles, array|int $groups, bool $refres // to query the database separately below. self::$query_during_construction = false; - // Initialize the objects. - $loaded[] = new self($profile, $group); + // Initialize the objects. The constructor registers each one in + // self::$loaded, which is where they are collected from below. + new self($profile, $group); // Restore this to its normal value. self::$query_during_construction = true; @@ -301,6 +302,9 @@ public static function load(array|int $profiles, array|int $groups, bool $refres // Board permissions. self::loadBoardPermissionData($profiles, $query_groups); + // A set that came from the cache takes the place of the instance the + // constructor registered, so the ones to hand back are whichever + // ended up in self::$loaded. foreach ($profiles as $profile) { foreach ($query_groups as $group) { if (isset(self::$loaded[$profile][$group])) {