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

Authx - Retain authentication outcome/metadata #20026

Merged
merged 1 commit into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ext/authx/CRM/Authx/Page/AJAX.php
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
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
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