feat(files_sharing): allow mixed values in share attributes and allow storing email arrays

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

8 files changed, 66 insertions, 25 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 72ebd39ea07..e2ea4012208 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -1986,10 +1986,17 @@ class ShareAPIController extends OCSController {
 			$formattedShareAttributes = \json_decode($attributesString, true);
 			if (is_array($formattedShareAttributes)) {
 				foreach ($formattedShareAttributes as $formattedAttr) {
+					// Legacy handling of the 'enabled' attribute
+					if ($formattedAttr['enabled']) {
+						$formattedAttr['value'] = is_string($formattedAttr['enabled'])
+							? (bool) \json_decode($formattedAttr['enabled'])
+							: $formattedAttr['enabled'];
+					}
+
 					$newShareAttributes->setAttribute(
 						$formattedAttr['scope'],
 						$formattedAttr['key'],
-						is_string($formattedAttr['enabled']) ? (bool) \json_decode($formattedAttr['enabled']) : $formattedAttr['enabled']
+						$formattedAttr['value'],
 					);
 				}
 			} else {
diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php
index b70c16872ee..8968f6fa4d1 100644
--- a/apps/files_sharing/lib/MountProvider.php
+++ b/apps/files_sharing/lib/MountProvider.php
@@ -215,7 +215,7 @@ class MountProvider implements IMountProvider {
 							continue;
 						}
 						// update supershare attributes with subshare attribute
-						$superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['enabled']);
+						$superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['value']);
 					}
 				}
 
diff --git a/apps/files_sharing/src/components/NewFileRequestDialog.vue b/apps/files_sharing/src/components/NewFileRequestDialog.vue
index 53b4408b263..7db7c75313f 100644
--- a/apps/files_sharing/src/components/NewFileRequestDialog.vue
+++ b/apps/files_sharing/src/components/NewFileRequestDialog.vue
@@ -244,7 +244,7 @@ export default defineComponent({
 			try {
 				const request = await axios.post(shareUrl, {
 					path: this.destination,
-					shareType: Type.SHARE_TYPE_LINK,
+					shareType: Type.SHARE_TYPE_EMAIL,
 					publicUpload: 'true',
 					password: this.password || undefined,
 					expireDate,
diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue
index 9410f980a54..210029f617b 100644
--- a/apps/files_sharing/src/components/SharingEntryLink.vue
+++ b/apps/files_sharing/src/components/SharingEntryLink.vue
@@ -554,7 +554,7 @@ export default {
 		},
 
 		canChangeHideDownload() {
-			const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.enabled === false
+			const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.value === false
 			return this.fileInfo.shareAttributes.some(hasDisabledDownload)
 		},
 	},
diff --git a/apps/files_sharing/src/mixins/ShareDetails.js b/apps/files_sharing/src/mixins/ShareDetails.js
index 36e0e6c03f7..6c936b393ea 100644
--- a/apps/files_sharing/src/mixins/ShareDetails.js
+++ b/apps/files_sharing/src/mixins/ShareDetails.js
@@ -46,7 +46,7 @@ export default {
 			const share = {
 				attributes: [
 					{
-						enabled: true,
+						value: true,
 						key: 'download',
 						scope: 'permissions',
 					},
diff --git a/apps/files_sharing/src/models/Share.js b/apps/files_sharing/src/models/Share.js
index 72e85773855..479d55ca241 100644
--- a/apps/files_sharing/src/models/Share.js
+++ b/apps/files_sharing/src/models/Share.js
@@ -535,7 +535,7 @@ export default class Share {
 		for (const i in this._share.attributes) {
 			const attr = this._share.attributes[i]
 			if (attr.scope === 'permissions' && attr.key === 'download') {
-				return attr.enabled
+				return attr.value
 			}
 		}
 
@@ -546,11 +546,11 @@ export default class Share {
 		this.setAttribute('permissions', 'download', !!enabled)
 	}
 
-	setAttribute(scope, key, enabled) {
+	setAttribute(scope, key, value) {
 		const attrUpdate = {
 			scope,
 			key,
-			enabled,
+			value,
 		}
 
 		// try and replace existing
diff --git a/apps/files_sharing/src/views/SharingDetailsTab.vue b/apps/files_sharing/src/views/SharingDetailsTab.vue
index a9ade99f556..610f9b99eac 100644
--- a/apps/files_sharing/src/views/SharingDetailsTab.vue
+++ b/apps/files_sharing/src/views/SharingDetailsTab.vue
@@ -421,13 +421,13 @@ export default {
 		 */
 		canDownload: {
 			get() {
-				return this.share.attributes.find(attr => attr.key === 'download')?.enabled || false
+				return this.share.attributes.find(attr => attr.key === 'download')?.value || false
 			},
 			set(checked) {
 				// Find the 'download' attribute and update its value
 				const downloadAttr = this.share.attributes.find(attr => attr.key === 'download')
 				if (downloadAttr) {
-					downloadAttr.enabled = checked
+					downloadAttr.value = checked
 				}
 			},
 		},
@@ -678,7 +678,7 @@ export default {
 			return OC.appswebroots.spreed !== undefined
 		},
 		canChangeHideDownload() {
-			const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.enabled === false
+			const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.value === false
 			return this.fileInfo.shareAttributes.some(hasDisabledDownload)
 		},
 		customPermissionsList() {
diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index 63f80f5db5f..ae71d86028a 100644
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -5,6 +5,7 @@
  */
 namespace OCA\ShareByMail;
 
+use OC\Share20\DefaultShareProvider;
 use OC\Share20\Exception\InvalidShare;
 use OC\Share20\Share;
 use OC\User\NoUserException;
@@ -29,6 +30,7 @@ use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IAttributes;
 use OCP\Share\IManager as IShareManager;
 use OCP\Share\IShare;
 use OCP\Share\IShareProviderWithNotification;
@@ -39,7 +41,7 @@ use Psr\Log\LoggerInterface;
  *
  * @package OCA\ShareByMail
  */
-class ShareByMailProvider implements IShareProviderWithNotification {
+class ShareByMailProvider extends DefaultShareProvider implements IShareProviderWithNotification {
 	/**
 	 * Return the identifier of this provider.
 	 *
@@ -76,11 +78,10 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 	 */
 	public function create(IShare $share): IShare {
 		$shareWith = $share->getSharedWith();
-		/*
-		 * Check if file is not already shared with the remote user
-		 */
+		// Check if file is not already shared with the given email,
+		// if we have an email at all.
 		$alreadyShared = $this->getSharedWith($shareWith, IShare::TYPE_EMAIL, $share->getNode(), 1, 0);
-		if (!empty($alreadyShared)) {
+		if ($shareWith !== '' && !empty($alreadyShared)){
 			$message = 'Sharing %1$s failed, because this item is already shared with the account %2$s';
 			$message_t = $this->l->t('Sharing %1$s failed, because this item is already shared with the account %2$s', [$share->getNode()->getName(), $shareWith]);
 			$this->logger->debug(sprintf($message, $share->getNode()->getName(), $shareWith), ['app' => 'Federated File Sharing']);
@@ -224,7 +225,8 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 			$share->getHideDownload(),
 			$share->getLabel(),
 			$share->getExpirationDate(),
-			$share->getNote()
+			$share->getNote(),
+			$share->getAttributes(),
 		);
 	}
 
@@ -233,11 +235,17 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 	 */
 	public function sendMailNotification(IShare $share): bool {
 		$shareId = $share->getId();
-		if (!$this->mailer->validateMailAddress($share->getSharedWith())) {
+
+		$emails = $this->getSharedWithEmails($share);
+		$validEmails = array_filter($emails, function ($email) {
+			return $this->mailer->validateMailAddress($email);
+		});
+
+		if (count($validEmails) === 0) {
 			$this->removeShareFromTable($shareId);
-			$e = new HintException('Failed to send share by mail. Got an invalid email address: ' . $share->getSharedWith(),
+			$e = new HintException('Failed to send share by mail. Could not find a valid email address: ' . join(', ', $emails),
 				$this->l->t('Failed to send share by email. Got an invalid email address'));
-			$this->logger->error('Failed to send share by mail. Got an invalid email address ' . $share->getSharedWith(), [
+			$this->logger->error('Failed to send share by mail. Could not find a valid email address ' . join(', ', $emails), [
 				'app' => 'sharebymail',
 				'exception' => $e,
 			]);
@@ -250,7 +258,7 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 				$share->getNode()->getName(),
 				$link,
 				$share->getSharedBy(),
-				$share->getSharedWith(),
+				$emails,
 				$share->getExpirationDate(),
 				$share->getNote()
 			);
@@ -293,7 +301,7 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 	 * @param string $filename file/folder name
 	 * @param string $link link to the file/folder
 	 * @param string $initiator user ID of share sender
-	 * @param string $shareWith email addresses
+	 * @param string[] $shareWith email addresses
 	 * @param \DateTime|null $expiration expiration date
 	 * @param string $note note
 	 * @throws \Exception If mail couldn't be sent
@@ -302,7 +310,7 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 		string $filename,
 		string $link,
 		string $initiator,
-		string $shareWith,
+		array $shareWith,
 		?\DateTime $expiration = null,
 		string $note = '',
 	): void {
@@ -337,7 +345,13 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 			$link
 		);
 
-		$message->setTo([$shareWith]);
+		// If multiple recipients are given, we send the mail to all of them
+		if (count($shareWith) > 1) {
+			// We do not want to expose the email addresses of the other recipients
+			$message->setBcc($shareWith);
+		} else {
+			$message->setTo($shareWith);
+		}
 
 		// The "From" contains the sharers name
 		$instanceName = $this->defaults->getName();
@@ -618,7 +632,8 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 		?bool $hideDownload,
 		?string $label,
 		?\DateTimeInterface $expirationTime,
-		?string $note = ''
+		?string $note = '',
+		?IAttributes $attributes = null,
 	): int {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->insert('share')
@@ -640,6 +655,10 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 			->setValue('note', $qb->createNamedParameter($note))
 			->setValue('mail_send', $qb->createNamedParameter(1));
 
+		// set share attributes
+		$shareAttributes = $this->formatShareAttributes($attributes);
+
+		$qb->setValue('attributes', $qb->createNamedParameter($shareAttributes));
 		if ($expirationTime !== null) {
 			$qb->setValue('expiration', $qb->createNamedParameter($expirationTime, IQueryBuilder::PARAM_DATE));
 		}
@@ -955,6 +974,8 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 			}
 		}
 
+		$share = $this->updateShareAttributes($share, $data['attributes']);
+
 		$share->setNodeId((int)$data['file_source']);
 		$share->setNodeType($data['item_type']);
 
@@ -1137,4 +1158,17 @@ class ShareByMailProvider implements IShareProviderWithNotification {
 		}
 		$cursor->closeCursor();
 	}
+
+	/**
+	 * Extract the emails from the share
+	 * It can be a single email, from the share_with field
+	 * or a list of emails from the emails attributes field.
+	 */
+	protected function getSharedWithEmails(IShare $share) {
+		$attributes = $share->getAttributes()->getAttribute('sharedWith', 'emails');
+		if (isset($attributes) && is_array($attributes) && !empty($attributes)) {
+			return $attributes;
+		}
+		return [$share->getSharedWith()];
+	}
 }