feat(files_sharing): allow mixed values in share attributes and allow…

… storing email arrays

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

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -1986,10 +1986,17 @@ private function setShareAttributes(IShare $share, ?string $attributesString) {
 			$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 {

apps/files_sharing/lib/MountProvider.php

@@ -215,7 +215,7 @@ private function buildSuperShares(array $allShares, \OCP\IUser $user) {
 							continue;
 						}
 						// update supershare attributes with subshare attribute
-						$superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['enabled']);
+						$superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['value']);
 					}
 				}
 

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,

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)
 		},
 	},

apps/files_sharing/src/mixins/ShareDetails.js

@@ -46,7 +46,7 @@ export default {
 			const share = {
 				attributes: [
 					{
-						enabled: true,
+						value: true,
 						key: 'download',
 						scope: 'permissions',
 					},

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

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() {

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\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 @@
  *
  * @package OCA\ShareByMail
  */
-class ShareByMailProvider implements IShareProviderWithNotification {
+class ShareByMailProvider extends DefaultShareProvider implements IShareProviderWithNotification {
 	/**
 	 * Return the identifier of this provider.
 	 *
@@ -76,11 +78,10 @@ public function __construct(
 	 */
 	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 @@ protected function createMailShare(IShare $share): int {
 			$share->getHideDownload(),
 			$share->getLabel(),
 			$share->getExpirationDate(),
-			$share->getNote()
+			$share->getNote(),
+			$share->getAttributes(),
 		);
 	}
 
@@ -233,11 +235,17 @@ protected function createMailShare(IShare $share): int {
 	 */
 	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 @@ public function sendMailNotification(IShare $share): bool {
 				$share->getNode()->getName(),
 				$link,
 				$share->getSharedBy(),
-				$share->getSharedWith(),
+				$emails,
 				$share->getExpirationDate(),
 				$share->getNote()
 			);
@@ -293,7 +301,7 @@ public function sendMailNotification(IShare $share): bool {
 	 * @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 @@ protected function sendEmail(
 		string $filename,
 		string $link,
 		string $initiator,
-		string $shareWith,
+		array $shareWith,
 		?\DateTime $expiration = null,
 		string $note = '',
 	): void {
@@ -337,7 +345,13 @@ protected function sendEmail(
 			$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 @@ protected function addShareToDB(
 		?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 @@ protected function addShareToDB(
 			->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 @@ protected function createShareObject(array $data): IShare {
 			}
 		}
 
+		$share = $this->updateShareAttributes($share, $data['attributes']);
+
 		$share->setNodeId((int)$data['file_source']);
 		$share->setNodeType($data['item_type']);
 
@@ -1137,4 +1158,17 @@ public function getAllShares(): iterable {
 		}
 		$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()];
+	}
 }

lib/private/Share20/DefaultShareProvider.php

@@ -1382,7 +1382,7 @@ private function propagateNote(IShare $share) {
 		}
 	}
 
-	public function sendMailNotification(IShare $share): true {
+	public function sendMailNotification(IShare $share): bool {
 		try {
 			// Check user
 			$user = $this->userManager->get($share->getSharedWith());
@@ -1616,7 +1616,7 @@ public function getAllShares(): iterable {
 	 *
 	 * @return IShare the modified share
 	 */
-	private function updateShareAttributes(IShare $share, ?string $data): IShare {
+	protected function updateShareAttributes(IShare $share, ?string $data): IShare {
 		if ($data !== null && $data !== '') {
 			$attributes = new ShareAttributes();
 			$compressedAttributes = \json_decode($data, true);
@@ -1639,7 +1639,7 @@ private function updateShareAttributes(IShare $share, ?string $data): IShare {
 	/**
 	 * Format IAttributes to database format (JSON string)
 	 */
-	private function formatShareAttributes(?IAttributes $attributes): ?string {
+	protected function formatShareAttributes(?IAttributes $attributes): ?string {
 		if ($attributes === null || empty($attributes->toArray())) {
 			return null;
 		}
@@ -1649,7 +1649,7 @@ private function formatShareAttributes(?IAttributes $attributes): ?string {
 			$compressedAttributes[] = [
 				0 => $attribute['scope'],
 				1 => $attribute['key'],
-				2 => $attribute['enabled']
+				2 => $attribute['value']
 			];
 		}
 		return \json_encode($compressedAttributes);

lib/private/Share20/ShareAttributes.php

@@ -20,11 +20,11 @@ public function __construct() {
 	/**
 	 * @inheritdoc
 	 */
-	public function setAttribute($scope, $key, $enabled) {
+	public function setAttribute($scope, $key, $value) {
 		if (!\array_key_exists($scope, $this->attributes)) {
 			$this->attributes[$scope] = [];
 		}
-		$this->attributes[$scope][$key] = $enabled;
+		$this->attributes[$scope][$key] = $value;
 		return $this;
 	}
 
@@ -45,11 +45,11 @@ public function getAttribute($scope, $key) {
 	public function toArray() {
 		$result = [];
 		foreach ($this->attributes as $scope => $keys) {
-			foreach ($keys as $key => $enabled) {
+			foreach ($keys as $key => $value) {
 				$result[] = [
 					"scope" => $scope,
 					"key" => $key,
-					"enabled" => $enabled
+					"value" => $value
 				];
 			}
 		}

lib/public/Share/IAttributes.php

@@ -17,7 +17,7 @@ interface IAttributes {
 	 *
 	 * @param string $scope scope
 	 * @param string $key key
-	 * @param bool $enabled enabled
+	 * @param bool|string|array|null $value value
 	 * @return IAttributes The modified object
 	 * @since 25.0.0
 	 */
@@ -29,7 +29,7 @@ public function setAttribute($scope, $key, $enabled);
 	 *
 	 * @param string $scope scope
 	 * @param string $key key
-	 * @return bool|null
+	 * @return bool|string|array|null
 	 * @since 25.0.0
 	 */
 	public function getAttribute($scope, $key);