Merge pull request #44838 from nextcloud/41229-expire-date-fed-share

fix: avoid douple `expireDate` parsing
This commit is contained in:
F. E Noel Nfebe 2024-04-18 02:26:30 +01:00 committed by GitHub
commit 4cbe82639c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 76 additions and 144 deletions

View File

@ -75,7 +75,6 @@ use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\Lock\ILockingProvider;
@ -87,6 +86,7 @@ use OCP\Share\IManager;
use OCP\Share\IShare;
use OCP\UserStatus\IManager as IUserStatusManager;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
/**
@ -96,32 +96,8 @@ use Psr\Log\LoggerInterface;
*/
class ShareAPIController extends OCSController {
/** @var IManager */
private $shareManager;
/** @var IGroupManager */
private $groupManager;
/** @var IUserManager */
private $userManager;
/** @var IRootFolder */
private $rootFolder;
/** @var IURLGenerator */
private $urlGenerator;
/** @var string */
private $currentUser;
/** @var IL10N */
private $l;
/** @var \OCP\Files\Node */
private $lockedNode;
/** @var IConfig */
private $config;
/** @var IAppManager */
private $appManager;
/** @var IServerContainer */
private $serverContainer;
/** @var IUserStatusManager */
private $userStatusManager;
/** @var IPreview */
private $previewManager;
private ?Node $lockedNode = null;
private string $currentUser;
/**
* Share20OCS constructor.
@ -129,35 +105,23 @@ class ShareAPIController extends OCSController {
public function __construct(
string $appName,
IRequest $request,
IManager $shareManager,
IGroupManager $groupManager,
IUserManager $userManager,
IRootFolder $rootFolder,
IURLGenerator $urlGenerator,
?string $userId = null,
IL10N $l10n,
IConfig $config,
IAppManager $appManager,
IServerContainer $serverContainer,
IUserStatusManager $userStatusManager,
IPreview $previewManager,
private IManager $shareManager,
private IGroupManager $groupManager,
private IUserManager $userManager,
private IRootFolder $rootFolder,
private IURLGenerator $urlGenerator,
private IL10N $l,
private IConfig $config,
private IAppManager $appManager,
private ContainerInterface $serverContainer,
private IUserStatusManager $userStatusManager,
private IPreview $previewManager,
private IDateTimeZone $dateTimeZone,
private LoggerInterface $logger,
?string $userId = null
) {
parent::__construct($appName, $request);
$this->shareManager = $shareManager;
$this->userManager = $userManager;
$this->groupManager = $groupManager;
$this->request = $request;
$this->rootFolder = $rootFolder;
$this->urlGenerator = $urlGenerator;
$this->currentUser = $userId;
$this->l = $l10n;
$this->config = $config;
$this->appManager = $appManager;
$this->serverContainer = $serverContainer;
$this->userStatusManager = $userStatusManager;
$this->previewManager = $previewManager;
}
/**
@ -375,7 +339,7 @@ class ShareAPIController extends OCSController {
'strict_search' => true,
]);
} catch (Exception $e) {
Server::get(LoggerInterface::class)->error(
$this->logger->error(
$e->getMessage(),
['exception' => $e]
);
@ -457,7 +421,7 @@ class ShareAPIController extends OCSController {
try {
$slaveService = Server::get(\OCA\GlobalSiteSelector\Service\SlaveService::class);
} catch (\Throwable $e) {
Server::get(LoggerInterface::class)->error(
$this->logger->error(
$e->getMessage(),
['exception' => $e]
);
@ -681,6 +645,16 @@ class ShareAPIController extends OCSController {
$share = $this->setShareAttributes($share, $attributes);
}
//Expire date
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
$share->setSharedBy($this->currentUser);
$this->checkInheritedAttributes($share);
@ -767,15 +741,6 @@ class ShareAPIController extends OCSController {
$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
$share->setSharedWithDisplayName($this->getCachedFederatedDisplayName($shareWith, false));
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
@ -788,14 +753,6 @@ class ShareAPIController extends OCSController {
$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
} elseif ($shareType === IShare::TYPE_CIRCLE) {
if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled'));
@ -831,16 +788,6 @@ class ShareAPIController extends OCSController {
throw new OCSBadRequestException($this->l->t('Unknown share type'));
}
//Expire date
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
$share->setShareType($shareType);
if ($note !== '') {
@ -850,11 +797,11 @@ class ShareAPIController extends OCSController {
try {
$share = $this->shareManager->createShare($share);
} catch (GenericShareException $e) {
\OCP\Server::get(LoggerInterface::class)->error($e->getMessage(), ['exception' => $e]);
$this->logger->error($e->getMessage(), ['exception' => $e]);
$code = $e->getCode() === 0 ? 403 : $e->getCode();
throw new OCSException($e->getHint(), $code);
} catch (\Exception $e) {
\OCP\Server::get(LoggerInterface::class)->error($e->getMessage(), ['exception' => $e]);
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new OCSForbiddenException($e->getMessage(), $e);
}

View File

@ -48,9 +48,10 @@ use OCP\IDateTimeZone;
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\Share\IShare;
use OCP\UserStatus\IManager as IUserStatusManager;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
/**
* Class ApiTest
@ -120,10 +121,11 @@ class ApiTest extends TestCase {
});
$config = $this->createMock(IConfig::class);
$appManager = $this->createMock(IAppManager::class);
$serverContainer = $this->createMock(IServerContainer::class);
$serverContainer = $this->createMock(ContainerInterface::class);
$userStatusManager = $this->createMock(IUserStatusManager::class);
$previewManager = $this->createMock(IPreview::class);
$dateTimeZone = $this->createMock(IDateTimeZone::class);
$logger = $this->createMock(LoggerInterface::class);
$dateTimeZone->method('getTimeZone')->willReturn(new \DateTimeZone(date_default_timezone_get()));
return new ShareAPIController(
@ -134,7 +136,6 @@ class ApiTest extends TestCase {
\OC::$server->getUserManager(),
\OC::$server->getRootFolder(),
\OC::$server->getURLGenerator(),
$userId,
$l,
$config,
$appManager,
@ -142,6 +143,8 @@ class ApiTest extends TestCase {
$userStatusManager,
$previewManager,
$dateTimeZone,
$logger,
$userId,
);
}

View File

@ -53,7 +53,6 @@ use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
@ -63,6 +62,8 @@ use OCP\Share\IAttributes as IShareAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use OCP\UserStatus\IManager as IUserStatusManager;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use Test\TestCase;
/**
@ -73,53 +74,23 @@ use Test\TestCase;
*/
class ShareAPIControllerTest extends TestCase {
/** @var string */
private $appName = 'files_sharing';
/** @var \OC\Share20\Manager|\PHPUnit\Framework\MockObject\MockObject */
private $shareManager;
/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
private $groupManager;
/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
private $userManager;
/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
private $request;
/** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject */
private $rootFolder;
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
private $urlGenerator;
/** @var string|\PHPUnit\Framework\MockObject\MockObject */
private $currentUser;
/** @var ShareAPIController */
private $ocs;
/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
private $l;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
private $config;
/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
private $appManager;
/** @var IServerContainer|\PHPUnit\Framework\MockObject\MockObject */
private $serverContainer;
/** @var IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject */
private $userStatusManager;
/** @var IPreview|\PHPUnit\Framework\MockObject\MockObject */
private $previewManager;
/** @var IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject */
private $dateTimeZone;
private string $appName = 'files_sharing';
private \OC\Share20\Manager|\PHPUnit\Framework\MockObject\MockObject $shareManager;
private IGroupManager|\PHPUnit\Framework\MockObject\MockObject $groupManager;
private IUserManager|\PHPUnit\Framework\MockObject\MockObject $userManager;
private IRequest|\PHPUnit\Framework\MockObject\MockObject $request;
private IRootFolder|\PHPUnit\Framework\MockObject\MockObject $rootFolder;
private IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGenerator;
private string|\PHPUnit\Framework\MockObject\MockObject $currentUser;
private ShareAPIController $ocs;
private IL10N|\PHPUnit\Framework\MockObject\MockObject $l;
private IConfig|\PHPUnit\Framework\MockObject\MockObject $config;
private IAppManager|\PHPUnit\Framework\MockObject\MockObject $appManager;
private IServerContainer|\PHPUnit\Framework\MockObject\MockObject $serverContainer;
private IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject $userStatusManager;
private IPreview|\PHPUnit\Framework\MockObject\MockObject $previewManager;
private IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject $dateTimeZone;
private LoggerInterface $logger;
protected function setUp(): void {
$this->shareManager = $this->createMock(IManager::class);
@ -144,7 +115,7 @@ class ShareAPIControllerTest extends TestCase {
});
$this->config = $this->createMock(IConfig::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->serverContainer = $this->createMock(IServerContainer::class);
$this->serverContainer = $this->createMock(ContainerInterface::class);
$this->userStatusManager = $this->createMock(IUserStatusManager::class);
$this->previewManager = $this->createMock(IPreview::class);
$this->previewManager->method('isAvailable')
@ -152,6 +123,7 @@ class ShareAPIControllerTest extends TestCase {
return $fileInfo->getMimeType() === 'mimeWithPreview';
});
$this->dateTimeZone = $this->createMock(IDateTimeZone::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->ocs = new ShareAPIController(
$this->appName,
@ -161,7 +133,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -169,6 +140,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
);
}
@ -185,7 +158,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -193,6 +165,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
}
@ -786,7 +760,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -794,6 +767,9 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['canAccessShare'])
->getMock();
@ -1416,7 +1392,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -1424,6 +1399,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
@ -1756,7 +1733,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -1764,6 +1740,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
@ -1851,7 +1829,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -1859,6 +1836,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
@ -2270,7 +2249,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -2278,6 +2256,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
@ -2337,7 +2317,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -2345,6 +2324,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
@ -2577,7 +2558,6 @@ class ShareAPIControllerTest extends TestCase {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
@ -2585,6 +2565,8 @@ class ShareAPIControllerTest extends TestCase {
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();