From: Tim Otten <totten@civicrm.org>
Date: Fri, 9 Apr 2021 23:29:10 +0000 (-0700)
Subject: Authx - Retain authentication outcome/metadata as a session variable. Update tests.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=ef36a3dba26d3db7d4fc072f806a17feefead13e;p=civicrm-core.git

Authx - Retain authentication outcome/metadata as a session variable. Update tests.
---

diff --git a/ext/authx/CRM/Authx/Page/AJAX.php b/ext/authx/CRM/Authx/Page/AJAX.php
index ecdba043b2..2f8220922d 100644
--- a/ext/authx/CRM/Authx/Page/AJAX.php
+++ b/ext/authx/CRM/Authx/Page/AJAX.php
@@ -12,9 +12,13 @@ class CRM_Authx_Page_AJAX {
   public static function getId() {
     $authxUf = _authx_uf();
 
+    /** @var array $authx */
+    $authx = CRM_Core_Session::singleton()->get('authx');
     $response = [
       'contact_id' => CRM_Core_Session::getLoggedInContactID(),
       'user_id' => $authxUf->getCurrentUserId(),
+      'flow' => $authx['flow'] ?? NULL,
+      'cred' => $authx['credType'] ?? NULL,
     ];
 
     CRM_Utils_JSON::output($response);
diff --git a/ext/authx/Civi/Authx/Authenticator.php b/ext/authx/Civi/Authx/Authenticator.php
index e7559dbbb1..49f103e8a1 100644
--- a/ext/authx/Civi/Authx/Authenticator.php
+++ b/ext/authx/Civi/Authx/Authenticator.php
@@ -213,6 +213,7 @@ class Authenticator {
     // Post-login Civi stuff...
 
     $session = \CRM_Core_Session::singleton();
+    $session->set('authx', $tgt->createRedacted());
     $session->set('ufID', $tgt->userId);
     $session->set('userID', $tgt->contactId);
 
@@ -348,4 +349,26 @@ class AuthenticatorTarget {
     return !empty($this->userId) || !empty($this->contactId);
   }
 
+  /**
+   * Create a variant of the authentication record which omits any secret values. It may be
+   * useful to examining metadata and outcomes.
+   *
+   * The redacted version may be retained in the (real or fake) session and consulted by more
+   * fine-grained access-controls.
+   *
+   * @return array
+   */
+  public function createRedacted(): array {
+    return [
+      // omit: cred
+      // omit: siteKey
+      'flow' => $this->flow,
+      'credType' => $this->credType,
+      'jwt' => $this->jwt,
+      'useSession' => $this->useSession,
+      'userId' => $this->userId,
+      'contactId' => $this->contactId,
+    ];
+  }
+
 }
diff --git a/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php b/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
index 0db6f7d44b..d2e2ea5f61 100644
--- a/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
+++ b/ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
@@ -131,7 +131,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     // Phase 2: Request succeeds if this credential type is enabled
     \Civi::settings()->set("authx_{$flowType}_cred", [$credType]);
     $response = $http->send($request);
-    $this->assertMyContact($this->getLebowskiCID(), NULL, $response);
+    $this->assertMyContact($this->getLebowskiCID(), NULL, $credType, $flowType, $response);
     if (!in_array('sendsExcessCookies', $this->quirks)) {
       $this->assertNoCookies($response);
     }
@@ -162,7 +162,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     // Phase 2: Request succeeds if this credential type is enabled
     \Civi::settings()->set("authx_{$flowType}_cred", [$credType]);
     $response = $http->send($request);
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $response);
     if (!in_array('sendsExcessCookies', $this->quirks)) {
       $this->assertNoCookies($response);
     }
@@ -196,12 +196,12 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     // 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);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $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);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $response);
 
     // Request fails. Policy requires site_key, but we don't have the wrong value.
     \Civi::settings()->set("authx_guards", ['site_key']);
@@ -240,12 +240,12 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     $response = $http->post('civicrm/authx/login', [
       'form_params' => ['_authx' => $this->$credFunc($this->getDemoCID())],
     ]);
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $response);
     $this->assertHasCookies($response);
 
     // Phase 3: We can use cookies to request other pages
     $response = $http->get('civicrm/authx/id');
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $response);
     $response = $http->get('civicrm/user');
     $this->assertDashboardOk();
 
@@ -304,7 +304,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     $this->assertEquals(0, $cookieJar->count());
     $response = $http->send($request);
     $this->assertTrue($cookieJar->count() >= 1);
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $credType, $flowType, $response);
 
     // FIXME: Assert that re-using cookies yields correct result.
   }
@@ -350,10 +350,10 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
     $response = $http->post('civicrm/authx/login', [
       'form_params' => ['_authx' => $this->credApikey($this->getDemoCID())],
     ]);
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), 'api_key', 'login', $response);
     $this->assertHasCookies($response);
     $response = $http->get('civicrm/authx/id');
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), 'api_key', 'login', $response);
 
     // Phase 2: Make a single, stateless request with different creds
     /** @var \Psr\Http\Message\RequestInterface $request */
@@ -367,7 +367,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
 
     // Phase 3: Original session is still valid
     $response = $http->get('civicrm/authx/id');
-    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response);
+    $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), 'api_key', 'login', $response);
   }
 
   /**
@@ -393,7 +393,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
         case 'L':
           $request = $this->applyAuth($this->requestMyContact(), 'api_key', 'header', $this->getLebowskiCID());
           $response = $http->send($request);
-          $this->assertMyContact($this->getLebowskiCID(), NULL, $response, 'Expected Lebowski in step #' . $i);
+          $this->assertMyContact($this->getLebowskiCID(), NULL, 'api_key', 'header', $response, 'Expected Lebowski in step #' . $i);
           $actualSteps .= 'L';
           break;
 
@@ -407,7 +407,7 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
         case 'D':
           $request = $this->applyAuth($this->requestMyContact(), 'api_key', 'header', $this->getDemoCID());
           $response = $http->send($request);
-          $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), $response, 'Expected demo in step #' . $i);
+          $this->assertMyContact($this->getDemoCID(), $this->getDemoUID(), 'api_key', 'header', $response, 'Expected demo in step #' . $i);
           $actualSteps .= 'D';
           break;
 
@@ -463,15 +463,23 @@ class AllFlowsTest extends \PHPUnit\Framework\TestCase implements EndToEndInterf
    *   The expected contact ID
    * @param int|null $uid
    *   The expected user ID
+   * @param string $credType
+   * @param string $flow
    * @param \Psr\Http\Message\ResponseInterface $response
    */
-  public function assertMyContact($cid, $uid, ResponseInterface $response): void {
+  public function assertMyContact($cid, $uid, $credType, $flow, ResponseInterface $response): void {
     $this->assertContentType('application/json', $response);
     $this->assertStatusCode(200, $response);
     $j = json_decode((string) $response->getBody(), 1);
     $formattedFailure = $this->formatFailure($response);
     $this->assertEquals($cid, $j['contact_id'], "Response did not give expected contact ID\n" . $formattedFailure);
     $this->assertEquals($uid, $j['user_id'], "Response did not give expected user ID\n" . $formattedFailure);
+    if ($flow !== NULL) {
+      $this->assertEquals($flow, $j['flow'], "Response did not give expected flow type\n" . $formattedFailure);
+    }
+    if ($credType !== NULL) {
+      $this->assertEquals($credType, $j['cred'], "Response did not give expected cred type\n" . $formattedFailure);
+    }
   }
 
   /**