Skip to content

Commit

Permalink
Authx - Retain authentication outcome/metadata as a session variable.…
Browse files Browse the repository at this point in the history 
… Update tests.
  • Loading branch information
@totten
totten committed Apr 9, 2021
1 parent 59553ea commit ef36a3d
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 13 deletions.
4 changes: 4 additions & 0 deletions ext/authx/CRM/Authx/Page/AJAX.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
23 changes: 23 additions & 0 deletions ext/authx/Civi/Authx/Authenticator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ protected function login(AuthenticatorTarget $tgt) {
// Post-login Civi stuff...

$session = \CRM_Core_Session::singleton();
$session->set('authx', $tgt->createRedacted());
$session->set('ufID', $tgt->userId);
$session->set('userID', $tgt->contactId);

Expand Down Expand Up @@ -348,4 +349,26 @@ public function hasPrincipal(): bool {
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,
];
}

}
34 changes: 21 additions & 13 deletions ext/authx/tests/phpunit/Civi/Authx/AllFlowsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public function testStatelessContactOnly($credType, $flowType): void {
// 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);
}
Expand Down Expand Up @@ -162,7 +162,7 @@ public function testStatelessUserContact($credType, $flowType): void {
// 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);
}
Expand Down Expand Up @@ -196,12 +196,12 @@ public function testStatelessGuardSiteKey() {
// 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']);
Expand Down Expand Up @@ -240,12 +240,12 @@ public function testStatefulLoginAllowed($credType): void {
$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();

Expand Down Expand Up @@ -304,7 +304,7 @@ public function testStatefulAutoAllowed($credType): void {
$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.
}
Expand Down Expand Up @@ -350,10 +350,10 @@ public function testStatefulStatelessOverlap(): void {
$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 */
Expand All @@ -367,7 +367,7 @@ public function testStatefulStatelessOverlap(): void {

// 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);
}

/**
Expand All @@ -393,7 +393,7 @@ public function testMultipleStateless(): void {
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;

Expand All @@ -407,7 +407,7 @@ public function testMultipleStateless(): void {
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;

Expand Down Expand Up @@ -463,15 +463,23 @@ public function requestMyContact() {
* 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);
}
}

/**
Expand Down

0 comments on commit ef36a3d

Please sign in to comment.