TwoFactorMiddleware::beforeController() - Code Metrics - Inspection of "Downstream 2016-06-09" - nextcloud/server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#31)

by Blizzz

created 2016-06-09 16:45 UTC

TwoFactorMiddleware::beforeController() C

↳ Parent: TwoFactorMiddleware

Complexity

Conditions	7
Paths	6

Size

Total Lines	24
Code Lines	11

Duplication

Lines	0
Ratio	0 %

Importance

Changes	3
Bugs	0	Features	0

Metric	Value
cc	7
eloc	11
c	3
b	0
f	0
nc	6
nop	2
dl	0
loc	24
rs	6.7272

<?php

/**
 * @author Christoph Wurst <[email protected]>
 *
 * @copyright Copyright (c) 2016, ownCloud, Inc.
 * @license AGPL-3.0
 *
 * This code is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License, version 3,
 * as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License, version 3,
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
 *
 */

namespace OC\Core\Middleware;

use Exception;
use OC\Authentication\Exceptions\TwoFactorAuthRequiredException;
use OC\Authentication\Exceptions\UserAlreadyLoggedInException;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Controller\TwoFactorChallengeController;
use OC\User\Session;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Middleware;
use OCP\AppFramework\Utility\IControllerMethodReflector;
use OCP\IRequest;
use OCP\ISession;
use OCP\IURLGenerator;

class TwoFactorMiddleware extends Middleware {

	/** @var Manager */
	private $twoFactorManager;

	/** @var Session */
	private $userSession;

	/** @var ISession */
	private $session;

	/** @var IURLGenerator */
	private $urlGenerator;

	/** @var IControllerMethodReflector */
	private $reflector;

	/** @var IRequest */
	private $request;

	/**
	 * @param Manager $twoFactorManager
	 * @param Session $userSession
	 * @param ISession $session
	 * @param IURLGenerator $urlGenerator
	 */
	public function __construct(Manager $twoFactorManager, Session $userSession, ISession $session,

		IURLGenerator $urlGenerator, IControllerMethodReflector $reflector, IRequest $request) {
		$this->twoFactorManager = $twoFactorManager;
		$this->userSession = $userSession;
		$this->session = $session;
		$this->urlGenerator = $urlGenerator;
		$this->reflector = $reflector;
		$this->request = $request;
	}

	/**
	 * @param Controller $controller
	 * @param string $methodName
	 */
	public function beforeController($controller, $methodName) {
		if ($this->reflector->hasAnnotation('PublicPage')) {
			// Don't block public pages
			return;
		}

		if ($controller instanceof \OC\Core\Controller\LoginController && $methodName === 'logout') {
			// Don't block the logout page, to allow canceling the 2FA
			return;
		}

		if ($this->userSession->isLoggedIn()) {
			$user = $this->userSession->getUser();

			if ($this->twoFactorManager->isTwoFactorAuthenticated($user)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
				$this->checkTwoFactor($controller, $methodName);
			} else if ($controller instanceof TwoFactorChallengeController) {
				// Allow access to the two-factor controllers only if two-factor authentication
				// is in progress.
				throw new UserAlreadyLoggedInException();
			}
		}
		// TODO: dont check/enforce 2FA if a auth token is used
	}

	private function checkTwoFactor($controller, $methodName) {

		// If two-factor auth is in progress disallow access to any controllers
		// defined within "LoginController".
		$needsSecondFactor = $this->twoFactorManager->needsSecondFactor();
		$twoFactor = $controller instanceof TwoFactorChallengeController;

		// Disallow access to any controller if 2FA needs to be checked
		if ($needsSecondFactor && !$twoFactor) {
			throw new TwoFactorAuthRequiredException();
		}

		// Allow access to the two-factor controllers only if two-factor authentication
		// is in progress.
		if (!$needsSecondFactor && $twoFactor) {
			throw new UserAlreadyLoggedInException();
		}
	}

	public function afterException($controller, $methodName, Exception $exception) {
		if ($exception instanceof TwoFactorAuthRequiredException) {
			return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', [
					'redirect_url' => urlencode($this->request->server['REQUEST_URI']),
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
			]));
		}
		if ($exception instanceof UserAlreadyLoggedInException) {
			return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index'));
		}
	}

}


1		<?php
2
3		/**
4		* @author Christoph Wurst <[email protected]>
5		*
6		* @copyright Copyright (c) 2016, ownCloud, Inc.
7		* @license AGPL-3.0
8		*
9		* This code is free software: you can redistribute it and/or modify
10		* it under the terms of the GNU Affero General Public License, version 3,
11		* as published by the Free Software Foundation.
12		*
13		* This program is distributed in the hope that it will be useful,
14		* but WITHOUT ANY WARRANTY; without even the implied warranty of
15		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16		* GNU Affero General Public License for more details.
17		*
18		* You should have received a copy of the GNU Affero General Public License, version 3,
19		* along with this program. If not, see <http://www.gnu.org/licenses/>
20		*
21		*/
22
23		namespace OC\Core\Middleware;
24
25		use Exception;
26		use OC\Authentication\Exceptions\TwoFactorAuthRequiredException;
27		use OC\Authentication\Exceptions\UserAlreadyLoggedInException;
28		use OC\Authentication\TwoFactorAuth\Manager;
29		use OC\Core\Controller\TwoFactorChallengeController;
30		use OC\User\Session;
31		use OCP\AppFramework\Controller;
32		use OCP\AppFramework\Http\RedirectResponse;
33		use OCP\AppFramework\Middleware;
34		use OCP\AppFramework\Utility\IControllerMethodReflector;
35		use OCP\IRequest;
36		use OCP\ISession;
37		use OCP\IURLGenerator;
38
39		class TwoFactorMiddleware extends Middleware {
40
41		/** @var Manager */
42		private $twoFactorManager;
43
44		/** @var Session */
45		private $userSession;
46
47		/** @var ISession */
48		private $session;
49
50		/** @var IURLGenerator */
51		private $urlGenerator;
52
53		/** @var IControllerMethodReflector */
54		private $reflector;
55
56		/** @var IRequest */
57		private $request;
58
59		/**
60		* @param Manager $twoFactorManager
61		* @param Session $userSession
62		* @param ISession $session
63		* @param IURLGenerator $urlGenerator
64		*/
65	View Code Duplication	public function __construct(Manager $twoFactorManager, Session $userSession, ISession $session,
		0 ignored issues – show Duplication introduced 2016-06-09 16:58 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
66		IURLGenerator $urlGenerator, IControllerMethodReflector $reflector, IRequest $request) {
67		$this->twoFactorManager = $twoFactorManager;
68		$this->userSession = $userSession;
69		$this->session = $session;
70		$this->urlGenerator = $urlGenerator;
71		$this->reflector = $reflector;
72		$this->request = $request;
73		}
74
75		/**
76		* @param Controller $controller
77		* @param string $methodName
78		*/
79		public function beforeController($controller, $methodName) {
80		if ($this->reflector->hasAnnotation('PublicPage')) {
81		// Don't block public pages
82		return;
83		}
84
85		if ($controller instanceof \OC\Core\Controller\LoginController && $methodName === 'logout') {
86		// Don't block the logout page, to allow canceling the 2FA
87		return;
88		}
89
90		if ($this->userSession->isLoggedIn()) {
91		$user = $this->userSession->getUser();
92
93		if ($this->twoFactorManager->isTwoFactorAuthenticated($user)) {
		0 ignored issues – show Bug introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->userSession->getUser()` on line `91` can be `null`; however, `OC\Authentication\TwoFac...woFactorAuthenticated()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
94		$this->checkTwoFactor($controller, $methodName);
95		} else if ($controller instanceof TwoFactorChallengeController) {
96		// Allow access to the two-factor controllers only if two-factor authentication
97		// is in progress.
98		throw new UserAlreadyLoggedInException();
99		}
100		}
101		// TODO: dont check/enforce 2FA if a auth token is used
102		}
103
104		private function checkTwoFactor($controller, $methodName) {
		0 ignored issues – show Unused Code introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report The parameter `$methodName` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
105		// If two-factor auth is in progress disallow access to any controllers
106		// defined within "LoginController".
107		$needsSecondFactor = $this->twoFactorManager->needsSecondFactor();
108		$twoFactor = $controller instanceof TwoFactorChallengeController;
109
110		// Disallow access to any controller if 2FA needs to be checked
111		if ($needsSecondFactor && !$twoFactor) {
112		throw new TwoFactorAuthRequiredException();
113		}
114
115		// Allow access to the two-factor controllers only if two-factor authentication
116		// is in progress.
117		if (!$needsSecondFactor && $twoFactor) {
118		throw new UserAlreadyLoggedInException();
119		}
120		}
121
122		public function afterException($controller, $methodName, Exception $exception) {
123		if ($exception instanceof TwoFactorAuthRequiredException) {
124		return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', [
125		'redirect_url' => urlencode($this->request->server['REQUEST_URI']),
		0 ignored issues – show Bug introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report Accessing `server` on the interface `OCP\IRequest` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
126		]));
127		}
128		if ($exception instanceof UserAlreadyLoggedInException) {
129		return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index'));
130		}
131		}
132
133		}
134

nextcloud / server

Pull Request — master (#31)

TwoFactorMiddleware::beforeController() C

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like