Skip to content

Commit 4bef954

Browse files
committed
fix(manager): restore XHR CSRF responses and update tab restoration tests
1 parent 0d4b2cb commit 4bef954

4 files changed

Lines changed: 42 additions & 13 deletions

File tree

‎core/src/Middleware/VerifyCsrfToken.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,12 @@ protected function getActionId($request)
154154
}
155155

156156
/**
157-
* Builds the rejection response, matching the format the caller expects.
157+
* Builds the rejection response. Legacy XHR callers require JSON even when
158+
* their Accept header prefers HTML; ordinary page loads receive plain text.
158159
*/
159160
protected function reject($request)
160161
{
161-
if ($request->expectsJson()) {
162+
if ($request->ajax() || $request->wantsJson()) {
162163
return Response::json(['error' => 'CSRF token mismatch', 'code' => 'csrf_token_mismatch'], 403);
163164
}
164165

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
<?php
22

3-
test('manager frame loads main target link helper before modx js', function () {
3+
test('manager frame loads navigation and restoration helpers before versioned evo js', function () {
44
$template = file_get_contents(dirname(__DIR__, 4) . '/manager/views/frame/1.blade.php');
55

66
$treeHelper = 'media/script/tree-drop-guard-helper.js?v={{evo()->getVersionData(\'version\')}}';
77
$mainTargetHelper = 'media/script/main-target-link-helper.js?v={{evo()->getVersionData(\'version\')}}';
8-
$modxScript = '{{ManagerTheme::getThemeUrl()}}js/evo.js?v={{evo()->getVersionData(\'version\')}}';
8+
$tabHelper = "@revision(MGR_DIR . '/media/script/manager-tab-state.js')";
9+
$evoScript = "@revision(MGR_DIR . '/' . ManagerTheme::getThemeDir(false) . 'js/evo.js')";
910

1011
expect(str_contains($template, $treeHelper))->toBeTrue();
1112
expect(str_contains($template, $mainTargetHelper))->toBeTrue();
12-
expect(strpos($template, $mainTargetHelper))->toBeLessThan(strpos($template, $modxScript));
13+
expect($template)->toContain($tabHelper)->toContain($evoScript);
14+
expect(strpos($template, $mainTargetHelper))->toBeLessThan(strpos($template, $tabHelper));
15+
expect(strpos($template, $tabHelper))->toBeLessThan(strpos($template, $evoScript));
1316
});
Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,36 @@
11
<?php
22

3-
test('manager global tabs refresh persisted csrf tokens before restoring urls', function () {
3+
test('manager global tabs delegate token-free storage and audited restoration to the shared helper', function () {
44
$frame = file_get_contents(dirname(__DIR__, 4) . '/manager/views/frame/1.blade.php');
55

66
expect($frame)
7-
->toContain('csrf_token: @js(csrf_token())');
7+
->toMatch('~<meta\s+name="csrf-token"\s+content="\{\{ csrf_token\(\) \}\}"\s*/?>~')
8+
->toContain("config('cms.manager_tab_restore.modules'")
9+
->toContain('tab_restore_user:');
810

911
$script = file_get_contents(dirname(__DIR__, 4) . '/manager/media/style/default/js/evo.js');
1012

1113
expect($script)
12-
->toContain("tabsCsrfPlaceholder: '__EVO_CSRF_TOKEN__'")
14+
->not->toContain('__EVO_CSRF_TOKEN__')
1315
->toContain('tabUrlForStorage: function (url)')
1416
->toContain('tabUrlForRestore: function (url)')
17+
->toContain('w.evoManagerTabState.storage(url, evo.tabsRestoreOptions())')
18+
->toContain('w.evoManagerTabState.restore(url, evo.tabsRestoreOptions())')
1519
->toContain('url: evo.tabUrlForStorage(url)')
1620
->toContain('active: evo.tabUrlForStorage(activeUrl)')
17-
->toContain('url: evo.tabUrlForRestore(tab.url)')
18-
->toContain('url: evo.tabUrlForRestore(data.active)')
19-
->toContain('evo.tabUrlForRestore(href)')
21+
->toContain('var tabUrl = evo.tabUrlForRestore(tab.url)')
22+
->toContain('var activeUrl = evo.tabUrlForRestore(data.active)')
23+
->toContain('startupUrl = evo.tabUrlForRestore(w.location.href)')
24+
->toContain('token: evo.tabsCsrfToken()')
25+
->toContain('modules: evo.config.tab_restore_modules')
26+
->toContain("'EVO_Tabs:' + encodeURIComponent(evo.EVO_MANAGER_URL) + ':' + evo.config.tab_restore_user")
2027
->toContain("localStorage.setItem('page_url', evo.tabUrlForStorage(b))");
28+
29+
// Runtime URL validation and stale-token cases live in manager-tab-state.test.js.
30+
$helper = file_get_contents(dirname(__DIR__, 4) . '/manager/media/script/manager-tab-state.js');
31+
expect($helper)
32+
->toContain("target.searchParams.delete('_token')")
33+
->toContain("target.searchParams.set('_token', options.token)")
34+
->toContain('var target = read(url, options)')
35+
->toContain("if (!target || !options.token) return ''");
2136
});

‎core/tests/Unit/Security/ManagerCsrfMiddlewareTest.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,11 @@ function csrfPassed(Request $request): bool
205205
expect(csrfPassed($request))->toBeTrue();
206206
});
207207

208-
it('answers XHR callers with JSON and page requests with plain text', function () {
209-
$xhr = csrfRequest('POST', ['a' => 109], ['HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest']);
208+
it('answers XHR callers with JSON and page requests with plain text', function (string $accept) {
209+
$xhr = csrfRequest('POST', ['a' => 109], [
210+
'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest',
211+
'HTTP_ACCEPT' => $accept,
212+
]);
210213
$page = csrfRequest('GET', ['a' => 6, 'id' => 3]);
211214

212215
$middleware = new VerifyCsrfToken();
@@ -216,6 +219,13 @@ function csrfPassed(Request $request): bool
216219
->toMatchArray(['kind' => 'json', 'status' => 403])
217220
->and($middleware->handle($page, $noop))
218221
->toMatchArray(['kind' => 'text', 'status' => 403]);
222+
})->with(['text/html', '*/*', 'application/json']);
223+
224+
it('answers non-XHR JSON clients with a JSON rejection', function () {
225+
$request = csrfRequest('POST', ['a' => 109], ['HTTP_ACCEPT' => 'application/json']);
226+
227+
expect((new VerifyCsrfToken())->handle($request, fn () => 'next'))
228+
->toMatchArray(['kind' => 'json', 'status' => 403]);
219229
});
220230

221231
it('lets HEAD and OPTIONS through for ordinary navigation', function (string $method) {

0 commit comments

Comments
 (0)