Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(dev/core#2077) Make 'civicrm/ajax/rest' interoperable with 'extern/rest.php' parameters #19727

Merged
merged 9 commits into from
Feb 5, 2022
46 changes: 34 additions & 12 deletions CRM/Utils/REST.php
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,7 @@ public static function loadTemplate() {
unset($param['q']);
$smarty->assign_by_ref("request", $param);

if (!array_key_exists('HTTP_X_REQUESTED_WITH', $_SERVER) ||
$_SERVER['HTTP_X_REQUESTED_WITH'] != "XMLHttpRequest"
) {
if (!self::isWebServiceRequest()) {

$smarty->assign('tplFile', $tpl);
$config = CRM_Core_Config::singleton();
Expand Down Expand Up @@ -434,10 +432,7 @@ public static function ajaxJson() {

require_once 'api/v3/utils.php';
$config = CRM_Core_Config::singleton();
if (!$config->debug && (!array_key_exists('HTTP_X_REQUESTED_WITH', $_SERVER) ||
$_SERVER['HTTP_X_REQUESTED_WITH'] != "XMLHttpRequest"
)
) {
if (!$config->debug && !self::isWebServiceRequest()) {
$error = civicrm_api3_create_error("SECURITY ALERT: Ajax requests can only be issued by javascript clients, eg. CRM.api3().",
[
'IP' => $_SERVER['REMOTE_ADDR'],
Expand Down Expand Up @@ -499,11 +494,7 @@ public static function ajax() {
// restrict calls to this etc
// the request has to be sent by an ajax call. First line of protection against csrf
$config = CRM_Core_Config::singleton();
if (!$config->debug &&
(!array_key_exists('HTTP_X_REQUESTED_WITH', $_SERVER) ||
$_SERVER['HTTP_X_REQUESTED_WITH'] != "XMLHttpRequest"
)
) {
if (!$config->debug && !self::isWebServiceRequest()) {
require_once 'api/v3/utils.php';
$error = civicrm_api3_create_error("SECURITY ALERT: Ajax requests can only be issued by javascript clients, eg. CRM.api3().",
[
Expand Down Expand Up @@ -636,4 +627,35 @@ public function loadCMSBootstrap() {
}
}

/**
* Does this request appear to be a web-service request?
*
* This is used to mitigate CSRF risks.
*
* @return bool
* TRUE if the current request appears to either XMLHttpRequest or non-browser-based.
* Indicated by either (a) custom headers like `X-Request-With`/`X-Civi-Auth`
* or (b) strong-secret-params that could theoretically appear in URL bar but which
* cannot be meaningfully forged for CSRF purposes (like `?api_key=SECRET` or `?_authx=SECRET`).
* FALSE if the current request looks like a standard browser request. This request may be generated by
* <A HREF>, <IFRAME>, <IMG>, `Location:`, or similar CSRF vector.
*/
protected static function isWebServiceRequest(): bool {
if (($_SERVER['HTTP_X_REQUESTED_WITH'] ?? NULL) === 'XMLHttpRequest') {
return TRUE;
}

$authx = \CRM_Core_Session::singleton()->get('authx');
colemanw marked this conversation as resolved.
Show resolved Hide resolved
$allowFlows = ['legacyrest', 'param', 'xheader'];
// <legacyrest> Current request has valid `?api_key=SECRET&key=SECRET` ==> Strong-secret params
// <param> Current request has valid `?_authx=SECRET` ==> Strong-secret param
// <xheader> Current request has valid `X-Civi-Auth:` ==> Custom header AND strong-secret param
// NOTE: Prohibited flows: `login`, `auto`, and `header` are driven by standard headers (`Cookie:`/`Authorization:`)
if (!empty($authx) && in_array($authx['flow'], $allowFlows)) {
return TRUE;
}

return FALSE;
}

}
42 changes: 42 additions & 0 deletions ext/authx/Civi/Authx/LegacyRestAuthenticator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php
/*
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC. All rights reserved. |
| |
| This work is published under the GNU AGPLv3 license with some |
| permitted exceptions and without any warranty. For full license |
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/

namespace Civi\Authx;

use GuzzleHttp\Psr7\Response;

/**
* Historically, 'extern/rest.php' and 'civicrm/ajax/rest' were similar interfaces
* based on the same controller, but they used different authentication styles.
*
* This authenticator is activated if one requests 'civicrm/ajax/rest' using the
* authentication style of 'extern/rest.php'.
*
* @package Civi\Authx
*/
class LegacyRestAuthenticator extends Authenticator {

protected function reject($message = 'Authentication failed') {
$data = ["error_message" => "FATAL: $message", "is_error" => 1];
$r = new Response(200, ['Content-Type' => 'text/javascript'], json_encode($data));
\CRM_Utils_System::sendResponse($r);
}

protected function login(AuthenticatorTarget $tgt) {
parent::login($tgt);
\Civi::dispatcher()->addListener('hook_civicrm_permission_check', function ($e) {
if ($e->permission === 'access AJAX API') {
$e->granted = TRUE;
}
});
}

}
4 changes: 4 additions & 0 deletions ext/authx/authx.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
_authx_redact(['_authx']);
}
}

if (count($e->args) > 2 && $e->args[1] === 'ajax' && $e->args[2] === 'rest' && (!empty($_REQUEST['api_key']) || !empty($_REQUEST['key']))) {
return (new \Civi\Authx\LegacyRestAuthenticator())->auth($e, ['flow' => 'legacyrest', 'cred' => 'Bearer ' . $_REQUEST['api_key'] ?? '', 'siteKey' => $_REQUEST['key'] ?? NULL]);
}
});

/**
Expand Down
5 changes: 4 additions & 1 deletion ext/authx/settings/authx.setting.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* @copyright CiviCRM LLC https://civicrm.org/licensing
*/
$_authx_settings = function() {
$flows = ['param', 'header', 'xheader', 'login', 'auto', 'script', 'pipe'];
$flows = ['param', 'header', 'xheader', 'login', 'auto', 'script', 'pipe', 'legacyrest'];
$basic = [
'group_name' => 'CiviCRM Preferences',
'group' => 'authx',
Expand Down Expand Up @@ -77,6 +77,9 @@
];
}

// Override defaults for a few specific elements
$s['authx_legacyrest_cred']['default'] = ['jwt', 'api_key'];
$s['authx_legacyrest_user']['default'] = 'require';
$s['authx_param_cred']['default'] = ['jwt', 'api_key'];
$s['authx_header_cred']['default'] = ['jwt', 'api_key'];
$s['authx_xheader_cred']['default'] = ['jwt', 'api_key'];
Expand Down
45 changes: 45 additions & 0 deletions tests/phpunit/E2E/Extern/AuthxRestTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php
/*
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC. All rights reserved. |
| |
| This work is published under the GNU AGPLv3 license with some |
| permitted exceptions and without any warranty. For full license |
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/

/**
* Verify that the REST API bindings correctly parse and authenticate requests.
*
* @group e2e
*/
class E2E_Extern_AuthxRestTest extends E2E_Extern_BaseRestTest {

public static function setUpBeforeClass(): void {
parent::setUpBeforeClass();
\Civi\Test::e2e()
->install(['authx'])
->callback(
function() {
\CRM_Utils_System::synchronizeUsers();
},
'synchronizeUsers'
)
->apply();
}

protected function getRestUrl() {
return CRM_Utils_System::url('civicrm/ajax/rest', NULL, TRUE, NULL, FALSE, TRUE);
}

public function apiTestCases() {
$r = parent::apiTestCases();
$r = array_filter($r, function($case) {
// The 'civicrm/ajax/rest' end-point does not support '?q' inputs.
return !isset($case[0]['q']);
});
return $r;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@
+--------------------------------------------------------------------+
*/

use Civi\Test\HttpTestTrait;

/**
* Verify that the REST API bindings correctly parse and authenticate requests.
*
* @group e2e
*/
class E2E_Extern_RestTest extends CiviEndToEndTestCase {
abstract class E2E_Extern_BaseRestTest extends CiviEndToEndTestCase {

use HttpTestTrait;

protected $url;
protected static $api_key;
protected $session_id;
Expand Down Expand Up @@ -46,10 +51,7 @@ protected function setUp(): void {
$this->old_api_keys = [];
}

protected function getRestUrl() {
return CRM_Core_Resources::singleton()
->getUrl('civicrm', 'extern/rest.php');
}
abstract protected function getRestUrl();

protected function tearDown(): void {
if (!empty($this->old_api_keys)) {
Expand Down Expand Up @@ -211,9 +213,11 @@ public function apiTestCases() {
public function testAPICalls($query, $is_error) {
$this->updateAdminApiKey();

$client = CRM_Utils_HttpClient::singleton();
list($status, $data) = $client->post($this->getRestUrl(), $query);
$this->assertEquals(CRM_Utils_HttpClient::STATUS_OK, $status);
$http = $this->createGuzzle(['http_errors' => FALSE]);
$response = $http->post($this->getRestUrl(), ['form_params' => $query]);
$this->assertStatusCode(200, $response);
$data = (string) $response->getBody();

$result = json_decode($data, TRUE);
if ($result === NULL) {
$msg = print_r(array(
Expand All @@ -231,7 +235,7 @@ public function testAPICalls($query, $is_error) {
* a real user. Submit in "?entity=X&action=X" notation
*/
public function testNotCMSUser_entityAction() {
$client = CRM_Utils_HttpClient::singleton();
$http = $this->createGuzzle(['http_errors' => FALSE]);

//Create contact with api_key
$test_key = "testing1234";
Expand All @@ -251,9 +255,10 @@ public function testNotCMSUser_entityAction() {
"json" => "1",
"api_key" => $test_key,
);
list($status, $data) = $client->post($this->getRestUrl(), $params);
$this->assertEquals(CRM_Utils_HttpClient::STATUS_OK, $status);
$result = json_decode($data, TRUE);

$response = $http->post($this->getRestUrl(), ['form_params' => $params]);
$this->assertStatusCode(200, $response);
$result = json_decode((string) $response->getBody(), TRUE);
$this->assertNotNull($result);
$this->assertAPIErrorCode($result, 1);
}
Expand All @@ -264,7 +269,7 @@ public function testNotCMSUser_entityAction() {
*/
public function testGetCorrectUserBack() {
$this->updateAdminApiKey();
$client = CRM_Utils_HttpClient::singleton();
$http = $this->createGuzzle(['http_errors' => FALSE]);

//Create contact with api_key
// The key associates with a real contact but not a real user
Expand All @@ -276,9 +281,9 @@ public function testGetCorrectUserBack() {
"api_key" => self::getApiKey(),
"id" => "user_contact_id",
);
list($status, $data) = $client->post($this->getRestUrl(), $params);
$this->assertEquals(CRM_Utils_HttpClient::STATUS_OK, $status);
$result = json_decode($data, TRUE);
$response = $http->post($this->getRestUrl(), ['form_params' => $params]);
$this->assertStatusCode(200, $response);
$result = json_decode((string) $response->getBody(), TRUE);
$this->assertNotNull($result);
$this->assertEquals($result['id'], $this->adminContactId);
}
Expand All @@ -288,7 +293,7 @@ public function testGetCorrectUserBack() {
* a real user. Submit in "?q=civicrm/$entity/$action" notation
*/
public function testNotCMSUser_q() {
$client = CRM_Utils_HttpClient::singleton();
$http = $this->createGuzzle(['http_errors' => FALSE]);

//Create contact with api_key
$test_key = "testing1234";
Expand All @@ -307,9 +312,10 @@ public function testNotCMSUser_q() {
"json" => "1",
"api_key" => $test_key,
);
list($status, $data) = $client->post($this->getRestUrl(), $params);
$this->assertEquals(CRM_Utils_HttpClient::STATUS_OK, $status);
$result = json_decode($data, TRUE);
$response = $http->post($this->getRestUrl(), ['form_params' => $params]);

$this->assertStatusCode(200, $response);
$result = json_decode((string) $response->getBody(), TRUE);
$this->assertNotNull($result);
$this->assertAPIErrorCode($result, 1);
}
Expand Down
31 changes: 31 additions & 0 deletions tests/phpunit/E2E/Extern/LegacyRestTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
/*
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC. All rights reserved. |
| |
| This work is published under the GNU AGPLv3 license with some |
| permitted exceptions and without any warranty. For full license |
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/

/**
* Verify that the REST API bindings correctly parse and authenticate requests.
*
* @group e2e
*/
class E2E_Extern_LegacyRestTest extends E2E_Extern_BaseRestTest {

protected function setUp(): void {
if (CIVICRM_UF === 'Drupal8') {
$this->markTestSkipped('Legacy extern/rest.php does not apply to Drupal 8+');
}
parent::setUp();
}

protected function getRestUrl() {
return CRM_Core_Resources::singleton()
->getUrl('civicrm', 'extern/rest.php');
}

}