Skip checking MFA status for WebDAV and CardDAV requests.

This commit is contained in:
Harald Eilertsen
2024-06-05 07:59:42 +00:00
committed by Mario
parent 9d56bb952e
commit 350f84913a
4 changed files with 119 additions and 2 deletions

View File

@@ -176,6 +176,40 @@ function log_failed_login($errormsg) {
@file_put_contents($authlog, datetime_convert() . ':' . session_id() . ' ' . $errormsg . PHP_EOL, FILE_APPEND);
}
/**
* Determines if checking for multifactor authentication needs to be checked.
*
* Checks that multi factor authentication is enabled for the given account_id,
* and whether it's already authenticated or not.
*
* Some modules needs to be excluded from the mfa checks for various reasons:
*
* - `totp_check` is used by the mfa module itself.
* - `dav` provides WebDAV access, and has no way of providing a mfa code.
* - `cdav` is accessed both via CardDAV which has the same limitations as
* the `dav` module, but may also be accessed via a web browser over http.
* We only exclude it if it's not being accessed via a web browser.
*
* @param int $account_id The id of the account we're verifying.
* @param string $module The requested module.
* @param string $arg The first arg passed to the module (or empty if none.)
*
* @return bool `true` if mfa status needs to be checked, `false` otherwise.
*/
function requires_mfa_check(int $account_id, string $module, string $arg): bool {
if (in_array($module, ['totp_check', 'dav'], true)) {
return false;
}
if ($module === 'cdav' && !in_array($arg, ['addressbook', 'calendar'], true)) {
return false;
}
$multiFactor = AConfig::Get($account_id, 'system', 'mfa_enabled');
return $multiFactor && empty($_SESSION['2FA_VERIFIED']);
}
/**
* Inline - not a function
* look for auth parameters or re-validate an existing session
@@ -267,8 +301,7 @@ if((isset($_SESSION)) && (x($_SESSION, 'authenticated')) &&
$login_refresh = true;
}
$multiFactor = AConfig::Get(App::$account['account_id'], 'system', 'mfa_enabled');
if ($multiFactor && empty($_SESSION['2FA_VERIFIED']) && App::$module !== 'totp_check') {
if (requires_mfa_check(App::$account['account_id'], App::$module, argv(1))) {
$o = new Totp_check;
echo $o->get();
killme();

View File

@@ -31,6 +31,7 @@ use PHPUnit\Framework\TestCase;
*/
require_once __DIR__ . '/../../boot.php';
require_once 'include/dba/dba_driver.php' ;
require_once 'include/dba/dba_transaction.php';
/**
* Base class for our Unit Tests.

View File

@@ -0,0 +1,81 @@
<?php
/**
* Tests for the authentication code used in Hubzilla.
*
* SPDX-FileCopyrightText: 2024 Hubzilla Community
* SPDX-FileContributor: Harald Eilertsen
*
* SPDX-License-Identifier: MIT
*/
namespace Zotlabs\Tests\Unit\Includes;
use App;
use Zotlabs\Lib\AConfig;
use Zotlabs\Tests\Unit\UnitTestCase;
use PHPUnit\Framework\Attributes\{DataProvider, RunTestsInSeparateProcesses};
/**
* Test class containing the test for the Hubzilla authentication code.
*
* Since the main authentication code is executed in the global scope on
* inclusion of the `includes/auth.php` file, we need to run each test in a
* separate process to make sure we can excersize the code as we need.
*/
#[RunTestsInSeparateProcesses]
class AuthTest extends UnitTestCase {
/**
* Check that mfa status is not checked for certain modules.
*
* This causes issues with things like WebDAV and CardDAV, as there's
* currently no way for these modules to signal that a TOTP code is needed
* back to the connecting client.
*/
#[DataProvider('modules_excluded_from_mfa')]
public function test_mfa_is_not_checked_for_excluded_modules(string $module, array $args): void {
$account_id = $this->fixtures['account']['0']['account_id'];
$_SESSION = [
'authenticated' => true,
'account_id' => $account_id,
// Trick the code to not warn that $_SESSION['uid'] is not set,
// but also not trigger the code that tries to change to the
// given channel. *Remove when code is fixed!*
'uid' => 0,
];
$_SERVER['REMOTE_ADDR'] = '127.0.0.1';
App::$session = $this->create_session_stub();
App::$module = $module;
App::$argv = $args;
App::$argc = count($args);
// Enable multi factor authentication for this account
AConfig::Set($account_id, 'system', 'mfa_enabled', true);
require 'include/auth.php';
$this->assertEquals(1, $_SESSION['authenticated']);
}
/**
* Data provider for testing modules excluded from mfa
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
*/
public static function modules_excluded_from_mfa(): array {
return [
['totp_check', []],
['cdav', []],
['cdav', ['calendar']],
['cdav', ['addressbook']],
['dav', []],
];
}
private function create_session_stub(): \Zotlabs\Web\Session {
return $this->createStub('Zotlabs\Web\Session');
}
}

View File

@@ -3,7 +3,9 @@ account:
account_id: 42
account_email: "hubzilla@example.com"
account_language: "no"
account_flags: 0
-
account_id: 43
account_email: "hubzilla@example.org"
account_language: "de"
account_flags: 1