Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IProviderFactory;
use OCP\Share\IShare;
use OCP\Util;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -68,7 +66,6 @@
private ICloudIdManager $cloudIdManager,
private readonly ISignatureManager $signatureManager,
private readonly OCMSignatoryManager $signatoryManager,
private readonly IProviderFactory $shareProviderFactory,
) {
parent::__construct($appName, $request);
}
Expand Down Expand Up @@ -238,15 +235,15 @@
// if request is signed and well signed, no exception are thrown
// if request is not signed and host is known for not supporting signed request, no exception are thrown
$signedRequest = $this->getSignedRequest();
$this->confirmShareOrigin($signedRequest, $notification['sharedSecret'] ?? '');
$this->confirmNotificationIdentity($signedRequest, $resourceType, $notification['sharedSecret'] ?? '');
} catch (IncomingRequestException $e) {
$this->logger->warning('incoming request exception', ['exception' => $e]);
return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
}

// check if all required parameters are set
if ($notificationType === null ||

Check failure on line 245 in apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainNull

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php:245:7: TypeDoesNotContainNull: Type string for $resourceType is never null (see https://psalm.dev/090)
$resourceType === null ||

Check failure on line 246 in apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainNull

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php:246:4: TypeDoesNotContainNull: string does not contain null (see https://psalm.dev/090)
$providerId === null ||
!is_array($notification)
) {
Expand Down Expand Up @@ -387,50 +384,60 @@
}
}


/**
* confirm that the value related to share token is in format userid@hostname
* and compare hostname with the origin of the signed request.
* confirm identity of the remote instance on notification, based on the share token.
*
* If request is not signed, we still verify that the hostname from the extracted value does,
* actually, not support signed request
*
* @param IIncomingSignedRequest|null $signedRequest
* @param string $resourceType
* @param string $token
*
* @throws IncomingRequestException
* @throws BadRequestException
*/
private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, string $token): void {
private function confirmNotificationIdentity(?IIncomingSignedRequest $signedRequest, string $resourceType, string $token): void {
if ($token === '') {
throw new BadRequestException(['sharedSecret']);
}

$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
$share = $provider->getShareByToken($token);
try {
$this->confirmShareEntry($signedRequest, $share->getSharedWith());
} catch (IncomingRequestException $e) {
// notification might come from the instance that owns the share
$this->logger->debug('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')', ['exception' => $e]);
$provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType);
$identities = $provider->getFederationIdFromToken($token);
} catch (\Exception $e) {
throw new IncomingRequestException($e->getMessage());
}

if (!is_array($identities)) {
$identities = [$identities];
}

$confirmed = false;
foreach ($identities as $identity) {
try {
$this->confirmShareEntry($signedRequest, $share->getShareOwner());
} catch (IncomingRequestException $f) {
// if both entry are failing, we log first exception as warning and second exception
// will be logged as warning by the controller
$this->logger->warning('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')', ['exception' => $e]);
throw $f;
$this->confirmNotificationEntry($signedRequest, $identity);
$confirmed = true;
continue;
} catch (IncomingRequestException $e) {
$this->logger->notice('could not confirm notification entry', ['exception' => $e, 'identity' => $identity, $signedRequest => $signedRequest]);

Check failure on line 423 in apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidArgument

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php:423:67: InvalidArgument: Argument 2 of Psr\Log\LoggerInterface::notice expects array<array-key, mixed>, but array{exception: NCU\Security\Signature\Exceptions\IncomingRequestException, identity: mixed|string, ...<NCU\Security\Signature\IIncomingSignedRequest|null, NCU\Security\Signature\IIncomingSignedRequest|null>} provided (see https://psalm.dev/004)
}
}

if (!$confirmed) {
throw new IncomingRequestException('cannot confirm identity');
}
}


/**
* @param IIncomingSignedRequest|null $signedRequest
* @param string $entry
*
* @return void
* @throws IncomingRequestException
*/
private function confirmShareEntry(?IIncomingSignedRequest $signedRequest, string $entry): void {
private function confirmNotificationEntry(?IIncomingSignedRequest $signedRequest, string $entry): void {
$instance = $this->getHostFromFederationId($entry);
if ($signedRequest === null) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
namespace OCA\FederatedFileSharing\OCM;

use NCU\Security\Signature\Exceptions\IncomingRequestException;
use NCU\Security\Signature\IIncomingSignedRequest;
use OC\AppFramework\Http;
use OC\Files\Filesystem;
use OCA\FederatedFileSharing\AddressHandler;
Expand Down Expand Up @@ -37,6 +39,7 @@
use OCP\Server;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;
use OCP\Share\IProviderFactory;
use OCP\Share\IShare;
use OCP\Util;
use Psr\Log\LoggerInterface;
Expand All @@ -63,6 +66,7 @@ public function __construct(
private Manager $externalShareManager,
private LoggerInterface $logger,
private IFilenameValidator $filenameValidator,
private readonly IProviderFactory $shareProviderFactory,
) {
}

Expand Down Expand Up @@ -747,4 +751,18 @@ public function getUserDisplayName(string $userId): string {

return $slaveService->getUserDisplayName($this->cloudIdManager->removeProtocolFromUrl($userId), false);
}

/**
* @inheritDoc
*
* @param string $token
* @return string|array
*/
public function getFederationIdFromToken(string $token): string|array {
$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
$share = $provider->getShareByToken($token);

// in case of reshare, notification comes from the instance that owns the share
return [$share->getSharedWith(), $share->getShareOwner()];
}
}
8 changes: 8 additions & 0 deletions lib/public/Federation/ICloudFederationProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,12 @@ public function notificationReceived($notificationType, $providerId, array $noti
* @since 14.0.0
*/
public function getSupportedShareTypes();

/**
* get federationId (one or multiple) in direct relation (recipient, author) of a share token
*
* @param string $token
* @return string|array
*/
public function getFederationIdFromToken(string $token): string|array;
}
Loading