Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/app_controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,14 @@ protected function _checkCsrfReferer() {
'sysparameters/delete',
);

// Non-existent actions will result in a 404; skip CSRF checking for them.
// Use the same predicate as the dispatcher ($this->methods excludes base
// Controller methods, which are also not dispatchable as actions).
$dispatchableMethods = array_flip($this->methods);
if (!isset($dispatchableMethods[strtolower($this->params['action'])])) {
return true;
}

$action = strtolower($this->params['controller'] . '/' . $this->params['action']);
$method = strtoupper(env('REQUEST_METHOD'));

Expand Down
33 changes: 33 additions & 0 deletions app/tests/cases/controllers/csrf_controller.test.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ function redirect($url, $status = null, $exit = true) {
$this->redirectUrl = $url;
}
}

// Expose _checkCsrfReferer for direct unit testing without going through
// the full dispatch cycle (which would kill the process via missingAction
// for non-existent actions).
function checkCsrfRefererForTest($action) {
$this->params['action'] = $action;
return $this->_checkCsrfReferer();
}
}

class CsrfControllerTestCase extends ExtendedAuthTestCase {
Expand Down Expand Up @@ -136,4 +144,29 @@ function testSameHostDifferentPortIsAllowed() {
$this->testAction('/departments/index', array('method' => 'post'));
$this->assertNull($this->controller->redirectUrl);
}

function testCrossOriginPostToNonExistentActionIsNotCsrfBlocked() {
// A request to a non-existent action will 404; CSRF checking should be
// skipped so the user gets the 404, not a misleading CSRF error.
// Use the direct proxy to avoid the full dispatch cycle, which would
// kill the process via missingAction for non-existent actions.
$_SERVER['HTTP_REFERER'] = 'http://other.com/url';
$_SERVER['REQUEST_METHOD'] = 'POST';
$result = $this->controller->checkCsrfRefererForTest('thisActionDoesNotExist');
$this->assertTrue($result);
$this->assertNull($this->controller->redirectUrl);
}

function testCrossOriginPostToBaseControllerMethodIsNotCsrfBlocked() {
// Base Controller methods like "render" are not dispatchable actions; a
// request for them will 404. CSRF checking must be skipped for these too,
// since method_exists() returns true for inherited methods even though the
// dispatcher will reject them the same as any other missing action.
$_SERVER['HTTP_REFERER'] = 'http://other.com/url';
$_SERVER['REQUEST_METHOD'] = 'POST';
$result = $this->controller->checkCsrfRefererForTest('render');
$this->assertTrue($result);
$this->assertNull($this->controller->redirectUrl);
}

}
Loading