Merge pull request #1943 from nextcloud/log-query-builder-queries

Log QueryBuilder queries

lib/private/DB/Connection.php

@@ -67,7 +67,11 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 	 * @return \OCP\DB\QueryBuilder\IQueryBuilder
 	 */
 	public function getQueryBuilder() {
-		return new QueryBuilder($this);
+		return new QueryBuilder(
+			$this,
+			\OC::$server->getSystemConfig(),
+			\OC::$server->getLogger()
+		);
 	}
 
 	/**

lib/private/DB/QueryBuilder/QueryBuilder.php

@@ -31,16 +31,24 @@ use OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder;
 use OC\DB\QueryBuilder\ExpressionBuilder\MySqlExpressionBuilder;
 use OC\DB\QueryBuilder\ExpressionBuilder\OCIExpressionBuilder;
 use OC\DB\QueryBuilder\ExpressionBuilder\PgSqlExpressionBuilder;
+use OC\SystemConfig;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\DB\QueryBuilder\IQueryFunction;
 use OCP\DB\QueryBuilder\IParameter;
 use OCP\IDBConnection;
+use OCP\ILogger;
 
 class QueryBuilder implements IQueryBuilder {
 
 	/** @var \OCP\IDBConnection */
 	private $connection;
 
+	/** @var SystemConfig */
+	private $systemConfig;
+
+	/** @var ILogger */
+	private $logger;
+
 	/** @var \Doctrine\DBAL\Query\QueryBuilder */
 	private $queryBuilder;
 
@@ -56,10 +64,14 @@ class QueryBuilder implements IQueryBuilder {
 	/**
 	 * Initializes a new QueryBuilder.
 	 *
-	 * @param \OCP\IDBConnection $connection
+	 * @param IDBConnection $connection
+	 * @param SystemConfig $systemConfig
+	 * @param ILogger $logger
 	 */
-	public function __construct(IDBConnection $connection) {
+	public function __construct(IDBConnection $connection, SystemConfig $systemConfig, ILogger $logger) {
 		$this->connection = $connection;
+		$this->systemConfig = $systemConfig;
+		$this->logger = $logger;
 		$this->queryBuilder = new \Doctrine\DBAL\Query\QueryBuilder($this->connection);
 		$this->helper = new QuoteHelper();
 	}
@@ -139,6 +151,29 @@ class QueryBuilder implements IQueryBuilder {
 	 * @return \Doctrine\DBAL\Driver\Statement|int
 	 */
 	public function execute() {
+		if ($this->systemConfig->getValue('log_query', false)) {
+			$params = [];
+			foreach ($this->getParameters() as $placeholder => $value) {
+				if (is_array($value)) {
+					$params[] = $placeholder . ' => (\'' . implode('\', \'', $value) . '\')';
+				} else {
+					$params[] = $placeholder . ' => \'' . $value . '\'';
+				}
+			}
+			if (empty($params)) {
+				$this->logger->debug('DB QueryBuilder: \'{query}\'', [
+					'query' => $this->getSQL(),
+					'app' => 'core',
+				]);
+			} else {
+				$this->logger->debug('DB QueryBuilder: \'{query}\' with parameters: {params}', [
+					'query' => $this->getSQL(),
+					'params' => implode(', ', $params),
+					'app' => 'core',
+				]);
+			}
+		}
+
 		return $this->queryBuilder->execute();
 	}
 

lib/private/Security/CSRF/TokenStorage/SessionStorage.php

@@ -40,6 +40,13 @@ class SessionStorage {
 		$this->session = $session;
 	}
 
+	/**
+	 * @param ISession $session
+	 */
+	public function setSession(ISession $session) {
+		$this->session = $session;
+	}
+
 	/**
 	 * Returns the current token or throws an exception if none is found.
 	 *

lib/private/Server.php

@@ -710,13 +710,15 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerService('CsrfTokenManager', function (Server $c) {
 			$tokenGenerator = new CsrfTokenGenerator($c->getSecureRandom());
-			$sessionStorage = new SessionStorage($c->getSession());
 
 			return new CsrfTokenManager(
 				$tokenGenerator,
-				$sessionStorage
+				$c->query(SessionStorage::class)
 			);
 		});
+		$this->registerService(SessionStorage::class, function (Server $c) {
+			return new SessionStorage($c->getSession());
+		});
 		$this->registerService('ContentSecurityPolicyManager', function (Server $c) {
 			return new ContentSecurityPolicyManager();
 		});
@@ -945,6 +947,7 @@ class Server extends ServerContainer implements IServerContainer {
 	 * @param \OCP\ISession $session
 	 */
 	public function setSession(\OCP\ISession $session) {
+		$this->query(SessionStorage::class)->setSession($session);
 		return $this->query('UserSession')->setSession($session);
 	}
 

tests/lib/DB/QueryBuilder/QueryBuilderTest.php

@@ -25,7 +25,9 @@ use Doctrine\DBAL\Query\Expression\CompositeExpression;
 use OC\DB\QueryBuilder\Literal;
 use OC\DB\QueryBuilder\Parameter;
 use OC\DB\QueryBuilder\QueryBuilder;
+use OC\SystemConfig;
 use OCP\IDBConnection;
+use OCP\ILogger;
 
 /**
  * Class QueryBuilderTest
@@ -41,11 +43,19 @@ class QueryBuilderTest extends \Test\TestCase {
 	/** @var IDBConnection */
 	protected $connection;
 
+	/** @var SystemConfig|\PHPUnit_Framework_MockObject_MockObject */
+	protected $config;
+
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
+	protected $logger;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->connection = \OC::$server->getDatabaseConnection();
-		$this->queryBuilder = new QueryBuilder($this->connection);
+		$this->config = $this->createMock(SystemConfig::class);
+		$this->logger = $this->createMock(ILogger::class);
+		$this->queryBuilder = new QueryBuilder($this->connection, $this->config, $this->logger);
 	}
 
 	protected function createTestingRows($appId = 'testFirstResult') {
@@ -166,7 +176,9 @@ class QueryBuilderTest extends \Test\TestCase {
 	}
 
 	public function dataSelect() {
-		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection());
+		$config = $this->createMock(SystemConfig::class);
+		$logger = $this->createMock(ILogger::class);
+		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection(), $config, $logger);
 		return [
 			// select('column1')
 			[['configvalue'], ['configvalue' => '99']],
@@ -232,7 +244,9 @@ class QueryBuilderTest extends \Test\TestCase {
 	}
 
 	public function dataSelectAlias() {
-		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection());
+		$config = $this->createMock(SystemConfig::class);
+		$logger = $this->createMock(ILogger::class);
+		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection(), $config, $logger);
 		return [
 			['configvalue', 'cv', ['cv' => '99']],
 			[$queryBuilder->expr()->literal('column1'), 'thing', ['thing' => 'column1']],
@@ -301,7 +315,9 @@ class QueryBuilderTest extends \Test\TestCase {
 	}
 
 	public function dataAddSelect() {
-		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection());
+		$config = $this->createMock(SystemConfig::class);
+		$logger = $this->createMock(ILogger::class);
+		$queryBuilder = new QueryBuilder(\OC::$server->getDatabaseConnection(), $config, $logger);
 		return [
 			// addSelect('column1')
 			[['configvalue'], ['appid' => 'testFirstResult', 'configvalue' => '99']],
@@ -1200,4 +1216,130 @@ class QueryBuilderTest extends \Test\TestCase {
 			$this->queryBuilder->getColumnName($column, $prefix)
 		);
 	}
+
+	public function testExecuteWithoutLogger() {
+		$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
+		$queryBuilder
+			->expects($this->once())
+			->method('execute')
+			->willReturn(3);
+		$this->logger
+			->expects($this->never())
+			->method('debug');
+		$this->config
+			->expects($this->once())
+			->method('getValue')
+			->with('log_query', false)
+			->willReturn(false);
+
+		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->assertEquals(3, $this->queryBuilder->execute());
+	}
+
+	public function testExecuteWithLoggerAndNamedArray() {
+		$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
+		$queryBuilder
+			->expects($this->at(0))
+			->method('getParameters')
+			->willReturn([
+				'foo' => 'bar',
+				'key' => 'value',
+			]);
+		$queryBuilder
+			->expects($this->at(1))
+			->method('getSQL')
+			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+		$queryBuilder
+			->expects($this->once())
+			->method('execute')
+			->willReturn(3);
+		$this->logger
+			->expects($this->once())
+			->method('debug')
+			->with(
+				'DB QueryBuilder: \'{query}\' with parameters: {params}',
+				[
+					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'params' => 'foo => \'bar\', key => \'value\'',
+					'app' => 'core',
+				]
+			);
+		$this->config
+			->expects($this->once())
+			->method('getValue')
+			->with('log_query', false)
+			->willReturn(true);
+
+		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->assertEquals(3, $this->queryBuilder->execute());
+	}
+
+	public function testExecuteWithLoggerAndUnnamedArray() {
+		$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
+		$queryBuilder
+			->expects($this->at(0))
+			->method('getParameters')
+			->willReturn(['Bar']);
+		$queryBuilder
+			->expects($this->at(1))
+			->method('getSQL')
+			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+		$queryBuilder
+			->expects($this->once())
+			->method('execute')
+			->willReturn(3);
+		$this->logger
+			->expects($this->once())
+			->method('debug')
+			->with(
+				'DB QueryBuilder: \'{query}\' with parameters: {params}',
+				[
+					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'params' => '0 => \'Bar\'',
+					'app' => 'core',
+				]
+			);
+		$this->config
+			->expects($this->once())
+			->method('getValue')
+			->with('log_query', false)
+			->willReturn(true);
+
+		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->assertEquals(3, $this->queryBuilder->execute());
+	}
+
+	public function testExecuteWithLoggerAndNoParams() {
+		$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
+		$queryBuilder
+			->expects($this->at(0))
+			->method('getParameters')
+			->willReturn([]);
+		$queryBuilder
+			->expects($this->at(1))
+			->method('getSQL')
+			->willReturn('SELECT * FROM FOO WHERE BAR = ?');
+		$queryBuilder
+			->expects($this->once())
+			->method('execute')
+			->willReturn(3);
+		$this->logger
+			->expects($this->once())
+			->method('debug')
+			->with(
+				'DB QueryBuilder: \'{query}\'',
+				[
+					'query' => 'SELECT * FROM FOO WHERE BAR = ?',
+					'app' => 'core',
+				]
+			);
+		$this->config
+			->expects($this->once())
+			->method('getValue')
+			->with('log_query', false)
+			->willReturn(true);
+
+		$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
+		$this->assertEquals(3, $this->queryBuilder->execute());
+	}
 }

tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php

@@ -21,6 +21,8 @@
 
 namespace Test\Security\CSRF\TokenStorage;
 
+use OCP\ISession;
+
 class SessionStorageTest extends \Test\TestCase {
 	/** @var \OCP\ISession */
 	private $session;
@@ -106,4 +108,15 @@ class SessionStorageTest extends \Test\TestCase {
 			->willReturn(false);
 		$this->assertSame(false, $this->sessionStorage->hasToken());
 	}
+
+	public function testSetSession() {
+		$session = $this->createMock(ISession::class);
+		$session
+			->expects($this->once())
+			->method('get')
+			->with('requesttoken')
+			->willReturn('MyToken');
+		$this->sessionStorage->setSession($session);
+		$this->assertSame('MyToken', $this->sessionStorage->getToken());
+	}
 }

tests/lib/Security/CredentialsManagerTest.php

@@ -21,6 +21,8 @@
 
 namespace Test\Security;
 
+use OC\SystemConfig;
+use OCP\ILogger;
 use \OCP\Security\ICrypto;
 use \OCP\IDBConnection;
 use \OC\Security\CredentialsManager;
@@ -45,7 +47,7 @@ class CredentialsManagerTest extends \Test\TestCase {
 		$this->manager = new CredentialsManager($this->crypto, $this->dbConnection);
 	}
 
-	private function getQeuryResult($row) {
+	private function getQueryResult($row) {
 		$result = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement')
 			->disableOriginalConstructor()
 			->getMock();
@@ -87,12 +89,16 @@ class CredentialsManagerTest extends \Test\TestCase {
 			->willReturn(json_encode('bar'));
 
 		$qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder')
-			->setConstructorArgs([$this->dbConnection])
+			->setConstructorArgs([
+				$this->dbConnection,
+				$this->createMock(SystemConfig::class),
+				$this->createMock(ILogger::class),
+			])
 			->setMethods(['execute'])
 			->getMock();
 		$qb->expects($this->once())
 			->method('execute')
-			->willReturn($this->getQeuryResult(['credentials' => 'baz']));
+			->willReturn($this->getQueryResult(['credentials' => 'baz']));
 
 		$this->dbConnection->expects($this->once())
 			->method('getQueryBuilder')