Fix disabling send password by Talk without new password in mail shares
When "send password by Talk" was disabled in a mail share it was possible to keep the same password as before, as it does not pose any security issue (unlike keeping it when "send password by Talk" is enabled, as in that case the password was already disclosed by mail). However, if a mail share is updated but the password is not set again only the hashed password will be available. In that case it would not make sense to send the password by mail, so now the password must be changed when disabling "send password by Talk". Note that, even if explicitly setting the same password again along with the "send password by Talk" property would work, this was also prevented for simplicity. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
parent
8e5aa03834
commit
6ca312eec9
|
@ -983,11 +983,9 @@ class Manager implements IManager {
|
|||
}
|
||||
} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) {
|
||||
// The new password is not set again if it is the same as the old
|
||||
// one, unless when switching from sending by Talk to sending by
|
||||
// mail.
|
||||
// one.
|
||||
$plainTextPassword = $share->getPassword();
|
||||
if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare) &&
|
||||
!($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk())) {
|
||||
if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare)) {
|
||||
$plainTextPassword = null;
|
||||
}
|
||||
if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) {
|
||||
|
@ -995,6 +993,8 @@ class Manager implements IManager {
|
|||
// would already have access to the share without having to call
|
||||
// the sharer to verify her identity
|
||||
throw new \InvalidArgumentException('Can’t enable sending the password by Talk without setting a new password');
|
||||
} elseif (empty($plainTextPassword) && $originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()) {
|
||||
throw new \InvalidArgumentException('Can’t disable sending the password by Talk without setting a new password');
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -3339,6 +3339,9 @@ class ManagerTest extends \Test\TestCase {
|
|||
}
|
||||
|
||||
public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword() {
|
||||
$this->expectException(\InvalidArgumentException::class);
|
||||
$this->expectExceptionMessage('Can’t disable sending the password by Talk without setting a new password');
|
||||
|
||||
$manager = $this->createManagerMock()
|
||||
->setMethods([
|
||||
'canShare',
|
||||
|
@ -3380,8 +3383,8 @@ 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('generalCreateChecks')->with($share);
|
||||
$manager->expects($this->once())->method('pathCreateChecks')->with($file);
|
||||
$manager->expects($this->never())->method('verifyPassword');
|
||||
$manager->expects($this->never())->method('pathCreateChecks');
|
||||
$manager->expects($this->never())->method('linkCreateChecks');
|
||||
$manager->expects($this->never())->method('validateExpirationDate');
|
||||
|
||||
|
@ -3393,10 +3396,8 @@ class ManagerTest extends \Test\TestCase {
|
|||
$this->hasher->expects($this->never())
|
||||
->method('hash');
|
||||
|
||||
$this->defaultProvider->expects($this->once())
|
||||
->method('update')
|
||||
->with($share, 'password')
|
||||
->willReturn($share);
|
||||
$this->defaultProvider->expects($this->never())
|
||||
->method('update');
|
||||
|
||||
$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
|
||||
\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
|
||||
|
@ -3413,6 +3414,79 @@ class ManagerTest extends \Test\TestCase {
|
|||
$manager->updateShare($share);
|
||||
}
|
||||
|
||||
public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassword() {
|
||||
$this->expectException(\InvalidArgumentException::class);
|
||||
$this->expectExceptionMessage('Can’t disable sending the password by Talk without setting a new password');
|
||||
|
||||
$manager = $this->createManagerMock()
|
||||
->setMethods([
|
||||
'canShare',
|
||||
'getShareById',
|
||||
'generalCreateChecks',
|
||||
'verifyPassword',
|
||||
'pathCreateChecks',
|
||||
'linkCreateChecks',
|
||||
'validateExpirationDate',
|
||||
])
|
||||
->getMock();
|
||||
|
||||
$originalShare = $this->manager->newShare();
|
||||
$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
|
||||
->setPermissions(\OCP\Constants::PERMISSION_ALL)
|
||||
->setPassword('passwordHash')
|
||||
->setSendPasswordByTalk(true);
|
||||
|
||||
$tomorrow = new \DateTime();
|
||||
$tomorrow->setTime(0,0,0);
|
||||
$tomorrow->add(new \DateInterval('P1D'));
|
||||
|
||||
$file = $this->createMock(File::class);
|
||||
$file->method('getId')->willReturn(100);
|
||||
|
||||
$share = $this->manager->newShare();
|
||||
$share->setProviderId('foo')
|
||||
->setId('42')
|
||||
->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
|
||||
->setToken('token')
|
||||
->setSharedBy('owner')
|
||||
->setShareOwner('owner')
|
||||
->setPassword('passwordHash')
|
||||
->setSendPasswordByTalk(false)
|
||||
->setExpirationDate($tomorrow)
|
||||
->setNode($file)
|
||||
->setPermissions(\OCP\Constants::PERMISSION_ALL);
|
||||
|
||||
$manager->expects($this->once())->method('canShare')->willReturn(true);
|
||||
$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
|
||||
$manager->expects($this->once())->method('generalCreateChecks')->with($share);
|
||||
$manager->expects($this->never())->method('verifyPassword');
|
||||
$manager->expects($this->never())->method('pathCreateChecks');
|
||||
$manager->expects($this->never())->method('linkCreateChecks');
|
||||
$manager->expects($this->never())->method('validateExpirationDate');
|
||||
|
||||
$this->hasher->expects($this->never())
|
||||
->method('verify');
|
||||
|
||||
$this->hasher->expects($this->never())
|
||||
->method('hash');
|
||||
|
||||
$this->defaultProvider->expects($this->never())
|
||||
->method('update');
|
||||
|
||||
$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
|
||||
\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
|
||||
$hookListner->expects($this->never())->method('post');
|
||||
|
||||
$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
|
||||
\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
|
||||
$hookListner2->expects($this->never())->method('post');
|
||||
|
||||
$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
|
||||
\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
|
||||
$hookListner3->expects($this->never())->method('post');
|
||||
|
||||
$manager->updateShare($share);
|
||||
}
|
||||
|
||||
public function testMoveShareLink() {
|
||||
$this->expectException(\InvalidArgumentException::class);
|
||||
|
|
Loading…
Reference in New Issue