Skip to content

Commit

Permalink
test(Sharing): Integration test for no expiration set date for share
Browse files Browse the repository at this point in the history
- Verify that explicitly sending empty `expireDate` param to server overwrite default

and sets not expiry date, if non is enforced.

- Update tests to avoid converting empty string to date.

Signed-off-by: fenn-cs <[email protected]>

[skip ci]

Signed-off-by: Louis Chemineau <[email protected]>
  • Loading branch information
nfebe authored and artonge committed May 23, 2024
1 parent e132870 commit 0eca300
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 29 deletions.
8 changes: 4 additions & 4 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
$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'] = [];

Expand Down Expand Up @@ -332,7 +332,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array

$result['attributes'] = null;
if ($attributes = $share->getAttributes()) {
$result['attributes'] = \json_encode($attributes->toArray());
$result['attributes'] = \json_encode($attributes->toArray());
}

return $result;
Expand Down Expand Up @@ -1275,8 +1275,8 @@ public function updateShare(
}

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'));
}
}
Expand Down
22 changes: 13 additions & 9 deletions build/integration/features/bootstrap/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ public function asCreatingAShareWith($user, $body) {
$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;
}
Expand Down Expand Up @@ -270,13 +272,13 @@ public function updatingLastShare($body) {
}

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 = [
Expand Down Expand Up @@ -328,7 +330,9 @@ public function createShare($user,
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) {
Expand Down
18 changes: 18 additions & 0 deletions build/integration/sharing_features/sharing-v1.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
29 changes: 14 additions & 15 deletions lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ protected function generalCreateChecks(IShare $share, bool $isUpdate = false) {

$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
Expand Down Expand Up @@ -406,7 +406,7 @@ protected function validateExpirationDateInternal(IShare $share) {

// 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 (!($share->getNoExpirationDate() && !$isEnforced)) {
if ($expirationDate != null) {
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);
Expand Down Expand Up @@ -486,11 +486,11 @@ protected function validateExpirationDateLink(IShare $share) {

// 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 (!($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) {
Expand All @@ -506,24 +506,24 @@ protected function validateExpirationDateLink(IShare $share) {
} 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);

$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'));
}

// 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'));
Expand All @@ -532,7 +532,6 @@ protected function validateExpirationDateLink(IShare $share) {
throw new GenericShareException($message, $message, 404);
}
}

}

$accepted = true;
Expand Down Expand Up @@ -907,12 +906,12 @@ public function createShare(IShare $share) {
* @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;

Expand Down
4 changes: 3 additions & 1 deletion lib/private/Share20/Share.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -102,6 +102,8 @@ class Share implements IShare {
/** @var bool */
private $hideDownload = false;

private bool $noExpirationDate = false;

public function __construct(IRootFolder $rootFolder, IUserManager $userManager) {
$this->rootFolder = $rootFolder;
$this->userManager = $userManager;
Expand Down

0 comments on commit 0eca300

Please sign in to comment.