From 75ca4944c9e9a1b20bdb1e01f5acf59f2c77afe2 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Fri, 5 Jul 2024 14:27:49 +0200 Subject: [PATCH] feat(files_sharing): allow mixed values in share attributes and allow storing email arrays Signed-off-by: skjnldsv --- .../lib/Controller/ShareAPIController.php | 9 ++- apps/files_sharing/lib/MountProvider.php | 2 +- .../src/components/NewFileRequestDialog.vue | 2 +- .../src/components/SharingEntryLink.vue | 2 +- apps/files_sharing/src/mixins/ShareDetails.js | 2 +- apps/files_sharing/src/models/Share.js | 6 +- .../src/views/SharingDetailsTab.vue | 6 +- apps/sharebymail/lib/ShareByMailProvider.php | 62 ++++++++++++++----- lib/private/Share20/DefaultShareProvider.php | 8 +-- lib/private/Share20/ShareAttributes.php | 8 +-- lib/public/Share/IAttributes.php | 4 +- 11 files changed, 76 insertions(+), 35 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()]; + } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 875c2a85188..d1d818222e2 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -1382,7 +1382,7 @@ class DefaultShareProvider implements IShareProviderWithNotification { } } - 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 @@ class DefaultShareProvider implements IShareProviderWithNotification { * * @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 @@ class DefaultShareProvider implements IShareProviderWithNotification { /** * 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 @@ class DefaultShareProvider implements IShareProviderWithNotification { $compressedAttributes[] = [ 0 => $attribute['scope'], 1 => $attribute['key'], - 2 => $attribute['enabled'] + 2 => $attribute['value'] ]; } return \json_encode($compressedAttributes); diff --git a/lib/private/Share20/ShareAttributes.php b/lib/private/Share20/ShareAttributes.php index abbbd36759b..4011278bb39 100644 --- a/lib/private/Share20/ShareAttributes.php +++ b/lib/private/Share20/ShareAttributes.php @@ -20,11 +20,11 @@ class ShareAttributes implements IAttributes { /** * @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 @@ class ShareAttributes implements IAttributes { 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 ]; } } diff --git a/lib/public/Share/IAttributes.php b/lib/public/Share/IAttributes.php index af7277c39c0..7ad2765354f 100644 --- a/lib/public/Share/IAttributes.php +++ b/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 @@ interface IAttributes { * * @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);