[TASK] Migrate ext:saltedpasswords to use Doctrine DBAL. 92/48492/5
authorGleb Levitin <gleb.levitin@dkd.de>
Tue, 7 Jun 2016 09:29:17 +0000 (11:29 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Wed, 8 Jun 2016 15:29:51 +0000 (17:29 +0200)
Resolves: #76471
Releases: master
Change-Id: Ib3a504b6c1ddd875bbe87c6110b905631a0c6731
Reviewed-on: https://review.typo3.org/48492
Reviewed-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Tested-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/saltedpasswords/Classes/SaltedPasswordService.php
typo3/sysext/saltedpasswords/Classes/Task/BulkUpdateTask.php
typo3/sysext/saltedpasswords/Classes/Utility/SaltedPasswordsUtility.php
typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml [new file with mode: 0644]
typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/fe_users.xml [new file with mode: 0644]
typo3/sysext/saltedpasswords/Tests/Functional/SaltedPasswordServiceTest.php [new file with mode: 0644]
typo3/sysext/saltedpasswords/Tests/Functional/Task/BulkUpdateTaskTest.php [new file with mode: 0644]
typo3/sysext/saltedpasswords/Tests/Functional/Utility/SaltedPasswordsUtilityTest.php [new file with mode: 0644]

index abb4bc0..813c015 100644 (file)
@@ -13,6 +13,7 @@ namespace TYPO3\CMS\Saltedpasswords;
  *
  * The TYPO3 project - inspiring people to share!
  */
+use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
@@ -216,8 +217,20 @@ class SaltedPasswordService extends \TYPO3\CMS\Sv\AbstractAuthenticationService
      */
     protected function updatePassword($uid, $updateFields)
     {
-        $GLOBALS['TYPO3_DB']->exec_UPDATEquery($this->pObj->user_table, sprintf('uid = %u', $uid), $updateFields);
-        GeneralUtility::devLog(sprintf('Automatic password update for user record in %s with uid %u', $this->pObj->user_table, $uid), $this->extKey, 1);
+        $connection = GeneralUtility::makeInstance(ConnectionPool::class)
+            ->getConnectionForTable($this->pObj->user_table);
+
+        $connection->update(
+            $this->pObj->user_table,
+            $updateFields,
+            ['uid' => (int)$uid]
+        );
+
+        GeneralUtility::devLog(
+            sprintf('Automatic password update for user record in %s with uid %u', $this->pObj->user_table, $uid),
+            $this->extKey,
+            1
+        );
     }
 
     /**
index 7228e19..0928779 100644 (file)
@@ -14,6 +14,9 @@ namespace TYPO3\CMS\Saltedpasswords\Task;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 /**
  * Update plaintext and hashed passwords of existing users to salted passwords.
  */
@@ -108,7 +111,19 @@ class BulkUpdateTask extends \TYPO3\CMS\Scheduler\Task\AbstractTask
      */
     protected function findUsersToUpdate($mode)
     {
-        $usersToUpdate = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows('uid, password', strtolower($mode) . '_users', '1 = 1', '', 'uid ASC', $this->userRecordPointer[$mode] . ', ' . $this->numberOfRecords);
+        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
+            ->getQueryBuilderForTable($this->getTablename($mode));
+        $queryBuilder->getRestrictions()->removeAll();
+
+        $usersToUpdate = $queryBuilder
+            ->select('uid', 'password')
+            ->from($this->getTablename($mode))
+            ->orderBy('uid')
+            ->setFirstResult($this->userRecordPointer[$mode])
+            ->setMaxResults($this->numberOfRecords)
+            ->execute()
+            ->fetchAll();
+
         return $usersToUpdate;
     }
 
@@ -155,9 +170,14 @@ class BulkUpdateTask extends \TYPO3\CMS\Scheduler\Task\AbstractTask
                 $newPassword = 'M' . $newPassword;
             }
             // Persist updated password
-            $GLOBALS['TYPO3_DB']->exec_UPDATEquery(strtolower($mode) . '_users', 'uid = ' . $user['uid'], array(
-                'password' => $newPassword
-            ));
+
+            GeneralUtility::makeInstance(ConnectionPool::class)
+                ->getConnectionForTable($this->getTablename($mode))
+                ->update(
+                    $this->getTablename($mode),
+                    ['password' => $newPassword],
+                    ['uid' => (int)$user['uid']]
+                );
         }
     }
 
@@ -272,4 +292,24 @@ class BulkUpdateTask extends \TYPO3\CMS\Scheduler\Task\AbstractTask
     {
         return $this->numberOfRecords;
     }
+
+    /**
+     * @param string $mode FE for fe_users, BE for be_users
+     * @return string
+     * @throws \InvalidArgumentException
+     */
+    protected function getTablename(string $mode): string
+    {
+        $mode = strtolower($mode);
+        if ($mode === 'be') {
+            return 'be_users';
+        } elseif ($mode === 'fe') {
+            return 'fe_users';
+        } else {
+            throw new \InvalidArgumentException(
+                'Invalid mode "' . $mode . '" for salted passwords bulk update',
+                1465392861
+            );
+        }
+    }
 }
index 2b656fa..4e90fde 100644 (file)
@@ -14,6 +14,9 @@ namespace TYPO3\CMS\Saltedpasswords\Utility;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 /**
  * General library class.
  */
@@ -25,20 +28,26 @@ class SaltedPasswordsUtility
     const EXTKEY = 'saltedpasswords';
 
     /**
-     * Calculates number of backend users, who have no saltedpasswords
-     * protection.
+     * Calculates number of backend users, who have no saltedpasswords protection.
      *
      * @return int
      */
     public static function getNumberOfBackendUsersWithInsecurePassword()
     {
-        $userCount = $GLOBALS['TYPO3_DB']->exec_SELECTcountRows(
-            '*',
-            'be_users',
-            'password != \'\''
-                . ' AND password NOT LIKE ' . $GLOBALS['TYPO3_DB']->fullQuoteStr('$%', 'be_users')
-                . ' AND password NOT LIKE ' . $GLOBALS['TYPO3_DB']->fullQuoteStr('M$%', 'be_users')
-        );
+        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('be_users');
+        $queryBuilder->getRestrictions()->removeAll();
+
+        $userCount = $queryBuilder
+            ->count('*')
+            ->from('be_users')
+            ->where(
+                $queryBuilder->expr()->neq('password', $queryBuilder->quote('')),
+                $queryBuilder->expr()->notLike('password', $queryBuilder->quote('$%')),
+                $queryBuilder->expr()->notLike('password', $queryBuilder->quote('M$%'))
+            )
+            ->execute()
+            ->fetchColumn();
+
         return $userCount;
     }
 
diff --git a/typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml b/typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml
new file mode 100644 (file)
index 0000000..884707e
--- /dev/null
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="utf-8"?>
+<dataset>
+       <be_users>
+               <uid>1</uid>
+               <username>user_with_salted_password_1</username>
+               <password>$P$CDmA2Juu2h9/9MNaKaxtgzZgIVmjkh/</password> <!-- salted hash starting with $ -->
+               <disable>0</disable>
+               <deleted>0</deleted>
+       </be_users>
+       <be_users>
+               <uid>2</uid>
+               <username>user_with_salted_password_2</username>
+               <password>M$v2AultVYItaCpb.tpdx2aGAue8eL3/</password> <!-- salted hash starting with M$ -->
+               <disable>0</disable>
+               <deleted>0</deleted>
+       </be_users>
+       <be_users>
+               <uid>3</uid>
+               <username>user_with_insecure_password_1</username>
+               <password>5f4dcc3b5aa765d61d8327deb882cf99</password> <!-- simple md5 hash -->
+               <disable>0</disable>
+               <deleted>0</deleted>
+       </be_users>
+       <be_users>
+               <uid>4</uid>
+               <username>user_with_empty_password</username>
+               <password></password> <!-- empty password -->
+               <disable>0</disable>
+               <deleted>0</deleted>
+       </be_users>
+       <be_users>
+               <uid>5</uid>
+               <username>deleted_user_with_insecure_password</username>
+               <password>819b0643d6b89dc9b579fdfc9094f28e</password> <!-- simple md5 hash -->
+               <disable>0</disable>
+               <deleted>1</deleted> <!-- user is marked as deleted -->
+       </be_users>
+       <be_users>
+               <uid>6</uid>
+               <username>disabled_user_with_insecure_password</username>
+               <password>34cc93ece0ba9e3f6f235d4af979b16c</password> <!-- simple md5 hash -->
+               <disable>1</disable> <!-- user is marked as disabled -->
+               <deleted>0</deleted>
+       </be_users>
+</dataset>
diff --git a/typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/fe_users.xml b/typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/fe_users.xml
new file mode 100644 (file)
index 0000000..598fb5a
--- /dev/null
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<dataset>
+       <fe_users>
+               <uid>1</uid>
+               <username>user_with_salted_password</username>
+               <password>$P$CDmA2Juu2h9/9MNaKaxtgzZgIVmjkh/</password> <!-- salted hash starting with $ -->
+               <deleted>0</deleted>
+               <disable>0</disable>
+       </fe_users>
+       <fe_users>
+               <uid>2</uid>
+               <username>user_with_unsecured_password</username>
+               <password>5f4dcc3b5aa765d61d8327deb882cf99</password>
+               <deleted>0</deleted>
+               <disable>0</disable>
+       </fe_users>
+</dataset>
diff --git a/typo3/sysext/saltedpasswords/Tests/Functional/SaltedPasswordServiceTest.php b/typo3/sysext/saltedpasswords/Tests/Functional/SaltedPasswordServiceTest.php
new file mode 100644 (file)
index 0000000..6df6f4d
--- /dev/null
@@ -0,0 +1,80 @@
+<?php
+namespace TYPO3\CMS\Saltedpasswords\Tests\Functional;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\SaltedPasswordService;
+
+/**
+ * Test case for \TYPO3\CMS\Saltedpasswords\SaltedPasswordService
+ */
+class SaltedPasswordServiceTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase
+{
+
+    /**
+     * XML database fixtures to be loaded into database.
+     *
+     * @var array
+     */
+    protected $xmlDatabaseFixtures = [
+        'typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml'
+    ];
+
+    /**
+     * @var SaltedPasswordService
+     */
+    protected $subject;
+
+    /**
+     * Sets up this test suite.
+     *
+     * @return void
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+        foreach ($this->xmlDatabaseFixtures as $fixture) {
+            $this->importDataSet($fixture);
+        }
+        $this->subject = GeneralUtility::makeInstance(SaltedPasswordService::class);
+    }
+
+    /**
+     * Check if service updates backend user password
+     *
+     * @test
+     */
+    public function checkIfServiceUpdatesBackendUserPassword()
+    {
+        $newPassword = array('password' => '008c5926ca861023c1d2a36653fd88e2');
+
+        $this->subject->pObj = new \stdClass();
+        $this->subject->pObj->user_table = 'be_users';
+
+        $this->callInaccessibleMethod($this->subject, 'updatePassword', 3, $newPassword);
+
+        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('be_users');
+
+        $currentPassword = $queryBuilder
+            ->select('password')
+            ->from('be_users')
+            ->where($queryBuilder->expr()->eq('uid', 3))
+            ->execute()
+            ->fetchColumn();
+
+        $this->assertEquals($newPassword['password'], $currentPassword);
+    }
+}
diff --git a/typo3/sysext/saltedpasswords/Tests/Functional/Task/BulkUpdateTaskTest.php b/typo3/sysext/saltedpasswords/Tests/Functional/Task/BulkUpdateTaskTest.php
new file mode 100644 (file)
index 0000000..e4abf83
--- /dev/null
@@ -0,0 +1,153 @@
+<?php
+namespace TYPO3\CMS\Saltedpasswords\Tests\Functional\Task;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\Task\BulkUpdateTask;
+
+/**
+ * Test case for \TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility
+ */
+class BulkUpdateTaskTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase
+{
+    /**
+     * XML database fixtures to be loaded into database.
+     *
+     * @var array
+     */
+    protected $xmlDatabaseFixtures = [
+        'typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml',
+        'typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/fe_users.xml'
+    ];
+
+    /**
+     * Core extensions to load
+     *
+     * @var array
+     */
+    protected $testExtensionsToLoad = [
+        'typo3/sysext/scheduler'
+    ];
+
+    /**
+     * @var BulkUpdateTask
+     */
+    protected $subject;
+
+    /**
+     * Sets up this test suite.
+     *
+     * @return void
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+        foreach ($this->xmlDatabaseFixtures as $fixture) {
+            $this->importDataSet($fixture);
+        }
+        $this->subject = GeneralUtility::makeInstance(BulkUpdateTask::class);
+    }
+
+    /**
+     * Test if bulk update task finds backend users to be updated
+     *
+     * @test
+     */
+    public function testIfBulkUpdateTaskFindsBackendUsersToBeUpdated()
+    {
+        $expectedOutput = [
+            [
+                'uid' => 1,
+                'password' => '$P$CDmA2Juu2h9/9MNaKaxtgzZgIVmjkh/'
+            ],
+            [
+                'uid' => 2,
+                'password' => 'M$v2AultVYItaCpb.tpdx2aGAue8eL3/'
+            ],
+            [
+                'uid' => 3,
+                'password' => '5f4dcc3b5aa765d61d8327deb882cf99'
+            ],
+            [
+                'uid' => 4,
+                'password' => ''
+            ],
+            [
+                'uid' => 5,
+                'password' => '819b0643d6b89dc9b579fdfc9094f28e'
+            ],
+            [
+                'uid' => 6,
+                'password' => '34cc93ece0ba9e3f6f235d4af979b16c'
+            ]
+        ];
+
+        $this->assertEquals($expectedOutput, $this->callInaccessibleMethod($this->subject, 'findUsersToUpdate', 'BE'));
+    }
+
+    /**
+     * Test if bulk update task finds frontend users to be updated
+     *
+     * @test
+     */
+    public function testIfBulkUpdateTaskFindsFrontendUsersToBeUpdated()
+    {
+        $expectedOutput = [
+            [
+                'uid' => 1,
+                'password' => '$P$CDmA2Juu2h9/9MNaKaxtgzZgIVmjkh/'
+            ],
+            [
+                'uid' => 2,
+                'password' => '5f4dcc3b5aa765d61d8327deb882cf99'
+            ]
+        ];
+
+        $this->assertEquals($expectedOutput, $this->callInaccessibleMethod($this->subject, 'findUsersToUpdate', 'FE'));
+    }
+
+    /**
+     * Test, if passwords are updated with salted hashes for a given user list
+     *
+     * @test
+     */
+    public function testIfPasswordsAreUpdatedWithSaltedHashesForGivenUserList()
+    {
+        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('fe_users');
+        $usersToBeUpdated = $queryBuilder
+            ->select('uid', 'password')
+            ->from('fe_users')
+            ->where($queryBuilder->expr()->eq('uid', 2))
+            ->execute()
+            ->fetchAll();
+
+        $originalMd5Password = $usersToBeUpdated[0]['password'];
+
+        $this->callInaccessibleMethod($this->subject, 'updatePasswords', 'FE', $usersToBeUpdated);
+
+        $saltedPassword = $queryBuilder
+            ->select('password')
+            ->from('fe_users')
+            ->where($queryBuilder->expr()->eq('uid', 2))
+            ->execute()
+            ->fetchColumn();
+
+        $this->assertNotEquals($originalMd5Password, $saltedPassword);
+
+        $saltedPasswordsInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance(substr($saltedPassword, 1));
+        $this->assertTrue($saltedPasswordsInstance->checkPassword($originalMd5Password, substr($saltedPassword, 1)));
+    }
+}
diff --git a/typo3/sysext/saltedpasswords/Tests/Functional/Utility/SaltedPasswordsUtilityTest.php b/typo3/sysext/saltedpasswords/Tests/Functional/Utility/SaltedPasswordsUtilityTest.php
new file mode 100644 (file)
index 0000000..9f6d24b
--- /dev/null
@@ -0,0 +1,56 @@
+<?php
+namespace TYPO3\CMS\Saltedpasswords\Tests\Functional\Utility;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility;
+
+/**
+ * Test case for \TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility
+ */
+class SaltedPasswordsUtilityTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase
+{
+
+    /**
+     * XML database fixtures to be loaded into database.
+     *
+     * @var array
+     */
+    protected $xmlDatabaseFixtures = [
+        'typo3/sysext/saltedpasswords/Tests/Functional/Fixtures/be_users.xml'
+    ];
+
+    /**
+     * Sets up this test suite.
+     *
+     * @return void
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+        foreach ($this->xmlDatabaseFixtures as $fixture) {
+            $this->importDataSet($fixture);
+        }
+    }
+
+    /**
+     * Check if salted password utility returns the correct number of backend users with insecure passwords
+     *
+     * @test
+     */
+    public function checkIfNumberOfBackendUsersWithInsecurePasswordsIsFetchedCorrectly()
+    {
+        $this->assertEquals(3, SaltedPasswordsUtility::getNumberOfBackendUsersWithInsecurePassword());
+    }
+}