Skip to content

Commit 97c25e0

Browse files
committed
[BUGFIX] Prevent c:0 variant and content leakage on fe_group-restricted pages
Fixes two interrelated bugs in the page indexing flow that cause access-protected pages to be indexed with incorrect Solr documents: - Bug 1 (PageIndexer::index): the findUserGroups indexer request collects fe_group values from all content rendered for that request, including global-template content (footer/nav) which typically has fe_group=0. Group 0 then reaches accessGroups and triggers an unwanted c:0 variant via indexPage, making the page findable without the required access group. Group 0 is now filtered out when the page has fe_group > 0, and the page's own group is ensured to be present so restricted-but-eligible users can still find the page. - Bug 2 (FrontendGroupsModifier): the event listener faked pageUserGroup into the frontend groups unconditionally. During the c:0 indexer request this granted access to fe_group-restricted content elements, leaking protected bodytext into the c:0 Solr document. pageUserGroup is now only faked in when userGroup > 0; the c:0 request runs with anonymous groups. The new fixture mirrors the production scenario: a protected page (fe_group=1) with a protected CE (fe_group=1) alongside a footer/nav CE (pid=1, fe_group=0) rendered via a second CONTENT object outside the TYPO3SEARCH markers — the same shape global template content has in production. Three existing data-provider yields had been silently encoding the bug as expected ('2:1/c:0' on a page with fe_group=1) and are corrected to '2:1/c:1', the semantically correct access variant. Files: - Classes/IndexQueue/PageIndexer.php - In index(): read pageUserGroup from the page record (via enablecolumns.fe_group). For protected pages, filter group 0 from accessGroups and ensure pageUserGroup is in the list. - Classes/EventListener/PageIndexer/FrontendGroupsModifier.php - Add a userGroup > 0 guard to the pageUserGroup faking block. - Tests/Integration/IndexQueue/PageIndexerTest.php - Added data-provider yield "protected page: c:0 must not contain same-group protected content (isolation bug)" expecting only c:1. - Corrected three protected-page yields ("protected page", "protected page with protected content", "translation of protected page with protected content"): expectedAccessFieldValues '2:1/c:0' -> '2:1/c:1'. - Tests/Integration/IndexQueue/Fixtures/can_index_protected_page_with_public_and_same_group_protected_content.csv - New fixture; sys_template inlined to render bodytext and include the second CONTENT object outside TYPO3SEARCH markers. Partially ports: #4559 Closes: #4642
1 parent 50674c9 commit 97c25e0

4 files changed

Lines changed: 89 additions & 5 deletions

File tree

Classes/EventListener/PageIndexer/FrontendGroupsModifier.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ public function __invoke(ModifyResolvedFrontendGroupsEvent $event): void
8686
);
8787
}
8888

89-
if ((int)$pageIndexerRequest->getParameter('pageUserGroup') > 0) {
89+
if ((int)$pageIndexerRequest->getParameter('userGroup') > 0
90+
&& (int)$pageIndexerRequest->getParameter('pageUserGroup') > 0
91+
) {
9092
$groups[] = (int)$pageIndexerRequest->getParameter('pageUserGroup');
9193
}
9294
$groupData = [];

Classes/IndexQueue/PageIndexer.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,23 @@ public function index(Item $item): bool
6767
);
6868
}
6969

70+
$feGroupColumn = $GLOBALS['TCA']['pages']['ctrl']['enablecolumns']['fe_group'] ?? '';
71+
$pageUserGroup = $feGroupColumn !== '' ? (int)($item->getRecord()[$feGroupColumn] ?? 0) : 0;
7072
foreach ($systemLanguageUids as $systemLanguageUid) {
7173
$contentAccessGroups = $this->getAccessGroupsFromContent($item, $systemLanguageUid);
74+
if ($pageUserGroup > 0) {
75+
// A page with fe_group restriction has no publicly-accessible content variants:
76+
// remove group 0 (which may originate from global template CEs, not page content).
77+
// Ensure the page's own group is in accessGroups so restricted-but-eligible users
78+
// can still find the page in search results.
79+
$contentAccessGroups = array_values(array_filter(
80+
$contentAccessGroups,
81+
static fn(int $group): bool => $group !== 0,
82+
));
83+
if (!in_array($pageUserGroup, $contentAccessGroups, true)) {
84+
$contentAccessGroups[] = $pageUserGroup;
85+
}
86+
}
7287
foreach ($contentAccessGroups as $userGroup) {
7388
$this->indexPage($item, $systemLanguageUid, (int)$userGroup);
7489
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
"fe_groups",
2+
,"uid","pid","title"
3+
,1,1,"group 1"
4+
"pages",
5+
,"uid","pid","is_siteroot","doktype","slug","title","subtitle","crdate","tstamp","fe_group"
6+
,2,1,0,1,"/protected_page","protected page","",1449151778,1449151778,1
7+
"tt_content",
8+
,"uid","pid","CType","header","bodytext","fe_group","sorting"
9+
,11,"2","text","protected ce ","protected bodytext ",1,20
10+
,20,"1","text","footer nav ce ","footer nav bodytext ",0,10
11+
"sys_template",
12+
,"uid","pid","root","clear","sorting","config"
13+
,100,1,1,3,50,"
14+
@import 'EXT:solr/Tests/Integration/Fixtures/sites_setup_and_data_set/Integration.setup.typoscript'
15+
16+
# Replicate what addSimpleFrontendRenderingToTypoScriptRendering adds in setUp()
17+
page = PAGE
18+
page.typeNum = 0
19+
config.index_enable = 1
20+
page.10 = CONTENT
21+
page.10 {
22+
table = tt_content
23+
select.orderBy = sorting
24+
select.where = colPos=0
25+
renderObj = COA
26+
renderObj {
27+
10 = TEXT
28+
10.field = bodytext
29+
}
30+
}
31+
page.10.renderObj.5 = TEXT
32+
page.10.renderObj.5.field = header
33+
page.10.stdWrap.dataWrap = <!--TYPO3SEARCH_begin-->|<!--TYPO3SEARCH_end-->
34+
35+
# Simulates global template content (footer/nav) rendered outside TYPO3SEARCH markers.
36+
# In production, such content from other pages causes UserGroupDetector to collect
37+
# their fe_group values (often 0/empty), which triggers the c:0 indexing variant.
38+
page.20 = CONTENT
39+
page.20 {
40+
table = tt_content
41+
select.pidInList = 1
42+
select.orderBy = sorting
43+
select.where = colPos=0
44+
renderObj = COA
45+
renderObj {
46+
10 = TEXT
47+
10.field = bodytext
48+
}
49+
}
50+
"
51+
"tx_solr_indexqueue_item",
52+
,"uid","root","item_type","item_uid","indexing_configuration","changed","indexed","has_indexing_properties","indexing_priority","indexed","errors"
53+
,4711,1,"pages",2,"pages",1449151778,0,0,0,0,0

Tests/Integration/IndexQueue/PageIndexerTest.php

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public function canIndexPageWithAccessProtectedContentIntoSolr(
105105
true,
106106
);
107107

108-
self::assertEquals($expectedNumFound, $solrContent['response']['numFound'] ?? 0, 'Could not index documents into Solr');
108+
self::assertEquals($expectedNumFound, $solrContent['response']['numFound'] ?? 0, 'Unexpected count of documents in Solr index.');
109109
foreach ($expectedAccessFieldValues as $index => $expectedAccessFieldValue) {
110110
self::assertEquals(
111111
$expectedAccessFieldValue,
@@ -143,7 +143,7 @@ public static function canIndexPageWithAccessProtectedContentIntoSolrDataProvide
143143
'fixture' => 'can_index_access_protected_page',
144144
'expectedNumFound' => 1,
145145
'expectedAccessFieldValues' => [
146-
'2:1/c:0',
146+
'2:1/c:1',
147147
],
148148
'expectedContents' => [
149149
'public content of protected page',
@@ -171,7 +171,7 @@ public static function canIndexPageWithAccessProtectedContentIntoSolrDataProvide
171171
'fixture' => 'can_index_access_protected_page_with_protected_contents',
172172
'expectedNumFound' => 2,
173173
'expectedAccessFieldValues' => [
174-
'2:1/c:0',
174+
'2:1/c:1',
175175
'2:1/c:2',
176176
],
177177
'expectedContents' => [
@@ -187,7 +187,7 @@ public static function canIndexPageWithAccessProtectedContentIntoSolrDataProvide
187187
'fixture' => 'can_index_access_protected_page_with_protected_contents',
188188
'expectedNumFound' => 2,
189189
'expectedAccessFieldValues' => [
190-
'2:1/c:0',
190+
'2:1/c:1',
191191
'2:1/c:2',
192192
],
193193
'expectedContents' => [
@@ -232,6 +232,20 @@ public static function canIndexPageWithAccessProtectedContentIntoSolrDataProvide
232232
'expectedNumFoundLoggedInUser' => 2,
233233
];
234234

235+
yield 'protected page: c:0 must not contain same-group protected content (isolation bug)' => [
236+
'fixture' => 'can_index_protected_page_with_public_and_same_group_protected_content',
237+
'expectedNumFound' => 1,
238+
'expectedAccessFieldValues' => [
239+
'2:1/c:1',
240+
],
241+
'expectedContents' => [
242+
'protected ce protected bodytext',
243+
],
244+
'expectedNumFoundAnonymousUser' => 0,
245+
'userGroupToCheckAccessFilter' => '0,1',
246+
'expectedNumFoundLoggedInUser' => 1,
247+
];
248+
235249
yield 'page protected by extend to subpages' => [
236250
'fixture' => 'can_index_sub_page_of_protected_page_with_extend_to_subpage',
237251
'expectedNumFound' => 1,

0 commit comments

Comments
 (0)