Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 14 additions & 7 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType;
use OCP\App\IAppManager;
use OCP\Constants;
use OCP\Encryption\Exceptions\GenericEncryptionException;
use OCP\Files;
use OCP\Files\EntityTooLargeException;
Expand Down Expand Up @@ -539,18 +540,24 @@ public function getContentType() {
}

/**
* @return array|bool
* @throws NotFoundException
* @throws NotPermittedException
*/
public function getDirectDownload() {
public function getDirectDownload(): array|false {
if (Server::get(IAppManager::class)->isEnabledForUser('encryption')) {
return [];
return false;
}
[$storage, $internalPath] = $this->fileView->resolvePath($this->path);
if (is_null($storage)) {
return [];
$node = $this->getNode();
$storage = $node->getStorage();
if (!$storage) {
return false;
}

if (!($node->getPermissions() & Constants::PERMISSION_READ)) {
return false;
}

return $storage->getDirectDownload($internalPath);
return $storage->getDirectDownloadById((string)$node->getId());
}

/**
Expand Down
27 changes: 20 additions & 7 deletions apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class FilesPlugin extends ServerPlugin {
public const OCM_SHARE_PERMISSIONS_PROPERTYNAME = '{http://open-cloud-mesh.org/ns}share-permissions';
public const SHARE_ATTRIBUTES_PROPERTYNAME = '{http://nextcloud.org/ns}share-attributes';
public const DOWNLOADURL_PROPERTYNAME = '{http://owncloud.org/ns}downloadURL';
public const DOWNLOADURL_EXPIRATION_PROPERTYNAME = '{http://nextcloud.org/ns}download-url-expiration';
public const SIZE_PROPERTYNAME = '{http://owncloud.org/ns}size';
public const GETETAG_PROPERTYNAME = '{DAV:}getetag';
public const LASTMODIFIED_PROPERTYNAME = '{DAV:}lastmodified';
Expand Down Expand Up @@ -120,6 +121,7 @@ public function initialize(Server $server) {
$server->protectedProperties[] = self::SHARE_ATTRIBUTES_PROPERTYNAME;
$server->protectedProperties[] = self::SIZE_PROPERTYNAME;
$server->protectedProperties[] = self::DOWNLOADURL_PROPERTYNAME;
$server->protectedProperties[] = self::DOWNLOADURL_EXPIRATION_PROPERTYNAME;
$server->protectedProperties[] = self::OWNER_ID_PROPERTYNAME;
$server->protectedProperties[] = self::OWNER_DISPLAY_NAME_PROPERTYNAME;
$server->protectedProperties[] = self::CHECKSUMS_PROPERTYNAME;
Expand Down Expand Up @@ -471,19 +473,30 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
}

if ($node instanceof File) {
$propFind->handle(self::DOWNLOADURL_PROPERTYNAME, function () use ($node) {
$requestProperties = $propFind->getRequestedProperties();

if (in_array(self::DOWNLOADURL_PROPERTYNAME, $requestProperties, true)
|| in_array(self::DOWNLOADURL_EXPIRATION_PROPERTYNAME, $requestProperties, true)) {
try {
$directDownloadUrl = $node->getDirectDownload();
if (isset($directDownloadUrl['url'])) {
} catch (StorageNotAvailableException|ForbiddenException) {
$directDownloadUrl = null;
}

$propFind->handle(self::DOWNLOADURL_PROPERTYNAME, function () use ($node, $directDownloadUrl) {
if ($directDownloadUrl && isset($directDownloadUrl['url'])) {
return $directDownloadUrl['url'];
}
} catch (StorageNotAvailableException $e) {
return false;
} catch (ForbiddenException $e) {
});

$propFind->handle(self::DOWNLOADURL_EXPIRATION_PROPERTYNAME, function () use ($node, $directDownloadUrl) {
if ($directDownloadUrl && isset($directDownloadUrl['expiration'])) {
return $directDownloadUrl['expiration'];
}
return false;
}
return false;
});
});
}

$propFind->handle(self::CHECKSUMS_PROPERTYNAME, function () use ($node) {
$checksum = $node->getChecksum();
Expand Down
10 changes: 9 additions & 1 deletion apps/files_sharing/lib/SharedStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
use OCP\Server;
use OCP\Share\IShare;
use OCP\Util;
use Override;
use Psr\Log\LoggerInterface;

/**
Expand Down Expand Up @@ -558,8 +559,15 @@ public function getUnjailedPath(string $path): string {
return parent::getUnjailedPath($path);
}

#[Override]
public function getDirectDownload(string $path): array|false {
// disable direct download for shares
return [];
return false;
}

#[Override]
public function getDirectDownloadById(string $fileId): array|false {
// disable direct download for shares
return false;
}
}
4 changes: 4 additions & 0 deletions lib/private/Files/ObjectStore/Azure.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,8 @@ public function objectExists($urn) {
public function copyObject($from, $to) {
$this->getBlobClient()->copyBlob($this->containerName, $to, $this->containerName, $from);
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
19 changes: 19 additions & 0 deletions lib/private/Files/ObjectStore/ObjectStoreStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCP\Files\Storage\IStorage;
use OCP\IDBConnection;
use OCP\Server;
use Override;
use Psr\Log\LoggerInterface;

class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFileWrite {
Expand Down Expand Up @@ -844,4 +845,22 @@ public function free_space(string $path): int|float|false {

return $available;
}

#[Override]
public function getDirectDownloadById(string $fileId): array|false {
$expiration = new \DateTimeImmutable('+60 minutes');
$url = $this->objectStore->preSignedUrl($this->getURN((int)$fileId), $expiration);
return $url ? ['url' => $url, 'expiration' => $expiration->getTimestamp()] : false;
}

#[Override]
public function getDirectDownload(string $path): array|false {
$path = $this->normalizePath($path);
$cacheEntry = $this->getCache()->get($path);

if (!$cacheEntry || $cacheEntry->getMimeType() === FileInfo::MIMETYPE_FOLDER) {
return false;
}
return $this->getDirectDownloadById((string)$cacheEntry->getId());
}
}
9 changes: 9 additions & 0 deletions lib/private/Files/ObjectStore/S3ConnectionTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ trait S3ConnectionTrait {

protected ?S3Client $connection = null;

private bool $usePresignedUrl = false;

protected function parseParams($params) {
if (empty($params['bucket'])) {
throw new \Exception('Bucket has to be configured.');
Expand Down Expand Up @@ -93,12 +95,15 @@ public function getConnection() {
)
);

$this->usePresignedUrl = $this->params['use_presigned_url'] ?? false;

$options = [
'version' => $this->params['version'] ?? 'latest',
'credentials' => $provider,
'endpoint' => $base_url,
'region' => $this->params['region'],
'use_path_style_endpoint' => isset($this->params['use_path_style']) ? $this->params['use_path_style'] : false,
'proxy' => isset($this->params['proxy']) ? $this->params['proxy'] : false,
'signature_provider' => \Aws\or_chain([self::class, 'legacySignatureProvider'], ClientResolver::_default_signature_provider()),
'csm' => false,
'use_arn_region' => false,
Expand Down Expand Up @@ -256,4 +261,8 @@ protected function getSSECParameters(bool $copy = false): array {
'SSECustomerKeyMD5' => md5($rawKey, true)
];
}

public function isUsePresignedUrl(): bool {
return $this->usePresignedUrl;
}
}
20 changes: 20 additions & 0 deletions lib/private/Files/ObjectStore/S3ObjectTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace OC\Files\ObjectStore;

use Aws\Command;
use Aws\Exception\AwsException;
use Aws\Exception\MultipartUploadException;
use Aws\S3\Exception\S3MultipartUploadException;
use Aws\S3\MultipartCopy;
Expand Down Expand Up @@ -291,4 +292,23 @@ public function copyObject($from, $to, array $options = []) {
], $options));
}
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
$command = $this->getConnection()->getCommand('GetObject', [
'Bucket' => $this->getBucket(),
'Key' => $urn,
]);

if (!$this->isUsePresignedUrl()) {
return null;
}

try {
return (string)$this->getConnection()->createPresignedRequest($command, $expiration, [
Copy link
Member

Choose a reason for hiding this comment

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

just to make sure, have we confirmed that creating presigned urls is cheap enough to do for every file in a directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked with blackfire, creating a pre-signed urls has a minimal impact on performance since it's only doing an hash.

The only thing expensive is creating the connection object, because we are doing a HEAD request to the s3 server all the time, which I am fixing here: https://github.com/nextcloud/server/pull/56395/files

Copy link

Choose a reason for hiding this comment

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

I would like to comment that efficiency is not the only important parameter, as there are cases where the nextcloud server itself is unable to serve files because of the company policies which are forcing S3 servers to face the customer instead

'signPayload' => true,
])->getUri();
} catch (AwsException) {
return null;
}
}
}
4 changes: 4 additions & 0 deletions lib/private/Files/ObjectStore/StorageObjectStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,8 @@ public function objectExists($urn) {
public function copyObject($from, $to) {
$this->storage->copy($from, $to);
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
3 changes: 3 additions & 0 deletions lib/private/Files/ObjectStore/Swift.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,7 @@ public function copyObject($from, $to) {
'destination' => $this->getContainer()->name . '/' . $to
]);
}
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
14 changes: 8 additions & 6 deletions lib/private/Files/Storage/Common.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
use OCP\Server;
use Override;
use Psr\Log\LoggerInterface;

/**
Expand Down Expand Up @@ -445,13 +446,14 @@ public function instanceOfStorage(string $class): bool {
return is_a($this, $class);
}

/**
* A custom storage implementation can return an url for direct download of a give file.
*
* For now the returned array can hold the parameter url - in future more attributes might follow.
*/
#[Override]
public function getDirectDownload(string $path): array|false {
return [];
return false;
}

#[Override]
public function getDirectDownloadById(string $fileId): array|false {
return false;
}

public function verifyPath(string $path, string $fileName): void {
Expand Down
4 changes: 4 additions & 0 deletions lib/private/Files/Storage/FailedStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ public function getDirectDownload(string $path): never {
throw new StorageNotAvailableException($this->e->getMessage(), $this->e->getCode(), $this->e);
}

public function getDirectDownloadById(string $fileId): never {
throw new StorageNotAvailableException($this->e->getMessage(), $this->e->getCode(), $this->e);
}

public function verifyPath(string $path, string $fileName): void {
}

Expand Down
4 changes: 4 additions & 0 deletions lib/private/Files/Storage/Wrapper/Availability.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ public function getDirectDownload(string $path): array|false {
return $this->handleAvailability('getDirectDownload', $path);
}

public function getDirectDownloadById(string $fileId): array|false {
return $this->handleAvailability('getDirectDownloadById', $fileId);
}

public function copyFromStorage(IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool {
return $this->handleAvailability('copyFromStorage', $sourceStorage, $sourceInternalPath, $targetInternalPath);
}
Expand Down
7 changes: 7 additions & 0 deletions lib/private/Files/Storage/Wrapper/Wrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use OCP\Files\Storage\IWriteStreamStorage;
use OCP\Lock\ILockingProvider;
use OCP\Server;
use Override;
use Psr\Log\LoggerInterface;

class Wrapper implements \OC\Files\Storage\Storage, ILockingStorage, IWriteStreamStorage {
Expand Down Expand Up @@ -258,10 +259,16 @@ public function __call(string $method, array $args) {
return call_user_func_array([$this->getWrapperStorage(), $method], $args);
}

#[Override]
public function getDirectDownload(string $path): array|false {
return $this->getWrapperStorage()->getDirectDownload($path);
}

#[Override]
public function getDirectDownloadById(string $fileId): array|false {
return $this->getWrapperStorage()->getDirectDownloadById($fileId);
}

public function getAvailability(): array {
return $this->getWrapperStorage()->getAvailability();
}
Expand Down
4 changes: 4 additions & 0 deletions lib/private/Lockdown/Filesystem/NullStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ public function getDirectDownload(string $path): array|false {
return false;
}

public function getDirectDownloadById(string $fileId): array|false {
return false;
}

public function copyFromStorage(IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath, bool $preserveMtime = false): never {
throw new \OC\ForbiddenException('This request is not allowed to access the filesystem');
}
Expand Down
6 changes: 6 additions & 0 deletions lib/public/Files/ObjectStore/IObjectStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,10 @@ public function objectExists($urn);
* @since 21.0.0
*/
public function copyObject($from, $to);

/**
* Get pre signed url for an object
* @since 33.0.0
*/
public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string;
}
19 changes: 16 additions & 3 deletions lib/public/Files/Storage/IStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,15 +302,28 @@ public function isLocal();
public function instanceOfStorage(string $class);

/**
* A custom storage implementation can return an url for direct download of a give file.
* A custom storage implementation can return a url for direct download of a give file.
*
* For now the returned array can hold the parameter url - in future more attributes might follow.
* For now the returned array can hold the parameter url and expiration - in future more attributes might follow.
*
* @return array|false
* @param string $path Either the path or the fileId
* @return array{url: ?string, expiration: ?int}|false
* @since 9.0.0
* @deprecated Use IStorage::getDirectDownloadById instead.
*/
public function getDirectDownload(string $path);

/**
* A custom storage implementation can return a url for direct download of a give file.
*
* For now the returned array can hold the parameter url and expiration - in future more attributes might follow.
*
* @param string $fileId The fileId of the file.
* @return array{url: ?string, expiration: ?int}|false
* @since 33.0.0
*/
public function getDirectDownloadById(string $fileId): array|false;

/**
* @return void
* @throws InvalidPathException
Expand Down
4 changes: 4 additions & 0 deletions tests/lib/Files/Mount/ObjectHomeMountProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,4 +268,8 @@ public function objectExists($urn) {

public function copyObject($from, $to) {
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
4 changes: 4 additions & 0 deletions tests/lib/Files/ObjectStore/FailDeleteObjectStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,8 @@ public function objectExists($urn) {
public function copyObject($from, $to) {
$this->objectStore->copyObject($from, $to);
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
4 changes: 4 additions & 0 deletions tests/lib/Files/ObjectStore/FailWriteObjectStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@ public function objectExists($urn) {
public function copyObject($from, $to) {
$this->objectStore->copyObject($from, $to);
}

public function preSignedUrl(string $urn, \DateTimeInterface $expiration): ?string {
return null;
}
}
Loading