From f63535501f8ec04f717d759117295a041eed3ee8 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 19:03:59 -0700 Subject: [PATCH 01/14] GuzzleMiddleware::curlLog() - Compatibility with Guzzle 7 This only runs when you enable debugging for e2e tests (DEBUG=1). --- CRM/Utils/GuzzleMiddleware.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CRM/Utils/GuzzleMiddleware.php b/CRM/Utils/GuzzleMiddleware.php index 0c9815361265..c8b76cce88ca 100644 --- a/CRM/Utils/GuzzleMiddleware.php +++ b/CRM/Utils/GuzzleMiddleware.php @@ -227,7 +227,7 @@ public static function curlLog(\Psr\Log\LoggerInterface $logger) { $curlFmt = new class() extends \GuzzleHttp\MessageFormatter { - public function format(RequestInterface $request, ResponseInterface $response = NULL, \Exception $error = NULL) { + public function format(RequestInterface $request, ?ResponseInterface $response = NULL, ?\Throwable $error = NULL): string { $cmd = '$ curl'; if ($request->getMethod() !== 'GET') { $cmd .= ' -X ' . escapeshellarg($request->getMethod()); From b882ec6cd70a4f52b1b978654883b28afc388fff Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 13:41:31 -0700 Subject: [PATCH 02/14] (REF) MockPublicFormTest - Extract method assertUrlStartsSession() --- .../E2E/AfformMock/MockPublicFormTest.php | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php index d9fe9ac341ad..f0008320dcbe 100644 --- a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php @@ -82,13 +82,7 @@ public function testAuthenticatedUrlToken_Plain() { $url = $m[1]; $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); - // Going to this page will cause us to authenticate as the target contact - $http = $this->createGuzzle(['http_errors' => FALSE, 'cookies' => new \GuzzleHttp\Cookie\CookieJar()]); - $response = $http->get($url); - $r = (string) $response->getBody(); - $this->assertStatusCode(200, $response); - $response = $http->get('civicrm/authx/id'); - $this->assertContactJson($lebowski, $response); + $this->assertUrlStartsSession($url, $lebowski); } /** @@ -108,12 +102,7 @@ public function testAuthenticatedUrlToken_Html() { $url = html_entity_decode($m[1]); $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); - // Going to this page will cause us to authenticate as the target contact - $http = $this->createGuzzle(['cookies' => new \GuzzleHttp\Cookie\CookieJar()]); - $response = $http->get($url); - $this->assertStatusCode(200, $response); - $response = $http->get('civicrm/authx/id'); - $this->assertContactJson($lebowski, $response); + $this->assertUrlStartsSession($url, $lebowski); } /** @@ -135,12 +124,7 @@ public function testAuthenticatedLinkToken_Html() { $url = $item->getAttribute('href'); } - // Going to this page will cause us to authenticate as the target contact - $http = $this->createGuzzle(['cookies' => new \GuzzleHttp\Cookie\CookieJar()]); - $response = $http->get($url); - $this->assertStatusCode(200, $response); - $response = $http->get('civicrm/authx/id'); - $this->assertContactJson($lebowski, $response); + $this->assertUrlStartsSession($url, $lebowski); } protected function renderTokens($cid, $body, $format) { @@ -151,7 +135,7 @@ protected function renderTokens($cid, $body, $format) { return $tp->getRow(0)->render('example'); } - protected function getLebowskiCID() { + protected function getLebowskiCID(): int { $contact = \civicrm_api3('Contact', 'create', [ 'contact_type' => 'Individual', 'first_name' => 'Jeffrey', @@ -179,4 +163,25 @@ public function assertContactJson($cid, $response) { $this->assertEquals($cid, $j['contact_id'], "Response did not give expected contact ID\n" . $formattedFailure); } + /** + * Opening $url + * + * @param string $url + * @param int $contactId + * + * @return void + * @throws \GuzzleHttp\Exception\GuzzleException + */ + protected function assertUrlStartsSession(string $url, int $contactId): void { + $http = $this->createGuzzle([ + 'http_errors' => FALSE, + 'cookies' => new \GuzzleHttp\Cookie\CookieJar(), + ]); + $response = $http->get($url); + $r = (string) $response->getBody(); + $this->assertStatusCode(200, $response); + $response = $http->get('civicrm/authx/id'); + $this->assertContactJson($contactId, $response); + } + } From 41ec310227ee9b625281a27bfaee8aa84730ca18 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 15:49:59 -0700 Subject: [PATCH 03/14] (REF) MockPublicFormTest - Shorter assertion --- .../phpunit/E2E/AfformMock/MockPublicFormTest.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php index f0008320dcbe..d1526bbfebf3 100644 --- a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php @@ -70,9 +70,7 @@ public function testPublicEditDisallowed() { * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. */ public function testAuthenticatedUrlToken_Plain() { - if (!function_exists('authx_civicrm_config')) { - $this->fail('Cannot test without authx'); - } + $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); $lebowski = $this->getLebowskiCID(); $text = $this->renderTokens($lebowski, 'Please go to {afform.mockPublicFormUrl}', 'text/plain'); @@ -89,9 +87,7 @@ public function testAuthenticatedUrlToken_Plain() { * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. */ public function testAuthenticatedUrlToken_Html() { - if (!function_exists('authx_civicrm_config')) { - $this->fail('Cannot test without authx'); - } + $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); $lebowski = $this->getLebowskiCID(); $html = $this->renderTokens($lebowski, 'Please go to <a href="{afform.mockPublicFormUrl}">my form</a>', 'text/html'); @@ -109,9 +105,7 @@ public function testAuthenticatedUrlToken_Html() { * The email token `{afform.mockPublicFormLink}` should evaluate to an authenticated URL. */ public function testAuthenticatedLinkToken_Html() { - if (!function_exists('authx_civicrm_config')) { - $this->fail('Cannot test without authx'); - } + $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); $lebowski = $this->getLebowskiCID(); $html = $this->renderTokens($lebowski, 'Please go to {afform.mockPublicFormLink}', 'text/html'); From 255659f894a3730bd794cefaf01310775f0d7dd4 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 16:11:51 -0700 Subject: [PATCH 04/14] (REF) MockPublicFormTest - Reorganize/simplify tests Before: Separate tests for different permutations of token-name/output-media After: One test checks consistency across token-names/output-media -- asserting that they all point to the same URL. Another test checks whether that URL behaves corectly. --- .../E2E/AfformMock/MockPublicFormTest.php | 64 ++++++++----------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php index d1526bbfebf3..7a0e525d09bd 100644 --- a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php @@ -67,56 +67,48 @@ public function testPublicEditDisallowed() { } /** - * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. + * There are two tokens ({afform.mockPublicFormUrl} and {afform.mockPublicFormLink}) + * which are rendered in two contexts (text and HTML). + * + * Make sure that the resulting URLs point to the same place, regardless of which + * variant or environment is used. + * + * @return void */ - public function testAuthenticatedUrlToken_Plain() { - $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); - + public function testWellFormedTokens() { $lebowski = $this->getLebowskiCID(); - $text = $this->renderTokens($lebowski, 'Please go to {afform.mockPublicFormUrl}', 'text/plain'); - if (!preg_match(';Please go to ([^\s]+);', $text, $m)) { - $this->fail('Plain text message did not have URL in expected place: ' . $text); - } - $url = $m[1]; - $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); + $messages = \CRM_Core_TokenSmarty::render([ + 'text' => 'url=({afform.mockPublicFormUrl}) link=({afform.mockPublicFormLink})', + 'html' => '<p>url=({afform.mockPublicFormUrl}) link=({afform.mockPublicFormLink})</p>', + ], ['contactId' => $lebowski]); - $this->assertUrlStartsSession($url, $lebowski); - } + $httpTextUrl = '(https?:[a-zA-Z0-9_/\.\?\-\+:=#&]+)'; + $httpHtmlUrl = '(https?:[a-zA-Z0-9_/\.\?\-\+:=#&\;]+)'; + $textPattern = ";url=\($httpTextUrl\) link=\(\[My public form\]\($httpTextUrl\)\); "; + $htmlPattern = ";\<p\>url=\($httpHtmlUrl\) link=\(<a href=\"$httpHtmlUrl\">My public form</a>\)\</p\>;"; - /** - * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. - */ - public function testAuthenticatedUrlToken_Html() { - $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); + $this->assertMatchesRegularExpression($textPattern, $messages['text']); + $this->assertMatchesRegularExpression($htmlPattern, $messages['html']); - $lebowski = $this->getLebowskiCID(); - $html = $this->renderTokens($lebowski, 'Please go to <a href="{afform.mockPublicFormUrl}">my form</a>', 'text/html'); + preg_match($textPattern, $messages['text'], $textMatches); + preg_match($htmlPattern, $messages['html'], $htmlMatches); - if (!preg_match(';a href="([^"]+)";', $html, $m)) { - $this->fail('HTML message did not have URL in expected place: ' . $html); - } - $url = html_entity_decode($m[1]); - $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); + $this->assertEquals($textMatches[1], html_entity_decode($htmlMatches[1]), 'Text and HTML values of {afform.mockPublicFormUrl} should point to same place'); + $this->assertEquals($textMatches[2], html_entity_decode($htmlMatches[2]), 'Text and HTML values of {afform.mockPublicFormLink} should point to same place'); - $this->assertUrlStartsSession($url, $lebowski); + $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $textMatches[1], "URL should look plausible"); + $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $textMatches[2], "URL should look plausible"); } /** - * The email token `{afform.mockPublicFormLink}` should evaluate to an authenticated URL. + * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. */ - public function testAuthenticatedLinkToken_Html() { + public function testAuthenticatedUrlToken() { $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); $lebowski = $this->getLebowskiCID(); - $html = $this->renderTokens($lebowski, 'Please go to {afform.mockPublicFormLink}', 'text/html'); - $doc = \phpQuery::newDocument($html, 'text/html'); - $this->assertEquals(1, $doc->find('a')->count(), 'Document should have hyperlink'); - foreach ($doc->find('a') as $item) { - /** @var \DOMElement $item */ - $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $item->getAttribute('href')); - $this->assertEquals('My public form', $item->firstChild->data); - $url = $item->getAttribute('href'); - } + $url = $this->renderTokens($lebowski, '{afform.mockPublicFormUrl}', 'text/plain'); + $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); $this->assertUrlStartsSession($url, $lebowski); } From 526bffbb2d58e0d02e416c0abc527f7c60db5cef Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Mon, 4 Dec 2023 13:06:26 +0000 Subject: [PATCH 05/14] (dev/core#4462) Authenticator, CheckCredentialEvent - Add property $requestPath --- ext/authx/Civi/Authx/Authenticator.php | 19 +++++++++++-- ext/authx/Civi/Authx/CheckCredentialEvent.php | 27 ++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/ext/authx/Civi/Authx/Authenticator.php b/ext/authx/Civi/Authx/Authenticator.php index 246d62bdff47..88fbe2751d58 100644 --- a/ext/authx/Civi/Authx/Authenticator.php +++ b/ext/authx/Civi/Authx/Authenticator.php @@ -125,6 +125,7 @@ public function auth($e, $details) { 'cred' => $details['cred'] ?? NULL, 'siteKey' => $details['siteKey'] ?? NULL, 'useSession' => $details['useSession'] ?? FALSE, + 'requestPath' => empty($e->args) ? '*' : implode('/', $e->args), ]); if (isset($tgt->cred)) { @@ -161,6 +162,7 @@ public function validate(array $details): array { 'cred' => $details['cred'] ?? NULL, 'siteKey' => $details['siteKey'] ?? NULL, 'useSession' => $details['useSession'] ?? FALSE, + 'requestPath' => $details['requestPath'] ?? '*', ]); if ($principal = $this->checkCredential($tgt)) { @@ -186,7 +188,7 @@ protected function checkCredential($tgt) { // 1. Accept the cred, which stops event propagation and further checks; // 2. Reject the cred, which stops event propagation and further checks; // 3. Neither accept nor reject, letting the event continue on to the next. - $checkEvent = new CheckCredentialEvent($tgt->cred); + $checkEvent = new CheckCredentialEvent($tgt->cred, $tgt->requestPath); \Civi::dispatcher()->dispatch('civi.authx.checkCredential', $checkEvent); if ($checkEvent->getRejection()) { @@ -343,6 +345,12 @@ class AuthenticatorTarget { */ public $flow; + /** + * @var string|null + * Ex: 'civicrm/dashboard' + */ + public $requestPath; + /** * @var bool */ @@ -396,7 +404,13 @@ class AuthenticatorTarget { * @return $this */ public static function create($args = []) { - return (new static())->set($args); + $tgt = (new static())->set($args); + if ($tgt->useSession || $tgt->requestPath === NULL) { + // If requesting access to a session (or using anything that isn't specifically tied + // to an HTTP route), then we are effectively asking for any/all routes. + $tgt->requestPath = '*'; + } + return $tgt; } /** @@ -470,6 +484,7 @@ public function createRedacted(): array { // omit: cred // omit: siteKey 'flow' => $this->flow, + 'requestPath' => $this->requestPath, 'credType' => $this->credType, 'jwt' => $this->jwt, 'useSession' => $this->useSession, diff --git a/ext/authx/Civi/Authx/CheckCredentialEvent.php b/ext/authx/Civi/Authx/CheckCredentialEvent.php index 87379f7e816e..a3475edbfcc1 100644 --- a/ext/authx/Civi/Authx/CheckCredentialEvent.php +++ b/ext/authx/Civi/Authx/CheckCredentialEvent.php @@ -31,6 +31,16 @@ class CheckCredentialEvent extends \Civi\Core\Event\GenericHookEvent { */ public $credValue; + /** + * @var string + * Ex: 'civicrm/dashboard' or '*' + * + * This identifies the path(s) that the requestor wants to access. + * For a stateless HTTP request, that's a specific path. + * For stateful HTTP session or CLI pipe, that's a wildcard. + */ + protected $requestPath; + /** * Authenticated principal. * @@ -49,9 +59,16 @@ class CheckCredentialEvent extends \Civi\Core\Event\GenericHookEvent { /** * @param string $cred * Ex: 'Basic ABCD1234' or 'Bearer ABCD1234' + * @param string $requestPath + * Ex: 'civicrm/dashboard' or '*' + * + * This identifies the path(s) that the requestor wants to access. + * For a stateless HTTP request, that's a specific path. + * For stateful HTTP session or CLI pipe, that's a wildcard. */ - public function __construct(string $cred) { + public function __construct(string $cred, string $requestPath) { [$this->credFormat, $this->credValue] = explode(' ', $cred, 2); + $this->requestPath = $requestPath; } /** @@ -123,4 +140,12 @@ public function getRejection(): ?string { return $this->rejection; } + /** + * @return string + * Ex: 'civicrm/dashboard' + */ + public function getRequestPath(): string { + return $this->requestPath; + } + } From 8d1e2dab0845e2ea45ff327ccc70786e5c892754 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Mon, 4 Dec 2023 13:09:22 +0000 Subject: [PATCH 06/14] (dev/core#4462) Afform - Allow server to receive page-level auth tokens --- .../core/Civi/Afform/PageTokenCredential.php | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 ext/afform/core/Civi/Afform/PageTokenCredential.php diff --git a/ext/afform/core/Civi/Afform/PageTokenCredential.php b/ext/afform/core/Civi/Afform/PageTokenCredential.php new file mode 100644 index 000000000000..259dcdf5b3cb --- /dev/null +++ b/ext/afform/core/Civi/Afform/PageTokenCredential.php @@ -0,0 +1,188 @@ +<?php + +namespace Civi\Afform; + +use Civi\Authx\CheckCredentialEvent; +use Civi\Core\Service\AutoService; +use Civi\Crypto\Exception\CryptoException; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * Allow Afform-based pages to accept page-level access token + * + * Example: + * - Create a JWT with `[scope => afform, afform => MY_FORM_NAME, sub=>cid:123]`. + * This is defined to support "Afform.prefill" and "Afform.submit" on behalf of contact #123. + * - Navigate to `civicrm/my-form?_aff=Bearer+MY_JWT` + * - Within the page-view, each AJAX call sets `X-Civi-Auth: MY_JWT`. + * + * @service civi.afform.page_token + */ +class PageTokenCredential extends AutoService implements EventSubscriberInterface { + + public static function getSubscribedEvents(): array { + $events = []; + $events['&civi.invoke.auth'][] = ['onInvoke', 105]; + $events['civi.authx.checkCredential'][] = ['afformPageToken', -400]; + return $events; + } + + /** + * If you visit a top-level page like "civicrm/my-custom-form?_aff=XXX", then + * all embedded AJAX calls should "_authx=XXX". + * + * @param array $path + * @return void + */ + public function onInvoke(array $path) { + $token = $_REQUEST['_aff'] ?? NULL; + + if (empty($token)) { + return; + } + if (!preg_match(';^[a-zA-Z0-9\.\-_ ]+$;', $token)) { + throw new \CRM_Core_Exception("Malformed page token"); + } + + \CRM_Core_Region::instance('page-header')->add([ + 'callback' => function() use ($token) { + $ajaxSetup = [ + 'headers' => ['X-Civi-Auth' => $token], + + // Sending cookies is counter-productive. For same-origin AJAX, there doesn't seem to be an opt-out. + // The main mitigating factor is that AuthX calls useFakeSession() for our use-case. + // 'xhrFields' => ['withCredentials' => FALSE], + // 'crossDomain' => TRUE, + ]; + $script = sprintf('CRM.$.ajaxSetup(%s);', json_encode($ajaxSetup)); + return "<script type='text/javascript'>\n$script\n</script>"; + }, + ]); + } + + /** + * If we get a JWT with `[scope=>afform, afformName=>xyz]`, then setup + * the current fake-session to allow limited page-views. + * + * @param \Civi\Authx\CheckCredentialEvent $check + * + * @return void + */ + public function afformPageToken(CheckCredentialEvent $check) { + if ($check->credFormat === 'Bearer') { + try { + $claims = \Civi::service('crypto.jwt')->decode($check->credValue); + $scopes = isset($claims['scope']) ? explode(' ', $claims['scope']) : []; + if (!in_array('afform', $scopes)) { + // This is not an afform JWT. Proceed to check any other token sources. + return; + } + if (empty($claims['sub']) || substr($claims['sub'], 0, 4) !== 'cid:') { + $check->reject('Malformed JWT. Must specify the contact ID.'); + } + else { + $contactId = substr($claims['sub'], 4); + if ($this->checkAllowedRoute($check->getRequestPath(), $claims)) { + $check->accept(['contactId' => $contactId, 'credType' => 'jwt', 'jwt' => $claims]); + } + else { + $check->reject('JWT specifies a different form or route'); + } + } + } + catch (CryptoException $e) { + if (strpos($e->getMessage(), 'Expired token') !== FALSE) { + $check->reject('Expired token'); + } + + // Not a valid AuthX JWT. Proceed to check any other token sources. + } + } + + } + + /** + * When processing CRM_Core_Invoke, check to see if our token allows us to handle this request. + * + * @param string $route + * @param array $jwt + * @return bool + * @throws \CRM_Core_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + public function checkAllowedRoute(string $route, array $jwt): bool { + $allowedForm = $jwt['afform']; + + $ajaxRoutes = $this->getAllowedRoutes(); + foreach ($ajaxRoutes as $regex => $routeInfo) { + if (preg_match($regex, $route)) { + $parsed = json_decode(\CRM_Utils_Request::retrieve('params', 'String'), 1); + if (empty($parsed)) { + \Civi::log()->warning("Malformed request. Routes matching $regex must submit params as JSON field."); + return FALSE; + } + + $extraFields = array_diff(array_keys($parsed), $routeInfo['allowFields']); + if (!empty($extraFields)) { + \Civi::log()->warning("Malformed request. Routes matching $regex only support these input fields: " . json_encode($routeInfo['allowFields'])); + return FALSE; + } + + $actualForm = $parsed[$routeInfo['nameField']] ?? NULL; + if ($actualForm !== $allowedForm) { + \Civi::log()->warning("Malformed request. Requested form ($actualForm) does not match allowed name ($allowedForm)."); + return FALSE; + } + + return TRUE; + } + } + + // Actually, we may not need this? aiming for model where top page-request auth is irrelevant to subrequests... + $allowedFormRoute = \Civi\Api4\Afform::get(FALSE)->addWhere('name', '=', $allowedForm) + ->addSelect('name', 'server_route') + ->execute() + ->single(); + if ($allowedFormRoute['server_route'] === $route) { + return TRUE; + } + + return FALSE; + } + + /** + * @return array[] + */ + protected function getAllowedRoutes(): array { + return [ + // ';civicrm/path/to/some/page;' => [ + // + // // All the fields that are allowed for this API call. + // // N.B. Fields like "chain" are NOT allowed. + // 'allowFields' => ['field_1', 'field_2', ...] + // + // // The specific field which identifies the target form. Must match our page-token. + // 'nameField' => 'field_1', + // + // ], + + ';^civicrm/ajax/api4/Afform/prefill$;' => [ + 'allowFields' => ['name', 'args'], + 'nameField' => 'name', + ], + ';^civicrm/ajax/api4/Afform/submit$;' => [ + 'allowFields' => ['name', 'args', 'values'], + 'nameField' => 'name', + ], + ';^civicrm/ajax/api4/Afform/submitFile$;' => [ + 'allowFields' => ['name'], /* FIXME */ + 'nameField' => 'name', + ], + ';^civicrm/ajax/api4/\w+/autocomplete$;' => [ + 'allowFields' => ['formName'], /* FIXME */ + 'nameField' => 'formName', + ], + ]; + } + +} From 3d598cae6e62e4907e51292218f8f2294d2944ad Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 15:04:29 -0700 Subject: [PATCH 07/14] (dev/core#4462) Afform - Define setting `afform_mail_auth_token` (`page` or `session`) --- ext/afform/core/CRM/Afform/Utils.php | 32 +++++++++++++++++++++ ext/afform/core/info.xml | 2 ++ ext/afform/core/settings/Afform.setting.php | 27 +++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 ext/afform/core/CRM/Afform/Utils.php create mode 100644 ext/afform/core/settings/Afform.setting.php diff --git a/ext/afform/core/CRM/Afform/Utils.php b/ext/afform/core/CRM/Afform/Utils.php new file mode 100644 index 000000000000..3057a7c197fb --- /dev/null +++ b/ext/afform/core/CRM/Afform/Utils.php @@ -0,0 +1,32 @@ +<?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 | + +--------------------------------------------------------------------+ + */ + +use CRM_Afform_ExtensionUtil as E; + +/** + * + */ +class CRM_Afform_Utils { + + /** + * Get a list of authentication options for `afform_mail_auth_token`. + * + * @return array + * Array (string $machineName => string $label). + */ + public static function getMailAuthOptions(): array { + return [ + 'session' => E::ts('Session-level authentication'), + 'page' => E::ts('Page-level authentication'), + ]; + } + +} diff --git a/ext/afform/core/info.xml b/ext/afform/core/info.xml index 8f7331699006..155a2c2caaeb 100644 --- a/ext/afform/core/info.xml +++ b/ext/afform/core/info.xml @@ -40,6 +40,8 @@ <mixin>smarty@1.0.0</mixin> <mixin>entity-types-php@2.0.0</mixin> <mixin>menu-xml@1.0.0</mixin> + <mixin>setting-php@1.0.0</mixin> + <mixin>setting-admin@1.0.1</mixin> </mixins> <upgrader>CiviMix\Schema\Afform\AutomaticUpgrader</upgrader> </extension> diff --git a/ext/afform/core/settings/Afform.setting.php b/ext/afform/core/settings/Afform.setting.php new file mode 100644 index 000000000000..ac1ac74bc266 --- /dev/null +++ b/ext/afform/core/settings/Afform.setting.php @@ -0,0 +1,27 @@ +<?php +use CRM_Afform_ExtensionUtil as E; + +return [ + 'afform_mail_auth_token' => [ + 'group_name' => 'Afform Preferences', + 'group' => 'afform', + 'name' => 'afform_mail_auth_token', + 'type' => 'String', + 'html_type' => 'select', + 'html_attributes' => [ + 'class' => 'crm-select2', + ], + 'pseudoconstant' => [ + 'callback' => 'CRM_Afform_Utils::getMailAuthOptions', + ], + // Traditional default. Might be nice to change, but need to tend to upgrade process. + 'default' => 'session', + 'add' => '4.7', + 'title' => E::ts('Mail Authentication Tokens'), + 'is_domain' => 1, + 'is_contact' => 0, + 'description' => E::ts('How do authenticated email hyperlinks work?'), + 'help_text' => NULL, + 'settings_pages' => ['afform' => ['weight' => 10]], + ], +]; From 34b83986fd25fcb6f01f538ba9dd6a2bc4208f2b Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 15:20:49 -0700 Subject: [PATCH 08/14] (dev/core#4462) Afform - Use mockPublicForm for testing session-level and page-level auth --- ext/afform/mock/ang/mockPublicForm.aff.html | 12 +-- ext/afform/mock/ang/mockPublicForm.aff.php | 2 + .../E2E/AfformMock/MockPublicFormTest.php | 99 +++++++++++++++++-- 3 files changed, 98 insertions(+), 15 deletions(-) diff --git a/ext/afform/mock/ang/mockPublicForm.aff.html b/ext/afform/mock/ang/mockPublicForm.aff.html index 593154b507be..f4908e25fcf2 100644 --- a/ext/afform/mock/ang/mockPublicForm.aff.html +++ b/ext/afform/mock/ang/mockPublicForm.aff.html @@ -1,14 +1,8 @@ <!-- This example is entirely public; anonymous users may use it to submit a `Contact`, but they cannot view or modify data. --> <af-form ctrl="afform"> - <af-entity data="{contact_type: 'Individual', source: 'Hello'}" url-autofill="1" security="FBAC" actions="{create: true, update: false}" type="Contact" name="me" label="Myself" /> - <fieldset af-fieldset="me"> - <legend class="af-text">Individual 1</legend> - <div class="af-container"> - <div class="af-container af-layout-inline"> - <af-field name="first_name" /> - <af-field name="last_name" /> - </div> - </div> + <af-entity data="{contact_type: 'Individual', source: 'Mock Public Form'}" type="Contact" name="me" label="Myself" actions="{create: true, update: true}" security="FBAC" url-autofill="0" autofill="user" /> + <fieldset af-fieldset="me" class="af-container" af-title="Myself"> + <afblock-name-individual></afblock-name-individual> </fieldset> <button class="af-button btn-primary" crm-icon="fa-check" ng-click="afform.submit()">Submit</button> </af-form> diff --git a/ext/afform/mock/ang/mockPublicForm.aff.php b/ext/afform/mock/ang/mockPublicForm.aff.php index 533d45e70155..341629eea2b2 100644 --- a/ext/afform/mock/ang/mockPublicForm.aff.php +++ b/ext/afform/mock/ang/mockPublicForm.aff.php @@ -3,6 +3,8 @@ 'type' => 'form', 'title' => ts('My public form'), 'server_route' => 'civicrm/mock-public-form', + 'is_public' => TRUE, 'permission' => '*always allow*', 'is_token' => TRUE, + 'create_submission' => FALSE, ]; diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php index 7a0e525d09bd..91a1f7c622da 100644 --- a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php @@ -5,6 +5,11 @@ use CRM_Core_DAO; /** + * Perform some tests against `mockPublicForm.aff.html`. + * + * This test uses Guzzle and checks more low-level behaviors. For more comprehensive + * tests that also cover browser/Chrome/JS behaviors, see MockPublicFormBrowserTest. + * * @group e2e * @group ang */ @@ -101,10 +106,11 @@ public function testWellFormedTokens() { } /** - * The email token `{afform.mockPublicFormUrl}` should evaluate to an authenticated URL. + * Evaluate the email token `{afform.mockPublicFormUrl}`. The output should be a session-level auth token. */ - public function testAuthenticatedUrlToken() { + public function testAuthenticatedUrlToken_Session() { $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); + Civi::settings()->set('afform_mail_auth_token', 'session'); $lebowski = $this->getLebowskiCID(); $url = $this->renderTokens($lebowski, '{afform.mockPublicFormUrl}', 'text/plain'); @@ -113,6 +119,47 @@ public function testAuthenticatedUrlToken() { $this->assertUrlStartsSession($url, $lebowski); } + /** + * Evaluate the email token `{afform.mockPublicFormUrl}`. The output should be a page-level auth token. + */ + public function testAuthenticatedUrlToken_Page() { + $this->assertTrue(function_exists('authx_civicrm_config'), 'Cannot test without authx'); + Civi::settings()->set('afform_mail_auth_token', 'page'); + + $lebowski = $this->getLebowskiCID(); + $url = $this->renderTokens($lebowski, '{afform.mockPublicFormUrl}', 'text/plain'); + $this->assertMatchesRegularExpression(';^https?:.*civicrm/mock-public-form.*;', $url, "URL should look plausible"); + + // This URL doesn't specifically log you in to a durable sesion. + // $this->assertUrlStartsSession($url, NULL); + + // However, there is an auth token. + $query = parse_url($url, PHP_URL_QUERY); + parse_str($query, $queryParams); + $token = $queryParams['_aff']; + $this->assertNotEmpty($token); + $auth = ['_authx' => $token]; + + // This token cannot be used for any random API... + $body = $this->callApi4AuthTokenFailure($auth, 'Contact', 'get', ['limit' => 5]); + $this->assertMatchesRegularExpression('/JWT specifies a different form or route/', $body, 'Response should have error message'); + + // The token can be used for Afform.prefill and Afform.submit... + $response = $this->callApi4AuthTokenSuccess($auth, 'Afform', 'prefill', [ + 'name' => $this->getFormName(), + ]); + $this->assertEquals('me', $response['values'][0]['name']); + $this->assertEquals($lebowski, $response['values'][0]['values'][0]['fields']['id'], 'Afform.prefill should return id'); + $this->assertEquals('Lebowski', $response['values'][0]['values'][0]['fields']['last_name'], 'Afform.prefill should return last_name'); + + // But the token cannot be used for Afform calls with sneaky params... + $body = $this->callApi4AuthTokenFailure($auth, 'Afform', 'prefill', [ + 'name' => $this->getFormName(), + 'chain' => ['name_me_0' => ['Contact', 'get', []]], + ]); + $this->assertMatchesRegularExpression('/JWT specifies a different form or route/', $body, 'Response should have error message'); + } + protected function renderTokens($cid, $body, $format) { $tp = new \Civi\Token\TokenProcessor(\Civi::dispatcher(), []); $tp->addRow()->context('contactId', $cid); @@ -150,15 +197,14 @@ public function assertContactJson($cid, $response) { } /** - * Opening $url + * Opening $url may generate a session-cookie. Does that cookie authenticate you as $contactId? * * @param string $url - * @param int $contactId - * + * @param int|null $contactId * @return void * @throws \GuzzleHttp\Exception\GuzzleException */ - protected function assertUrlStartsSession(string $url, int $contactId): void { + protected function assertUrlStartsSession(string $url, ?int $contactId): void { $http = $this->createGuzzle([ 'http_errors' => FALSE, 'cookies' => new \GuzzleHttp\Cookie\CookieJar(), @@ -166,8 +212,49 @@ protected function assertUrlStartsSession(string $url, int $contactId): void { $response = $http->get($url); $r = (string) $response->getBody(); $this->assertStatusCode(200, $response); + + // We make another request in the same session. Is it the expected contact? $response = $http->get('civicrm/authx/id'); $this->assertContactJson($contactId, $response); } + protected function callApi4AuthTokenSuccess(array $auth, string $entity, string $action, $params = []) { + $response = $this->callApi4AuthToken($auth, $entity, $action, $params); + $this->assertContentType('application/json', $response); + $this->assertStatusCode(200, $response); + $result = json_decode((string) $response->getBody(), 1); + if (json_last_error() !== JSON_ERROR_NONE) { + $this->fail("Failed to decode APIv4 JSON.\n" . $this->formatFailure($response)); + } + return $result; + } + + protected function callApi4AuthTokenFailure(array $auth, string $entity, string $action, $params = []): string { + $httpResponse = $this->callApi4AuthToken($auth, $entity, $action, $params); + $this->assertEquals(401, $httpResponse->getStatusCode(), "HTTP status code should be 401"); + return (string) $httpResponse->getBody(); + } + + /** + * @param array $auth + * @param string $entity + * @param string $action + * @param array $params + * + * @return \Psr\Http\Message\ResponseInterface + * @throws \GuzzleHttp\Exception\GuzzleException + */ + protected function callApi4AuthToken(array $auth, string $entity, string $action, array $params = []): \Psr\Http\Message\ResponseInterface { + $http = $this->createGuzzle(['http_errors' => FALSE]); + $method = str_starts_with($action, 'get') ? 'GET' : 'POST'; + + $response = $http->request($method, "civicrm/ajax/api4/$entity/$action", [ + 'headers' => ['X-Requested-With' => 'XMLHttpRequest'], + // This should probably be 'form_params', but 'query' is more representative of frontend. + ($method === 'GET' ? 'query' : 'form_params') => array_merge(['params' => json_encode($params)], $auth), + 'http_errors' => FALSE, + ]); + return $response; + } + } From 6634157cbe025dad318fc206dda884f36cb53951 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Mon, 4 Dec 2023 13:23:23 +0000 Subject: [PATCH 09/14] (dev/core#4462) Afform - Allow generating mail tokens with either session-level or page-level auth --- ext/afform/core/Civi/Afform/Tokens.php | 53 ++++++++++++++----- .../E2E/AfformMock/MockPublicFormTest.php | 6 +++ 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/ext/afform/core/Civi/Afform/Tokens.php b/ext/afform/core/Civi/Afform/Tokens.php index c7b901f94428..d63a76c4d2a0 100644 --- a/ext/afform/core/Civi/Afform/Tokens.php +++ b/ext/afform/core/Civi/Afform/Tokens.php @@ -206,20 +206,47 @@ public static function createUrl($afform, $contactId): string { /** @var \Civi\Crypto\CryptoJwt $jwt */ $jwt = \Civi::service('crypto.jwt'); - $bearerToken = "Bearer " . $jwt->encode([ - 'exp' => $expires, - 'sub' => "cid:" . $contactId, - 'scope' => 'authx', - ]); - - $url = \CRM_Utils_System::url($afform['server_route'], - ['_authx' => $bearerToken, '_authxSes' => 1], - TRUE, - NULL, - FALSE, - $afform['is_public'] ?? TRUE - ); + $url = \Civi::url() + ->setScheme($afform['is_public'] ? 'frontend' : 'backend') + ->setPath($afform['server_route']) + ->setPreferFormat('absolute'); + + switch (static::getTokenType($afform, $contactId)) { + case 'session': + $bearerToken = "Bearer " . $jwt->encode([ + 'exp' => $expires, + 'sub' => "cid:" . $contactId, + 'scope' => 'authx', + ]); + return $url->addQuery(['_authx' => $bearerToken, '_authxSes' => 1]); + + case 'page': + $bearerToken = "Bearer " . $jwt->encode([ + 'exp' => $expires, + 'sub' => "cid:" . $contactId, + 'scope' => 'afform', + 'afform' => $afform['name'], + ]); + return $url->addQuery(['_aff' => $bearerToken]); + + default: + throw new \CRM_Core_Exception("Unrecognized authentication token type"); + } + return $url; } + /** + * Determine what kind of authentication-token to use for the given form/contact. + * + * @param array $afform + * @param int $contactId + * @return string + * One of: 'session', 'page' + */ + public static function getTokenType(array $afform, int $contactId): string { + return \Civi::settings()->get('afform_mail_auth_token'); + // Or maybe... read the $afform and determine its specific settings... + } + } diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php index 91a1f7c622da..22f3c40f1812 100644 --- a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormTest.php @@ -2,6 +2,7 @@ namespace E2E\AfformMock; +use Civi; use CRM_Core_DAO; /** @@ -17,6 +18,11 @@ class MockPublicFormTest extends \Civi\AfformMock\FormTestCase { protected $formName = 'mockPublicForm'; + protected function setUp(): void { + parent::setUp(); + Civi::settings()->revert('afform_mail_auth_token'); + } + public function testGetPage() { $r = $this->createGuzzle()->get('civicrm/mock-public-form'); $this->assertContentType('text/html', $r); From 0de954665205f262305c02fe74d1461c3e9e721e Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Sun, 30 Jun 2024 22:34:52 -0700 Subject: [PATCH 10/14] Afform - Add a full mink/browser test for viewing authenticated email links --- .../AfformMock/MockPublicFormBrowserTest.php | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormBrowserTest.php diff --git a/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormBrowserTest.php b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormBrowserTest.php new file mode 100644 index 000000000000..2963f3d539a0 --- /dev/null +++ b/ext/afform/mock/tests/phpunit/E2E/AfformMock/MockPublicFormBrowserTest.php @@ -0,0 +1,111 @@ +<?php + +namespace E2E\AfformMock; + +use Civi; + +/** + * Perform some tests against `mockPublicForm.aff.html`. + * + * This test uses Chrome and checks more high-level behaviors. For lower-level checks, + * see MockPublicFormTest. + * + * @group e2e + */ +class MockPublicFormBrowserTest extends Civi\Test\MinkBase { + + protected $formName = 'mockPublicForm'; + + public static function setUpBeforeClass(): void { + \Civi\Test::e2e() + ->install(['org.civicrm.afform', 'org.civicrm.afform-mock']) + ->apply(); + } + + protected function setUp(): void { + parent::setUp(); + } + + protected function tearDown(): void { + parent::tearDown(); + Civi::settings()->revert('afform_mail_auth_token'); + } + + /** + * Create a contact with middle name "Donald". Use the custom form to change the middle + * name to "Donny". + * + * @return void + * @throws \Behat\Mink\Exception\ElementNotFoundException + * @throws \CRM_Core_Exception + */ + public function testUpdateMiddleName() { + Civi::settings()->set('afform_mail_auth_token', 'page'); + + $donny = $this->initializeTheodoreKerabatsos(); + $this->assertEquals('Donald', $this->getContact($donny)['middle_name'], 'Middle name has original value'); + + $session = $this->mink->getSession(); + $url = $this->renderToken('{afform.mockPublicFormUrl}', $donny); + $this->visit($url); + + // Goal: Wait for the fields to be populated. But how...? + // $session->wait(5000, 'document.querySelectorAll("input#middle-name-1").length > 0'); + // $session->wait(5000, '!!document.querySelectorAll("input#first-name-0").length && !!document.querySelectorAll("input#first-name-0")[0].value'); + // $session->wait(5000, '!!document.querySelectorAll("input#middle-name-1").length && document.querySelectorAll("input#middle-name-1")[0].value.length > 0'); + // $session->wait(5000, 'CRM.$(\'#middle-name-1:contains("Donald")\').length > 0'); + // $session->wait(5000, 'window.CRM.$(\'#middle-name-1:contains("Donald")\').length > 0'); + $session->wait(2000); /* FIXME: Cannot get wait-condition to be meaningfully used */ + + $middleName = $this->assertSession()->elementExists('css', 'input#middle-name-1'); + $this->assertEquals('Donald', $middleName->getValue()); + $middleName->setValue('Donny'); + + $submit = $this->assertSession()->elementExists('css', 'button.af-button.btn-primary'); + $submit->click(); + + // Goal: Wait for the "Saved" status message. But how...? + // $session->wait(5000, 'document.querySelectorAll(".crm-status-box-msg").length > 0'); + // $session->wait(5000, 'CRM.$(\'.crm-status-box-msg:contains("Saved")\').length > 1'); + $session->wait(2000); /* FIXME: Cannot get wait-condition to be meaningfully used */ + + $this->assertEquals('Donny', $this->getContact($donny)['middle_name'], 'Middle name has been updated'); + } + + protected function renderToken(string $token, int $cid): string { + $tp = new \Civi\Token\TokenProcessor(\Civi::dispatcher(), []); + $tp->addRow()->context('contactId', $cid); + $tp->addMessage('example', $token, 'text/plain'); + $tp->evaluate(); + return $tp->getRow(0)->render('example'); + } + + protected function initializeTheodoreKerabatsos(): int { + $record = [ + 'contact_type' => 'Individual', + 'first_name' => 'Theodore', + 'middle_name' => 'Donald', + 'last_name' => 'Kerabatsos', + 'external_identifier' => __CLASS__, + ]; + $contact = \Civi\Api4\Contact::save(FALSE) + ->setRecords([$record]) + ->setMatch(['external_identifier']) + ->execute(); + return $contact[0]['id']; + } + + /** + * @param int $contactId + * @return string + * @throws \CRM_Core_Exception + */ + protected function getContact(int $contactId): array { + return Civi\Api4\Contact::get(FALSE) + ->addWhere('id', '=', $contactId) + ->addSelect('id', 'first_name', 'middle_name', 'last_name') + ->execute() + ->single(); + } + +} From f27d3cc890611ebf6491db6c3128b7465028d87c Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Thu, 5 Sep 2024 13:58:27 -0700 Subject: [PATCH 11/14] PageTokenCredential - Admit more fields for APIv4 Autocomplete --- ext/afform/core/Civi/Afform/PageTokenCredential.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/afform/core/Civi/Afform/PageTokenCredential.php b/ext/afform/core/Civi/Afform/PageTokenCredential.php index 259dcdf5b3cb..de6641a7af1d 100644 --- a/ext/afform/core/Civi/Afform/PageTokenCredential.php +++ b/ext/afform/core/Civi/Afform/PageTokenCredential.php @@ -179,7 +179,7 @@ protected function getAllowedRoutes(): array { 'nameField' => 'name', ], ';^civicrm/ajax/api4/\w+/autocomplete$;' => [ - 'allowFields' => ['formName'], /* FIXME */ + 'allowFields' => ['fieldName', 'filters', 'formName', 'input', 'page', 'values'], 'nameField' => 'formName', ], ]; From ab79da07429bcc6ecc2aa481309f141a029d3777 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Thu, 5 Sep 2024 16:37:00 -0700 Subject: [PATCH 12/14] PageTokenCredential - Prohibit immortality --- ext/afform/core/Civi/Afform/PageTokenCredential.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ext/afform/core/Civi/Afform/PageTokenCredential.php b/ext/afform/core/Civi/Afform/PageTokenCredential.php index de6641a7af1d..9f16331a7b74 100644 --- a/ext/afform/core/Civi/Afform/PageTokenCredential.php +++ b/ext/afform/core/Civi/Afform/PageTokenCredential.php @@ -77,6 +77,9 @@ public function afformPageToken(CheckCredentialEvent $check) { // This is not an afform JWT. Proceed to check any other token sources. return; } + if (empty($claims['exp'])) { + $check->reject('Malformed JWT. Must specify an expiration time.'); + } if (empty($claims['sub']) || substr($claims['sub'], 0, 4) !== 'cid:') { $check->reject('Malformed JWT. Must specify the contact ID.'); } From 33c3a7d554c129bad193f2f38ae99268ce117c76 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Thu, 5 Sep 2024 19:50:09 -0700 Subject: [PATCH 13/14] PageTokenCredential - Convert nameField to checkRequest. Fix more edge-cases. --- .../core/Civi/Afform/PageTokenCredential.php | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/ext/afform/core/Civi/Afform/PageTokenCredential.php b/ext/afform/core/Civi/Afform/PageTokenCredential.php index 9f16331a7b74..520404589867 100644 --- a/ext/afform/core/Civi/Afform/PageTokenCredential.php +++ b/ext/afform/core/Civi/Afform/PageTokenCredential.php @@ -131,9 +131,12 @@ public function checkAllowedRoute(string $route, array $jwt): bool { return FALSE; } - $actualForm = $parsed[$routeInfo['nameField']] ?? NULL; - if ($actualForm !== $allowedForm) { - \Civi::log()->warning("Malformed request. Requested form ($actualForm) does not match allowed name ($allowedForm)."); + if (empty($routeInfo['checkRequest'])) { + throw new \LogicException("Route ($regex) doesn't define checkRequest."); + } + $checkRequest = $routeInfo['checkRequest']; + if (!$checkRequest($parsed, $jwt)) { + \Civi::log()->warning("Malformed request. Requested form does not match allowed name ($allowedForm)."); return FALSE; } @@ -157,6 +160,9 @@ public function checkAllowedRoute(string $route, array $jwt): bool { * @return array[] */ protected function getAllowedRoutes(): array { + // These params are common to many Afform actions. + $abstractProcessorParams = ['name', 'args', 'fillMode']; + return [ // ';civicrm/path/to/some/page;' => [ // @@ -164,27 +170,33 @@ protected function getAllowedRoutes(): array { // // N.B. Fields like "chain" are NOT allowed. // 'allowFields' => ['field_1', 'field_2', ...] // - // // The specific field which identifies the target form. Must match our page-token. - // 'nameField' => 'field_1', + // // Inspect the API-request and assert that the JWT allows these values. + // // Generally, check that the JWT's allowed-form-name matches REST's actual-form-name. + // 'checkRequest' => function(array $request, array $jwt): bool, // // ], ';^civicrm/ajax/api4/Afform/prefill$;' => [ - 'allowFields' => ['name', 'args'], - 'nameField' => 'name', + 'allowFields' => $abstractProcessorParams, + 'checkRequest' => fn($request, $jwt) => ($request['name'] === $jwt['afform']), ], ';^civicrm/ajax/api4/Afform/submit$;' => [ - 'allowFields' => ['name', 'args', 'values'], - 'nameField' => 'name', + 'allowFields' => [...$abstractProcessorParams, 'values'], + 'checkRequest' => fn($request, $jwt) => ($request['name'] === $jwt['afform']), ], ';^civicrm/ajax/api4/Afform/submitFile$;' => [ - 'allowFields' => ['name'], /* FIXME */ - 'nameField' => 'name', + 'allowFields' => $abstractProcessorParams, + 'checkRequest' => fn($request, $jwt) => ($request['name'] === $jwt['afform']), ], ';^civicrm/ajax/api4/\w+/autocomplete$;' => [ - 'allowFields' => ['fieldName', 'filters', 'formName', 'input', 'page', 'values'], - 'nameField' => 'formName', + 'allowFields' => ['fieldName', 'filters', 'formName', 'ids', 'input', 'page', 'values'], + 'checkRequest' => fn($request, $jwt) => ('afform:' . $jwt['afform']) === $request['formName'], ], + // It's been hypothesized that we'll also need this. Haven't seen it yet. + // ';^civicrm/ajax/api4/Afform/getFields;' => [ + // 'allowFields' => [], + // 'checkRequest' => fn($expected, $request) => TRUE, + // ], ]; } From a345655527ce337794408d882b59bcb8dc8c5232 Mon Sep 17 00:00:00 2001 From: Tim Otten <totten@civicrm.org> Date: Fri, 6 Sep 2024 02:42:02 -0700 Subject: [PATCH 14/14] (WIP) Allow _aff to be used to authenticate the main page-load --- .../core/Civi/Afform/PageTokenCredential.php | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/ext/afform/core/Civi/Afform/PageTokenCredential.php b/ext/afform/core/Civi/Afform/PageTokenCredential.php index 520404589867..653d25c7a23f 100644 --- a/ext/afform/core/Civi/Afform/PageTokenCredential.php +++ b/ext/afform/core/Civi/Afform/PageTokenCredential.php @@ -3,6 +3,7 @@ namespace Civi\Afform; use Civi\Authx\CheckCredentialEvent; +use Civi\Core\Event\GenericHookEvent; use Civi\Core\Service\AutoService; use Civi\Crypto\Exception\CryptoException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -22,7 +23,7 @@ class PageTokenCredential extends AutoService implements EventSubscriberInterfac public static function getSubscribedEvents(): array { $events = []; - $events['&civi.invoke.auth'][] = ['onInvoke', 105]; + $events['civi.invoke.auth'][] = ['onInvoke', 105]; $events['civi.authx.checkCredential'][] = ['afformPageToken', -400]; return $events; } @@ -31,19 +32,31 @@ public static function getSubscribedEvents(): array { * If you visit a top-level page like "civicrm/my-custom-form?_aff=XXX", then * all embedded AJAX calls should "_authx=XXX". * - * @param array $path + * @param \Civi\Core\Event\GenericHookEvent $e * @return void */ - public function onInvoke(array $path) { + public function onInvoke(GenericHookEvent $e) { $token = $_REQUEST['_aff'] ?? NULL; if (empty($token)) { return; } + if (!preg_match(';^[a-zA-Z0-9\.\-_ ]+$;', $token)) { throw new \CRM_Core_Exception("Malformed page token"); } + // FIXME: This would authenticate requests to the main page, but it also has the side-effect + // of making the user login. + + // \CRM_Core_Session::useFakeSession(); + // $params = ($_SERVER['REQUEST_METHOD'] === 'GET') ? $_GET : $_POST; + // $authenticated = \Civi::service('authx.authenticator')->auth($e, ['flow' => 'param', 'cred' => $params['_aff'], 'siteKey' => NULL]); + // _authx_redact(['_aff']); + // if (!$authenticated) { + // return; + // } + \CRM_Core_Region::instance('page-header')->add([ 'callback' => function() use ($token) { $ajaxSetup = [