From d8dcd72118f7f5f83874e3a54a7fbc02bda10ce6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 10 Apr 2017 18:36:23 +0200 Subject: [PATCH 1/9] allow admin to enforce password on mail shares Signed-off-by: Bjoern Schiessle --- apps/sharebymail/css/settings-admin.css | 3 + apps/sharebymail/js/settings-admin.js | 10 +- apps/sharebymail/lib/AppInfo/Application.php | 4 +- apps/sharebymail/lib/Settings.php | 15 ++ apps/sharebymail/lib/Settings/Admin.php | 3 +- .../lib/Settings/SettingsManager.php | 16 ++- apps/sharebymail/lib/ShareByMailProvider.php | 134 +++++++++++++++--- .../templates/altmailpasswordowner.php | 32 +++++ .../templates/mailpasswordowner.php | 59 ++++++++ apps/sharebymail/templates/settings-admin.php | 5 +- apps/sharebymail/tests/SettingsTest.php | 34 ++++- .../tests/ShareByMailProviderTest.php | 28 +++- core/js/shareconfigmodel.js | 1 + core/js/sharedialogshareelistview.js | 3 +- lib/private/Share20/ProviderFactory.php | 3 +- 15 files changed, 318 insertions(+), 32 deletions(-) create mode 100644 apps/sharebymail/css/settings-admin.css create mode 100644 apps/sharebymail/templates/altmailpasswordowner.php create mode 100644 apps/sharebymail/templates/mailpasswordowner.php diff --git a/apps/sharebymail/css/settings-admin.css b/apps/sharebymail/css/settings-admin.css new file mode 100644 index 0000000000..c7bfb122f3 --- /dev/null +++ b/apps/sharebymail/css/settings-admin.css @@ -0,0 +1,3 @@ +#ncShareByMailSettings p { + padding-top: 10px; +} diff --git a/apps/sharebymail/js/settings-admin.js b/apps/sharebymail/js/settings-admin.js index 7b43123303..35a0e9855a 100644 --- a/apps/sharebymail/js/settings-admin.js +++ b/apps/sharebymail/js/settings-admin.js @@ -24,7 +24,15 @@ $(function() { if ($(this).is(':checked')) { status = 'yes'; } - OC.AppConfig.setValue('sharebymail', 'sendpasswordmail', status); + OCP.AppConfig.setValue('sharebymail', 'sendpasswordmail', status); + }); + + $('#enforcePasswordProtection').on('change', function() { + var status = 'no'; + if ($(this).is(':checked')) { + status = 'yes'; + } + OCP.AppConfig.setValue('sharebymail', 'enforcePasswordProtection', status); }); }); diff --git a/apps/sharebymail/lib/AppInfo/Application.php b/apps/sharebymail/lib/AppInfo/Application.php index 98febf9dad..12419a8c3d 100644 --- a/apps/sharebymail/lib/AppInfo/Application.php +++ b/apps/sharebymail/lib/AppInfo/Application.php @@ -32,7 +32,8 @@ class Application extends App { public function __construct(array $urlParams = array()) { parent::__construct('sharebymail', $urlParams); - $settings = new Settings(); + $settingsManager = \OC::$server->query(Settings\SettingsManager::class); + $settings = new Settings($settingsManager); /** register capabilities */ $container = $this->getContainer(); @@ -40,6 +41,7 @@ class Application extends App { /** register hooks */ Util::connectHook('\OCP\Config', 'js', $settings, 'announceShareProvider'); + Util::connectHook('\OCP\Config', 'js', $settings, 'announceShareByMailSettings'); } } diff --git a/apps/sharebymail/lib/Settings.php b/apps/sharebymail/lib/Settings.php index 4ab1622425..e032bc43ff 100644 --- a/apps/sharebymail/lib/Settings.php +++ b/apps/sharebymail/lib/Settings.php @@ -23,8 +23,17 @@ namespace OCA\ShareByMail; +use OCA\ShareByMail\Settings\SettingsManager; + class Settings { + /** @var SettingsManager */ + private $settingsManager; + + public function __construct(SettingsManager $settingsManager) { + $this->settingsManager = $settingsManager; + } + /** * announce that the share-by-mail share provider is enabled * @@ -35,4 +44,10 @@ class Settings { $array['shareByMailEnabled'] = true; $settings['array']['oc_appconfig'] = json_encode($array); } + + public function announceShareByMailSettings(array $settings) { + $array = json_decode($settings['array']['oc_appconfig'], true); + $array['shareByMail']['enforcePasswordProtection'] = $this->settingsManager->enforcePasswordProtection(); + $settings['array']['oc_appconfig'] = json_encode($array); + } } diff --git a/apps/sharebymail/lib/Settings/Admin.php b/apps/sharebymail/lib/Settings/Admin.php index b6e7e5d3b4..93a8d3aafa 100644 --- a/apps/sharebymail/lib/Settings/Admin.php +++ b/apps/sharebymail/lib/Settings/Admin.php @@ -40,7 +40,8 @@ class Admin implements ISettings { public function getForm() { $parameters = [ - 'sendPasswordMail' => $this->settingsManager->sendPasswordByMail() + 'sendPasswordMail' => $this->settingsManager->sendPasswordByMail(), + 'enforcePasswordProtection' => $this->settingsManager->enforcePasswordProtection() ]; return new TemplateResponse('sharebymail', 'settings-admin', $parameters, ''); diff --git a/apps/sharebymail/lib/Settings/SettingsManager.php b/apps/sharebymail/lib/Settings/SettingsManager.php index 205b253f33..2b35e5833a 100644 --- a/apps/sharebymail/lib/Settings/SettingsManager.php +++ b/apps/sharebymail/lib/Settings/SettingsManager.php @@ -30,7 +30,9 @@ class SettingsManager { /** @var IConfig */ private $config; - private $defaultSetting = 'yes'; + private $sendPasswordByMailDefault = 'yes'; + + private $enforcePasswordProtectionDefault = 'no'; public function __construct(IConfig $config) { $this->config = $config; @@ -42,8 +44,18 @@ class SettingsManager { * @return bool */ public function sendPasswordByMail() { - $sendPasswordByMail = $this->config->getAppValue('sharebymail', 'sendpasswordmail', $this->defaultSetting); + $sendPasswordByMail = $this->config->getAppValue('sharebymail', 'sendpasswordmail', $this->sendPasswordByMailDefault); return $sendPasswordByMail === 'yes'; } + /** + * do we require a share by mail to be password protected + * + * @return bool + */ + public function enforcePasswordProtection() { + $enforcePassword = $this->config->getAppValue('sharebymail', 'enforcePasswordProtection', $this->enforcePasswordProtectionDefault); + return $enforcePassword === 'yes'; + } + } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 767bdc86a4..defcd5d4ac 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -37,6 +37,7 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IMailer; +use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OC\Share20\Share; use OCP\Share\Exceptions\ShareNotFound; @@ -84,6 +85,9 @@ class ShareByMailProvider implements IShareProvider { /** @var Defaults */ private $defaults; + /** @var IHasher */ + private $hasher; + /** * Return the identifier of this provider. * @@ -107,6 +111,7 @@ class ShareByMailProvider implements IShareProvider { * @param IManager $activityManager * @param SettingsManager $settingsManager * @param Defaults $defaults + * @param IHasher $hasher */ public function __construct( IDBConnection $connection, @@ -119,7 +124,8 @@ class ShareByMailProvider implements IShareProvider { IURLGenerator $urlGenerator, IManager $activityManager, SettingsManager $settingsManager, - Defaults $defaults + Defaults $defaults, + IHasher $hasher ) { $this->dbConnection = $connection; $this->secureRandom = $secureRandom; @@ -132,6 +138,7 @@ class ShareByMailProvider implements IShareProvider { $this->activityManager = $activityManager; $this->settingsManager = $settingsManager; $this->defaults = $defaults; + $this->hasher = $hasher; } /** @@ -156,13 +163,50 @@ class ShareByMailProvider implements IShareProvider { throw new \Exception($message_t); } + // if the admin enforces a password for all mail shares we create a + // random password and send it to the recipient + $password = ''; + $passwordEnforced = $this->settingsManager->enforcePasswordProtection(); + if ($passwordEnforced) { + $password = $this->autoGeneratePassword($share); + } + $shareId = $this->createMailShare($share); + $send = $this->sendPassword($share->getNode()->getName(), $share->getSharedBy(), $share->getSharedWith(), $password); + if ($passwordEnforced && $send === false) { + $this->sendPasswordToOwner($share->getNode()->getName(), $share->getSharedBy(), $shareWith, $password); + } $this->createActivity($share); $data = $this->getRawShare($shareId); + return $this->createShareObject($data); } + /** + * auto generate password in case of password enforcement on mail shares + * + * @param IShare $share + * @return string + * @throws \Exception + */ + protected function autoGeneratePassword($share) { + $initiatorUser = $this->userManager->get($share->getSharedBy()); + $initiatorEMailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; + $allowPasswordByMail = $this->settingsManager->sendPasswordByMail(); + + if ($initiatorEMailAddress === null && !$allowPasswordByMail) { + throw new \Exception( + $this->l->t("We can't send you the auto-generated password. Please set a valid email address in your personal settings and try again.") + ); + } + + $password = $this->generateToken(8); + $share->setPassword($this->hasher->hash($password)); + + return $password; + } + /** * create activity if a file/folder was shared by mail * @@ -230,7 +274,8 @@ class ShareByMailProvider implements IShareProvider { $share->getSharedBy(), $share->getShareOwner(), $share->getPermissions(), - $share->getToken() + $share->getToken(), + $share->getPassword() ); try { @@ -275,6 +320,7 @@ class ShareByMailProvider implements IShareProvider { $initiatorUser = $this->userManager->get($initiator); $ownerDisplayName = ($ownerUser instanceof IUser) ? $ownerUser->getDisplayName() : $owner; $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; + $initiatorEmailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; if ($owner === $initiator) { $subject = (string)$this->l->t('%s shared »%s« with you', array($ownerDisplayName, $filename)); } else { @@ -287,23 +333,25 @@ class ShareByMailProvider implements IShareProvider { $emailTemplate->addHeader(); $emailTemplate->addHeading($this->l->t('%s shared »%s« with you', [$ownerDisplayName, $filename]), false); - if ($owner === $initiator) { $text = $this->l->t('%s shared »%s« with you.', [$ownerDisplayName, $filename]); } else { $text= $this->l->t('%s shared »%s« with you on behalf of %s.', [$ownerDisplayName, $filename, $initiator]); } - $emailTemplate->addBodyText( $text . ' ' . $this->l->t('Click the button below to open it.'), $text ); - $emailTemplate->addBodyButton( $this->l->t('Open »%s«', [$filename]), $link ); + $emailTemplate->addFooter(); + if ($initiatorEmailAddress !== null) { + $message->setFrom([$initiatorEmailAddress => $initiatorDisplayName]); + } + $message->setTo([$shareWith]); // The "From" contains the sharers name @@ -339,49 +387,98 @@ class ShareByMailProvider implements IShareProvider { * @param string $filename * @param string $initiator * @param string $shareWith + * @param string $password + * @return bool */ protected function sendPassword($filename, $initiator, $shareWith, $password) { - if ($this->settingsManager->sendPasswordByMail() === false) { - return; + if ($password === '' || $this->settingsManager->sendPasswordByMail() === false) { + return false; } $initiatorUser = $this->userManager->get($initiator); $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; + $initiatorEmailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; + $subject = (string)$this->l->t('Password to access »%s« shared to you by %s', [$filename, $initiatorDisplayName]); + $plainBodyPart = $this->l->t("%s shared »%s« with you.\nYou should have already received a separate mail with a link to access it.\n", [$initiatorDisplayName, $filename]); + $htmlBodyPart = $this->l->t('%s shared »%s« with you. You should have already received a separate mail with a link to access it.', [$initiatorDisplayName, $filename]); $message = $this->mailer->createMessage(); $emailTemplate = $this->mailer->createEMailTemplate(); - $emailTemplate->addHeader(); - $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename])); - - $emailTemplate->addBodyText($this->l->t( - '%s shared »%s« with you. You should have already received a separate mail with a link to access it.', - [$initiatorDisplayName, $filename] - )); + $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false); + $emailTemplate->addBodyText($htmlBodyPart, $plainBodyPart); $emailTemplate->addBodyText($this->l->t('It is protected with the following password: %s', [$password])); - $emailTemplate->addFooter(); + if ($initiatorEmailAddress !== null) { + $message->setFrom([$initiatorEmailAddress => $initiatorDisplayName]); + } $message->setTo([$shareWith]); $message->setSubject($subject); $message->setBody($emailTemplate->renderText(), 'text/plain'); $message->setHtmlBody($emailTemplate->renderHtml()); $this->mailer->send($message); + return true; } + /** + * send auto generated password to the owner. This happens if the admin enforces + * a password for mail shares and forbid to send the password by mail to the recipient + * + * @param string $filename + * @param string $initiator + * @param string $shareWith + * @param string $password + * @throws \Exception + */ + protected function sendPasswordToOwner($filename, $initiator, $shareWith, $password) { + + $initiatorUser = $this->userManager->get($initiator); + $initiatorEMailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; + $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; + + if ($initiatorEMailAddress === null) { + throw new \Exception( + $this->l->t("We can't send you the auto-generated password. Please set a valid email address in your personal settings and try again.") + ); + } + + $subject = (string)$this->l->t('Password to access »%s« shared with %s', [$filename, $shareWith]); + $plainBodyPart = $this->l->t("You just shared »%s« with %s.\nThe share was already send to the recipient. Due to the security policies\neach share needs to be protected by password and it is not allowed to send the\npassword directly by mail to the recipient. Therefore you need to forward\nthe password manually to the recipient.", [$filename, $shareWith]); + $htmlBodyPart = $this->l->t('You just shared »%s« with %s. The share was already send to the recipient. Due to the security policies each share needs to be protected by password and it is not allowed to send the password directly by mail to the recipient. Therefore you need to forward the password manually to the recipient.', [$filename, $shareWith]); + + $message = $this->mailer->createMessage(); + $emailTemplate = $this->mailer->createEMailTemplate(); + + $emailTemplate->addHeader(); + $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false); + $emailTemplate->addBodyText($htmlBodyPart, $plainBodyPart); + $emailTemplate->addBodyText($this->l->t('This is the password: %s', [$password])); + $emailTemplate->addBodyText($this->l->t('You can chose a different password at any time in the share dialog.')); + $emailTemplate->addFooter(); + + if ($initiatorEMailAddress) { + $message->setFrom([$initiatorEMailAddress => $initiatorDisplayName]); + } + $message->setTo([$initiatorEMailAddress => $initiatorDisplayName]); + $message->setSubject($subject); + $message->setBody($emailTemplate->renderText(), 'text/plain'); + $message->setHtmlBody($emailTemplate->renderHTML()); + $this->mailer->send($message); + } /** * generate share token * * @return string */ - protected function generateToken() { + protected function generateToken($size = 15) { $token = $this->secureRandom->generate( - 15, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS); + $size, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS); return $token; } @@ -422,7 +519,7 @@ class ShareByMailProvider implements IShareProvider { * @param string $token * @return int */ - protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token) { + protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password) { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') ->setValue('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_EMAIL)) @@ -434,6 +531,7 @@ class ShareByMailProvider implements IShareProvider { ->setValue('uid_initiator', $qb->createNamedParameter($sharedBy)) ->setValue('permissions', $qb->createNamedParameter($permissions)) ->setValue('token', $qb->createNamedParameter($token)) + ->setValue('password', $qb->createNamedParameter($password)) ->setValue('stime', $qb->createNamedParameter(time())); /* diff --git a/apps/sharebymail/templates/altmailpasswordowner.php b/apps/sharebymail/templates/altmailpasswordowner.php new file mode 100644 index 0000000000..d9dd4f80e2 --- /dev/null +++ b/apps/sharebymail/templates/altmailpasswordowner.php @@ -0,0 +1,32 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +/** @var OC_Theme $theme */ +/** @var array $_ */ +print_unescaped($l->t("Hey there,\n\nyou just shared »%s« with %s.\nThe share was already send to the recipient. Due to the security policies\neach share needs to be protected by email and it is not allowed to send the\npassword directly by mail to the recipient. Therefore you need to forward\nthe password manually to the recipient.\n\nThis is the password: %s\n\nYou can chose a different password at any time in the share dialog.\n\n", [$_['filename'], $_['recipient'], $_['password']])); +// TRANSLATORS term at the end of a mail +p($l->t("Cheers!")); +print_unescaped("\n"); +?> + + -- +getName() . ' - ' . $theme->getSlogan()); ?> +getBaseUrl()); diff --git a/apps/sharebymail/templates/mailpasswordowner.php b/apps/sharebymail/templates/mailpasswordowner.php new file mode 100644 index 0000000000..cfd6131359 --- /dev/null +++ b/apps/sharebymail/templates/mailpasswordowner.php @@ -0,0 +1,59 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +/** @var OCA\Theming\ThemingDefaults $theme */ +/** @var array $_ */ +?> + + + +
+ + + + + + + + + + + + + + + + + +
+ <?php p($theme->getName()); ?> +
 
  + t('Hey there,

you just shared »%s« with %s.
The share was already send to the recipient. Due to the security policies each share needs to be protected by email and it is not allowed to send the password directly by mail to the recipient. Therefore you need to forward the password manually to the recipient.

This is the password: %s

You can chose a different password at any time in the share dialog.

', [$_['filename'], $_['recipient'], $_['password']])); + // TRANSLATORS term at the end of a mail + p($l->t('Cheers!')); + ?> +
 
 --
+ getName()); ?> - + getSlogan()); ?> +
getBaseUrl());?> +
 
+
diff --git a/apps/sharebymail/templates/settings-admin.php b/apps/sharebymail/templates/settings-admin.php index c4e4108606..3af98741e5 100644 --- a/apps/sharebymail/templates/settings-admin.php +++ b/apps/sharebymail/templates/settings-admin.php @@ -4,6 +4,7 @@ use OCA\Federation\TrustedServers; /** @var \OCP\IL10N $l */ script('sharebymail', 'settings-admin'); +style('sharebymail', 'settings-admin'); ?>

t('Share by mail')); ?>

@@ -11,7 +12,9 @@ script('sharebymail', 'settings-admin');

/> - +
+ /> +

diff --git a/apps/sharebymail/tests/SettingsTest.php b/apps/sharebymail/tests/SettingsTest.php index f415421b0c..8b2fc200d5 100644 --- a/apps/sharebymail/tests/SettingsTest.php +++ b/apps/sharebymail/tests/SettingsTest.php @@ -24,6 +24,7 @@ namespace OCA\ShareByMail\Tests; use OCA\ShareByMail\Settings; +use OCA\ShareByMail\Settings\SettingsManager; use Test\TestCase; class SettingsTest extends TestCase { @@ -31,10 +32,15 @@ class SettingsTest extends TestCase { /** @var Settings */ private $instance; + /** @var SettingsManager | \PHPUnit_Framework_MockObject_MockObject */ + private $settingsManager; + public function setUp() { parent::setUp(); - $this->instance = new Settings(); + $this->settingsManager = $this->getMockBuilder(SettingsManager::class) + ->disableOriginalConstructor()->getMock(); + $this->instance = new Settings($this->settingsManager); } public function testAnnounceShareProvider() { @@ -61,4 +67,30 @@ class SettingsTest extends TestCase { $this->assertSame($after, $before); } + + public function testAnnounceShareByMailSettings() { + $this->settingsManager->expects($this->once())->method('enforcePasswordProtection')->willReturn(true); + $before = [ + 'oc_appconfig' => + json_encode([ + 'key1' => 'value1', + 'key2' => 'value2' + ]), + 'oc_foo' => 'oc_bar' + ]; + + $after = [ + 'oc_appconfig' => + json_encode([ + 'key1' => 'value1', + 'key2' => 'value2', + 'shareByMail' => ['enforcePasswordProtection' => true] + ]), + 'oc_foo' => 'oc_bar' + ]; + + $this->instance->announceShareByMailSettings(['array' => &$before]); + $this->assertSame($after, $before); + } + } diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 581ca9b1b9..4061ef4eb5 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -27,6 +27,7 @@ use OC\Mail\Message; use OCA\ShareByMail\Settings\SettingsManager; use OCA\ShareByMail\ShareByMailProvider; use OCP\Defaults; +use OCP\Files\File; use OCP\Files\IRootFolder; use OCP\IDBConnection; use OCP\IL10N; @@ -36,6 +37,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; +use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Share\IManager; use OCP\Share\IShare; @@ -88,6 +90,9 @@ class ShareByMailProviderTest extends TestCase { /** @var Defaults|\PHPUnit_Framework_MockObject_MockObject */ private $defaults; + /** @var IHasher | \PHPUnit_Framework_MockObject_MockObject */ + private $hasher; + public function setUp() { parent::setUp(); @@ -109,6 +114,7 @@ class ShareByMailProviderTest extends TestCase { $this->activityManager = $this->getMockBuilder('OCP\Activity\IManager')->getMock(); $this->settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); $this->defaults = $this->createMock(Defaults::class); + $this->hasher = $this->getMockBuilder(IHasher::class)->getMock(); $this->userManager->expects($this->any())->method('userExists')->willReturn(true); } @@ -134,7 +140,8 @@ class ShareByMailProviderTest extends TestCase { $this->urlGenerator, $this->activityManager, $this->settingsManager, - $this->defaults + $this->defaults, + $this->hasher ] ); @@ -154,7 +161,8 @@ class ShareByMailProviderTest extends TestCase { $this->urlGenerator, $this->activityManager, $this->settingsManager, - $this->defaults + $this->defaults, + $this->hasher ); } @@ -167,15 +175,22 @@ class ShareByMailProviderTest extends TestCase { public function testCreate() { $share = $this->getMockBuilder('\OCP\Share\IShare')->getMock(); - $share->expects($this->once())->method('getSharedWith')->willReturn('user1'); + $share->expects($this->any())->method('getSharedWith')->willReturn('user1'); - $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createActivity']); + $node = $this->getMockBuilder(File::class)->getMock(); + $node->expects($this->any())->method('getName')->willReturn('filename'); + + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createActivity', 'sendPassword']); $instance->expects($this->once())->method('getSharedWith')->willReturn([]); $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); $instance->expects($this->once())->method('createActivity')->with($share); $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); + $instance->expects($this->any())->method('sendPassword')->willReturn(true); + $share->expects($this->any())->method('getNode')->willReturn($node); + $this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false); + $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $this->assertSame('shareObject', $instance->create($share) @@ -273,6 +288,7 @@ class ShareByMailProviderTest extends TestCase { $uidOwner = 'user2'; $permissions = 1; $token = 'token'; + $password = 'password'; $instance = $this->getInstance(); @@ -286,7 +302,8 @@ class ShareByMailProviderTest extends TestCase { $sharedBy, $uidOwner, $permissions, - $token + $token, + $password ] ); @@ -305,6 +322,7 @@ class ShareByMailProviderTest extends TestCase { $this->assertSame($uidOwner, $result[0]['uid_owner']); $this->assertSame($permissions, (int)$result[0]['permissions']); $this->assertSame($token, $result[0]['token']); + $this->assertSame($password, $result[0]['password']); } diff --git a/core/js/shareconfigmodel.js b/core/js/shareconfigmodel.js index 1ead631db4..16ab904ad4 100644 --- a/core/js/shareconfigmodel.js +++ b/core/js/shareconfigmodel.js @@ -29,6 +29,7 @@ isMailShareAllowed: oc_appconfig.shareByMailEnabled !== undefined, defaultExpireDate: oc_appconfig.core.defaultExpireDate, isResharingAllowed: oc_appconfig.core.resharingAllowed, + isPasswordForMailSharesRequired: (oc_appconfig.shareByMail === undefined) ? false : oc_appconfig.shareByMail.enforcePasswordProtection, allowGroupSharing: oc_appconfig.core.allowGroupSharing }, diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 6903dd57c3..726095cf93 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -100,7 +100,7 @@ '{{/if}}' + '
  • ' + '' + - '' + + '' + '' + '
    ' + ' ' + @@ -268,6 +268,7 @@ crudsLabel: t('core', 'Access control'), triangleSImage: OC.imagePath('core', 'actions/triangle-s'), isResharingAllowed: this.configModel.get('isResharingAllowed'), + isPasswordForMailSharesRequired: this.configModel.get('isPasswordForMailSharesRequired'), sharePermissionPossible: this.model.sharePermissionPossible(), editPermissionPossible: this.model.editPermissionPossible(), createPermissionPossible: this.model.createPermissionPossible(), diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index beb3a0965d..c9c0265c78 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -160,7 +160,8 @@ class ProviderFactory implements IProviderFactory { $this->serverContainer->getURLGenerator(), $this->serverContainer->getActivityManager(), $settingsManager, - $this->serverContainer->query(Defaults::class) + $this->serverContainer->query(Defaults::class), + $this->serverContainer->getHasher() ); } From 7a8f8cbb331f95c20980ab2516d372f65a8d4bd4 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 13 Apr 2017 11:32:59 +0200 Subject: [PATCH 2/9] templates no longer needed --- .../templates/altmailpasswordowner.php | 32 ---------- .../templates/mailpasswordowner.php | 59 ------------------- 2 files changed, 91 deletions(-) delete mode 100644 apps/sharebymail/templates/altmailpasswordowner.php delete mode 100644 apps/sharebymail/templates/mailpasswordowner.php diff --git a/apps/sharebymail/templates/altmailpasswordowner.php b/apps/sharebymail/templates/altmailpasswordowner.php deleted file mode 100644 index d9dd4f80e2..0000000000 --- a/apps/sharebymail/templates/altmailpasswordowner.php +++ /dev/null @@ -1,32 +0,0 @@ - - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ - -/** @var OC_Theme $theme */ -/** @var array $_ */ -print_unescaped($l->t("Hey there,\n\nyou just shared »%s« with %s.\nThe share was already send to the recipient. Due to the security policies\neach share needs to be protected by email and it is not allowed to send the\npassword directly by mail to the recipient. Therefore you need to forward\nthe password manually to the recipient.\n\nThis is the password: %s\n\nYou can chose a different password at any time in the share dialog.\n\n", [$_['filename'], $_['recipient'], $_['password']])); -// TRANSLATORS term at the end of a mail -p($l->t("Cheers!")); -print_unescaped("\n"); -?> - - -- -getName() . ' - ' . $theme->getSlogan()); ?> -getBaseUrl()); diff --git a/apps/sharebymail/templates/mailpasswordowner.php b/apps/sharebymail/templates/mailpasswordowner.php deleted file mode 100644 index cfd6131359..0000000000 --- a/apps/sharebymail/templates/mailpasswordowner.php +++ /dev/null @@ -1,59 +0,0 @@ - - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ - -/** @var OCA\Theming\ThemingDefaults $theme */ -/** @var array $_ */ -?> - - - -
    - - - - - - - - - - - - - - - - - -
    - <?php p($theme->getName()); ?> -
     
      - t('Hey there,

    you just shared »%s« with %s.
    The share was already send to the recipient. Due to the security policies each share needs to be protected by email and it is not allowed to send the password directly by mail to the recipient. Therefore you need to forward the password manually to the recipient.

    This is the password: %s

    You can chose a different password at any time in the share dialog.

    ', [$_['filename'], $_['recipient'], $_['password']])); - // TRANSLATORS term at the end of a mail - p($l->t('Cheers!')); - ?> -
     
     --
    - getName()); ?> - - getSlogan()); ?> -
    getBaseUrl());?> -
     
    -
    From f0651cb06601dda528c14a538c18c5484b7c6ef8 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 11 Apr 2017 21:29:54 +0200 Subject: [PATCH 3/9] allow to set a password for shares which where created without a password before the admin started to enforce the password Signed-off-by: Bjoern Schiessle --- core/js/sharedialogshareelistview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 726095cf93..3a481e53dd 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -100,7 +100,7 @@ '{{/if}}' + '
  • ' + '' + - '' + + '' + '' + '
    ' + ' ' + From f00c1eccf714379df8ffa316481967e5a0e993fb Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 19 Apr 2017 15:10:22 +0200 Subject: [PATCH 4/9] create activity if a password was send by mail Signed-off-by: Bjoern Schiessle --- apps/sharebymail/lib/Activity.php | 52 +++++++++++--- apps/sharebymail/lib/ShareByMailProvider.php | 73 +++++++++++++++----- 2 files changed, 101 insertions(+), 24 deletions(-) diff --git a/apps/sharebymail/lib/Activity.php b/apps/sharebymail/lib/Activity.php index 1c2f37dc38..acc3e59f92 100644 --- a/apps/sharebymail/lib/Activity.php +++ b/apps/sharebymail/lib/Activity.php @@ -58,6 +58,8 @@ class Activity implements IProvider { const SUBJECT_SHARED_EMAIL_SELF = 'shared_with_email_self'; const SUBJECT_SHARED_EMAIL_BY = 'shared_with_email_by'; + const SUBJECT_SHARED_EMAIL_PASSWORD_SEND = 'shared_with_email_password_send'; + const SUBJECT_SHARED_EMAIL_PASSWORD_SEND_SELF = 'shared_with_email_password_send_self'; /** * @param IFactory $languageFactory @@ -119,15 +121,26 @@ class Activity implements IProvider { ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_BY) { $event->setParsedSubject($this->l->t('Shared with %1$s by %2$s', [ - $parsedParameters['email']['name'], - $parsedParameters['actor']['name'], - ])) + $parsedParameters['email']['name'], + $parsedParameters['actor']['name'], + ])) ->setRichSubject($this->l->t('Shared with {email} by {actor}'), [ 'email' => $parsedParameters['email'], 'actor' => $parsedParameters['actor'], ]) ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); - + } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND) { + $event->setParsedSubject($this->l->t('Password for mail share send to %1$s', [ + $parsedParameters['email']['name'] + ])) + ->setRichSubject($this->l->t('Password for mail share send to {email}'), [ + 'email' => $parsedParameters['email'] + ]) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND_SELF) { + $event->setParsedSubject($this->l->t('Password for mail share send to you')) + ->setRichSubject($this->l->t('Password for mail share send to you')) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); } else { throw new \InvalidArgumentException(); } @@ -153,12 +166,26 @@ class Activity implements IProvider { ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_BY) { $event->setParsedSubject($this->l->t('%3$s shared %1$s with %2$s by mail', [ - $parsedParameters['file']['path'], - $parsedParameters['email']['name'], - $parsedParameters['actor']['name'], - ])) + $parsedParameters['file']['path'], + $parsedParameters['email']['name'], + $parsedParameters['actor']['name'], + ])) ->setRichSubject($this->l->t('{actor} shared {file} with {email} by mail'), $parsedParameters) ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND) { + $event->setParsedSubject($this->l->t('Password to access %1$s was send to %2s', [ + $parsedParameters['file']['path'], + $parsedParameters['email']['name'] + ])) + ->setRichSubject($this->l->t('Password to access {file} was send to {email}'), $parsedParameters) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + } else if ($event->getSubject() === self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND_SELF) { + $event->setParsedSubject( + $this->l->t('Password to access %1$s was send to you', + [$parsedParameters['file']['path']])) + ->setRichSubject($this->l->t('Password to access {file} was send to you'), $parsedParameters) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + } else { throw new \InvalidArgumentException(); } @@ -182,6 +209,15 @@ class Activity implements IProvider { 'email' => $this->generateEmailParameter($parameters[1]), 'actor' => $this->generateUserParameter($parameters[2]), ]; + case self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND: + return [ + 'file' => $this->generateFileParameter((int) $event->getObjectId(), $parameters[0]), + 'email' => $this->generateEmailParameter($parameters[1]), + ]; + case self::SUBJECT_SHARED_EMAIL_PASSWORD_SEND_SELF: + return [ + 'file' => $this->generateFileParameter((int) $event->getObjectId(), $parameters[0]), + ]; } throw new \InvalidArgumentException(); } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index defcd5d4ac..7093fd3dfe 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -172,11 +172,12 @@ class ShareByMailProvider implements IShareProvider { } $shareId = $this->createMailShare($share); - $send = $this->sendPassword($share->getNode()->getName(), $share->getSharedBy(), $share->getSharedWith(), $password); + $send = $this->sendPassword($share, $password); if ($passwordEnforced && $send === false) { - $this->sendPasswordToOwner($share->getNode()->getName(), $share->getSharedBy(), $shareWith, $password); + $this->sendPasswordToOwner($share, $password); } - $this->createActivity($share); + + $this->createShareActivity($share); $data = $this->getRawShare($shareId); return $this->createShareObject($data); @@ -212,7 +213,7 @@ class ShareByMailProvider implements IShareProvider { * * @param IShare $share */ - protected function createActivity(IShare $share) { + protected function createShareActivity(IShare $share) { $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy()); @@ -240,6 +241,37 @@ class ShareByMailProvider implements IShareProvider { } + /** + * create activity if a file/folder was shared by mail + * + * @param IShare $share + * @param string $sharedWith + * @param bool $sendToSelf + */ + protected function createPasswordSendActivity(IShare $share, $sharedWith, $sendToSelf) { + + $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy()); + + if ($sendToSelf) { + $this->publishActivity( + Activity::SUBJECT_SHARED_EMAIL_PASSWORD_SEND_SELF, + [$userFolder->getRelativePath($share->getNode()->getPath())], + $share->getSharedBy(), + $share->getNode()->getId(), + $userFolder->getRelativePath($share->getNode()->getPath()) + ); + } else { + $this->publishActivity( + Activity::SUBJECT_SHARED_EMAIL_PASSWORD_SEND, + [$userFolder->getRelativePath($share->getNode()->getPath()), $sharedWith], + $share->getSharedBy(), + $share->getNode()->getId(), + $userFolder->getRelativePath($share->getNode()->getPath()) + ); + } + } + + /** * publish activity if a file/folder was shared by mail * @@ -384,13 +416,15 @@ class ShareByMailProvider implements IShareProvider { /** * send password to recipient of a mail share * - * @param string $filename - * @param string $initiator - * @param string $shareWith + * @param IShare $share * @param string $password * @return bool */ - protected function sendPassword($filename, $initiator, $shareWith, $password) { + protected function sendPassword(IShare $share, $password) { + + $filename = $share->getNode()->getName(); + $initiator = $share->getSharedBy(); + $shareWith = $share->getSharedWith(); if ($password === '' || $this->settingsManager->sendPasswordByMail() === false) { return false; @@ -422,6 +456,8 @@ class ShareByMailProvider implements IShareProvider { $message->setHtmlBody($emailTemplate->renderHtml()); $this->mailer->send($message); + $this->createPasswordSendActivity($share, $shareWith, false); + return true; } @@ -429,17 +465,18 @@ class ShareByMailProvider implements IShareProvider { * send auto generated password to the owner. This happens if the admin enforces * a password for mail shares and forbid to send the password by mail to the recipient * - * @param string $filename - * @param string $initiator - * @param string $shareWith + * @param IShare $share * @param string $password + * @return bool * @throws \Exception */ - protected function sendPasswordToOwner($filename, $initiator, $shareWith, $password) { + protected function sendPasswordToOwner(IShare $share, $password) { - $initiatorUser = $this->userManager->get($initiator); - $initiatorEMailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; - $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; + $filename = $share->getNode()->getName(); + $initiator = $this->userManager->get($share->getSharedBy()); + $initiatorEMailAddress = ($initiator instanceof IUser) ? $initiator->getEMailAddress() : null; + $initiatorDisplayName = ($initiator instanceof IUser) ? $initiator->getDisplayName() : $share->getSharedBy(); + $shareWith = $share->getSharedWith(); if ($initiatorEMailAddress === null) { throw new \Exception( @@ -469,6 +506,10 @@ class ShareByMailProvider implements IShareProvider { $message->setBody($emailTemplate->renderText(), 'text/plain'); $message->setHtmlBody($emailTemplate->renderHTML()); $this->mailer->send($message); + + $this->createPasswordSendActivity($share, $shareWith, true); + + return true; } /** @@ -561,7 +602,7 @@ class ShareByMailProvider implements IShareProvider { $validPassword = $plainTextPassword !== null && $plainTextPassword !== ''; if($validPassword && $originalShare->getPassword() !== $share->getPassword()) { - $this->sendPassword($share->getNode()->getName(), $share->getSharedBy(), $share->getSharedWith(), $plainTextPassword); + $this->sendPassword($share, $plainTextPassword); } /* * We allow updating the permissions and password of mail shares From 428d7cdb5e7be1d5ede6d5084459660b22c84b2c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 19 Apr 2017 15:53:33 +0200 Subject: [PATCH 5/9] improve mail text Signed-off-by: Bjoern Schiessle --- apps/sharebymail/lib/ShareByMailProvider.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 7093fd3dfe..5307a8cb22 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -485,15 +485,14 @@ class ShareByMailProvider implements IShareProvider { } $subject = (string)$this->l->t('Password to access »%s« shared with %s', [$filename, $shareWith]); - $plainBodyPart = $this->l->t("You just shared »%s« with %s.\nThe share was already send to the recipient. Due to the security policies\neach share needs to be protected by password and it is not allowed to send the\npassword directly by mail to the recipient. Therefore you need to forward\nthe password manually to the recipient.", [$filename, $shareWith]); - $htmlBodyPart = $this->l->t('You just shared »%s« with %s. The share was already send to the recipient. Due to the security policies each share needs to be protected by password and it is not allowed to send the password directly by mail to the recipient. Therefore you need to forward the password manually to the recipient.', [$filename, $shareWith]); + $bodyPart = $this->l->t("You just shared »%s« with %s. The share was already send to the recipient. Due to the security policies defined by the administrator of %s each share needs to be protected by password and it is not allowed to send the password directly to the recipient. Therefore you need to forward the password manually to the recipient.", [$filename, $shareWith, $this->defaults->getName()]); $message = $this->mailer->createMessage(); $emailTemplate = $this->mailer->createEMailTemplate(); $emailTemplate->addHeader(); $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false); - $emailTemplate->addBodyText($htmlBodyPart, $plainBodyPart); + $emailTemplate->addBodyText($bodyPart); $emailTemplate->addBodyText($this->l->t('This is the password: %s', [$password])); $emailTemplate->addBodyText($this->l->t('You can chose a different password at any time in the share dialog.')); $emailTemplate->addFooter(); From 972b4c04e2ea3bf96533c111853a57177231c638 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 19 Apr 2017 16:56:34 +0200 Subject: [PATCH 6/9] respect password policy for auto generated passwords Signed-off-by: Bjoern Schiessle --- apps/sharebymail/lib/ShareByMailProvider.php | 35 +++++++++++++++++-- .../tests/ShareByMailProviderTest.php | 11 ++++-- lib/private/Share20/ProviderFactory.php | 4 ++- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 5307a8cb22..f31a774b1d 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -21,6 +21,7 @@ namespace OCA\ShareByMail; +use OC\CapabilitiesManager; use OC\HintException; use OC\Share20\Exception\InvalidShare; use OCA\ShareByMail\Settings\SettingsManager; @@ -43,7 +44,6 @@ use OC\Share20\Share; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IShare; use OCP\Share\IShareProvider; -use OCP\Template; /** * Class ShareByMail @@ -88,6 +88,9 @@ class ShareByMailProvider implements IShareProvider { /** @var IHasher */ private $hasher; + /** @var CapabilitiesManager */ + private $capabilitiesManager; + /** * Return the identifier of this provider. * @@ -112,6 +115,7 @@ class ShareByMailProvider implements IShareProvider { * @param SettingsManager $settingsManager * @param Defaults $defaults * @param IHasher $hasher + * @param CapabilitiesManager $capabilitiesManager */ public function __construct( IDBConnection $connection, @@ -125,7 +129,8 @@ class ShareByMailProvider implements IShareProvider { IManager $activityManager, SettingsManager $settingsManager, Defaults $defaults, - IHasher $hasher + IHasher $hasher, + CapabilitiesManager $capabilitiesManager ) { $this->dbConnection = $connection; $this->secureRandom = $secureRandom; @@ -139,6 +144,7 @@ class ShareByMailProvider implements IShareProvider { $this->settingsManager = $settingsManager; $this->defaults = $defaults; $this->hasher = $hasher; + $this->capabilitiesManager = $capabilitiesManager; } /** @@ -202,12 +208,35 @@ class ShareByMailProvider implements IShareProvider { ); } - $password = $this->generateToken(8); + $passwordPolicy = $this->getPasswordPolicy(); + $passwordCharset = ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS; + $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); + $share->setPassword($this->hasher->hash($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 * diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 4061ef4eb5..a3e5da82bf 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -23,6 +23,7 @@ namespace OCA\ShareByMail\Tests; +use OC\CapabilitiesManager; use OC\Mail\Message; use OCA\ShareByMail\Settings\SettingsManager; use OCA\ShareByMail\ShareByMailProvider; @@ -93,6 +94,9 @@ class ShareByMailProviderTest extends TestCase { /** @var IHasher | \PHPUnit_Framework_MockObject_MockObject */ private $hasher; + /** @var CapabilitiesManager | \PHPUnit_Framework_MockObject_MockObject */ + private $capabilitiesManager; + public function setUp() { parent::setUp(); @@ -115,6 +119,7 @@ class ShareByMailProviderTest extends TestCase { $this->settingsManager = $this->getMockBuilder(SettingsManager::class)->disableOriginalConstructor()->getMock(); $this->defaults = $this->createMock(Defaults::class); $this->hasher = $this->getMockBuilder(IHasher::class)->getMock(); + $this->capabilitiesManager = $this->getMockBuilder(CapabilitiesManager::class)->disableOriginalConstructor()->getMock(); $this->userManager->expects($this->any())->method('userExists')->willReturn(true); } @@ -141,7 +146,8 @@ class ShareByMailProviderTest extends TestCase { $this->activityManager, $this->settingsManager, $this->defaults, - $this->hasher + $this->hasher, + $this->capabilitiesManager ] ); @@ -162,7 +168,8 @@ class ShareByMailProviderTest extends TestCase { $this->activityManager, $this->settingsManager, $this->defaults, - $this->hasher + $this->hasher, + $this->capabilitiesManager ); } diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index c9c0265c78..c79f58f6ba 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -23,6 +23,7 @@ */ namespace OC\Share20; +use OC\CapabilitiesManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\DiscoveryManager; use OCA\FederatedFileSharing\FederatedShareProvider; @@ -161,7 +162,8 @@ class ProviderFactory implements IProviderFactory { $this->serverContainer->getActivityManager(), $settingsManager, $this->serverContainer->query(Defaults::class), - $this->serverContainer->getHasher() + $this->serverContainer->getHasher(), + $this->serverContainer->query(CapabilitiesManager::class) ); } From aa54d31bd2161f74ccccd375fab46bdf63c5207e Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 19 Apr 2017 17:08:52 +0200 Subject: [PATCH 7/9] fix unit tests Signed-off-by: Bjoern Schiessle --- apps/sharebymail/lib/ShareByMailProvider.php | 6 ------ apps/sharebymail/tests/ShareByMailProviderTest.php | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index f31a774b1d..459b1ffe7c 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -381,7 +381,6 @@ class ShareByMailProvider implements IShareProvider { $initiatorUser = $this->userManager->get($initiator); $ownerDisplayName = ($ownerUser instanceof IUser) ? $ownerUser->getDisplayName() : $owner; $initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator; - $initiatorEmailAddress = ($initiatorUser instanceof IUser) ? $initiatorUser->getEMailAddress() : null; if ($owner === $initiator) { $subject = (string)$this->l->t('%s shared »%s« with you', array($ownerDisplayName, $filename)); } else { @@ -408,11 +407,6 @@ class ShareByMailProvider implements IShareProvider { $link ); - $emailTemplate->addFooter(); - if ($initiatorEmailAddress !== null) { - $message->setFrom([$initiatorEmailAddress => $initiatorDisplayName]); - } - $message->setTo([$shareWith]); // The "From" contains the sharers name diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index a3e5da82bf..269f8e8f41 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -187,11 +187,11 @@ class ShareByMailProviderTest extends TestCase { $node = $this->getMockBuilder(File::class)->getMock(); $node->expects($this->any())->method('getName')->willReturn('filename'); - $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createActivity', 'sendPassword']); + $instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'sendPassword']); $instance->expects($this->once())->method('getSharedWith')->willReturn([]); $instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42); - $instance->expects($this->once())->method('createActivity')->with($share); + $instance->expects($this->once())->method('createShareActivity')->with($share); $instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare'); $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject'); $instance->expects($this->any())->method('sendPassword')->willReturn(true); @@ -661,7 +661,7 @@ class ShareByMailProviderTest extends TestCase { $userManager = \OC::$server->getUserManager(); $rootFolder = \OC::$server->getRootFolder(); - $provider = $this->getInstance(['sendMailNotification', 'createActivity']); + $provider = $this->getInstance(['sendMailNotification', 'createShareActivity']); $u1 = $userManager->createUser('testFed', md5(time())); $u2 = $userManager->createUser('testFed2', md5(time())); @@ -703,7 +703,7 @@ class ShareByMailProviderTest extends TestCase { $userManager = \OC::$server->getUserManager(); $rootFolder = \OC::$server->getRootFolder(); - $provider = $this->getInstance(['sendMailNotification', 'createActivity']); + $provider = $this->getInstance(['sendMailNotification', 'createShareActivity']); $u1 = $userManager->createUser('testFed', md5(time())); $u2 = $userManager->createUser('testFed2', md5(time())); From 9d07ec5a96d7995785f6d3d377024ba09977cb91 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 19 Apr 2017 17:54:17 -0500 Subject: [PATCH 8/9] Fix casing Signed-off-by: Morris Jobke --- apps/sharebymail/lib/ShareByMailProvider.php | 2 +- apps_socialsharing | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 160000 apps_socialsharing diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 459b1ffe7c..5634f77398 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -526,7 +526,7 @@ class ShareByMailProvider implements IShareProvider { $message->setTo([$initiatorEMailAddress => $initiatorDisplayName]); $message->setSubject($subject); $message->setBody($emailTemplate->renderText(), 'text/plain'); - $message->setHtmlBody($emailTemplate->renderHTML()); + $message->setHtmlBody($emailTemplate->renderHtml()); $this->mailer->send($message); $this->createPasswordSendActivity($share, $shareWith, true); diff --git a/apps_socialsharing b/apps_socialsharing new file mode 160000 index 0000000000..a1157abb8a --- /dev/null +++ b/apps_socialsharing @@ -0,0 +1 @@ +Subproject commit a1157abb8a37862f87f5012317f9323113467737 From 6c294c3407426340d06babd1a295209b7f1bae54 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 19 Apr 2017 18:06:40 -0500 Subject: [PATCH 9/9] fix typo Signed-off-by: Morris Jobke --- apps/sharebymail/lib/ShareByMailProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 5634f77398..7e0f7c5071 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -517,7 +517,7 @@ class ShareByMailProvider implements IShareProvider { $emailTemplate->addHeading($this->l->t('Password to access »%s«', [$filename]), false); $emailTemplate->addBodyText($bodyPart); $emailTemplate->addBodyText($this->l->t('This is the password: %s', [$password])); - $emailTemplate->addBodyText($this->l->t('You can chose a different password at any time in the share dialog.')); + $emailTemplate->addBodyText($this->l->t('You can choose a different password at any time in the share dialog.')); $emailTemplate->addFooter(); if ($initiatorEMailAddress) {