From 85bc5edb5e8dbbf6bbebf4637ac9e05cc9399f78 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 9 Aug 2018 13:43:00 +0200 Subject: [PATCH 1/2] Add integration/unit test for the double-insert of same values Signed-off-by: Christoph Wurst --- .../Db/ProviderUserAssignmentDaoTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php index 6323c2f0aa..2f74a81503 100644 --- a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php @@ -112,4 +112,23 @@ class ProviderUserAssignmentDaoTest extends TestCase { $this->assertCount(1, $data); } + public function testPersistSameStateTwice() { + $qb = $this->dbConn->getQueryBuilder(); + + $this->dao->persist('twofactor_totp', 'user123', 0); + $this->dao->persist('twofactor_totp', 'user123', 0); + + $q = $qb + ->select('*') + ->from(ProviderUserAssignmentDao::TABLE_NAME) + ->where($qb->expr()->eq('provider_id', $qb->createNamedParameter('twofactor_totp'))) + ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter('user123'))) + ->andWhere($qb->expr()->eq('enabled', $qb->createNamedParameter(1))); + $res = $q->execute(); + $data = $res->fetchAll(); + $res->closeCursor(); + + $this->assertCount(1, $data); + } + } From 8db66d5dfb8b85e395636f71be4db36df069c86e Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 9 Aug 2018 13:56:04 +0200 Subject: [PATCH 2/2] Fix double-inserts of the same provider state Signed-off-by: Christoph Wurst --- .../Db/ProviderUserAssignmentDao.php | 25 ++++++++++++++----- .../Db/ProviderUserAssignmentDaoTest.php | 4 +-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php b/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php index 0e9cd8045c..e04512b857 100644 --- a/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php +++ b/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php @@ -72,15 +72,26 @@ class ProviderUserAssignmentDao { public function persist(string $providerId, string $uid, int $enabled) { $qb = $this->conn->getQueryBuilder(); - // First, try to update an existing entry - $updateQuery = $qb->update(self::TABLE_NAME) - ->set('enabled', $qb->createNamedParameter($enabled)) + $this->conn->beginTransaction(); + // To prevent duplicate primary key, we have to first check if an INSERT + // or UPDATE is required + $query = $qb->select('*') + ->from(self::TABLE_NAME) ->where($qb->expr()->eq('provider_id', $qb->createNamedParameter($providerId))) ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); - $updatedRows = $updateQuery->execute(); + $result = $query->execute(); + $rowCount = count($result->fetchAll()); + $result->closeCursor(); - // If this (providerId, UID) key tuple is new, we have to insert it - if (0 === (int)$updatedRows) { + if ($rowCount > 0) { + // There is an entry -> update it + $updateQuery = $qb->update(self::TABLE_NAME) + ->set('enabled', $qb->createNamedParameter($enabled)) + ->where($qb->expr()->eq('provider_id', $qb->createNamedParameter($providerId))) + ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); + $updateQuery->execute(); + } else { + // Insert a new entry $insertQuery = $qb->insert(self::TABLE_NAME)->values([ 'provider_id' => $qb->createNamedParameter($providerId), 'uid' => $qb->createNamedParameter($uid), @@ -89,6 +100,8 @@ class ProviderUserAssignmentDao { $insertQuery->execute(); } + $this->conn->commit(); + } } diff --git a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php index 2f74a81503..b46bce719f 100644 --- a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php @@ -115,8 +115,8 @@ class ProviderUserAssignmentDaoTest extends TestCase { public function testPersistSameStateTwice() { $qb = $this->dbConn->getQueryBuilder(); - $this->dao->persist('twofactor_totp', 'user123', 0); - $this->dao->persist('twofactor_totp', 'user123', 0); + $this->dao->persist('twofactor_totp', 'user123', 1); + $this->dao->persist('twofactor_totp', 'user123', 1); $q = $qb ->select('*')