From a7f67e630c2b56f66539cbf5e9f9c24e64f0f77c Mon Sep 17 00:00:00 2001
From: Pedro Vodicka <pedro@xgw.cz>
Date: Mon, 15 Jun 2020 20:35:01 +0200
Subject: [PATCH] Log failed twofactor authentication in TwoFactorAuth\Manager

---
 changelog/unreleased/37401                      |  6 ++++++
 .../Authentication/TwoFactorAuth/Manager.php    | 17 ++++++++++++++++-
 lib/private/Server.php                          |  2 +-
 .../TwoFactorAuth/ManagerTest.php               | 10 +++++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)
 create mode 100644 changelog/unreleased/37401

diff --git a/changelog/unreleased/37401 b/changelog/unreleased/37401
new file mode 100644
index 000000000000..103c8a9c851a
--- /dev/null
+++ b/changelog/unreleased/37401
@@ -0,0 +1,6 @@
+Bugfix: Log failed twofactor authentication
+
+When user entered bad twofactor authentication (i.e. code) there was no message
+in application log. This change will log this failed authentication.
+
+https://github.com/owncloud/core/pull/37401
\ No newline at end of file
diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php
index bc5ffa2fe843..ab46f0b92c94 100644
--- a/lib/private/Authentication/TwoFactorAuth/Manager.php
+++ b/lib/private/Authentication/TwoFactorAuth/Manager.php
@@ -29,6 +29,8 @@
 use OCP\AppFramework\QueryException;
 use OCP\Authentication\TwoFactorAuth\IProvider;
 use OCP\IConfig;
+use OCP\IRequest;
+use OCP\ILogger;
 use OCP\ISession;
 use OCP\IUser;
 
@@ -44,15 +46,25 @@ class Manager {
 	/** @var IConfig */
 	private $config;
 
+	/** @var IRequest */
+	private $request;
+
+	/** @var ILogger */
+	private $logger;
+
 	/**
 	 * @param AppManager $appManager
 	 * @param ISession $session
 	 * @param IConfig $config
+	 * @param IRequest $request
+	 * @param ILogger $logger
 	 */
-	public function __construct(AppManager $appManager, ISession $session, IConfig $config) {
+	public function __construct(AppManager $appManager, ISession $session, IConfig $config, IRequest $request, ILogger $logger) {
 		$this->appManager = $appManager;
 		$this->session = $session;
 		$this->config = $config;
+		$this->request = $request;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -157,6 +169,9 @@ public function verifyChallenge($providerId, IUser $user, $challenge) {
 		$result = $provider->verifyChallenge($user, $challenge);
 		if ($result) {
 			$this->session->remove(self::SESSION_UID_KEY);
+		} else {
+			$this->logger->warning("Two factor verify failed: '{$user->getUserName()}' (Remote IP: '{$this->request->getRemoteAddress()}')",
+				['app' => 'core']);
 		}
 		return $result;
 	}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 8ed71a4d8691..3c33cd6b0d3c 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -387,7 +387,7 @@ public function __construct($webRoot, \OC\Config $config) {
 		});
 
 		$this->registerService('\OC\Authentication\TwoFactorAuth\Manager', function (Server $c) {
-			return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig());
+			return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig(), $c->getRequest(), $c->getLogger());
 		});
 
 		$this->registerService('NavigationManager', function (Server $c) {
diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
index c3f9a4674991..927b95e22914 100644
--- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
@@ -42,6 +42,12 @@ class ManagerTest extends TestCase {
 	/** @var \OCP\IConfig|\PHPUnit\Framework\MockObject\MockObject */
 	private $config;
 
+	/** @var \OCP\IRequest | \PHPUnit\Framework\MockObject\MockObject */
+	private $request;
+
+	/** @var \OCP\ILogger | \PHPUnit\Framework\MockObject\MockObject */
+	private $logger;
+
 	/** @var \OCP\Authentication\TwoFactorAuth\IProvider|\PHPUnit\Framework\MockObject\MockObject */
 	private $fakeProvider;
 
@@ -54,9 +60,11 @@ protected function setUp(): void {
 			->getMock();
 		$this->session = $this->createMock('\OCP\ISession');
 		$this->config = $this->createMock('\OCP\IConfig');
+		$this->request = $this->createMock('\OCP\IRequest');
+		$this->logger = $this->createMock('\OCP\ILogger');
 
 		$this->manager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager')
-			->setConstructorArgs([$this->appManager, $this->session, $this->config])
+			->setConstructorArgs([$this->appManager, $this->session, $this->config, $this->request, $this->logger])
 			->setMethods(['loadTwoFactorApp']) // Do not actually load the apps
 			->getMock();