Merge pull request #23709 from nextcloud/backport/23702/stable18

[stable18] fix sharer flag on ldap:show-remnants when user owned more than a single share

Author: blizzz

apps/user_ldap/lib/User/OfflineUser.php

@@ -89,7 +89,6 @@ public function __construct($ocName, IConfig $config, IDBConnection $db, UserMap
 		$this->config = $config;
 		$this->db = $db;
 		$this->mapping = $mapping;
-		$this->fetchDetails();
 	}
 
 	/**
@@ -131,6 +130,9 @@ public function getOCName() {
 	 * @return string
 	 */
 	public function getUID() {
+		if (!isset($this->uid)) {
+			$this->fetchDetails();
+		}
 		return $this->uid;
 	}
 
@@ -139,6 +141,9 @@ public function getUID() {
 	 * @return string
 	 */
 	public function getDN() {
+		if (!isset($this->dn)) {
+			$this->fetchDetails();
+		}
 		return $this->dn;
 	}
 
@@ -147,6 +152,9 @@ public function getDN() {
 	 * @return string
 	 */
 	public function getDisplayName() {
+		if (!isset($this->displayName)) {
+			$this->fetchDetails();
+		}
 		return $this->displayName;
 	}
 
@@ -155,6 +163,9 @@ public function getDisplayName() {
 	 * @return string
 	 */
 	public function getEmail() {
+		if (!isset($this->email)) {
+			$this->fetchDetails();
+		}
 		return $this->email;
 	}
 
@@ -163,6 +174,9 @@ public function getEmail() {
 	 * @return string
 	 */
 	public function getHomePath() {
+		if (!isset($this->homePath)) {
+			$this->fetchDetails();
+		}
 		return $this->homePath;
 	}
 
@@ -171,6 +185,9 @@ public function getHomePath() {
 	 * @return int
 	 */
 	public function getLastLogin() {
+		if (!isset($this->lastLogin)) {
+			$this->fetchDetails();
+		}
 		return (int)$this->lastLogin;
 	}
 
@@ -179,6 +196,9 @@ public function getLastLogin() {
 	 * @return int
 	 */
 	public function getDetectedOn() {
+		if (!isset($this->foundDeleted)) {
+			$this->fetchDetails();
+		}
 		return (int)$this->foundDeleted;
 	}
 
@@ -187,6 +207,9 @@ public function getDetectedOn() {
 	 * @return bool
 	 */
 	public function getHasActiveShares() {
+		if (!isset($this->hasActiveShares)) {
+			$this->fetchDetails();
+		}
 		return $this->hasActiveShares;
 	}
 
@@ -219,29 +242,22 @@ protected function fetchDetails() {
 	 */
 	protected function determineShares() {
 		$query = $this->db->prepare('
-			SELECT COUNT(`uid_owner`)
+			SELECT `uid_owner`
 			FROM `*PREFIX*share`
 			WHERE `uid_owner` = ?
 		', 1);
 		$query->execute(array($this->ocName));
-		$sResult = $query->fetchColumn(0);
-		if((int)$sResult === 1) {
+		if ($query->rowCount() > 0) {
 			$this->hasActiveShares = true;
 			return;
 		}
 
 		$query = $this->db->prepare('
-			SELECT COUNT(`owner`)
+			SELECT `owner`
 			FROM `*PREFIX*share_external`
 			WHERE `owner` = ?
 		', 1);
 		$query->execute(array($this->ocName));
-		$sResult = $query->fetchColumn(0);
-		if((int)$sResult === 1) {
-			$this->hasActiveShares = true;
-			return;
-		}
-
-		$this->hasActiveShares = false;
+		$this->hasActiveShares = $query->rowCount() > 0;
 	}
 }

apps/user_ldap/tests/User/OfflineUserTest.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Arthur Schiwon <[email protected]>
+ *
+ * @author Arthur Schiwon <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_LDAP\Tests\User;
+
+use Doctrine\DBAL\Driver\Statement;
+use OCA\User_LDAP\Mapping\UserMapping;
+use OCA\User_LDAP\User\OfflineUser;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use Test\TestCase;
+
+class OfflineUserTest extends TestCase {
+
+	/** @var OfflineUser */
+	protected $offlineUser;
+	/** @var UserMapping|\PHPUnit\Framework\MockObject\MockObject */
+	protected $mapping;
+	/** @var string */
+	protected $uid;
+	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
+	protected $config;
+	/** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */
+	protected $dbc;
+
+	public function setUp(): void {
+		$this->uid = 'deborah';
+		$this->config = $this->createMock(IConfig::class);
+		$this->dbc = $this->createMock(IDBConnection::class);
+		$this->mapping = $this->createMock(UserMapping::class);
+
+		$this->offlineUser = new OfflineUser(
+			$this->uid,
+			$this->config,
+			$this->dbc,
+			$this->mapping
+		);
+	}
+
+	public function shareOwnerProvider(): array {
+		// tests for none, one, many
+		return [
+			[ 0, 0, false],
+			[ 1, 0, true],
+			[ 0, 1, true],
+			[ 1, 1, true],
+			[ 2, 0, true],
+			[ 0, 2, true],
+			[ 2, 2, true],
+		];
+	}
+
+	/**
+	 * @dataProvider shareOwnerProvider
+	 */
+	public function testHasActiveShares(int $internalOwnerships, int $externalOwnerships, bool $expected) {
+		$queryMock = $this->createMock(Statement::class);
+		$queryMock->expects($this->atLeastOnce())
+			->method('execute');
+		$queryMock->expects($this->atLeastOnce())
+			->method('rowCount')
+			->willReturnOnConsecutiveCalls(
+				$internalOwnerships > 0 ? 1 : 0,
+				$externalOwnerships > 0 ? 1 : 0
+			);
+
+		$this->dbc->expects($this->atLeastOnce())
+			->method('prepare')
+			->willReturn($queryMock);
+
+		$this->assertSame($expected, $this->offlineUser->getHasActiveShares());
+	}
+}