From 695ccecf2643fa25fa5fd062f463438d453580ec 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 d45a376b71..552af9ad51 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) { @@ -762,7 +762,7 @@ class Manager implements IManager { ); //Verify the expiration date - $share = $this->validateExpirationDate($share); + $share = $this->validateExpirationDateLink($share); //Verify the password $this->verifyPassword($share->getPassword()); @@ -974,7 +974,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) { @@ -982,7 +982,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) { @@ -998,7 +998,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');