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

Completed

Pull Request — master (#32)

by Blizzz

created 2016-06-09 16:06 UTC

TwoFactorMiddleware::beforeController() B

↳ Parent: TwoFactorMiddleware

Complexity

Conditions	5
Paths	5

Size

Total Lines	19
Code Lines	9

Duplication

Lines	0
Ratio	0 %

Importance

Changes	2
Bugs	0	Features	0

Metric	Value
cc	5
eloc	9
c	2
b	0
f	0
nc	5
nop	2
dl	0
loc	19
rs	8.8571

<?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 ($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			public function __construct(Manager $twoFactorManager, Session $userSession, ISession $session,
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 ($this->userSession->isLoggedIn()) {
86			$user = $this->userSession->getUser();
87
88			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 `86` 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...
89			$this->checkTwoFactor($controller, $methodName);
90			} else if ($controller instanceof TwoFactorChallengeController) {
91			// Allow access to the two-factor controllers only if two-factor authentication
92			// is in progress.
93			throw new UserAlreadyLoggedInException();
94			}
95			}
96			// TODO: dont check/enforce 2FA if a auth token is used
97			}
98
99			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...
100			// If two-factor auth is in progress disallow access to any controllers
101			// defined within "LoginController".
102			$needsSecondFactor = $this->twoFactorManager->needsSecondFactor();
103			$twoFactor = $controller instanceof TwoFactorChallengeController;
104
105			// Disallow access to any controller if 2FA needs to be checked
106			if ($needsSecondFactor && !$twoFactor) {
107			throw new TwoFactorAuthRequiredException();
108			}
109
110			// Allow access to the two-factor controllers only if two-factor authentication
111			// is in progress.
112			if (!$needsSecondFactor && $twoFactor) {
113			throw new UserAlreadyLoggedInException();
114			}
115			}
116
117			public function afterException($controller, $methodName, Exception $exception) {
118			if ($exception instanceof TwoFactorAuthRequiredException) {
119			return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', [
120			'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...
121			]));
122			}
123			if ($exception instanceof UserAlreadyLoggedInException) {
124			return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index'));
125			}
126			}
127
128			}
129

nextcloud / server

Pull Request — master (#32)

TwoFactorMiddleware::beforeController() B

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like