From 74c2c63b31039f17ab79c50ae84b4abacbb06fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 29 Aug 2016 09:15:17 +0200 Subject: [PATCH] [stable9.1] Don't log credentials of LoginController::tryLogin (#25902) (#25935) * Don't log credentials of LoginController::tryLogin - fixes #25895 * Don't log password in loginWithPassword --- lib/private/Log.php | 8 ++++++-- tests/lib/LoggerTest.php | 43 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index 49223521916d..b65f48ec5181 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -66,6 +66,7 @@ class Log implements ILogger { 'checkPassword', 'updatePrivateKeyPassword', 'validateUserPass', + 'loginWithPassword', // TokenProvider 'getToken', @@ -84,6 +85,9 @@ class Log implements ILogger { 'calculateHMAC', 'encrypt', 'decrypt', + + //LoginController + 'tryLogin' ]; /** @@ -304,14 +308,14 @@ public function log($level, $message, array $context = array()) { * @since 8.2.0 */ public function logException($exception, array $context = array()) { - $exception = array( + $exception = [ 'Exception' => get_class($exception), 'Message' => $exception->getMessage(), 'Code' => $exception->getCode(), 'Trace' => $exception->getTraceAsString(), 'File' => $exception->getFile(), 'Line' => $exception->getLine(), - ); + ]; $exception['Trace'] = preg_replace('!(' . implode('|', $this->methodsWithSensitiveParameters) . ')\(.*\)!', '$1(*** sensitive parameters replaced ***)', $exception['Trace']); $msg = isset($context['message']) ? $context['message'] : 'Exception'; $msg .= ': ' . json_encode($exception); diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index 4b80c01f3436..43dc8fcd3513 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -9,14 +9,17 @@ namespace Test; use OC\Log; +use OCP\IConfig; +use OCP\Util; class LoggerTest extends TestCase { - /** - * @var \OCP\ILogger - */ + /** @var \OCP\ILogger */ private $logger; static private $logs = array(); + /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ + private $config; + protected function setUp() { parent::setUp(); @@ -40,7 +43,7 @@ public function testAppCondition() { $this->config->expects($this->any()) ->method('getValue') ->will(($this->returnValueMap([ - ['loglevel', \OCP\Util::WARN, \OCP\Util::WARN], + ['loglevel', Util::WARN, Util::WARN], ['log.condition', [], ['apps' => ['files']]] ]))); $logger = $this->logger; @@ -122,4 +125,36 @@ public function testDetectvalidateUserPass($user, $password) { $this->assertContains('validateUserPass(*** sensitive parameters replaced ***)', $logLine); } } + + /** + * @dataProvider userAndPasswordData + */ + public function testDetecttryLogin($user, $password) { + $e = new \Exception('test'); + $this->logger->logException($e); + $logLines = $this->getLogs(); + + foreach($logLines as $logLine) { + $this->assertNotContains($user, $logLine); + $this->assertNotContains($password, $logLine); + $this->assertContains('tryLogin(*** sensitive parameters replaced ***)', $logLine); + } + } + + //loginWithPassword + /** + * @dataProvider userAndPasswordData + */ + public function testDetectloginWithPassword($user, $password) { + $e = new \Exception('test'); + $this->logger->logException($e); + $logLines = $this->getLogs(); + + foreach($logLines as $logLine) { + $this->assertNotContains($user, $logLine); + $this->assertNotContains($password, $logLine); + $this->assertContains('loginWithPassword(*** sensitive parameters replaced ***)', $logLine); + } + } + }