Merge pull request #10611 from nextcloud/fix/2fa-provider-user-dao-duplicate-key

Fix duplicate key violation in 2FA provider registry DAO

2 files changed, 38 insertions, 6 deletions

diff --git a/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php b/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php
index 0e9cd8045cd..e04512b8575 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 6323c2f0aa4..b46bce719fa 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', 1);
+		$this->dao->persist('twofactor_totp', 'user123', 1);
+
+		$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);
+	}
+
 }