mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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>
This commit is contained in:
Vincent Petry 2021-03-18 17:12:28 +01:00 committed by backportbot[bot]
parent 682df0b788
commit 695ccecf26
2 changed files with 45 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/private/Share20/Manager.php
View File

@ -459,7 +459,7 @@ class Manager implements IManager {
* @throws \InvalidArgumentException * @throws \InvalidArgumentException
* @throws \Exception * @throws \Exception
*/ */
protected function validateExpirationDate(IShare $share) { protected function validateExpirationDateLink(IShare $share) {
$expirationDate = $share->getExpirationDate(); $expirationDate = $share->getExpirationDate();
if ($expirationDate !== null) { if ($expirationDate !== null) {
@ -762,7 +762,7 @@ class Manager implements IManager {
); );
//Verify the expiration date //Verify the expiration date
$share = $this->validateExpirationDate($share); $share = $this->validateExpirationDateLink($share);
//Verify the password //Verify the password
$this->verifyPassword($share->getPassword()); $this->verifyPassword($share->getPassword());
@ -974,7 +974,7 @@ class Manager implements IManager {
if ($share->getExpirationDate() != $originalShare->getExpirationDate()) { if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
//Verify the expiration date //Verify the expiration date
$this->validateExpirationDate($share); $this->validateExpirationDateInternal($share);
$expirationDateUpdated = true; $expirationDateUpdated = true;
} }
} elseif ($share->getShareType() === IShare::TYPE_GROUP) { } elseif ($share->getShareType() === IShare::TYPE_GROUP) {
@ -982,7 +982,7 @@ class Manager implements IManager {
if ($share->getExpirationDate() != $originalShare->getExpirationDate()) { if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
//Verify the expiration date //Verify the expiration date
$this->validateExpirationDate($share); $this->validateExpirationDateInternal($share);
$expirationDateUpdated = true; $expirationDateUpdated = true;
} }
} elseif ($share->getShareType() === IShare::TYPE_LINK) { } elseif ($share->getShareType() === IShare::TYPE_LINK) {
@ -998,7 +998,7 @@ class Manager implements IManager {
if ($share->getExpirationDate() != $originalShare->getExpirationDate()) { if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
//Verify the expiration date //Verify the expiration date
$this->validateExpirationDate($share); $this->validateExpirationDateLink($share);
$expirationDateUpdated = true; $expirationDateUpdated = true;
} }
} elseif ($share->getShareType() === IShare::TYPE_EMAIL) { } elseif ($share->getShareType() === IShare::TYPE_EMAIL) {

80
tests/lib/Share20/ManagerTest.php
View File

@ -1067,7 +1067,7 @@ class ManagerTest extends \Test\TestCase {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
$share->setExpirationDate($past); $share->setExpirationDate($past);
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
} }
public function testValidateExpirationDateEnforceButNotSet() { public function testValidateExpirationDateEnforceButNotSet() {
@ -1083,7 +1083,7 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_enforce_expire_date', 'no', 'yes'], ['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
]); ]);
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
} }
public function testValidateExpirationDateEnforceButNotEnabledAndNotSet() { public function testValidateExpirationDateEnforceButNotEnabledAndNotSet() {
@ -1095,7 +1095,7 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_enforce_expire_date', 'no', 'yes'], ['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
]); ]);
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertNull($share->getExpirationDate()); $this->assertNull($share->getExpirationDate());
} }
@ -1115,7 +1115,7 @@ class ManagerTest extends \Test\TestCase {
$expected->setTime(0,0,0); $expected->setTime(0,0,0);
$expected->add(new \DateInterval('P3D')); $expected->add(new \DateInterval('P3D'));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertNotNull($share->getExpirationDate()); $this->assertNotNull($share->getExpirationDate());
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
@ -1136,7 +1136,7 @@ class ManagerTest extends \Test\TestCase {
$expected->setTime(0,0,0); $expected->setTime(0,0,0);
$expected->add(new \DateInterval('P1D')); $expected->add(new \DateInterval('P1D'));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertNotNull($share->getExpirationDate()); $this->assertNotNull($share->getExpirationDate());
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
@ -1159,7 +1159,7 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_default_expire_date', 'no', 'yes'], ['core', 'shareapi_default_expire_date', 'no', 'yes'],
]); ]);
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
} }
public function testValidateExpirationDateEnforceValid() { public function testValidateExpirationDateEnforceValid() {
@ -1186,7 +1186,7 @@ class ManagerTest extends \Test\TestCase {
return $data['expirationDate'] == $future; return $data['expirationDate'] == $future;
})); }));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
} }
@ -1208,7 +1208,7 @@ class ManagerTest extends \Test\TestCase {
return $data['expirationDate'] == $expected && $data['passwordSet'] === false; return $data['expirationDate'] == $expected && $data['passwordSet'] === false;
})); }));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
} }
@ -1223,7 +1223,7 @@ class ManagerTest extends \Test\TestCase {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
$share->setPassword('password'); $share->setPassword('password');
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertNull($share->getExpirationDate()); $this->assertNull($share->getExpirationDate());
} }
@ -1248,7 +1248,7 @@ class ManagerTest extends \Test\TestCase {
return $data['expirationDate'] == $expected; return $data['expirationDate'] == $expected;
})); }));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
} }
@ -1277,7 +1277,7 @@ class ManagerTest extends \Test\TestCase {
return $data['expirationDate'] == $expected; return $data['expirationDate'] == $expected;
})); }));
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$this->assertEquals($expected, $share->getExpirationDate()); $this->assertEquals($expected, $share->getExpirationDate());
} }
@ -1298,7 +1298,7 @@ class ManagerTest extends \Test\TestCase {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
$share->setExpirationDate($nextWeek); $share->setExpirationDate($nextWeek);
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
$save->sub(new \DateInterval('P2D')); $save->sub(new \DateInterval('P2D'));
$this->assertEquals($save, $share->getExpirationDate()); $this->assertEquals($save, $share->getExpirationDate());
@ -1322,7 +1322,7 @@ class ManagerTest extends \Test\TestCase {
$data['message'] = 'Invalid date!'; $data['message'] = 'Invalid date!';
}); });
self::invokePrivate($this->manager, 'validateExpirationDate', [$share]); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
} }
public function testValidateExpirationDateExistingShareNoDefault() { public function testValidateExpirationDateExistingShareNoDefault() {
@ -1336,7 +1336,7 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_expire_after_n_days', '7', '6'], ['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()); $this->assertEquals(null, $share->getExpirationDate());
} }
@ -2040,7 +2040,7 @@ class ManagerTest extends \Test\TestCase {
'generalCreateChecks', 'generalCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
'verifyPassword', 'verifyPassword',
'setLinkParent', 'setLinkParent',
]) ])
@ -2082,7 +2082,7 @@ class ManagerTest extends \Test\TestCase {
->method('pathCreateChecks') ->method('pathCreateChecks')
->with($path); ->with($path);
$manager->expects($this->once()) $manager->expects($this->once())
->method('validateExpirationDate') ->method('validateExpirationDateLink')
->with($share) ->with($share)
->willReturn($share); ->willReturn($share);
$manager->expects($this->once()) $manager->expects($this->once())
@ -2165,7 +2165,7 @@ class ManagerTest extends \Test\TestCase {
'generalCreateChecks', 'generalCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
'verifyPassword', 'verifyPassword',
'setLinkParent', 'setLinkParent',
]) ])
@ -2201,7 +2201,7 @@ class ManagerTest extends \Test\TestCase {
->method('pathCreateChecks') ->method('pathCreateChecks')
->with($path); ->with($path);
$manager->expects($this->never()) $manager->expects($this->never())
->method('validateExpirationDate'); ->method('validateExpirationDateLink');
$manager->expects($this->never()) $manager->expects($this->never())
->method('verifyPassword'); ->method('verifyPassword');
$manager->expects($this->never()) $manager->expects($this->never())
@ -3015,7 +3015,7 @@ class ManagerTest extends \Test\TestCase {
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'verifyPassword', 'verifyPassword',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3044,7 +3044,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('canShare')->willReturn(true);
$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $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'); $manager->expects($this->once())->method('verifyPassword')->with('password');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
@ -3096,7 +3096,7 @@ class ManagerTest extends \Test\TestCase {
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'verifyPassword', 'verifyPassword',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3130,7 +3130,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('linkCreateChecks')->with($share); $manager->expects($this->once())->method('linkCreateChecks')->with($share);
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->never()) $this->hasher->expects($this->never())
->method('hash'); ->method('hash');
@ -3162,7 +3162,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3195,7 +3195,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('verifyPassword')->with('password'); $manager->expects($this->once())->method('verifyPassword')->with('password');
$manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('pathCreateChecks')->with($file);
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
->method('hash') ->method('hash')
@ -3237,7 +3237,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3273,7 +3273,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('verifyPassword')->with('password'); $manager->expects($this->once())->method('verifyPassword')->with('password');
$manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('pathCreateChecks')->with($file);
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
->method('hash') ->method('hash')
@ -3315,7 +3315,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3351,7 +3351,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('verifyPassword')->with('password'); $manager->expects($this->once())->method('verifyPassword')->with('password');
$manager->expects($this->once())->method('pathCreateChecks')->with($file); $manager->expects($this->once())->method('pathCreateChecks')->with($file);
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
->method('verify') ->method('verify')
@ -3401,7 +3401,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3437,7 +3437,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->never()) $this->hasher->expects($this->never())
->method('hash'); ->method('hash');
@ -3473,7 +3473,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3509,7 +3509,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->never()) $this->hasher->expects($this->never())
->method('hash'); ->method('hash');
@ -3545,7 +3545,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3581,7 +3581,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->never()) $this->hasher->expects($this->never())
->method('hash'); ->method('hash');
@ -3617,7 +3617,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3653,7 +3653,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
->method('verify') ->method('verify')
@ -3693,7 +3693,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3729,7 +3729,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->once()) $this->hasher->expects($this->once())
->method('verify') ->method('verify')
@ -3769,7 +3769,7 @@ class ManagerTest extends \Test\TestCase {
'verifyPassword', 'verifyPassword',
'pathCreateChecks', 'pathCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'validateExpirationDate', 'validateExpirationDateLink',
]) ])
->getMock(); ->getMock();
@ -3805,7 +3805,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->never())->method('verifyPassword'); $manager->expects($this->never())->method('verifyPassword');
$manager->expects($this->never())->method('pathCreateChecks'); $manager->expects($this->never())->method('pathCreateChecks');
$manager->expects($this->never())->method('linkCreateChecks'); $manager->expects($this->never())->method('linkCreateChecks');
$manager->expects($this->never())->method('validateExpirationDate'); $manager->expects($this->never())->method('validateExpirationDateLink');
$this->hasher->expects($this->never()) $this->hasher->expects($this->never())
->method('verify'); ->method('verify');