Skip to content

Commit cca99e8

Browse files
AltahrimAltahrim
committed
feat(share): save date and time for expiration
Because of timezones, not saving time can lead to unexpected behaviour when sharing an item sooner than timezone offset Example: sharing a file before 9am when in UTC+9 Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
1 parent 8822b16 commit cca99e8

File tree

7 files changed

+115
-44
lines changed

7 files changed

+115
-44
lines changed

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
240240

241241
$expiration = $share->getExpirationDate();
242242
if ($expiration !== null) {
243+
$expiration->setTimezone($this->dateTimeZone->getTimeZone());
243244
$result['expiration'] = $expiration->format('Y-m-d 00:00:00');
244245
}
245246

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

1705+
// Use server timezone to store the date
17021706
$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));
1703-
$date->setTime(0, 0, 0);
17041707

17051708
return $date;
17061709
}

‎apps/files_sharing/tests/ApiTest.php‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ private function createOCS($userId) {
124124
$userStatusManager = $this->createMock(IUserStatusManager::class);
125125
$previewManager = $this->createMock(IPreview::class);
126126
$dateTimeZone = $this->createMock(IDateTimeZone::class);
127+
$dateTimeZone->method('getTimeZone')->willReturn(new \DateTimeZone('Pacific/Auckland'));
127128

128129
return new ShareAPIController(
129130
self::APP_NAME,
@@ -1059,10 +1060,10 @@ public function testUpdateShareExpireDate() {
10591060
$config->setAppValue('core', 'shareapi_default_expire_date', 'yes');
10601061
$config->setAppValue('core', 'shareapi_enforce_expire_date', 'yes');
10611062

1062-
$dateWithinRange = new \DateTime();
1063+
$dateWithinRange = new \DateTime('now', new \DateTimeZone('Pacific/Auckland'));
10631064
$dateWithinRange->setTime(0, 0, 0);
10641065
$dateWithinRange->add(new \DateInterval('P5D'));
1065-
$dateOutOfRange = new \DateTime();
1066+
$dateOutOfRange = new \DateTime('now', new \DateTimeZone('Pacific/Auckland'));
10661067
$dateOutOfRange->setTime(0, 0, 0);
10671068
$dateOutOfRange->add(new \DateInterval('P8D'));
10681069

@@ -1286,13 +1287,15 @@ public function testShareStorageMountPoint() {
12861287
}
12871288

12881289
public function datesProvider() {
1289-
$date = new \DateTime();
1290+
$date = new \DateTime('now', new \DateTimeZone('Pacific/Auckland'));
1291+
$date->setTime(0, 0);
12901292
$date->add(new \DateInterval('P5D'));
1293+
$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));
12911294

12921295
return [
1293-
[$date->format('Y-m-d'), true],
1296+
[$date->format('Y-m-d H:i:s'), true],
12941297
['abc', false],
1295-
[$date->format('Y-m-d') . 'xyz', false],
1298+
[$date->format('Y-m-d H:i:s') . 'xyz', false],
12961299
];
12971300
}
12981301

@@ -1318,15 +1321,15 @@ public function testPublicLinkExpireDate($date, $valid) {
13181321

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

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

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

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

13311334
$this->shareManager->deleteShare($share);
13321335
}
@@ -1341,7 +1344,7 @@ public function testCreatePublicLinkExpireDateValid() {
13411344
$config->setAppValue('core', 'shareapi_default_expire_date', 'yes');
13421345
$config->setAppValue('core', 'shareapi_enforce_expire_date', 'yes');
13431346

1344-
$date = new \DateTime();
1347+
$date = new \DateTime('now', new \DateTimeZone('Pacific/Auckland'));
13451348
$date->add(new \DateInterval('P5D'));
13461349

13471350
$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
@@ -1350,7 +1353,7 @@ public function testCreatePublicLinkExpireDateValid() {
13501353

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

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

‎apps/files_sharing/tests/CapabilitiesTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use OCP\Files\IRootFolder;
3737
use OCP\Files\Mount\IMountManager;
3838
use OCP\IConfig;
39+
use OCP\IDateTimeZone;
3940
use OCP\IGroupManager;
4041
use OCP\IL10N;
4142
use OCP\IURLGenerator;
@@ -96,7 +97,8 @@ private function getResults(array $map) {
9697
$this->createMock(IEventDispatcher::class),
9798
$this->createMock(IUserSession::class),
9899
$this->createMock(KnownUserService::class),
99-
$this->createMock(ShareDisableChecker::class)
100+
$this->createMock(ShareDisableChecker::class),
101+
$this->createMock(IDateTimeZone::class),
100102
);
101103
$cap = new Capabilities($config, $shareManager);
102104
$result = $this->getFilesSharingPart($cap->getCapabilities());

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) {
845845
$this->groupManager->method('get')->willReturnMap([
846846
['group', $group],
847847
]);
848+
$this->dateTimeZone->method('getTimezone')->willReturn(new \DateTimeZone('UTC'));
848849

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

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

46514653
if (!$exception) {
46524654
$this->rootFolder->method('getById')

‎lib/private/Server.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,8 @@ public function __construct($webRoot, \OC\Config $config) {
12661266
$c->get(IEventDispatcher::class),
12671267
$c->get(IUserSession::class),
12681268
$c->get(KnownUserService::class),
1269-
$c->get(ShareDisableChecker::class)
1269+
$c->get(ShareDisableChecker::class),
1270+
$c->get(IDateTimeZone::class),
12701271
);
12711272

12721273
return $manager;

‎lib/private/Share20/Manager.php‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
use OCP\Files\Node;
5555
use OCP\HintException;
5656
use OCP\IConfig;
57+
use OCP\IDateTimeZone;
5758
use OCP\IGroupManager;
5859
use OCP\IL10N;
5960
use OCP\IURLGenerator;
@@ -120,6 +121,7 @@ class Manager implements IManager {
120121
/** @var KnownUserService */
121122
private $knownUserService;
122123
private ShareDisableChecker $shareDisableChecker;
124+
private IDateTimeZone $dateTimeZone;
123125

124126
public function __construct(
125127
LoggerInterface $logger,
@@ -139,7 +141,8 @@ public function __construct(
139141
IEventDispatcher $dispatcher,
140142
IUserSession $userSession,
141143
KnownUserService $knownUserService,
142-
ShareDisableChecker $shareDisableChecker
144+
ShareDisableChecker $shareDisableChecker,
145+
IDateTimeZone $dateTimeZone,
143146
) {
144147
$this->logger = $logger;
145148
$this->config = $config;
@@ -162,6 +165,7 @@ public function __construct(
162165
$this->userSession = $userSession;
163166
$this->knownUserService = $knownUserService;
164167
$this->shareDisableChecker = $shareDisableChecker;
168+
$this->dateTimeZone = $dateTimeZone;
165169
}
166170

167171
/**
@@ -382,10 +386,10 @@ protected function validateExpirationDateInternal(IShare $share) {
382386
$expirationDate = $share->getExpirationDate();
383387

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

388-
$date = new \DateTime();
392+
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
389393
$date->setTime(0, 0, 0);
390394
if ($date >= $expirationDate) {
391395
$message = $this->l->t('Expiration date is in the past');
@@ -413,9 +417,8 @@ protected function validateExpirationDateInternal(IShare $share) {
413417
$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
414418
}
415419
if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
416-
$expirationDate = new \DateTime();
420+
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
417421
$expirationDate->setTime(0, 0, 0);
418-
419422
$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
420423
if ($days > $defaultExpireDays) {
421424
$days = $defaultExpireDays;
@@ -429,7 +432,7 @@ protected function validateExpirationDateInternal(IShare $share) {
429432
throw new \InvalidArgumentException('Expiration date is enforced');
430433
}
431434

432-
$date = new \DateTime();
435+
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
433436
$date->setTime(0, 0, 0);
434437
$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
435438
if ($date < $expirationDate) {
@@ -469,10 +472,10 @@ protected function validateExpirationDateLink(IShare $share) {
469472
$expirationDate = $share->getExpirationDate();
470473

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

475-
$date = new \DateTime();
478+
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
476479
$date->setTime(0, 0, 0);
477480
if ($date >= $expirationDate) {
478481
$message = $this->l->t('Expiration date is in the past');
@@ -489,7 +492,7 @@ protected function validateExpirationDateLink(IShare $share) {
489492
}
490493

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

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

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

533+
if ($expirationDate instanceof \DateTimeInterface) {
534+
$expirationDate->setTimezone(new \DateTimeZone(date_default_timezone_get()));
535+
}
530536
$share->setExpirationDate($expirationDate);
531537

532538
return $share;

0 commit comments

Comments
 (0)