Merge pull request #22162 from nextcloud/enh/noid/password-generator-sharebymail

ShareByMail: Migrate to GenerateSecurePasswordEvent
This commit is contained in:
Morris Jobke 2020-08-11 21:59:21 +02:00 committed by GitHub
commit 138f47a1b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 54 deletions

View File

@ -36,7 +36,6 @@
namespace OCA\ShareByMail; namespace OCA\ShareByMail;
use OC\CapabilitiesManager;
use OC\HintException; use OC\HintException;
use OC\Share20\Exception\InvalidShare; use OC\Share20\Exception\InvalidShare;
use OC\Share20\Share; use OC\Share20\Share;
@ -45,6 +44,7 @@ use OCA\ShareByMail\Settings\SettingsManager;
use OCP\Activity\IManager; use OCP\Activity\IManager;
use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Defaults; use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Folder; use OCP\Files\Folder;
use OCP\Files\IRootFolder; use OCP\Files\IRootFolder;
use OCP\Files\Node; use OCP\Files\Node;
@ -55,6 +55,7 @@ use OCP\IURLGenerator;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\Mail\IMailer; use OCP\Mail\IMailer;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher; use OCP\Security\IHasher;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\GenericShareException;
@ -105,8 +106,8 @@ class ShareByMailProvider implements IShareProvider {
/** @var IHasher */ /** @var IHasher */
private $hasher; private $hasher;
/** @var CapabilitiesManager */ /** @var IEventDispatcher */
private $capabilitiesManager; private $eventDispatcher;
/** /**
* Return the identifier of this provider. * Return the identifier of this provider.
@ -117,23 +118,6 @@ class ShareByMailProvider implements IShareProvider {
return 'ocMailShare'; return 'ocMailShare';
} }
/**
* DefaultShareProvider constructor.
*
* @param IDBConnection $connection
* @param ISecureRandom $secureRandom
* @param IUserManager $userManager
* @param IRootFolder $rootFolder
* @param IL10N $l
* @param ILogger $logger
* @param IMailer $mailer
* @param IURLGenerator $urlGenerator
* @param IManager $activityManager
* @param SettingsManager $settingsManager
* @param Defaults $defaults
* @param IHasher $hasher
* @param CapabilitiesManager $capabilitiesManager
*/
public function __construct( public function __construct(
IDBConnection $connection, IDBConnection $connection,
ISecureRandom $secureRandom, ISecureRandom $secureRandom,
@ -147,7 +131,7 @@ class ShareByMailProvider implements IShareProvider {
SettingsManager $settingsManager, SettingsManager $settingsManager,
Defaults $defaults, Defaults $defaults,
IHasher $hasher, IHasher $hasher,
CapabilitiesManager $capabilitiesManager IEventDispatcher $eventDispatcher
) { ) {
$this->dbConnection = $connection; $this->dbConnection = $connection;
$this->secureRandom = $secureRandom; $this->secureRandom = $secureRandom;
@ -161,7 +145,7 @@ class ShareByMailProvider implements IShareProvider {
$this->settingsManager = $settingsManager; $this->settingsManager = $settingsManager;
$this->defaults = $defaults; $this->defaults = $defaults;
$this->hasher = $hasher; $this->hasher = $hasher;
$this->capabilitiesManager = $capabilitiesManager; $this->eventDispatcher = $eventDispatcher;
} }
/** /**
@ -227,33 +211,17 @@ class ShareByMailProvider implements IShareProvider {
); );
} }
$passwordPolicy = $this->getPasswordPolicy(); $passwordEvent = new GenerateSecurePasswordEvent();
$passwordCharset = ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS; $this->eventDispatcher->dispatchTyped($passwordEvent);
$passwordLength = 8;
if (!empty($passwordPolicy)) {
$passwordLength = (int)$passwordPolicy['minLength'] > 0 ? (int)$passwordPolicy['minLength'] : $passwordLength;
$passwordCharset .= $passwordPolicy['enforceSpecialCharacters'] ? ISecureRandom::CHAR_SYMBOLS : '';
}
$password = $this->secureRandom->generate($passwordLength, $passwordCharset); $password = $passwordEvent->getPassword();
if ($password === null) {
$password = $this->secureRandom->generate(8, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS);
}
return $password; return $password;
} }
/**
* get password policy
*
* @return array
*/
protected function getPasswordPolicy() {
$capabilities = $this->capabilitiesManager->getCapabilities();
if (isset($capabilities['password_policy'])) {
return $capabilities['password_policy'];
}
return [];
}
/** /**
* create activity if a file/folder was shared by mail * create activity if a file/folder was shared by mail
* *

View File

@ -30,11 +30,11 @@
namespace OCA\ShareByMail\Tests; namespace OCA\ShareByMail\Tests;
use OC\CapabilitiesManager;
use OC\Mail\Message; use OC\Mail\Message;
use OCA\ShareByMail\Settings\SettingsManager; use OCA\ShareByMail\Settings\SettingsManager;
use OCA\ShareByMail\ShareByMailProvider; use OCA\ShareByMail\ShareByMailProvider;
use OCP\Defaults; use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\File; use OCP\Files\File;
use OCP\Files\IRootFolder; use OCP\Files\IRootFolder;
use OCP\IDBConnection; use OCP\IDBConnection;
@ -46,10 +46,12 @@ use OCP\IUserManager;
use OCP\Mail\IEMailTemplate; use OCP\Mail\IEMailTemplate;
use OCP\Mail\IMailer; use OCP\Mail\IMailer;
use OCP\Mail\IMessage; use OCP\Mail\IMessage;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\IHasher; use OCP\Security\IHasher;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use OCP\Share\IManager; use OCP\Share\IManager;
use OCP\Share\IShare; use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase; use Test\TestCase;
/** /**
@ -102,8 +104,8 @@ class ShareByMailProviderTest extends TestCase {
/** @var IHasher | \PHPUnit_Framework_MockObject_MockObject */ /** @var IHasher | \PHPUnit_Framework_MockObject_MockObject */
private $hasher; private $hasher;
/** @var CapabilitiesManager | \PHPUnit_Framework_MockObject_MockObject */ /** @var IEventDispatcher */
private $capabilitiesManager; private $eventDispatcher;
protected function setUp(): void { protected function setUp(): void {
parent::setUp(); parent::setUp();
@ -127,7 +129,7 @@ class ShareByMailProviderTest extends TestCase {
$this->settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); $this->settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock();
$this->defaults = $this->createMock(Defaults::class); $this->defaults = $this->createMock(Defaults::class);
$this->hasher = $this->getMockBuilder(IHasher::class)->getMock(); $this->hasher = $this->getMockBuilder(IHasher::class)->getMock();
$this->capabilitiesManager = $this->getMockBuilder(CapabilitiesManager::class)->disableOriginalConstructor()->getMock(); $this->eventDispatcher = $this->getMockBuilder(IEventDispatcher::class)->getMock();
$this->userManager->expects($this->any())->method('userExists')->willReturn(true); $this->userManager->expects($this->any())->method('userExists')->willReturn(true);
} }
@ -154,7 +156,7 @@ class ShareByMailProviderTest extends TestCase {
$this->settingsManager, $this->settingsManager,
$this->defaults, $this->defaults,
$this->hasher, $this->hasher,
$this->capabilitiesManager $this->eventDispatcher
] ]
); );
@ -176,7 +178,7 @@ class ShareByMailProviderTest extends TestCase {
$this->settingsManager, $this->settingsManager,
$this->defaults, $this->defaults,
$this->hasher, $this->hasher,
$this->capabilitiesManager $this->eventDispatcher
); );
} }
@ -294,7 +296,15 @@ class ShareByMailProviderTest extends TestCase {
$node = $this->getMockBuilder(File::class)->getMock(); $node = $this->getMockBuilder(File::class)->getMock();
$node->expects($this->any())->method('getName')->willReturn('filename'); $node->expects($this->any())->method('getName')->willReturn('filename');
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']); $this->secureRandom->expects($this->once())
->method('generate')
->with(8, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS)
->willReturn('autogeneratedPassword');
$this->eventDispatcher->expects($this->once())
->method('dispatchTyped')
->with(new GenerateSecurePasswordEvent());
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'createPasswordSendActivity']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]); $instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
@ -310,7 +320,6 @@ class ShareByMailProviderTest extends TestCase {
// The autogenerated password should be mailed to the receiver of the share. // The autogenerated password should be mailed to the receiver of the share.
$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true); $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
$message = $this->createMock(IMessage::class); $message = $this->createMock(IMessage::class);
$message->expects($this->once())->method('setTo')->with(['receiver@example.com']); $message->expects($this->once())->method('setTo')->with(['receiver@example.com']);

View File

@ -30,7 +30,6 @@
namespace OC\Share20; namespace OC\Share20;
use OC\CapabilitiesManager;
use OC\Share20\Exception\ProviderException; use OC\Share20\Exception\ProviderException;
use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\AddressHandler;
use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\FederatedFileSharing\FederatedShareProvider;
@ -184,7 +183,7 @@ class ProviderFactory implements IProviderFactory {
$settingsManager, $settingsManager,
$this->serverContainer->query(Defaults::class), $this->serverContainer->query(Defaults::class),
$this->serverContainer->getHasher(), $this->serverContainer->getHasher(),
$this->serverContainer->query(CapabilitiesManager::class) $this->serverContainer->get(IEventDispatcher::class)
); );
} }