From 3d56dc09221391f6956bcbc55550730f09cb22f3 Mon Sep 17 00:00:00 2001 From: Andy Vandenberghe Date: Sun, 4 Oct 2026 21:37:07 +0200 Subject: [PATCH] Keep locked passworded forums out of Recent Topics get_forum_list() built the forum list from f_read / f_list_topics only. phpBB grants f_read on a passworded forum regardless of the password, so a forum the user had not unlocked was included, and its topic titles, last-post subjects and authors showed in the Recent Topics block on the index, on viewforum and on /app.php/rt. Drop the forums returned by user::get_passworded_forums(), the same check the index, viewforum, search and feeds use. It runs before the forum_recent_topics query, so the single-forum path is covered too. Closes #193 Co-Authored-By: Claude Opus 5.5 --- core/recenttopics.php | 4 ++ tests/core/forum_list_test.php | 128 +++++++++++++++++++++++++++++++++ tests/tests.md | 19 ++++- 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 tests/core/forum_list_test.php diff --git a/core/recenttopics.php b/core/recenttopics.php index 2521bee..68dc026 100644 --- a/core/recenttopics.php +++ b/core/recenttopics.php @@ -480,6 +480,10 @@ private function get_forum_list() } $this->forum_ids = array_unique($forum_ary); + // phpBB grants f_read on a passworded forum regardless of the password, so drop the ones + // this user has not unlocked, as the index, viewforum, search and feeds do (issue #193). + $this->forum_ids = array_diff($this->forum_ids, $this->user->get_passworded_forums()); + if (count($this->forum_ids) > 1) { $sql = 'SELECT forum_id diff --git a/tests/core/forum_list_test.php b/tests/core/forum_list_test.php new file mode 100644 index 0000000..0c1c5ff --- /dev/null +++ b/tests/core/forum_list_test.php @@ -0,0 +1,128 @@ + 1]; + } + + $auth = $this->createMock(\phpbb\auth\auth::class); + $auth->method('acl_getf')->willReturnCallback(function ($opt) use ($f_read) { + return $opt === 'f_read' ? $f_read : []; + }); + + $user = $this->createMock(\phpbb\user::class); + $user->method('get_passworded_forums') + ->willReturn(array_combine($passworded_forum_ids, $passworded_forum_ids) ?: []); + + $queried = []; + $db = $this->createMock(\phpbb\db\driver\driver_interface::class); + $db->method('sql_in_set')->willReturnCallback(function ($field, $ids) use (&$queried) { + $queried = array_values($ids); + return $field . ' IN (' . implode(',', $queried) . ')'; + }); + $db->method('sql_query')->willReturn('result'); + $db->method('sql_fetchrow')->willReturnCallback(function () use (&$queried) { + $forum_id = array_shift($queried); + return $forum_id === null ? false : ['forum_id' => $forum_id]; + }); + + return new \avathar\recenttopics\core\recenttopics( + $auth, + $this->createMock(\phpbb\cache\service::class), + new \phpbb\config\config([]), + $this->createMock(\phpbb\language\language::class), + $this->createMock(\phpbb\content_visibility::class), + $db, + $this->createMock(\phpbb\event\dispatcher_interface::class), + $this->createMock(\phpbb\pagination::class), + $this->createMock(\phpbb\request\request_interface::class), + $this->createMock(\phpbb\template\template::class), + $user, + '/', + 'php', + $this->createMock(\phpbb\config\db_text::class) + ); + } + + /** + * Run get_forum_list() and return the resulting forum ids, sorted. + * + * @param \avathar\recenttopics\core\recenttopics $rt + * @return array + */ + private function forum_ids($rt): array + { + $method = new \ReflectionMethod($rt, 'get_forum_list'); + $method->setAccessible(true); + $method->invoke($rt); + + $property = new \ReflectionProperty($rt, 'forum_ids'); + $property->setAccessible(true); + $forum_ids = array_values($property->getValue($rt)); + sort($forum_ids); + + return $forum_ids; + } + + /** + * Passworded forums the user has not unlocked must not feed the list (#193). + * phpBB grants f_read on them regardless of the password, so the ACL check + * alone lets their titles and authors through. + */ + public function test_locked_passworded_forum_is_excluded() + { + $rt = $this->make_rt([1, 2, 3], [2]); + + $this->assertSame([1, 3], $this->forum_ids($rt)); + } + + /** + * When excluding the locked forum leaves a single forum, that forum is + * still the only one listed. + */ + public function test_locked_passworded_forum_is_excluded_when_one_forum_remains() + { + $rt = $this->make_rt([1, 2], [2]); + + $this->assertSame([1], $this->forum_ids($rt)); + } + + /** + * A forum the user has unlocked, or one without a password, is unaffected. + */ + public function test_forums_without_locked_password_are_kept() + { + $rt = $this->make_rt([1, 2, 3], []); + + $this->assertSame([1, 2, 3], $this->forum_ids($rt)); + } +} diff --git a/tests/tests.md b/tests/tests.md index f5df972..bc64b0d 100644 --- a/tests/tests.md +++ b/tests/tests.md @@ -42,6 +42,7 @@ tests/ ├── controller/ │ └── page_controller_test.php Dedicated /rt and /rt/simple page controller ├── core/ +│ ├── forum_list_test.php Which forums the list may draw topics from │ └── recenttopics_events_test.php Public event API contract (contrib/events.md) └── functional/ └── recenttopics_test.php End-to-end browser tests @@ -157,7 +158,23 @@ Because that method requires a fully seeded database (topics, forums, user sessi --- -## 5. Functional tests (`functional/recenttopics_test.php`) +## 5. Forum list (`core/forum_list_test.php`) + +**What this code does:** +`get_forum_list()` in `core/recenttopics.php` decides which forums the list may take topics from: forums where the user has `f_read` or `f_list_topics`, minus passworded forums the user has not unlocked, minus forums the admin excluded in the ACP. + +**What this test file does:** +It builds `core\recenttopics` with a mocked `auth` (returning the readable forums), a mocked `user` (returning the passworded forums not yet unlocked) and a db stub that answers the ACP-exclusion query with every forum it is asked about. It calls `get_forum_list()` via `ReflectionMethod` and reads the private `forum_ids`. + +| Test | Scenario | What it verifies | +|------|----------|-----------------| +| `test_locked_passworded_forum_is_excluded` | Readable forums 1, 2, 3; forum 2 passworded and locked | Only 1 and 3 remain. phpBB grants `f_read` on a passworded forum regardless of the password, so without this the forum's titles and authors leak (#193) | +| `test_locked_passworded_forum_is_excluded_when_one_forum_remains` | Readable forums 1, 2; forum 2 locked | Only 1 remains, on the code path that skips the ACP-exclusion query | +| `test_forums_without_locked_password_are_kept` | Readable forums 1, 2, 3; none locked | All three remain | + +--- + +## 6. Functional tests (`functional/recenttopics_test.php`) **What this code does:** These tests start a real phpBB installation (using the test framework's built-in install), enable the extension, and make HTTP requests using a real browser-like crawler (Symfony DomCrawler). They check the actual rendered HTML for specific elements.