Merge pull request #45481 from nextcloud/backport/44485/stable27

[stable27] Respect empty `expiryDate` value in server

7 files changed, 185 insertions, 129 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index eb60a3a85f1..23a35a0550d 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -233,7 +233,7 @@ class ShareAPIController extends OCSController {
 			$result['share_with'] = $share->getSharedWith();
 			$result['share_with_displayname'] = $sharedWith !== null ? $sharedWith->getDisplayName() : $share->getSharedWith();
 			$result['share_with_displayname_unique'] = $sharedWith !== null ? (
-				 !empty($sharedWith->getSystemEMailAddress()) ? $sharedWith->getSystemEMailAddress() : $sharedWith->getUID()
+				!empty($sharedWith->getSystemEMailAddress()) ? $sharedWith->getSystemEMailAddress() : $sharedWith->getUID()
 			) : $share->getSharedWith();
 			$result['status'] = [];
 
@@ -332,7 +332,7 @@ class ShareAPIController extends OCSController {
 
 		$result['attributes'] = null;
 		if ($attributes = $share->getAttributes()) {
-			$result['attributes'] =  \json_encode($attributes->toArray());
+			$result['attributes'] = \json_encode($attributes->toArray());
 		}
 
 		return $result;
@@ -579,7 +579,7 @@ class ShareAPIController extends OCSController {
 		string $publicUpload = 'false',
 		string $password = '',
 		string $sendPasswordByTalk = null,
-		string $expireDate = '',
+		string $expireDate = null,
 		string $note = '',
 		string $label = '',
 		string $attributes = null
@@ -653,6 +653,21 @@ class ShareAPIController extends OCSController {
 			$share = $this->setShareAttributes($share, $attributes);
 		}
 
+		if ($expireDate !== null) {
+			if ($expireDate !== '') {
+				try {
+					$expireDate = $this->parseDate($expireDate);
+					$share->setExpirationDate($expireDate);
+				} catch (\Exception $e) {
+					throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
+				}
+			} else {
+				// Client sent empty string for expire date.
+				// Set noExpirationDate to true so overwrite is prevented.
+				$share->setNoExpirationDate(true);
+			}
+		}
+
 		$share->setSharedBy($this->currentUser);
 		$this->checkInheritedAttributes($share);
 
@@ -739,14 +754,6 @@ class ShareAPIController extends OCSController {
 
 			$share->setSharedWith($shareWith);
 			$share->setPermissions($permissions);
-			if ($expireDate !== '') {
-				try {
-					$expireDate = $this->parseDate($expireDate);
-					$share->setExpirationDate($expireDate);
-				} catch (\Exception $e) {
-					throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-				}
-			}
 
 			$share->setSharedWithDisplayName($this->getCachedFederatedDisplayName($shareWith, false));
 		} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
@@ -760,14 +767,7 @@ class ShareAPIController extends OCSController {
 
 			$share->setSharedWith($shareWith);
 			$share->setPermissions($permissions);
-			if ($expireDate !== '') {
-				try {
-					$expireDate = $this->parseDate($expireDate);
-					$share->setExpirationDate($expireDate);
-				} catch (\Exception $e) {
-					throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-				}
-			}
+
 		} elseif ($shareType === IShare::TYPE_CIRCLE) {
 			if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
 				throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled'));
@@ -803,16 +803,6 @@ class ShareAPIController extends OCSController {
 			throw new OCSBadRequestException($this->l->t('Unknown share type'));
 		}
 
-		//Expire date
-		if ($expireDate !== '') {
-			try {
-				$expireDate = $this->parseDate($expireDate);
-				$share->setExpirationDate($expireDate);
-			} catch (\Exception $e) {
-				throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-			}
-		}
-
 		$share->setShareType($shareType);
 
 		if ($note !== '') {
@@ -1285,8 +1275,8 @@ class ShareAPIController extends OCSController {
 				}
 
 				if (!$this->hasPermission($newPermissions, Constants::PERMISSION_READ) && (
-						$this->hasPermission($newPermissions, Constants::PERMISSION_UPDATE) || $this->hasPermission($newPermissions, Constants::PERMISSION_DELETE)
-					)) {
+					$this->hasPermission($newPermissions, Constants::PERMISSION_UPDATE) || $this->hasPermission($newPermissions, Constants::PERMISSION_DELETE)
+				)) {
 					throw new OCSBadRequestException($this->l->t('Share must have READ permission if UPDATE or DELETE permission is set'));
 				}
 			}
diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json
index cf81e358b5d..ea43488ec49 100644
--- a/apps/files_sharing/openapi.json
+++ b/apps/files_sharing/openapi.json
@@ -1563,10 +1563,10 @@
                     {
                         "name": "expireDate",
                         "in": "query",
-                        "description": "Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.",
+                        "description": "The expiry date of the share in the user's timezone (UTC) at 00:00. If $expireDate is not supplied or set to `null`, the system default will be used.",
                         "schema": {
                             "type": "string",
-                            "default": ""
+                            "nullable": true
                         }
                     },
                     {
diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php
index 2a6a509d65f..0f1b2215639 100644
--- a/build/integration/features/bootstrap/Sharing.php
+++ b/build/integration/features/bootstrap/Sharing.php
@@ -81,7 +81,9 @@ trait Sharing {
 			$fd = $body->getRowsHash();
 			if (array_key_exists('expireDate', $fd)) {
 				$dateModification = $fd['expireDate'];
-				$fd['expireDate'] = date('Y-m-d', strtotime($dateModification));
+				if (!empty($dateModification)) {
+					$fd['expireDate'] = date('Y-m-d', strtotime($dateModification));
+				}
 			}
 			$options['form_params'] = $fd;
 		}
@@ -270,13 +272,13 @@ trait Sharing {
 	}
 
 	public function createShare($user,
-								$path = null,
-								$shareType = null,
-								$shareWith = null,
-								$publicUpload = null,
-								$password = null,
-								$permissions = null,
-								$viewOnly = false) {
+		$path = null,
+		$shareType = null,
+		$shareWith = null,
+		$publicUpload = null,
+		$password = null,
+		$permissions = null,
+		$viewOnly = false) {
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares";
 		$client = new Client();
 		$options = [
@@ -328,7 +330,9 @@ trait Sharing {
 	public function isFieldInResponse($field, $contentExpected) {
 		$data = simplexml_load_string($this->response->getBody())->data[0];
 		if ((string)$field == 'expiration') {
-			$contentExpected = date('Y-m-d', strtotime($contentExpected)) . " 00:00:00";
+			if (!empty($contentExpected)) {
+				$contentExpected = date('Y-m-d', strtotime($contentExpected)) . " 00:00:00";
+			}
 		}
 		if (count($data->element) > 0) {
 			foreach ($data as $element) {
diff --git a/build/integration/sharing_features/sharing-v1.feature b/build/integration/sharing_features/sharing-v1.feature
index ca030bd3a31..2b5d4e89331 100644
--- a/build/integration/sharing_features/sharing-v1.feature
+++ b/build/integration/sharing_features/sharing-v1.feature
@@ -229,6 +229,24 @@ Feature: sharing
       | url | AN_URL |
       | mimetype | httpd/unix-directory |
 
+  Scenario: Creating a new share with expiration date removed, when default expiration is set
+    Given user "user0" exists
+    And user "user1" exists
+    And parameter "shareapi_default_expire_date" of app "core" is set to "yes"
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareWith | user1 |
+      | shareType | 0 |
+      | expireDate | |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And Getting info of last share
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And Share fields of last share match with
+      | expiration ||
+
   Scenario: Creating a new public share, updating its password and getting its info
     Given user "user0" exists
     And As an "user0"
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 62dfeafe26a..1bb4657ef79 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -298,7 +298,7 @@ class Manager implements IManager {
 
 		$isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage');
 		$permissions = 0;
-		
+
 		$isReshare = $share->getNode()->getOwner() && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy();
 		if (!$isReshare && $isUpdate) {
 			// in case of update on owner-less filesystem, we use share owner to improve reshare detection
@@ -392,26 +392,6 @@ class Manager implements IManager {
 
 		$expirationDate = $share->getExpirationDate();
 
-		if ($expirationDate !== null) {
-			$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
-			$expirationDate->setTime(0, 0, 0);
-
-			$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$date->setTime(0, 0, 0);
-			if ($date >= $expirationDate) {
-				$message = $this->l->t('Expiration date is in the past');
-				throw new GenericShareException($message, $message, 404);
-			}
-		}
-
-		// If expiredate is empty set a default one if there is a default
-		$fullId = null;
-		try {
-			$fullId = $share->getFullId();
-		} catch (\UnexpectedValueException $e) {
-			// This is a new share
-		}
-
 		if ($isRemote) {
 			$defaultExpireDate = $this->shareApiRemoteDefaultExpireDate();
 			$defaultExpireDays = $this->shareApiRemoteDefaultExpireDays();
@@ -423,28 +403,53 @@ class Manager implements IManager {
 			$configProp = 'internal_defaultExpDays';
 			$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
 		}
-		if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
-			$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$expirationDate->setTime(0, 0, 0);
-			$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
-			if ($days > $defaultExpireDays) {
-				$days = $defaultExpireDays;
+
+		// If $expirationDate is falsy, noExpirationDate is true and expiration not enforced
+		// Then skip expiration date validation as null is accepted
+		if (!($share->getNoExpirationDate() && !$isEnforced)) {
+			if ($expirationDate != null) {
+				$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
+				$expirationDate->setTime(0, 0, 0);
+
+				$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$date->setTime(0, 0, 0);
+				if ($date >= $expirationDate) {
+					$message = $this->l->t('Expiration date is in the past');
+					throw new GenericShareException($message, $message, 404);
+				}
 			}
-			$expirationDate->add(new \DateInterval('P' . $days . 'D'));
-		}
 
-		// If we enforce the expiration date check that is does not exceed
-		if ($isEnforced) {
-			if ($expirationDate === null) {
-				throw new \InvalidArgumentException('Expiration date is enforced');
+			// If expiredate is empty set a default one if there is a default
+			$fullId = null;
+			try {
+				$fullId = $share->getFullId();
+			} catch (\UnexpectedValueException $e) {
+				// This is a new share
+			}
+
+			if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
+				$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$expirationDate->setTime(0, 0, 0);
+				$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
+				if ($days > $defaultExpireDays) {
+					$days = $defaultExpireDays;
+				}
+				$expirationDate->add(new \DateInterval('P' . $days . 'D'));
 			}
 
-			$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$date->setTime(0, 0, 0);
-			$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
-			if ($date < $expirationDate) {
-				$message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $defaultExpireDays);
-				throw new GenericShareException($message, $message, 404);
+			// If we enforce the expiration date check that is does not exceed
+			if ($isEnforced) {
+				if (empty($expirationDate)) {
+					throw new \InvalidArgumentException('Expiration date is enforced');
+				}
+
+				$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$date->setTime(0, 0, 0);
+				$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
+				if ($date < $expirationDate) {
+					$message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $defaultExpireDays);
+					throw new GenericShareException($message, $message, 404);
+				}
 			}
 		}
 
@@ -477,50 +482,55 @@ class Manager implements IManager {
 	 */
 	protected function validateExpirationDateLink(IShare $share) {
 		$expirationDate = $share->getExpirationDate();
-
-		if ($expirationDate !== null) {
-			$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
-			$expirationDate->setTime(0, 0, 0);
-
-			$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$date->setTime(0, 0, 0);
-			if ($date >= $expirationDate) {
-				$message = $this->l->t('Expiration date is in the past');
-				throw new GenericShareException($message, $message, 404);
+		$isEnforced = $this->shareApiLinkDefaultExpireDateEnforced();
+
+		// If $expirationDate is falsy, noExpirationDate is true and expiration not enforced
+		// Then skip expiration date validation as null is accepted
+		if (!($share->getNoExpirationDate() && !$isEnforced)) {
+			if ($expirationDate !== null) {
+				$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
+				$expirationDate->setTime(0, 0, 0);
+
+				$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$date->setTime(0, 0, 0);
+				if ($date >= $expirationDate) {
+					$message = $this->l->t('Expiration date is in the past');
+					throw new GenericShareException($message, $message, 404);
+				}
 			}
-		}
 
-		// If expiredate is empty set a default one if there is a default
-		$fullId = null;
-		try {
-			$fullId = $share->getFullId();
-		} catch (\UnexpectedValueException $e) {
-			// This is a new share
-		}
+			// If expiredate is empty set a default one if there is a default
+			$fullId = null;
+			try {
+				$fullId = $share->getFullId();
+			} catch (\UnexpectedValueException $e) {
+				// This is a new share
+			}
 
-		if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
-			$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$expirationDate->setTime(0, 0, 0);
+			if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
+				$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$expirationDate->setTime(0, 0, 0);
 
-			$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());
-			if ($days > $this->shareApiLinkDefaultExpireDays()) {
-				$days = $this->shareApiLinkDefaultExpireDays();
+				$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());
+				if ($days > $this->shareApiLinkDefaultExpireDays()) {
+					$days = $this->shareApiLinkDefaultExpireDays();
+				}
+				$expirationDate->add(new \DateInterval('P' . $days . 'D'));
 			}
-			$expirationDate->add(new \DateInterval('P' . $days . 'D'));
-		}
 
-		// If we enforce the expiration date check that is does not exceed
-		if ($this->shareApiLinkDefaultExpireDateEnforced()) {
-			if ($expirationDate === null) {
-				throw new \InvalidArgumentException('Expiration date is enforced');
-			}
+			// If we enforce the expiration date check that is does not exceed
+			if ($isEnforced) {
+				if (empty($expirationDate)) {
+					throw new \InvalidArgumentException('Expiration date is enforced');
+				}
 
-			$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
-			$date->setTime(0, 0, 0);
-			$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
-			if ($date < $expirationDate) {
-				$message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $this->shareApiLinkDefaultExpireDays());
-				throw new GenericShareException($message, $message, 404);
+				$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
+				$date->setTime(0, 0, 0);
+				$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
+				if ($date < $expirationDate) {
+					$message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $this->shareApiLinkDefaultExpireDays());
+					throw new GenericShareException($message, $message, 404);
+				}
 			}
 		}
 
@@ -896,12 +906,12 @@ class Manager implements IManager {
 	 * @param \DateTime|null $expiration
 	 */
 	protected function sendMailNotification(IL10N $l,
-											$filename,
-											$link,
-											$initiator,
-											$shareWith,
-											\DateTime $expiration = null,
-											$note = '') {
+		$filename,
+		$link,
+		$initiator,
+		$shareWith,
+		\DateTime $expiration = null,
+		$note = '') {
 		$initiatorUser = $this->userManager->get($initiator);
 		$initiatorDisplayName = ($initiatorUser instanceof IUser) ? $initiatorUser->getDisplayName() : $initiator;
 
diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php
index 0a50fa0ccfb..b911a3b4623 100644
--- a/lib/private/Share20/Share.php
+++ b/lib/private/Share20/Share.php
@@ -29,8 +29,8 @@
  */
 namespace OC\Share20;
 
-use OCP\Files\File;
 use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\File;
 use OCP\Files\FileInfo;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
@@ -99,10 +99,11 @@ class Share implements IShare {
 
 	/** @var ICacheEntry|null */
 	private $nodeCacheEntry;
-
 	/** @var bool */
 	private $hideDownload = false;
 
+	private bool $noExpirationDate = false;
+
 	public function __construct(IRootFolder $rootFolder, IUserManager $userManager) {
 		$this->rootFolder = $rootFolder;
 		$this->userManager = $userManager;
@@ -424,6 +425,21 @@ class Share implements IShare {
 	/**
 	 * @inheritdoc
 	 */
+	public function setNoExpirationDate(bool $noExpirationDate) {
+		$this->noExpirationDate = $noExpirationDate;
+		return $this;
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function getNoExpirationDate(): bool {
+		return $this->noExpirationDate;
+	}
+
+	/**
+	 * @inheritdoc
+	 */
 	public function isExpired() {
 		return $this->getExpirationDate() !== null &&
 			$this->getExpirationDate() <= new \DateTime();
diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php
index 40548c6c73d..84388c00bbe 100644
--- a/lib/public/Share/IShare.php
+++ b/lib/public/Share/IShare.php
@@ -385,21 +385,39 @@ interface IShare {
 	/**
 	 * Set the expiration date
 	 *
-	 * @param null|\DateTime $expireDate
+	 * @param \DateTime|null $expireDate
 	 * @return \OCP\Share\IShare The modified object
 	 * @since 9.0.0
 	 */
-	public function setExpirationDate($expireDate);
+	public function setExpirationDate(\DateTime|null $expireDate);
 
 	/**
 	 * Get the expiration date
 	 *
-	 * @return \DateTime
+	 * @return \DateTime|null
 	 * @since 9.0.0
 	 */
 	public function getExpirationDate();
 
 	/**
+	 * Set overwrite flag for falsy expiry date vavlues
+	 *
+	 * @param bool $noExpirationDate
+	 * @return \OCP\Share\IShare The modified object
+	 * @since 27.1.10
+	 */
+	public function setNoExpirationDate(bool $noExpirationDate);
+
+
+	/**
+	 * Get value of overwrite falsy expiry date flag
+	 *
+	 * @return bool
+	 * @since 27.1.10
+	 */
+	public function getNoExpirationDate();
+
+	/**
 	 * Is the share expired ?
 	 *
 	 * @return boolean