From ecaa02678ab11f78a6626b85b084b1c789f3c47d Mon Sep 17 00:00:00 2001
From: Vincent Petry <vincent@nextcloud.com>
Date: Thu, 18 Mar 2021 17:12:28 +0100
Subject: [PATCH] Update user share must use correct expiration validation

Updating a user or group share now uses the correct method for the
validation of the expiration date. Instead of using the one from links
it uses the one for internal shares.

To avoid future confusion, the method "validateExpirationDate" has been
renamed to "validateExpirationDateLink".

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
---
 lib/private/Share20/Manager.php   | 10 ++--
 tests/lib/Share20/ManagerTest.php | 80 +++++++++++++++----------------
 2 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 0f70948201..1fad23f6d2 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -458,7 +458,7 @@ class Manager implements IManager {
 	 * @throws \InvalidArgumentException
 	 * @throws \Exception
 	 */
-	protected function validateExpirationDate(\OCP\Share\IShare $share) {
+	protected function validateExpirationDateLink(\OCP\Share\IShare $share) {
 		$expirationDate = $share->getExpirationDate();
 
 		if ($expirationDate !== null) {
@@ -761,7 +761,7 @@ class Manager implements IManager {
 			);
 
 			//Verify the expiration date
-			$share = $this->validateExpirationDate($share);
+			$share = $this->validateExpirationDateLink($share);
 
 			//Verify the password
 			$this->verifyPassword($share->getPassword());
@@ -969,7 +969,7 @@ class Manager implements IManager {
 
 			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
 				//Verify the expiration date
-				$this->validateExpirationDate($share);
+				$this->validateExpirationDateInternal($share);
 				$expirationDateUpdated = true;
 			}
 		} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
@@ -977,7 +977,7 @@ class Manager implements IManager {
 
 			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
 				//Verify the expiration date
-				$this->validateExpirationDate($share);
+				$this->validateExpirationDateInternal($share);
 				$expirationDateUpdated = true;
 			}
 		} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
@@ -993,7 +993,7 @@ class Manager implements IManager {
 
 			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
 				//Verify the expiration date
-				$this->validateExpirationDate($share);
+				$this->validateExpirationDateLink($share);
 				$expirationDateUpdated = true;
 			}
 		} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) {
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index adb283a2db..6c4effab42 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -1060,7 +1060,7 @@ class ManagerTest extends \Test\TestCase {
 		$share = $this->manager->newShare();
 		$share->setExpirationDate($past);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 	}
 
 	public function testValidateExpirationDateEnforceButNotSet() {
@@ -1076,7 +1076,7 @@ class ManagerTest extends \Test\TestCase {
 				['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
 			]);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 	}
 
 	public function testValidateExpirationDateEnforceButNotEnabledAndNotSet() {
@@ -1088,7 +1088,7 @@ class ManagerTest extends \Test\TestCase {
 				['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
 			]);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertNull($share->getExpirationDate());
 	}
@@ -1108,7 +1108,7 @@ class ManagerTest extends \Test\TestCase {
 		$expected->setTime(0,0,0);
 		$expected->add(new \DateInterval('P3D'));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertNotNull($share->getExpirationDate());
 		$this->assertEquals($expected, $share->getExpirationDate());
@@ -1129,7 +1129,7 @@ class ManagerTest extends \Test\TestCase {
 		$expected->setTime(0,0,0);
 		$expected->add(new \DateInterval('P1D'));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertNotNull($share->getExpirationDate());
 		$this->assertEquals($expected, $share->getExpirationDate());
@@ -1152,7 +1152,7 @@ class ManagerTest extends \Test\TestCase {
 				['core', 'shareapi_default_expire_date', 'no', 'yes'],
 			]);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 	}
 
 	public function testValidateExpirationDateEnforceValid() {
@@ -1179,7 +1179,7 @@ class ManagerTest extends \Test\TestCase {
 			return $data['expirationDate'] == $future;
 		}));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertEquals($expected, $share->getExpirationDate());
 	}
@@ -1201,7 +1201,7 @@ class ManagerTest extends \Test\TestCase {
 			return $data['expirationDate'] == $expected && $data['passwordSet'] === false;
 		}));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertEquals($expected, $share->getExpirationDate());
 	}
@@ -1216,7 +1216,7 @@ class ManagerTest extends \Test\TestCase {
 		$share = $this->manager->newShare();
 		$share->setPassword('password');
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertNull($share->getExpirationDate());
 	}
@@ -1241,7 +1241,7 @@ class ManagerTest extends \Test\TestCase {
 			return $data['expirationDate'] == $expected;
 		}));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertEquals($expected, $share->getExpirationDate());
 	}
@@ -1270,7 +1270,7 @@ class ManagerTest extends \Test\TestCase {
 			return $data['expirationDate'] == $expected;
 		}));
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertEquals($expected, $share->getExpirationDate());
 	}
@@ -1291,7 +1291,7 @@ class ManagerTest extends \Test\TestCase {
 		$share = $this->manager->newShare();
 		$share->setExpirationDate($nextWeek);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$save->sub(new \DateInterval('P2D'));
 		$this->assertEquals($save, $share->getExpirationDate());
@@ -1315,7 +1315,7 @@ class ManagerTest extends \Test\TestCase {
 			$data['message'] = 'Invalid date!';
 		});
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 	}
 
 	public function testValidateExpirationDateExistingShareNoDefault() {
@@ -1329,7 +1329,7 @@ class ManagerTest extends \Test\TestCase {
 				['core', 'shareapi_expire_after_n_days', '7', '6'],
 			]);
 
-		self::invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+		self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
 
 		$this->assertEquals(null, $share->getExpirationDate());
 	}
@@ -2033,7 +2033,7 @@ class ManagerTest extends \Test\TestCase {
 				'generalCreateChecks',
 				'linkCreateChecks',
 				'pathCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 				'verifyPassword',
 				'setLinkParent',
 			])
@@ -2075,7 +2075,7 @@ class ManagerTest extends \Test\TestCase {
 			->method('pathCreateChecks')
 			->with($path);
 		$manager->expects($this->once())
-			->method('validateExpirationDate')
+			->method('validateExpirationDateLink')
 			->with($share)
 			->willReturn($share);
 		$manager->expects($this->once())
@@ -2158,7 +2158,7 @@ class ManagerTest extends \Test\TestCase {
 				'generalCreateChecks',
 				'linkCreateChecks',
 				'pathCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 				'verifyPassword',
 				'setLinkParent',
 			])
@@ -2194,7 +2194,7 @@ class ManagerTest extends \Test\TestCase {
 			->method('pathCreateChecks')
 			->with($path);
 		$manager->expects($this->never())
-			->method('validateExpirationDate');
+			->method('validateExpirationDateLink');
 		$manager->expects($this->never())
 			->method('verifyPassword');
 		$manager->expects($this->never())
@@ -3008,7 +3008,7 @@ class ManagerTest extends \Test\TestCase {
 				'linkCreateChecks',
 				'pathCreateChecks',
 				'verifyPassword',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3037,7 +3037,7 @@ class ManagerTest extends \Test\TestCase {
 
 		$manager->expects($this->once())->method('canShare')->willReturn(true);
 		$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
-		$manager->expects($this->once())->method('validateExpirationDate')->with($share);
+		$manager->expects($this->once())->method('validateExpirationDateLink')->with($share);
 		$manager->expects($this->once())->method('verifyPassword')->with('password');
 
 		$this->hasher->expects($this->once())
@@ -3089,7 +3089,7 @@ class ManagerTest extends \Test\TestCase {
 				'linkCreateChecks',
 				'pathCreateChecks',
 				'verifyPassword',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3123,7 +3123,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->once())->method('linkCreateChecks')->with($share);
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->never())
 			->method('hash');
@@ -3155,7 +3155,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3188,7 +3188,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->once())->method('verifyPassword')->with('password');
 		$manager->expects($this->once())->method('pathCreateChecks')->with($file);
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->once())
 			->method('hash')
@@ -3230,7 +3230,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3266,7 +3266,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->once())->method('verifyPassword')->with('password');
 		$manager->expects($this->once())->method('pathCreateChecks')->with($file);
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->once())
 			->method('hash')
@@ -3308,7 +3308,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3344,7 +3344,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->once())->method('verifyPassword')->with('password');
 		$manager->expects($this->once())->method('pathCreateChecks')->with($file);
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->once())
 			->method('verify')
@@ -3394,7 +3394,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3430,7 +3430,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->never())
 			->method('hash');
@@ -3466,7 +3466,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3502,7 +3502,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->never())
 			->method('hash');
@@ -3538,7 +3538,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3574,7 +3574,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->never())
 			->method('hash');
@@ -3610,7 +3610,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3646,7 +3646,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->once())
 			->method('verify')
@@ -3686,7 +3686,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3722,7 +3722,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->once())
 			->method('verify')
@@ -3762,7 +3762,7 @@ class ManagerTest extends \Test\TestCase {
 				'verifyPassword',
 				'pathCreateChecks',
 				'linkCreateChecks',
-				'validateExpirationDate',
+				'validateExpirationDateLink',
 			])
 			->getMock();
 
@@ -3798,7 +3798,7 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('verifyPassword');
 		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
-		$manager->expects($this->never())->method('validateExpirationDate');
+		$manager->expects($this->never())->method('validateExpirationDateLink');
 
 		$this->hasher->expects($this->never())
 			->method('verify');