Skip checking MFA status for WebDAV and CardDAV requests.

4 files changed, 119 insertions, 2 deletions

diff --git a/include/auth.php b/include/auth.php
index 0cd48bce3..1fc2cc556 100644
--- a/include/auth.php
+++ b/include/auth.php
@@ -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();
diff --git a/tests/unit/UnitTestCase.php b/tests/unit/UnitTestCase.php
index 844746a51..afc309205 100644
--- a/tests/unit/UnitTestCase.php
+++ b/tests/unit/UnitTestCase.php
@@ -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.
diff --git a/tests/unit/includes/AuthTest.php b/tests/unit/includes/AuthTest.php
new file mode 100644
index 000000000..fa9726fe8
--- /dev/null
+++ b/tests/unit/includes/AuthTest.php
@@ -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');
+	}
+}
diff --git a/tests/unit/includes/dba/_files/account.yml b/tests/unit/includes/dba/_files/account.yml
index 344bdb799..88e59056e 100644
--- a/tests/unit/includes/dba/_files/account.yml
+++ b/tests/unit/includes/dba/_files/account.yml
@@ -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