Sharing link & mail parity

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

apps/files_sharing/src/components/SharingInput.vue

@@ -461,7 +461,7 @@ export default {
 			try {
 				let password = null
 
-				if (this.config.isPasswordForMailSharesRequired
+				if (this.config.enforcePasswordForPublicLink
 					&& value.shareType === this.SHARE_TYPES.SHARE_TYPE_EMAIL) {
 					password = await GeneratePassword()
 				}

apps/settings/templates/settings/admin/sharing.php

@@ -70,7 +70,7 @@
 			   value="1" <?php if ($_['allowLinks'] === 'yes') {
 	print_unescaped('checked="checked"');
 } ?> />
-		<label for="allowLinks"><?php p($l->t('Allow users to share via link'));?></label><br/>
+		<label for="allowLinks"><?php p($l->t('Allow users to share via link and emails'));?></label><br/>
 	</p>
 
 	<p id="publicLinkSettings" class="indent <?php if ($_['allowLinks'] !== 'yes' || $_['shareAPIEnabled'] === 'no') {
@@ -96,7 +96,7 @@
 			   value="1" <?php if ($_['shareDefaultExpireDateSet'] === 'yes') {
 	print_unescaped('checked="checked"');
 } ?> />
-		<label for="shareapiDefaultExpireDate"><?php p($l->t('Set default expiration date for link shares'));?></label><br/>
+		<label for="shareapiDefaultExpireDate"><?php p($l->t('Set default expiration date'));?></label><br/>
 
 	</p>
 	<p id="setDefaultExpireDate" class="double-indent <?php if ($_['allowLinks'] !== 'yes' || $_['shareDefaultExpireDateSet'] === 'no' || $_['shareAPIEnabled'] === 'no') {

apps/sharebymail/lib/Capabilities.php

@@ -29,13 +29,14 @@ namespace OCA\ShareByMail;
 
 use OCA\ShareByMail\Settings\SettingsManager;
 use OCP\Capabilities\ICapability;
+use OCP\Share\IManager;
 
 class Capabilities implements ICapability {
 
-	/** @var SettingsManager */
+	/** @var IManager */
 	private $manager;
 
-	public function __construct(SettingsManager $manager) {
+	public function __construct(IManager $manager) {
 		$this->manager = $manager;
 	}
 
@@ -45,16 +46,17 @@ class Capabilities implements ICapability {
 				[
 					'sharebymail' =>
 						[
-							'enabled' => true,
+							'enabled' => $this->manager->shareApiAllowLinks(),
 							'upload_files_drop' => [
 								'enabled' => true,
 							],
 							'password' => [
 								'enabled' => true,
-								'enforced' => $this->manager->enforcePasswordProtection(),
+								'enforced' => $this->manager->shareApiLinkEnforcePassword(),
 							],
 							'expire_date' => [
 								'enabled' => true,
+								'enforced' => $this->manager->shareApiLinkDefaultExpireDateEnforced(),
 							],
 						]
 				]

apps/sharebymail/lib/Settings/Admin.php

@@ -41,7 +41,6 @@ class Admin implements ISettings {
 	public function getForm() {
 		$parameters = [
 			'sendPasswordMail' => $this->settingsManager->sendPasswordByMail(),
-			'enforcePasswordProtection' => $this->settingsManager->enforcePasswordProtection(),
 			'replyToInitiator' => $this->settingsManager->replyToInitiator()
 		];
 

apps/sharebymail/lib/Settings/SettingsManager.php

@@ -36,8 +36,6 @@ class SettingsManager {
 
 	private $sendPasswordByMailDefault = 'yes';
 
-	private $enforcePasswordProtectionDefault = 'no';
-
 	private $replyToInitiatorDefault = 'yes';
 
 	public function __construct(IConfig $config) {
@@ -54,16 +52,6 @@ class SettingsManager {
 		return $sendPasswordByMail === 'yes';
 	}
 
-	/**
-	 * do we require a share by mail to be password protected
-	 *
-	 * @return bool
-	 */
-	public function enforcePasswordProtection(): bool {
-		$enforcePassword = $this->config->getAppValue('sharebymail', 'enforcePasswordProtection', $this->enforcePasswordProtectionDefault);
-		return $enforcePassword === 'yes';
-	}
-
 	/**
 	 * should add reply to with initiator mail
 	 *

apps/sharebymail/lib/ShareByMailProvider.php

@@ -61,6 +61,7 @@ use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IManager as IShareManager;
 use OCP\Share\IShare;
 use OCP\Share\IShareProvider;
 
@@ -110,6 +111,9 @@ class ShareByMailProvider implements IShareProvider {
 	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
+	/** @var IShareManager */
+	private $shareManager;
+
 	/**
 	 * Return the identifier of this provider.
 	 *
@@ -119,21 +123,20 @@ class ShareByMailProvider implements IShareProvider {
 		return 'ocMailShare';
 	}
 
-	public function __construct(
-		IDBConnection $connection,
-		ISecureRandom $secureRandom,
-		IUserManager $userManager,
-		IRootFolder $rootFolder,
-		IL10N $l,
-		ILogger $logger,
-		IMailer $mailer,
-		IURLGenerator $urlGenerator,
-		IManager $activityManager,
-		SettingsManager $settingsManager,
-		Defaults $defaults,
-		IHasher $hasher,
-		IEventDispatcher $eventDispatcher
-	) {
+	public function __construct(IDBConnection $connection,
+								ISecureRandom $secureRandom,
+								IUserManager $userManager,
+								IRootFolder $rootFolder,
+								IL10N $l,
+								ILogger $logger,
+								IMailer $mailer,
+								IURLGenerator $urlGenerator,
+								IManager $activityManager,
+								SettingsManager $settingsManager,
+								Defaults $defaults,
+								IHasher $hasher,
+								IEventDispatcher $eventDispatcher,
+								IShareManager $shareManager) {
 		$this->dbConnection = $connection;
 		$this->secureRandom = $secureRandom;
 		$this->userManager = $userManager;
@@ -147,6 +150,7 @@ class ShareByMailProvider implements IShareProvider {
 		$this->defaults = $defaults;
 		$this->hasher = $hasher;
 		$this->eventDispatcher = $eventDispatcher;
+		$this->shareManager = $shareManager;
 	}
 
 	/**
@@ -173,7 +177,7 @@ class ShareByMailProvider implements IShareProvider {
 		// if the admin enforces a password for all mail shares we create a
 		// random password and send it to the recipient
 		$password = $share->getPassword() ?: '';
-		$passwordEnforced = $this->settingsManager->enforcePasswordProtection();
+		$passwordEnforced = $this->shareManager->shareApiLinkEnforcePassword();
 		if ($passwordEnforced && empty($password)) {
 			$password = $this->autoGeneratePassword($share);
 		}

apps/sharebymail/templates/settings-admin.php

@@ -14,14 +14,11 @@ style('sharebymail', 'settings-admin');
 	p('checked');
 } ?> />
 		<label for="sendPasswordMail"><?php p($l->t('Send password by mail')); ?></label><br/>
-		<input id="enforcePasswordProtection" type="checkbox" class="checkbox" <?php if ($_['enforcePasswordProtection']) {
-	p('checked');
-} ?> />
-		<label for="enforcePasswordProtection"><?php p($l->t('Enforce password protection')); ?></label><br/>
-        <input id="replyToInitiator" type="checkbox" class="checkbox" <?php if ($_['replyToInitiator']) {
+
+		<input id="replyToInitiator" type="checkbox" class="checkbox" <?php if ($_['replyToInitiator']) {
 	p('checked');
 } ?> />
-        <label for="replyToInitiator"><?php p($l->t('Reply to initiator')); ?></label>
+		<label for="replyToInitiator"><?php p($l->t('Reply to initiator')); ?></label>
 	</p>
 
 </div>

build/integration/features/bootstrap/SharingContext.php

@@ -45,6 +45,5 @@ class SharingContext implements Context, SnippetAcceptingContext {
 		$this->deleteServerConfig('core', 'shareapi_default_expire_date');
 		$this->deleteServerConfig('core', 'shareapi_expire_after_n_days');
 		$this->deleteServerConfig('core', 'link_defaultExpDays');
-		$this->deleteServerConfig('sharebymail', 'enforcePasswordProtection');
 	}
 }

build/integration/sharing_features/sharing-v1.feature

@@ -84,7 +84,7 @@ Feature: sharing
   Scenario: Creating a new mail share with password when password protection is enforced
     Given dummy mail server is listening
     And As an "admin"
-    And parameter "enforcePasswordProtection" of app "sharebymail" is set to "yes"
+    And parameter "shareapi_enforce_links_password" of app "core" is set to "yes"
     And user "user0" exists
     And As an "user0"
     When creating a share with

lib/private/Share20/Manager.php

@@ -228,18 +228,19 @@ class Manager implements IManager {
 				throw new \InvalidArgumentException('SharedWith is not a valid group');
 			}
 		} elseif ($share->getShareType() === IShare::TYPE_LINK) {
+			// No check for TYPE_EMAIL here as we have a recipient for them
 			if ($share->getSharedWith() !== null) {
 				throw new \InvalidArgumentException('SharedWith should be empty');
 			}
-		} elseif ($share->getShareType() === IShare::TYPE_REMOTE) {
+		} elseif ($share->getShareType() === IShare::TYPE_EMAIL) {
 			if ($share->getSharedWith() === null) {
 				throw new \InvalidArgumentException('SharedWith should not be empty');
 			}
-		} elseif ($share->getShareType() === IShare::TYPE_REMOTE_GROUP) {
+		} elseif ($share->getShareType() === IShare::TYPE_REMOTE) {
 			if ($share->getSharedWith() === null) {
 				throw new \InvalidArgumentException('SharedWith should not be empty');
 			}
-		} elseif ($share->getShareType() === IShare::TYPE_EMAIL) {
+		} elseif ($share->getShareType() === IShare::TYPE_REMOTE_GROUP) {
 			if ($share->getSharedWith() === null) {
 				throw new \InvalidArgumentException('SharedWith should not be empty');
 			}
@@ -750,7 +751,8 @@ class Manager implements IManager {
 
 				//Verify the expiration date
 				$share = $this->validateExpirationDateInternal($share);
-			} elseif ($share->getShareType() === IShare::TYPE_LINK) {
+			} elseif ($share->getShareType() === IShare::TYPE_LINK
+				|| $share->getShareType() === IShare::TYPE_EMAIL) {
 				$this->linkCreateChecks($share);
 				$this->setLinkParent($share);
 
@@ -764,23 +766,16 @@ class Manager implements IManager {
 					)
 				);
 
-				//Verify the expiration date
+				// Verify the expiration date
 				$share = $this->validateExpirationDateLink($share);
 
-				//Verify the password
+				// Verify the password
 				$this->verifyPassword($share->getPassword());
 
 				// If a password is set. Hash it!
 				if ($share->getPassword() !== null) {
 					$share->setPassword($this->hasher->hash($share->getPassword()));
 				}
-			} elseif ($share->getShareType() === IShare::TYPE_EMAIL) {
-				$share->setToken(
-					$this->secureRandom->generate(
-						\OC\Share\Constants::TOKEN_LENGTH,
-						\OCP\Security\ISecureRandom::CHAR_HUMAN_READABLE
-					)
-				);
 			}
 
 			// Cannot share with the owner
@@ -988,7 +983,8 @@ class Manager implements IManager {
 				$this->validateExpirationDateInternal($share);
 				$expirationDateUpdated = true;
 			}
-		} elseif ($share->getShareType() === IShare::TYPE_LINK) {
+		} elseif ($share->getShareType() === IShare::TYPE_LINK
+			|| $share->getShareType() === IShare::TYPE_EMAIL) {
 			$this->linkCreateChecks($share);
 
 			$plainTextPassword = $share->getPassword();
@@ -1011,6 +1007,9 @@ class Manager implements IManager {
 			if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare)) {
 				$plainTextPassword = null;
 			}
+
+			$this->updateSharePasswordIfNeeded($share, $originalShare);
+
 			if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) {
 				// If the same password was already sent by mail the recipient
 				// would already have access to the share without having to call
@@ -1019,6 +1018,12 @@ class Manager implements IManager {
 			} elseif (empty($plainTextPassword) && $originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()) {
 				throw new \InvalidArgumentException('Can’t disable sending the password by Talk without setting a new password');
 			}
+
+			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
+				// Verify the expiration date
+				$this->validateExpirationDate($share);
+				$expirationDateUpdated = true;
+			}
 		}
 
 		$this->pathCreateChecks($share->getNode());
@@ -1218,7 +1223,8 @@ class Manager implements IManager {
 	 * @inheritdoc
 	 */
 	public function moveShare(IShare $share, $recipientId) {
-		if ($share->getShareType() === IShare::TYPE_LINK) {
+		if ($share->getShareType() === IShare::TYPE_LINK
+			|| $share->getShareType() === IShare::TYPE_EMAIL) {
 			throw new \InvalidArgumentException('Can’t change target of link share');
 		}
 
@@ -1480,10 +1486,10 @@ class Manager implements IManager {
 		$this->checkExpireDate($share);
 
 		/*
-		 * Reduce the permissions for link shares if public upload is not enabled
+		 * Reduce the permissions for link or email shares if public upload is not enabled
 		 */
-		if ($share->getShareType() === IShare::TYPE_LINK &&
-			!$this->shareApiLinkAllowPublicUpload()) {
+		if (($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL)
+			&& !$this->shareApiLinkAllowPublicUpload()) {
 			$share->setPermissions($share->getPermissions() & ~(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE));
 		}
 

lib/private/Share20/ProviderFactory.php

@@ -43,6 +43,7 @@ use OCA\ShareByMail\ShareByMailProvider;
 use OCP\Defaults;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IServerContainer;
+use OCP\Share\IManager;
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
 use OCP\Share\IShareProvider;
@@ -195,7 +196,8 @@ class ProviderFactory implements IProviderFactory {
 				$settingsManager,
 				$this->serverContainer->query(Defaults::class),
 				$this->serverContainer->getHasher(),
-				$this->serverContainer->get(IEventDispatcher::class)
+				$this->serverContainer->get(IEventDispatcher::class),
+				$this->serverContainer->get(IManager::class)
 			);
 		}