Skip to content

Commit c3d04f7

Browse files
konradmichalikkonradmichalik
authored
Merge pull request #134 from xima-media/feature/hardening
feat(hardening): service unit tests and defensive error handling
2 parents 0d0189a + bbb6e22 commit c3d04f7

8 files changed

Lines changed: 1146 additions & 17 deletions

File tree

‎Classes/Controller/AjaxController.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,18 @@ public function editInformationAction(ServerRequestInterface $request): JsonResp
9393

9494
$params = $request->getQueryParams();
9595

96-
$pid = (int) ($params['pid'] ?? 0);
97-
if (0 === $pid) {
98-
return new JsonResponse(['error' => 'Missing required parameter: pid'], 400);
96+
$pidParam = $params['pid'] ?? null;
97+
$pid = filter_var($pidParam, \FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]);
98+
if (false === $pid) {
99+
return new JsonResponse(['error' => 'Missing or invalid parameter: pid must be a positive integer'], 400);
100+
}
101+
102+
$languageParam = $params['language'] ?? 0;
103+
$languageUid = filter_var($languageParam, \FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]);
104+
if (false === $languageUid) {
105+
return new JsonResponse(['error' => 'Invalid parameter: language must be a non-negative integer'], 400);
99106
}
100107

101-
$languageUid = (int) ($params['language'] ?? 0);
102108
$returnUrl = (string) ($params['returnUrl'] ?? '');
103109
if ('' === $returnUrl) {
104110
return new JsonResponse(['error' => 'Missing required parameter: returnUrl'], 400);

‎Resources/Public/JavaScript/frontend_edit.js‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@
772772
*/
773773
const DataService = {
774774
getClosestContentElement(element) {
775+
if (!element) return null;
775776
while (element && !element.id.match(/c\d+/)) {
776777
element = element.parentElement;
777778
}
@@ -834,7 +835,8 @@
834835
async fetchContentElements(dataItems) {
835836
const config = document.getElementById('frontend-edit-toolbar-config');
836837
if (!config) {
837-
throw new Error('Frontend edit configuration not found');
838+
Logger.log('Frontend edit configuration element not found', null, 'warn');
839+
return {};
838840
}
839841

840842
const editInfoUrl = config.dataset.editInfoUrl;
@@ -849,21 +851,27 @@
849851

850852
Logger.log('Sending request to backend', { url: url.toString() });
851853

852-
const response = await fetch(url.toString(), {
853-
cache: 'no-cache',
854-
method: 'POST',
855-
headers: { 'Content-Type': 'application/json' },
856-
body: JSON.stringify(dataItems)
857-
});
854+
try {
855+
const response = await fetch(url.toString(), {
856+
cache: 'no-cache',
857+
method: 'POST',
858+
headers: { 'Content-Type': 'application/json' },
859+
body: JSON.stringify(dataItems)
860+
});
858861

859-
if (!response.ok) {
860-
throw new Error('Failed to fetch content elements');
861-
}
862+
if (!response.ok) {
863+
throw new Error('Failed to fetch content elements');
864+
}
862865

863-
const data = await response.json();
864-
Logger.log(`Backend response received with ${Object.keys(data).length} content element(s)`);
866+
const data = await response.json();
867+
Logger.log(`Backend response received with ${Object.keys(data).length} content element(s)`);
865868

866-
return data;
869+
return data;
870+
} catch (error) {
871+
Notification.show({ title: 'Frontend Edit', message: 'Failed to load edit information', severity: 'error' });
872+
Logger.log('Failed to fetch content elements', { error: error.message }, 'error');
873+
return {};
874+
}
867875
}
868876
};
869877

@@ -880,6 +888,12 @@
880888
let failed = 0;
881889

882890
for (let [uid, contentElement] of Object.entries(jsonResponse)) {
891+
if (!contentElement || !contentElement.menu || !contentElement.element) {
892+
Logger.log(`Skipping content element c${uid}: missing menu or element data`, null, 'warn');
893+
failed++;
894+
continue;
895+
}
896+
883897
let idElement = document.querySelector(`#c${uid}`);
884898

885899
// Handle translation mapping
@@ -1065,6 +1079,7 @@
10651079
error: error.message,
10661080
stack: error.stack
10671081
}, 'error');
1082+
Notification.show({ title: 'Frontend Edit', message: 'Initialization error', severity: 'warning' });
10681083
}
10691084
},
10701085

‎Tests/Unit/Service/Authentication/BackendUserServiceTest.php‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the "xima_typo3_frontend_edit" TYPO3 CMS extension.
7+
*
8+
* (c) 2024-2026 Konrad Michalik <hej@konradmichalik.dev>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Xima\XimaTypo3FrontendEdit\Tests\Unit\Service\Authentication;
15+
16+
use PHPUnit\Framework\Attributes\{CoversClass, Test};
17+
use PHPUnit\Framework\TestCase;
18+
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
19+
use Xima\XimaTypo3FrontendEdit\Configuration;
20+
use Xima\XimaTypo3FrontendEdit\Service\Authentication\BackendUserService;
21+
22+
/**
23+
* BackendUserServiceTest.
24+
*
25+
* @author Konrad Michalik <hej@konradmichalik.dev>
26+
* @license GPL-2.0-or-later
27+
*/
28+
#[CoversClass(BackendUserService::class)]
29+
final class BackendUserServiceTest extends TestCase
30+
{
31+
protected function tearDown(): void
32+
{
33+
unset($GLOBALS['BE_USER']);
34+
}
35+
36+
#[Test]
37+
public function getBackendUserReturnsNullWhenNotSet(): void
38+
{
39+
unset($GLOBALS['BE_USER']);
40+
41+
$service = new BackendUserService();
42+
43+
self::assertNull($service->getBackendUser());
44+
}
45+
46+
#[Test]
47+
public function getBackendUserReturnsBackendUserFromGlobals(): void
48+
{
49+
$backendUser = $this->createMock(BackendUserAuthentication::class);
50+
$backendUser->user = ['uid' => 1];
51+
$GLOBALS['BE_USER'] = $backendUser;
52+
53+
$service = new BackendUserService();
54+
55+
self::assertSame($backendUser, $service->getBackendUser());
56+
}
57+
58+
#[Test]
59+
public function getBackendUserCachesResult(): void
60+
{
61+
$backendUser = $this->createMock(BackendUserAuthentication::class);
62+
$backendUser->user = ['uid' => 1];
63+
$GLOBALS['BE_USER'] = $backendUser;
64+
65+
$service = new BackendUserService();
66+
67+
$first = $service->getBackendUser();
68+
$second = $service->getBackendUser();
69+
70+
self::assertSame($first, $second);
71+
}
72+
73+
#[Test]
74+
public function hasPageAccessReturnsFalseWhenNoBackendUser(): void
75+
{
76+
unset($GLOBALS['BE_USER']);
77+
78+
$service = new BackendUserService();
79+
80+
self::assertFalse($service->hasPageAccess(1));
81+
}
82+
83+
#[Test]
84+
public function hasPageAccessReturnsFalseWhenUserDataIsNull(): void
85+
{
86+
$backendUser = $this->createMock(BackendUserAuthentication::class);
87+
$backendUser->user = null;
88+
$GLOBALS['BE_USER'] = $backendUser;
89+
90+
$service = new BackendUserService();
91+
92+
self::assertFalse($service->hasPageAccess(1));
93+
}
94+
95+
#[Test]
96+
public function hasRecordEditAccessReturnsFalseWhenNoBackendUser(): void
97+
{
98+
unset($GLOBALS['BE_USER']);
99+
100+
$service = new BackendUserService();
101+
102+
self::assertFalse($service->hasRecordEditAccess('tt_content', ['uid' => 1]));
103+
}
104+
105+
#[Test]
106+
public function hasRecordEditAccessReturnsFalseWhenUserDataIsNull(): void
107+
{
108+
$backendUser = $this->createMock(BackendUserAuthentication::class);
109+
$backendUser->user = null;
110+
$GLOBALS['BE_USER'] = $backendUser;
111+
112+
$service = new BackendUserService();
113+
114+
self::assertFalse($service->hasRecordEditAccess('tt_content', ['uid' => 1]));
115+
}
116+
117+
#[Test]
118+
public function isFrontendEditDisabledReturnsTrueWhenNoBackendUser(): void
119+
{
120+
unset($GLOBALS['BE_USER']);
121+
122+
$service = new BackendUserService();
123+
124+
self::assertTrue($service->isFrontendEditDisabled());
125+
}
126+
127+
#[Test]
128+
public function isFrontendEditDisabledReturnsTrueWhenUserDataIsNull(): void
129+
{
130+
$backendUser = $this->createMock(BackendUserAuthentication::class);
131+
$backendUser->user = null;
132+
$GLOBALS['BE_USER'] = $backendUser;
133+
134+
$service = new BackendUserService();
135+
136+
self::assertTrue($service->isFrontendEditDisabled());
137+
}
138+
139+
#[Test]
140+
public function isFrontendEditDisabledReturnsFalseByDefault(): void
141+
{
142+
$backendUser = $this->createMock(BackendUserAuthentication::class);
143+
$backendUser->user = ['uid' => 1];
144+
$backendUser->uc = [];
145+
$GLOBALS['BE_USER'] = $backendUser;
146+
147+
$service = new BackendUserService();
148+
149+
self::assertFalse($service->isFrontendEditDisabled());
150+
}
151+
152+
#[Test]
153+
public function isFrontendEditDisabledReturnsTrueWhenUcKeyIsSet(): void
154+
{
155+
$backendUser = $this->createMock(BackendUserAuthentication::class);
156+
$backendUser->user = ['uid' => 1];
157+
$backendUser->uc = [Configuration::UC_KEY_DISABLED => true];
158+
$GLOBALS['BE_USER'] = $backendUser;
159+
160+
$service = new BackendUserService();
161+
162+
self::assertTrue($service->isFrontendEditDisabled());
163+
}
164+
165+
#[Test]
166+
public function isFrontendEditAllowedReturnsFalseWhenNoBackendUser(): void
167+
{
168+
unset($GLOBALS['BE_USER']);
169+
170+
$service = new BackendUserService();
171+
172+
self::assertFalse($service->isFrontendEditAllowed());
173+
}
174+
175+
#[Test]
176+
public function isFrontendEditAllowedReturnsFalseWhenUserDataIsNull(): void
177+
{
178+
$backendUser = $this->createMock(BackendUserAuthentication::class);
179+
$backendUser->user = null;
180+
$GLOBALS['BE_USER'] = $backendUser;
181+
182+
$service = new BackendUserService();
183+
184+
self::assertFalse($service->isFrontendEditAllowed());
185+
}
186+
187+
#[Test]
188+
public function isFrontendEditAllowedReturnsTrueByDefault(): void
189+
{
190+
$backendUser = $this->createMock(BackendUserAuthentication::class);
191+
$backendUser->user = ['uid' => 1];
192+
$backendUser->method('getTSConfig')->willReturn([]);
193+
$GLOBALS['BE_USER'] = $backendUser;
194+
195+
$service = new BackendUserService();
196+
197+
self::assertTrue($service->isFrontendEditAllowed());
198+
}
199+
200+
#[Test]
201+
public function isFrontendEditAllowedReturnsFalseWhenDisabledViaTsConfig(): void
202+
{
203+
$backendUser = $this->createMock(BackendUserAuthentication::class);
204+
$backendUser->user = ['uid' => 1];
205+
$backendUser->method('getTSConfig')->willReturn([
206+
Configuration::USER_TSCONFIG_KEY => [
207+
Configuration::USER_TSCONFIG_DISABLED => '1',
208+
],
209+
]);
210+
$GLOBALS['BE_USER'] = $backendUser;
211+
212+
$service = new BackendUserService();
213+
214+
self::assertFalse($service->isFrontendEditAllowed());
215+
}
216+
217+
#[Test]
218+
public function isFrontendEditAllowedReturnsTrueWhenExplicitlyEnabled(): void
219+
{
220+
$backendUser = $this->createMock(BackendUserAuthentication::class);
221+
$backendUser->user = ['uid' => 1];
222+
$backendUser->method('getTSConfig')->willReturn([
223+
Configuration::USER_TSCONFIG_KEY => [
224+
Configuration::USER_TSCONFIG_DISABLED => '0',
225+
],
226+
]);
227+
$GLOBALS['BE_USER'] = $backendUser;
228+
229+
$service = new BackendUserService();
230+
231+
self::assertTrue($service->isFrontendEditAllowed());
232+
}
233+
}

0 commit comments

Comments
 (0)