From 7d508d43b2fdeed3f7f2c05f8bdabc43892b2690 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Fri, 9 Feb 2018 12:07:32 +0530 Subject: [PATCH] [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 --- lib/private/Log.php | 16 +++++++++------ lib/private/User/User.php | 15 ++++++++++---- tests/lib/LoggerTest.php | 22 +++++++++++++++++++++ tests/lib/User/UserTest.php | 39 +++++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index 2244372a72a6..43e36eea674f 100644 --- 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 index 7ed18c2b9ae1..53febae93e92 100644 --- 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 index 59f172e0c83a..bfbb993ec736 100644 --- 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 index a0e02583e89f..c32af8e01ff9 100644 --- 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 */