From a9e3d2a10fe4926691065fe9a76e48e6a7e32a08 Mon Sep 17 00:00:00 2001 From: Vincent Petry 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 --- 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 ce1ec1d60f..1c875ad0c0 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -459,7 +459,7 @@ class Manager implements IManager { * @throws \InvalidArgumentException * @throws \Exception */ - protected function validateExpirationDate(IShare $share) { + protected function validateExpirationDateLink(IShare $share) { $expirationDate = $share->getExpirationDate(); if ($expirationDate !== null) { @@ -763,7 +763,7 @@ class Manager implements IManager { ); //Verify the expiration date - $share = $this->validateExpirationDate($share); + $share = $this->validateExpirationDateLink($share); //Verify the password $this->verifyPassword($share->getPassword()); @@ -975,7 +975,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() === IShare::TYPE_GROUP) { @@ -983,7 +983,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() === IShare::TYPE_LINK) { @@ -999,7 +999,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() === IShare::TYPE_EMAIL) { diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 372f49ddf8..fefbbc2b2a 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1067,7 +1067,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() { @@ -1083,7 +1083,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() { @@ -1095,7 +1095,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()); } @@ -1115,7 +1115,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()); @@ -1136,7 +1136,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()); @@ -1159,7 +1159,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() { @@ -1186,7 +1186,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()); } @@ -1208,7 +1208,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()); } @@ -1223,7 +1223,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()); } @@ -1248,7 +1248,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()); } @@ -1277,7 +1277,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()); } @@ -1298,7 +1298,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()); @@ -1322,7 +1322,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() { @@ -1336,7 +1336,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()); } @@ -2040,7 +2040,7 @@ class ManagerTest extends \Test\TestCase { 'generalCreateChecks', 'linkCreateChecks', 'pathCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', 'verifyPassword', 'setLinkParent', ]) @@ -2082,7 +2082,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()) @@ -2165,7 +2165,7 @@ class ManagerTest extends \Test\TestCase { 'generalCreateChecks', 'linkCreateChecks', 'pathCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', 'verifyPassword', 'setLinkParent', ]) @@ -2201,7 +2201,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()) @@ -3015,7 +3015,7 @@ class ManagerTest extends \Test\TestCase { 'linkCreateChecks', 'pathCreateChecks', 'verifyPassword', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3044,7 +3044,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()) @@ -3096,7 +3096,7 @@ class ManagerTest extends \Test\TestCase { 'linkCreateChecks', 'pathCreateChecks', 'verifyPassword', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3130,7 +3130,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'); @@ -3162,7 +3162,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3195,7 +3195,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') @@ -3237,7 +3237,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3273,7 +3273,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') @@ -3315,7 +3315,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3351,7 +3351,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') @@ -3401,7 +3401,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3437,7 +3437,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'); @@ -3473,7 +3473,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3509,7 +3509,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'); @@ -3545,7 +3545,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3581,7 +3581,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'); @@ -3617,7 +3617,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3653,7 +3653,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') @@ -3693,7 +3693,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3729,7 +3729,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') @@ -3769,7 +3769,7 @@ class ManagerTest extends \Test\TestCase { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3805,7 +3805,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');