From 476ba2b9960b37c85c05340be65c104bb4ff23f8 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 4 Mar 2021 04:41:34 -0800
Subject: [PATCH] authx - If 'authx_guards' is set, then enforce them
---
ext/authx/Civi/Authx/Authenticator.php | 25 ++++++++++
ext/authx/authx.php | 24 ++++++---
.../tests/phpunit/Civi/Authx/AllFlowsTest.php | 50 ++++++++++++++++++-
3 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/ext/authx/Civi/Authx/Authenticator.php b/ext/authx/Civi/Authx/Authenticator.php
index c83ac1d647..e7559dbbb1 100644
--- a/ext/authx/Civi/Authx/Authenticator.php
+++ b/ext/authx/Civi/Authx/Authenticator.php
@@ -60,6 +60,7 @@ class Authenticator {
$tgt = AuthenticatorTarget::create([
'flow' => $details['flow'],
'cred' => $details['cred'],
+ 'siteKey' => $details['siteKey'] ?? NULL,
'useSession' => $details['useSession'] ?? FALSE,
]);
if ($principal = $this->checkCredential($tgt)) {
@@ -148,6 +149,23 @@ class Authenticator {
}
break;
}
+
+ $useGuards = \Civi::settings()->get('authx_guards');
+ if (!empty($useGuards)) {
+ // array(string $credType => string $requiredPermissionToUseThisCred)
+ $perms['pass'] = 'authenticate with password';
+ $perms['api_key'] = 'authenticate with api key';
+
+ // If any one of these passes, then we allow the authentication.
+ $passGuard = [];
+ $passGuard[] = in_array('site_key', $useGuards) && defined('CIVICRM_SITE_KEY') && hash_equals(CIVICRM_SITE_KEY, $tgt->siteKey);
+ $passGuard[] = in_array('perm', $useGuards) && isset($perms[$tgt->credType]) && \CRM_Core_Permission::check($perms[$tgt->credType], $tgt->contactId);
+ // JWTs are signed by us. We don't need user to prove that they're allowed to use them.
+ $passGuard[] = ($tgt->credType === 'jwt');
+ if (!max($passGuard)) {
+ $this->reject(sprintf('Login not permitted. Must satisfy guard (%s).', implode(', ', $useGuards)));
+ }
+ }
}
/**
@@ -209,6 +227,7 @@ class Authenticator {
* @param string $message
*/
protected function reject($message = 'Authentication failed') {
+ \CRM_Core_Session::useFakeSession();
$r = new Response(401, ['Content-Type' => 'text/plain'], "HTTP 401 $message");
\CRM_Utils_System::sendResponse($r);
}
@@ -238,6 +257,12 @@ class AuthenticatorTarget {
*/
public $cred;
+ /**
+ * The raw site-key as submitted (if applicable).
+ * @var string
+ */
+ public $siteKey;
+
/**
* (Authenticated) The type of credential.
*
diff --git a/ext/authx/authx.php b/ext/authx/authx.php
index c01aaf3289..66ab1e41f0 100644
--- a/ext/authx/authx.php
+++ b/ext/authx/authx.php
@@ -6,22 +6,24 @@ use CRM_Authx_ExtensionUtil as E;
// phpcs:enable
Civi::dispatcher()->addListener('civi.invoke.auth', function($e) {
+ $params = ($_SERVER['REQUEST_METHOD'] === 'GET') ? $_GET : $_POST;
+ $siteKey = $_SERVER['HTTP_X_CIVI_KEY'] ?? $params['_authxSiteKey'] ?? NULL;
+
if (!empty($_SERVER['HTTP_X_CIVI_AUTH'])) {
- return (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'xheader', 'cred' => $_SERVER['HTTP_X_CIVI_AUTH']]);
+ return (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'xheader', 'cred' => $_SERVER['HTTP_X_CIVI_AUTH'], 'siteKey' => $siteKey]);
}
if (!empty($_SERVER['HTTP_AUTHORIZATION'])) {
- return (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'header', 'cred' => $_SERVER['HTTP_AUTHORIZATION']]);
+ return (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'header', 'cred' => $_SERVER['HTTP_AUTHORIZATION'], 'siteKey' => $siteKey]);
}
- $params = ($_SERVER['REQUEST_METHOD'] === 'GET') ? $_GET : $_POST;
if (!empty($params['_authx'])) {
if ((implode('/', $e->args) === 'civicrm/authx/login')) {
- (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'login', 'cred' => $params['_authx'], 'useSession' => TRUE]);
+ (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'login', 'cred' => $params['_authx'], 'useSession' => TRUE, 'siteKey' => $siteKey]);
_authx_redact(['_authx']);
}
elseif (!empty($params['_authxSes'])) {
- (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'auto', 'cred' => $params['_authx'], 'useSession' => TRUE]);
+ (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'auto', 'cred' => $params['_authx'], 'useSession' => TRUE, 'siteKey' => $siteKey]);
if ($_SERVER['REQUEST_METHOD'] === 'GET') {
_authx_reload(implode('/', $e->args), $_SERVER['QUERY_STRING']);
}
@@ -30,7 +32,7 @@ Civi::dispatcher()->addListener('civi.invoke.auth', function($e) {
}
}
else {
- (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'param', 'cred' => $params['_authx']]);
+ (new \Civi\Authx\Authenticator())->auth($e, ['flow' => 'param', 'cred' => $params['_authx'], 'siteKey' => $siteKey]);
_authx_redact(['_authx']);
}
}
@@ -212,6 +214,16 @@ function authx_civicrm_themes(&$themes) {
_authx_civix_civicrm_themes($themes);
}
+/**
+ * Implements hook_civicrm_permission().
+ *
+ * @see CRM_Utils_Hook::permission()
+ */
+function authx_civicrm_permission(&$permissions) {
+ $permissions['authenticate with password'] = ts('AuthX: Authenticate to services with password');
+ $permissions['authenticate with api key'] = ts('AuthX: Authenticate to services with API key');
+}
+
// --- Functions below this ship commented out. Uncomment as required. ---
/**
diff --git a/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php b/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
index eb6eff39b7..20055f6d36 100644
--- a/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
+++ b/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
@@ -56,10 +56,12 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
parent::setUp();
$this->settingsBackup = [];
foreach (\Civi\Authx\Meta::getFlowTypes() as $flowType) {
- foreach (["authx_{$flowType}_cred", "authx_{$flowType}_user"] as $setting) {
+ foreach (["authx_{$flowType}_cred", "authx_{$flowType}_user", "authx_guards"] as $setting) {
$this->settingsBackup[$setting] = \Civi::settings()->get($setting);
}
}
+
+ \Civi::settings()->set('authx_guards', []);
}
public function tearDown() {
@@ -166,6 +168,52 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
}
}
+ /**
+ * The setting "authx_guard" may be used to require (or not require) the site_key.
+ *
+ * @throws \CiviCRM_API3_Exception
+ * @throws \GuzzleHttp\Exception\GuzzleException
+ */
+ public function testStatelessGuardSiteKey() {
+ if (!defined('CIVICRM_SITE_KEY')) {
+ $this->markTestIncomplete("Cannot run test without CIVICRM_SITE_KEY");
+ }
+
+ $addParam = function($request, $key, $value) {
+ $query = $request->getUri()->getQuery();
+ return $request->withUri(
+ $request->getUri()->withQuery($query . '&' . urlencode($key) . '=' . urlencode($value))
+ );
+ };
+
+ [$credType, $flowType] = ['pass', 'header'];
+ $http = $this->createGuzzle(['http_errors' => FALSE]);
+ \Civi::settings()->set("authx_{$flowType}_cred", [$credType]);
+
+ /** @var \Psr\Http\Message\RequestInterface $request */
+ $request = $this->applyAuth($this->requestMyContact(), $credType, $flowType, $this->getDemoCID());
+
+ // Request OK. Policy requires site_key, and we have one.
+ \Civi::settings()->set("authx_guards", ['site_key']);
+ $response = $http->send($request->withHeader('X-Civi-Key', CIVICRM_SITE_KEY));
+ $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+
+ // Request OK. Policy does not require site_key, and we do not have one
+ \Civi::settings()->set("authx_guards", []);
+ $response = $http->send($request);
+ $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+
+ // Request fails. Policy requires site_key, but we don't have the wrong value.
+ \Civi::settings()->set("authx_guards", ['site_key']);
+ $response = $http->send($request->withHeader('X-Civi-Key', 'not-the-site-key'));
+ $this->assertFailedDueToProhibition($response);
+
+ // Request fails. Policy requires site_key, but we don't have one.
+ \Civi::settings()->set("authx_guards", ['site_key']);
+ $response = $http->send($request);
+ $this->assertFailedDueToProhibition($response);
+ }
+
/**
* The login flow allows you use 'civicrm/authx/login' and 'civicrm/authx/logout'
* to setup/teardown a session.
--
2.25.1