Skip to content

Commit

Permalink
fix(share): use user timezone to parse share expiration date
Browse files Browse the repository at this point in the history
If an user in UTC+1 try to create a share at 00:00, it's day D for him, but
D-1 for the server (UTC).
This fix aims to apply the correct offset

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
  • Loading branch information
Altahrim committed Jan 9, 2024
1 parent d72db91 commit cc3a2c3
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
23 changes: 6 additions & 17 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IPreview;
Expand Down Expand Up @@ -124,20 +125,6 @@ class ShareAPIController extends OCSController {

/**
* Share20OCS constructor.
*
* @param string $appName
* @param IRequest $request
* @param IManager $shareManager
* @param IGroupManager $groupManager
* @param IUserManager $userManager
* @param IRootFolder $rootFolder
* @param IURLGenerator $urlGenerator
* @param string $userId
* @param IL10N $l10n
* @param IConfig $config
* @param IAppManager $appManager
* @param IServerContainer $serverContainer
* @param IUserStatusManager $userStatusManager
*/
public function __construct(
string $appName,
Expand All @@ -153,7 +140,8 @@ public function __construct(
IAppManager $appManager,
IServerContainer $serverContainer,
IUserStatusManager $userStatusManager,
IPreview $previewManager
IPreview $previewManager,
private IDateTimeZone $dateTimeZone,
) {
parent::__construct($appName, $request);

Expand Down Expand Up @@ -597,7 +585,7 @@ public function deleteShare(string $id): DataResponse {
* @param string $publicUpload If public uploading is allowed
* @param string $password Password for the share
* @param string|null $sendPasswordByTalk Send the password for the share over Talk
* @param string $expireDate Expiry date of the share
* @param string $expireDate Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.
* @param string $note Note for the share
* @param string $label Label for the share (only used in link and email)
* @param string|null $attributes Additional attributes for the share
Expand Down Expand Up @@ -1706,11 +1694,12 @@ protected function canDeleteShareFromSelf(\OCP\Share\IShare $share): bool {
*/
private function parseDate(string $expireDate): \DateTime {
try {
$date = new \DateTime(trim($expireDate, "\""));
$date = new \DateTime(trim($expireDate, "\""), $this->dateTimeZone->getTimeZone());
} catch (\Exception $e) {
throw new \Exception('Invalid date. Format must be YYYY-MM-DD');
}

$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));
$date->setTime(0, 0, 0);

return $date;
Expand Down
2 changes: 1 addition & 1 deletion apps/files_sharing/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1899,7 +1899,7 @@
{
"name": "expireDate",
"in": "query",
"description": "Expiry date of the share",
"description": "Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.",
"schema": {
"type": "string",
"default": ""
Expand Down
5 changes: 4 additions & 1 deletion apps/files_sharing/tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
Expand Down Expand Up @@ -122,6 +123,7 @@ private function createOCS($userId) {
$serverContainer = $this->createMock(IServerContainer::class);
$userStatusManager = $this->createMock(IUserStatusManager::class);
$previewManager = $this->createMock(IPreview::class);
$dateTimeZone = $this->createMock(IDateTimeZone::class);

return new ShareAPIController(
self::APP_NAME,
Expand All @@ -137,7 +139,8 @@ private function createOCS($userId) {
$appManager,
$serverContainer,
$userStatusManager,
$previewManager
$previewManager,
$dateTimeZone,
);
}

Expand Down
16 changes: 15 additions & 1 deletion apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
use OCP\Files\NotFoundException;
use OCP\Files\Storage;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IL10N;
Expand Down Expand Up @@ -117,6 +118,9 @@ class ShareAPIControllerTest extends TestCase {
/** @var IPreview|\PHPUnit\Framework\MockObject\MockObject */
private $previewManager;

/** @var IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject */
private $dateTimeZone;

protected function setUp(): void {
$this->shareManager = $this->createMock(IManager::class);
$this->shareManager
Expand Down Expand Up @@ -147,6 +151,7 @@ protected function setUp(): void {
->willReturnCallback(function ($fileInfo) {
return $fileInfo->getMimeType() === 'mimeWithPreview';
});
$this->dateTimeZone = $this->createMock(IDateTimeZone::class);

$this->ocs = new ShareAPIController(
$this->appName,
Expand All @@ -162,7 +167,8 @@ protected function setUp(): void {
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager
$this->previewManager,
$this->dateTimeZone,
);
}

Expand All @@ -186,6 +192,7 @@ private function mockFormatShare() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();
}
Expand Down Expand Up @@ -783,6 +790,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['canAccessShare'])
->getMock();

Expand Down Expand Up @@ -1407,6 +1415,7 @@ public function testGetShares(array $getSharesParameters, array $shares, array $
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -1746,6 +1755,7 @@ public function testCreateShareUser() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -1840,6 +1850,7 @@ public function testCreateShareGroup() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2249,6 +2260,7 @@ public function testCreateShareRemote() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2315,6 +2327,7 @@ public function testCreateShareRemoteGroup() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2554,6 +2567,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

Expand Down

0 comments on commit cc3a2c3

Please sign in to comment.