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 d45a376b71972..552af9ad5169d 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -459,7 +459,7 @@ protected function validateExpirationDateInternal(IShare $share) { * @throws \InvalidArgumentException * @throws \Exception */ - protected function validateExpirationDate(IShare $share) { + protected function validateExpirationDateLink(IShare $share) { $expirationDate = $share->getExpirationDate(); if ($expirationDate !== null) { @@ -762,7 +762,7 @@ public function createShare(IShare $share) { ); //Verify the expiration date - $share = $this->validateExpirationDate($share); + $share = $this->validateExpirationDateLink($share); //Verify the password $this->verifyPassword($share->getPassword()); @@ -974,7 +974,7 @@ public function updateShare(IShare $share) { 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 @@ public function updateShare(IShare $share) { 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 @@ public function updateShare(IShare $share) { 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 372f49ddf87ee..fefbbc2b2ae3f 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1067,7 +1067,7 @@ public function testValidateExpirationDateInPast() { $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 @@ public function testValidateExpirationDateEnforceButNotSet() { ['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 @@ public function testValidateExpirationDateEnforceButNotEnabledAndNotSet() { ['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 @@ public function testValidateExpirationDateEnforceButNotSetNewShare() { $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 @@ public function testValidateExpirationDateEnforceRelaxedDefaultButNotSetNewShare $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 @@ public function testValidateExpirationDateEnforceTooFarIntoFuture() { ['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 @@ public function testValidateExpirationDateEnforceValid() { 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 @@ public function testValidateExpirationDateNoDefault() { 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 @@ public function testValidateExpirationDateNoDateNoDefault() { $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 @@ public function testValidateExpirationDateNoDateDefault() { 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 @@ public function testValidateExpirationDateDefault() { 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 @@ public function testValidateExpirationDateHookModification() { $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 @@ public function testValidateExpirationDateHookException() { $data['message'] = 'Invalid date!'; }); - self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); + self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]); } public function testValidateExpirationDateExistingShareNoDefault() { @@ -1336,7 +1336,7 @@ public function testValidateExpirationDateExistingShareNoDefault() { ['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 @@ public function testCreateShareLink() { 'generalCreateChecks', 'linkCreateChecks', 'pathCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', 'verifyPassword', 'setLinkParent', ]) @@ -2082,7 +2082,7 @@ public function testCreateShareLink() { ->method('pathCreateChecks') ->with($path); $manager->expects($this->once()) - ->method('validateExpirationDate') + ->method('validateExpirationDateLink') ->with($share) ->willReturn($share); $manager->expects($this->once()) @@ -2165,7 +2165,7 @@ public function testCreateShareMail() { 'generalCreateChecks', 'linkCreateChecks', 'pathCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', 'verifyPassword', 'setLinkParent', ]) @@ -2201,7 +2201,7 @@ public function testCreateShareMail() { ->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 @@ public function testUpdateShareLink() { 'linkCreateChecks', 'pathCreateChecks', 'verifyPassword', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3044,7 +3044,7 @@ public function testUpdateShareLink() { $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 @@ public function testUpdateShareLinkEnableSendPasswordByTalkWithNoPassword() { 'linkCreateChecks', 'pathCreateChecks', 'verifyPassword', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3130,7 +3130,7 @@ public function testUpdateShareLinkEnableSendPasswordByTalkWithNoPassword() { $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 @@ public function testUpdateShareMail() { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3195,7 +3195,7 @@ public function testUpdateShareMail() { $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 @@ public function testUpdateShareMailEnableSendPasswordByTalk() { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3273,7 +3273,7 @@ public function testUpdateShareMailEnableSendPasswordByTalk() { $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 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithDifferentPassword 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3351,7 +3351,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithDifferentPassword $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 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword() { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3437,7 +3437,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword() { $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 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword() { 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3509,7 +3509,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword() { $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 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithE 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3581,7 +3581,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithE $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 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword( 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3653,7 +3653,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword( $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 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3729,7 +3729,7 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword $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 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassw 'verifyPassword', 'pathCreateChecks', 'linkCreateChecks', - 'validateExpirationDate', + 'validateExpirationDateLink', ]) ->getMock(); @@ -3805,7 +3805,7 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassw $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');