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.

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

[stable26] fix(share): use user timezone to parse share expiration date #42808

Merged
merged 1 commit into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 9 additions & 27 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,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 All @@ -90,7 +91,6 @@
* @package OCA\Files_Sharing\API
*/
class ShareAPIController extends OCSController {

/** @var IManager */
private $shareManager;
/** @var IGroupManager */
Expand Down Expand Up @@ -120,20 +120,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 @@ -149,7 +135,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 @@ -244,7 +231,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 All @@ -265,7 +252,6 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
$result['share_with'] = $share->getSharedWith();
$result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith();
} elseif ($share->getShareType() === IShare::TYPE_LINK) {

// "share_with" and "share_with_displayname" for passwords of link
// shares was deprecated in Nextcloud 15, use "password" instead.
$result['share_with'] = $share->getPassword();
Expand Down Expand Up @@ -343,7 +329,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 @@ -600,7 +586,6 @@ public function createShare(
if ($permissions === null) {
if ($shareType === IShare::TYPE_LINK
|| $shareType === IShare::TYPE_EMAIL) {

// to keep legacy default behaviour, we ignore the setting below for link shares
$permissions = Constants::PERMISSION_READ;
} else {
Expand Down Expand Up @@ -687,7 +672,6 @@ public function createShare(
$share->setPermissions($permissions);
} elseif ($shareType === IShare::TYPE_LINK
|| $shareType === IShare::TYPE_EMAIL) {

// Can we even share links?
if (!$this->shareManager->shareApiAllowLinks()) {
throw new OCSNotFoundException($this->l->t('Public link sharing is disabled by the administrator'));
Expand Down Expand Up @@ -1104,7 +1088,6 @@ private function getFormattedShares(
* @throws SharingRightsException
*/
public function getInheritedShares(string $path): DataResponse {

// get Node from (string) path.
$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
try {
Expand Down Expand Up @@ -1258,7 +1241,6 @@ public function updateShare(
*/
if ($share->getShareType() === IShare::TYPE_LINK
|| $share->getShareType() === IShare::TYPE_EMAIL) {

/**
* We do not allow editing link shares that the current user
* doesn't own. This is confusing and lead to errors when
Expand Down Expand Up @@ -1296,8 +1278,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 Expand Up @@ -1679,11 +1661,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 Expand Up @@ -2097,6 +2080,5 @@ private function checkInheritedAttributes(IShare $share): void {
}
}
}

}
}
6 changes: 4 additions & 2 deletions apps/files_sharing/tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@

use OC\Files\Cache\Scanner;
use OC\Files\Filesystem;
use OC\Files\SetupManager;
use OCA\Files_Sharing\Controller\ShareAPIController;
use OCP\App\IAppManager;
use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSException;
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 @@ -123,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 @@ -138,7 +139,8 @@ private function createOCS($userId) {
$appManager,
$serverContainer,
$userStatusManager,
$previewManager
$previewManager,
$dateTimeZone,
);
}

Expand Down
34 changes: 23 additions & 11 deletions apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
use OCP\Files\Mount\IMountPoint;
use OCP\Files\NotFoundException;
use OCP\Files\Storage;
use OCP\Files\Storage\IStorage;
use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IL10N;
Expand All @@ -72,7 +72,6 @@
* @group DB
*/
class ShareAPIControllerTest extends TestCase {

/** @var string */
private $appName = 'files_sharing';

Expand Down Expand Up @@ -118,6 +117,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 @@ -148,6 +150,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 @@ -163,7 +166,8 @@ protected function setUp(): void {
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager
$this->previewManager,
$this->dateTimeZone,
);
}

Expand All @@ -187,6 +191,7 @@ private function mockFormatShare() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();
}
Expand Down Expand Up @@ -775,6 +780,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 @@ -1399,6 +1405,7 @@ public function testGetShares(array $getSharesParameters, array $shares, array $
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

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

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

Expand Down Expand Up @@ -2175,7 +2184,7 @@ public function testCreateShareValidExpireDate() {
$this->shareManager->expects($this->once())->method('createShare')->with(
$this->callback(function (\OCP\Share\IShare $share) use ($path) {
$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

return $share->getNode() === $path &&
$share->getShareType() === IShare::TYPE_LINK &&
Expand Down Expand Up @@ -2241,6 +2250,7 @@ public function testCreateShareRemote() {
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
])->setMethods(['formatShare'])
->getMock();

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

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

Expand Down Expand Up @@ -2734,7 +2746,7 @@ public function testUpdateLinkShareSet() {
$this->shareManager->expects($this->once())->method('updateShare')->with(
$this->callback(function (\OCP\Share\IShare $share) {
$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

return $share->getPermissions() === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) &&
$share->getPassword() === 'password' &&
Expand Down Expand Up @@ -3020,7 +3032,7 @@ public function testUpdateLinkSharePasswordDoesNotChangeOther(): void {
$ocs = $this->mockFormatShare();

$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

[$userFolder, $node] = $this->getNonSharedUserFolder();
$node->method('getId')->willReturn(42);
Expand Down Expand Up @@ -3072,7 +3084,7 @@ public function testUpdateLinkShareSendPasswordByTalkDoesNotChangeOther() {
$ocs = $this->mockFormatShare();

$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

[$userFolder, $node] = $this->getNonSharedUserFolder();
$userFolder->method('getById')
Expand Down Expand Up @@ -3130,7 +3142,7 @@ public function testUpdateLinkShareSendPasswordByTalkWithTalkDisabledDoesNotChan
$ocs = $this->mockFormatShare();

$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

[$userFolder, $node] = $this->getNonSharedUserFolder();
$userFolder->method('getById')
Expand Down Expand Up @@ -3170,7 +3182,7 @@ public function testUpdateLinkShareDoNotSendPasswordByTalkDoesNotChangeOther() {
$ocs = $this->mockFormatShare();

$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

[$userFolder, $node] = $this->getNonSharedUserFolder();
$userFolder->method('getById')
Expand Down Expand Up @@ -3224,7 +3236,7 @@ public function testUpdateLinkShareDoNotSendPasswordByTalkWithTalkDisabledDoesNo
$ocs = $this->mockFormatShare();

$date = new \DateTime('2000-01-01');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

[$userFolder, $node] = $this->getNonSharedUserFolder();
$node->method('getId')
Expand Down Expand Up @@ -3319,7 +3331,7 @@ public function testUpdateLinkShareExpireDateDoesNotChangeOther() {
$this->shareManager->expects($this->once())->method('updateShare')->with(
$this->callback(function (\OCP\Share\IShare $share) {
$date = new \DateTime('2010-12-23');
$date->setTime(0,0,0);
$date->setTime(0, 0, 0);

return $share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
$share->getPassword() === 'password' &&
Expand Down