Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(share): save date and time for expiration #43428

Merged
merged 1 commit into from
Feb 26, 2024
Merged

feat(share): save date and time for expiration #43428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/files_sharing/lib/Controller/ShareAPIController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array

$expiration = $share->getExpirationDate();
if ($expiration !== null) {
$expiration->setTimezone($this->dateTimeZone->getTimeZone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that the timezone is relative to the current user, logged in or not? So, in the case of a public share, the timezone of the current user will be used instead of the one from the share owner. Maybe this is only ever called from the owner perspective, so it might not be a problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.
It's mainly use for display, so an user will see the right expiration date for him, even if time will probably be false.

If a share expire at 00:00:00 UTC on day D, an user in UTC-2 will see D-1 00:00:00. Time is false but ensure API compatibility.

$result['expiration'] = $expiration->format('Y-m-d 00:00:00');
}

Expand Down Expand Up @@ -1695,12 +1696,14 @@ protected function canDeleteShareFromSelf(\OCP\Share\IShare $share): bool {
private function parseDate(string $expireDate): \DateTime {
try {
$date = new \DateTime(trim($expireDate, "\""), $this->dateTimeZone->getTimeZone());
// Make sure it expires at midnight in owner timezone
$date->setTime(0, 0, 0);
} catch (\Exception $e) {
throw new \Exception('Invalid date. Format must be YYYY-MM-DD');
}

// Use server timezone to store the date
$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always UTC (see base.php)

$date->setTime(0, 0, 0);

return $date;
}
Expand Down
20 changes: 12 additions & 8 deletions apps/files_sharing/tests/ApiTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ private function createOCS($userId) {
$userStatusManager = $this->createMock(IUserStatusManager::class);
$previewManager = $this->createMock(IPreview::class);
$dateTimeZone = $this->createMock(IDateTimeZone::class);
$dateTimeZone->method('getTimeZone')->willReturn(new \DateTimeZone(date_default_timezone_get()));

return new ShareAPIController(
self::APP_NAME,
Expand Down Expand Up @@ -1060,10 +1061,9 @@ public function testUpdateShareExpireDate() {
$config->setAppValue('core', 'shareapi_enforce_expire_date', 'yes');

$dateWithinRange = new \DateTime();
$dateWithinRange->setTime(0, 0, 0);
$dateWithinRange->add(new \DateInterval('P5D'));
$dateWithinRange->add(new \DateInterval('P6D'));

$dateOutOfRange = new \DateTime();
$dateOutOfRange->setTime(0, 0, 0);
$dateOutOfRange->add(new \DateInterval('P8D'));

// update expire date to a valid value
Expand All @@ -1074,6 +1074,8 @@ public function testUpdateShareExpireDate() {
$share1 = $this->shareManager->getShareById($share1->getFullId());

// date should be changed
$dateWithinRange->setTime(0, 0, 0);
$dateWithinRange->setTimezone(new \DateTimeZone(date_default_timezone_get()));
$this->assertEquals($dateWithinRange, $share1->getExpirationDate());

// update expire date to a value out of range
Expand Down Expand Up @@ -1287,12 +1289,14 @@ public function testShareStorageMountPoint() {

public function datesProvider() {
$date = new \DateTime();
$date->setTime(0, 0);
$date->add(new \DateInterval('P5D'));
$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));

return [
[$date->format('Y-m-d'), true],
[$date->format('Y-m-d H:i:s'), true],
['abc', false],
[$date->format('Y-m-d') . 'xyz', false],
[$date->format('Y-m-d H:i:s') . 'xyz', false],
];
}

Expand All @@ -1318,15 +1322,15 @@ public function testPublicLinkExpireDate($date, $valid) {

$data = $result->getData();
$this->assertTrue(is_string($data['token']));
$this->assertEquals($date, substr($data['expiration'], 0, 10));
$this->assertEquals(substr($date, 0, 10), substr($data['expiration'], 0, 10));

// check for correct link
$url = \OC::$server->getURLGenerator()->getAbsoluteURL('/index.php/s/' . $data['token']);
$this->assertEquals($url, $data['url']);

$share = $this->shareManager->getShareById('ocinternal:'.$data['id']);

$this->assertEquals($date, $share->getExpirationDate()->format('Y-m-d'));
$this->assertEquals($date, $share->getExpirationDate()->format('Y-m-d H:i:s'));

$this->shareManager->deleteShare($share);
}
Expand All @@ -1350,7 +1354,7 @@ public function testCreatePublicLinkExpireDateValid() {

$data = $result->getData();
$this->assertTrue(is_string($data['token']));
$this->assertEquals($date->format('Y-m-d') . ' 00:00:00', $data['expiration']);
$this->assertEquals($date->format('Y-m-d 00:00:00'), $data['expiration']);

// check for correct link
$url = \OC::$server->getURLGenerator()->getAbsoluteURL('/index.php/s/' . $data['token']);
Expand Down
4 changes: 3 additions & 1 deletion apps/files_sharing/tests/CapabilitiesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use OCP\Files\IRootFolder;
use OCP\Files\Mount\IMountManager;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IURLGenerator;
Expand Down Expand Up @@ -96,7 +97,8 @@ private function getResults(array $map) {
$this->createMock(IEventDispatcher::class),
$this->createMock(IUserSession::class),
$this->createMock(KnownUserService::class),
$this->createMock(ShareDisableChecker::class)
$this->createMock(ShareDisableChecker::class),
$this->createMock(IDateTimeZone::class),
);
$cap = new Capabilities($config, $shareManager);
$result = $this->getFilesSharingPart($cap->getCapabilities());
Expand Down
2 changes: 2 additions & 0 deletions apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) {
$this->groupManager->method('get')->willReturnMap([
['group', $group],
]);
$this->dateTimeZone->method('getTimezone')->willReturn(new \DateTimeZone('UTC'));

$d = $ocs->getShare($share->getId())->getData()[0];

Expand Down Expand Up @@ -4647,6 +4648,7 @@ public function testFormatShare(array $expects, \OCP\Share\IShare $share, array
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturnSelf();
$this->dateTimeZone->method('getTimezone')->willReturn(new \DateTimeZone('UTC'));

if (!$exception) {
$this->rootFolder->method('getById')
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,8 @@ public function __construct($webRoot, \OC\Config $config) {
$c->get(IEventDispatcher::class),
$c->get(IUserSession::class),
$c->get(KnownUserService::class),
$c->get(ShareDisableChecker::class)
$c->get(ShareDisableChecker::class),
$c->get(IDateTimeZone::class),
);

return $manager;
Expand Down
26 changes: 16 additions & 10 deletions lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
use OCP\Files\Node;
use OCP\HintException;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IURLGenerator;
Expand Down Expand Up @@ -120,6 +121,7 @@ class Manager implements IManager {
/** @var KnownUserService */
private $knownUserService;
private ShareDisableChecker $shareDisableChecker;
private IDateTimeZone $dateTimeZone;

public function __construct(
LoggerInterface $logger,
Expand All @@ -139,7 +141,8 @@ public function __construct(
IEventDispatcher $dispatcher,
IUserSession $userSession,
KnownUserService $knownUserService,
ShareDisableChecker $shareDisableChecker
ShareDisableChecker $shareDisableChecker,
IDateTimeZone $dateTimeZone,
) {
$this->logger = $logger;
$this->config = $config;
Expand All @@ -162,6 +165,7 @@ public function __construct(
$this->userSession = $userSession;
$this->knownUserService = $knownUserService;
$this->shareDisableChecker = $shareDisableChecker;
$this->dateTimeZone = $dateTimeZone;
}

/**
Expand Down Expand Up @@ -382,10 +386,10 @@ protected function validateExpirationDateInternal(IShare $share) {
$expirationDate = $share->getExpirationDate();

if ($expirationDate !== null) {
//Make sure the expiration date is a date
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);

$date = new \DateTime();
$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');
Expand Down Expand Up @@ -413,9 +417,8 @@ protected function validateExpirationDateInternal(IShare $share) {
$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
}
if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
$expirationDate = new \DateTime();
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);
Altahrim marked this conversation as resolved.
Show resolved Hide resolved

$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
if ($days > $defaultExpireDays) {
$days = $defaultExpireDays;
Expand All @@ -429,7 +432,7 @@ protected function validateExpirationDateInternal(IShare $share) {
throw new \InvalidArgumentException('Expiration date is enforced');
}

$date = new \DateTime();
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
$date->setTime(0, 0, 0);
Altahrim marked this conversation as resolved.
Show resolved Hide resolved
$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
if ($date < $expirationDate) {
Expand Down Expand Up @@ -469,10 +472,10 @@ protected function validateExpirationDateLink(IShare $share) {
$expirationDate = $share->getExpirationDate();

if ($expirationDate !== null) {
//Make sure the expiration date is a date
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);

$date = new \DateTime();
$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');
Expand All @@ -489,7 +492,7 @@ protected function validateExpirationDateLink(IShare $share) {
}

if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
$expirationDate = new \DateTime();
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);

$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());
Expand All @@ -505,7 +508,7 @@ protected function validateExpirationDateLink(IShare $share) {
throw new \InvalidArgumentException('Expiration date is enforced');
}

$date = new \DateTime();
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
$date->setTime(0, 0, 0);
$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
if ($date < $expirationDate) {
Expand All @@ -527,6 +530,9 @@ protected function validateExpirationDateLink(IShare $share) {
throw new \Exception($message);
}

if ($expirationDate instanceof \DateTime) {
$expirationDate->setTimezone(new \DateTimeZone(date_default_timezone_get()));
Fixed Show fixed Hide fixed
}
$share->setExpirationDate($expirationDate);

return $share;
Expand Down
Loading
Loading