[stable10] Check the return value of getUserSession before using it

Checking if the value returned by getUserSession and getUser is null or not before using it. Signed-off-by: Sujith H <sharidasan@owncloud.com>
owncloud · Feb 9, 2018 · 7d508d4 · 7d508d4
1 parent aefb9f7
commit 7d508d4
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 10 deletions.
diff --git a/lib/private/Log.php b/lib/private/Log.php
@@ -35,6 +35,7 @@
 use InterfaSys\LogNormalizer\Normalizer;
 
 use \OCP\ILogger;
+use OCP\IUserSession;
 use OCP\Util;
 
 /**
@@ -292,12 +293,15 @@ public function log($level, $message, array $context = []) {
 
 					// check for user
 					if (!empty($logCondition['users'])) {
-						$user = \OC::$server->getUserSession()->getUser();
-
-						// if the user matches set the log condition to satisfied
-						if ($user !== null && in_array($user->getUID(), $logCondition['users'], true)) {
-							$this->logConditionSatisfied = true;
-							break;
+						$userSession = \OC::$server->getUserSession();
+						if ($userSession instanceof IUserSession) {
+							$user = $userSession->getUser();
+
+							// if the user matches set the log condition to satisfied
+							if ($user !== null && in_array($user->getUID(), $logCondition['users'], true)) {
+								$this->logConditionSatisfied = true;
+								break;
+							}
 						}
 					}
 				}

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
@@ -41,6 +41,7 @@
 use OCP\IUser;
 use OCP\IConfig;
 use OCP\IUserBackend;
+use OCP\IUserSession;
 use OCP\User\IChangePasswordBackend;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\GenericEvent;
@@ -331,10 +332,16 @@ public function canChangePassword() {
 	 * @return bool
 	 */
 	public function canChangeDisplayName() {
-		if (($this->config->getSystemValue('allow_user_to_change_display_name') === false) &&
-			(!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) &&
-			(!$this->groupManager->getSubAdmin()->isSubAdmin($this->userSession->getUser()))) {
-			return false;
+		if ($this->userSession instanceof IUserSession) {
+			$user = $this->userSession->getUser();
+			if (
+				($this->config->getSystemValue('allow_user_to_change_display_name') === false) &&
+				(($user !== null) && (!$this->groupManager->isAdmin($user->getUID()))) &&
+				(($user !== null) && (!$this->groupManager->getSubAdmin()->isSubAdmin($user)))
+			) {
+				return false;
+			}
+
 		}
 		$backend = $this->account->getBackendInstance();
 		if (is_null($backend)) {

diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php
@@ -10,6 +10,7 @@
 
 use OC\Log;
 use OCP\IConfig;
+use OCP\IUserSession;
 use OCP\Util;
 
 class LoggerTest extends TestCase {
@@ -77,6 +78,27 @@ public function testAppLogCondition() {
 		$this->assertEquals($expected, $this->getLogs());
 	}
 
+	public function testNullUserSession() {
+		$userSession = $this->createMock(IUserSession::class);
+		$userSession->expects($this->any())
+			->method('getUser')
+			->willReturn(null);
+		$this->config->expects($this->any())
+			->method('getValue')
+			->will(($this->returnValueMap([
+				['loglevel', Util::WARN, Util::WARN],
+				['log.conditions', [], [['users' => ['foo'], 'apps' => ['files'], 'logfile' => '/tmp/test.log']]]
+			])));
+		$logger = $this->logger;
+
+		$logger->warning('Don\'t display info messages');
+
+		$expected = [
+			'2 Don\'t display info messages',
+			];
+		$this->assertEquals($expected, $this->getLogs());
+	}
+
 	private function getLogs() {
 		return self::$logs;
 	}

diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
@@ -313,6 +313,45 @@ public function testCanChangeDisplayName($expected, $implements) {
 		$this->assertEquals($expected, $user->setDisplayName('Foo'));
 	}
 
+	public function provideNullorFalseData() {
+		return [
+			[null, null, false],
+			[null, true, false],
+			[null, true, true]
+		];
+	}
+
+	/**
+	 * @dataProvider provideNullorFalseData
+	 * @param $user
+	 * @param $backendinstance
+	 * @param $setDisplayName
+	 */
+	public function testCanChangeDisplayNameWhenNullSession($getUser, $backendinstance, $setDisplayName) {
+		$this->sessionUser->method('getUser')
+			->willReturn($getUser);
+		$backend = $this->getMockBuilder(Database::class)
+			->setMethods(['implementsActions'])
+			->getMock();
+
+		/** @var Account | \PHPUnit_Framework_MockObject_MockObject $account */
+		$account = $this->getMockBuilder(Account::class)
+			->setMethods(['getBackendInstance', 'getDisplayName', 'setDisplayName'])
+			->getMock();
+		if ($backendinstance !== null) {
+			$backendinstance = $backend;
+		}
+		$account->expects($this->any())->method('getBackendInstance')->willReturn($backendinstance);
+		$account->expects($this->any())->method('getDisplayName')->willReturn('admin');
+		$account->expects($this->any())->method('setDisplayName')->willReturn($setDisplayName);
+		$user = new User($account, $this->accountMapper, null, $this->config, null, null, $this->groupManager, null);
+		if ($setDisplayName !== true) {
+			$this->assertEquals($setDisplayName, $user->canChangeDisplayName());
+		} else {
+			$this->assertEquals(null, $user->canChangeDisplayName());
+		}
+	}
+
 	/**
 	 * don't allow display names containing whitespaces only
 	 */