From 6af2efb67931863b27d96c74cdff1d2ca2615e52 Mon Sep 17 00:00:00 2001
From: Christoph Wurst <christoph@winzerhof-wurst.at>
Date: Wed, 24 Aug 2016 10:42:07 +0200
Subject: prevent infinite redirect loops if the there is no 2fa provider to
 pass

This fixes infinite loops that are caused whenever a user is about to solve a 2FA
challenge, but the provider app is disabled at the same time. Since the session
value usually indicates that the challenge needs to be solved before we grant access
we have to remove that value instead in this special case.
---
 lib/private/Authentication/TwoFactorAuth/Manager.php | 18 ++++++++++++++++--
 lib/private/legacy/api.php                           |  2 +-
 lib/private/legacy/json.php                          |  2 +-
 lib/private/legacy/util.php                          |  2 +-
 4 files changed, 19 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php
index 66bcafbce71..143fe7dc927 100644
--- a/lib/private/Authentication/TwoFactorAuth/Manager.php
+++ b/lib/private/Authentication/TwoFactorAuth/Manager.php
@@ -165,10 +165,24 @@ class Manager {
 	/**
 	 * Check if the currently logged in user needs to pass 2FA
 	 *
+	 * @param IUser $user the currently logged in user
 	 * @return boolean
 	 */
-	public function needsSecondFactor() {
-		return $this->session->exists(self::SESSION_UID_KEY);
+	public function needsSecondFactor(IUser $user = null) {
+		if (is_null($user) || !$this->session->exists(self::SESSION_UID_KEY)) {
+			return false;
+		}
+
+		if (!$this->isTwoFactorAuthenticated($user)) {
+			// There is no second factor any more -> let the user pass
+			//   This prevents infinite redirect loops when a user is about
+			//   to solve the 2FA challenge, and the provider app is
+			//   disabled the same time
+			$this->session->remove(self::SESSION_UID_KEY);
+			return false;
+		}
+
+		return true;
 	}
 
 	/**
diff --git a/lib/private/legacy/api.php b/lib/private/legacy/api.php
index 30083294861..17ee9c5d468 100644
--- a/lib/private/legacy/api.php
+++ b/lib/private/legacy/api.php
@@ -311,7 +311,7 @@ class OC_API {
 		// reuse existing login
 		$loggedIn = \OC::$server->getUserSession()->isLoggedIn();
 		if ($loggedIn === true) {
-			if (\OC::$server->getTwoFactorAuthManager()->needsSecondFactor()) {
+			if (\OC::$server->getTwoFactorAuthManager()->needsSecondFactor(\OC::$server->getUserSession()->getUser())) {
 				// Do not allow access to OCS until the 2FA challenge was solved successfully
 				return false;
 			}
diff --git a/lib/private/legacy/json.php b/lib/private/legacy/json.php
index 2882ac94ea9..f386d03ab1b 100644
--- a/lib/private/legacy/json.php
+++ b/lib/private/legacy/json.php
@@ -68,7 +68,7 @@ class OC_JSON{
 	public static function checkLoggedIn() {
 		$twoFactorAuthManger = \OC::$server->getTwoFactorAuthManager();
 		if( !OC_User::isLoggedIn()
-			|| $twoFactorAuthManger->needsSecondFactor()) {
+			|| $twoFactorAuthManger->needsSecondFactor(\OC::$server->getUserSession()->getUser())) {
 			$l = \OC::$server->getL10N('lib');
 			http_response_code(\OCP\AppFramework\Http::STATUS_UNAUTHORIZED);
 			self::error(array( 'data' => array( 'message' => $l->t('Authentication error'), 'error' => 'authentication_error' )));
diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php
index a975da39271..7341331518d 100644
--- a/lib/private/legacy/util.php
+++ b/lib/private/legacy/util.php
@@ -975,7 +975,7 @@ class OC_Util {
 			exit();
 		}
 		// Redirect to index page if 2FA challenge was not solved yet
-		if (\OC::$server->getTwoFactorAuthManager()->needsSecondFactor()) {
+		if (\OC::$server->getTwoFactorAuthManager()->needsSecondFactor(\OC::$server->getUserSession()->getUser())) {
 			header('Location: ' . \OCP\Util::linkToAbsolute('', 'index.php'));
 			exit();
 		}
-- 
cgit v1.2.3