Merge pull request #49646 from nextcloud/fix/noid/trigger-field-insert

fix(signed-request): trigger metadata insert with default value manually

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

@@ -5,6 +5,7 @@
  */
 namespace OCA\CloudFederationAPI\Controller;
 
+use NCU\Federation\ISignedCloudFederationProvider;
 use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
 use NCU\Security\Signature\Exceptions\IncomingRequestException;
 use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
@@ -37,8 +38,6 @@ use OCP\IRequest;
 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;
 
@@ -68,7 +67,6 @@ class RequestHandlerController extends Controller {
 		private ICloudIdManager $cloudIdManager,
 		private readonly ISignatureManager $signatureManager,
 		private readonly OCMSignatoryManager $signatoryManager,
-		private readonly IProviderFactory $shareProviderFactory,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -234,16 +232,6 @@ class RequestHandlerController extends Controller {
 	#[PublicPage]
 	#[BruteForceProtection(action: 'receiveFederatedShareNotification')]
 	public function receiveNotification($notificationType, $resourceType, $providerId, ?array $notification) {
-		try {
-			// 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'] ?? '');
-		} 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 ||
 			$resourceType === null ||
@@ -259,6 +247,16 @@ class RequestHandlerController extends Controller {
 			);
 		}
 
+		try {
+			// 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->confirmNotificationIdentity($signedRequest, $resourceType, $notification);
+		} catch (IncomingRequestException $e) {
+			$this->logger->warning('incoming request exception', ['exception' => $e]);
+			return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
+		}
+
 		try {
 			$provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType);
 			$result = $provider->notificationReceived($notificationType, $providerId, $notification);
@@ -387,42 +385,45 @@ class RequestHandlerController extends Controller {
 		}
 	}
 
-
 	/**
-	 *  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 $token
+	 * @param string $resourceType
+	 * @param string $sharedSecret
 	 *
 	 * @throws IncomingRequestException
+	 * @throws BadRequestException
 	 */
-	private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, string $token): void {
-		if ($token === '') {
+	private function confirmNotificationIdentity(
+		?IIncomingSignedRequest $signedRequest,
+		string $resourceType,
+		array $notification,
+	): void {
+		$sharedSecret = $notification['sharedSecret'] ?? '';
+		if ($sharedSecret === '') {
 			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]);
-			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;
+			$provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType);
+			if ($provider instanceof ISignedCloudFederationProvider) {
+				$identity = $provider->getFederationIdFromSharedSecret($sharedSecret, $notification);
+			} else {
+				$this->logger->debug('cloud federation provider {provider} does not implements ISignedCloudFederationProvider', ['provider' => $provider::class]);
+				return;
 			}
+		} catch (\Exception $e) {
+			throw new IncomingRequestException($e->getMessage());
 		}
+
+		$this->confirmNotificationEntry($signedRequest, $identity);
 	}
 
+
 	/**
 	 * @param IIncomingSignedRequest|null $signedRequest
 	 * @param string $entry
@@ -430,7 +431,7 @@ class RequestHandlerController extends Controller {
 	 * @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 {
@@ -440,7 +441,7 @@ class RequestHandlerController extends Controller {
 				return;
 			}
 		} elseif ($instance !== $signedRequest->getOrigin()) {
-			throw new IncomingRequestException('token sharedWith (' . $instance . ') not linked to origin (' . $signedRequest->getOrigin() . ')');
+			throw new IncomingRequestException('remote instance ' . $instance . ' not linked to origin ' . $signedRequest->getOrigin());
 		}
 	}
 

apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php

@@ -5,6 +5,7 @@
  */
 namespace OCA\FederatedFileSharing\OCM;
 
+use NCU\Federation\ISignedCloudFederationProvider;
 use OC\AppFramework\Http;
 use OC\Files\Filesystem;
 use OCA\FederatedFileSharing\AddressHandler;
@@ -21,7 +22,6 @@ use OCP\Federation\Exceptions\AuthenticationFailedException;
 use OCP\Federation\Exceptions\BadRequestException;
 use OCP\Federation\Exceptions\ProviderCouldNotAddShareException;
 use OCP\Federation\ICloudFederationFactory;
-use OCP\Federation\ICloudFederationProvider;
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudFederationShare;
 use OCP\Federation\ICloudIdManager;
@@ -37,11 +37,13 @@ use OCP\Notification\IManager as INotificationManager;
 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;
+use SensitiveParameter;
 
-class CloudFederationProviderFiles implements ICloudFederationProvider {
+class CloudFederationProviderFiles implements ISignedCloudFederationProvider {
 	/**
 	 * CloudFederationProvider constructor.
 	 */
@@ -63,6 +65,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
 		private Manager $externalShareManager,
 		private LoggerInterface $logger,
 		private IFilenameValidator $filenameValidator,
+		private readonly IProviderFactory $shareProviderFactory,
 	) {
 	}
 
@@ -747,4 +750,32 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
 
 		return $slaveService->getUserDisplayName($this->cloudIdManager->removeProtocolFromUrl($userId), false);
 	}
+
+	/**
+	 * @inheritDoc
+	 *
+	 * @param string $sharedSecret
+	 * @param array $payload
+	 * @return string
+	 */
+	public function getFederationIdFromSharedSecret(
+		#[SensitiveParameter]
+		string $sharedSecret,
+		array $payload,
+	): string {
+		$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
+		try {
+			$share = $provider->getShareByToken($sharedSecret);
+		} catch (ShareNotFound) {
+			return '';
+		}
+
+		// if uid_owner is a local account, the request comes from the recipient
+		// if not, request comes from the instance that owns the share and recipient is the re-sharer
+		if ($this->userManager->get($share->getShareOwner()) !== null) {
+			return $share->getSharedWith();
+		} else {
+			return $share->getShareOwner();
+		}
+	}
 }

lib/composer/composer/autoload_classmap.php

@@ -12,6 +12,7 @@ return array(
     'NCU\\Config\\Exceptions\\UnknownKeyException' => $baseDir . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
     'NCU\\Config\\IUserConfig' => $baseDir . '/lib/unstable/Config/IUserConfig.php',
     'NCU\\Config\\ValueType' => $baseDir . '/lib/unstable/Config/ValueType.php',
+    'NCU\\Federation\\ISignedCloudFederationProvider' => $baseDir . '/lib/unstable/Federation/ISignedCloudFederationProvider.php',
     'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
     'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
     'NCU\\Security\\Signature\\Enum\\SignatoryType' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',

lib/composer/composer/autoload_static.php

@@ -53,6 +53,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'NCU\\Config\\Exceptions\\UnknownKeyException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
         'NCU\\Config\\IUserConfig' => __DIR__ . '/../../..' . '/lib/unstable/Config/IUserConfig.php',
         'NCU\\Config\\ValueType' => __DIR__ . '/../../..' . '/lib/unstable/Config/ValueType.php',
+        'NCU\\Federation\\ISignedCloudFederationProvider' => __DIR__ . '/../../..' . '/lib/unstable/Federation/ISignedCloudFederationProvider.php',
         'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
         'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
         'NCU\\Security\\Signature\\Enum\\SignatoryType' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',

lib/private/OCM/OCMDiscoveryService.php

@@ -9,6 +9,7 @@ declare(strict_types=1);
 
 namespace OC\OCM;
 
+use GuzzleHttp\Exception\ConnectException;
 use JsonException;
 use OCP\AppFramework\Http;
 use OCP\Http\Client\IClientService;
@@ -50,7 +51,7 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 			// if scheme not specified, we test both;
 			try {
 				return $this->discover('https://' . $remote, $skipCache);
-			} catch (OCMProviderException) {
+			} catch (OCMProviderException|ConnectException) {
 				return $this->discover('http://' . $remote, $skipCache);
 			}
 		}

lib/private/Security/Signature/SignatureManager.php

@@ -142,7 +142,7 @@ class SignatureManager implements ISignatureManager {
 			if ($ttlSignatory > 0 && $knownSignatory->getLastUpdated() < (time() - $ttlSignatory)) {
 				$signatory = $this->getSaneRemoteSignatory($signatoryManager, $signedRequest);
 				$this->updateSignatoryMetadata($signatory);
-				$knownSignatory->setMetadata($signatory->getMetadata());
+				$knownSignatory->setMetadata($signatory->getMetadata() ?? []);
 			}
 
 			$signedRequest->setSignatory($knownSignatory);
@@ -353,6 +353,7 @@ class SignatureManager implements ISignatureManager {
 		$time = time();
 		$signatory->setCreation($time);
 		$signatory->setLastUpdated($time);
+		$signatory->setMetadata($signatory->getMetadata() ?? []); // trigger insert on field metadata using current or default value
 		$this->mapper->insert($signatory);
 	}
 

lib/unstable/Federation/ISignedCloudFederationProvider.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace NCU\Federation;
+
+use OCP\Federation\ICloudFederationProvider;
+
+/**
+ * Interface ICloudFederationProvider
+ *
+ * Enable apps to create their own cloud federation provider
+ *
+ * @experimental 31.0.0
+ */
+interface ISignedCloudFederationProvider extends ICloudFederationProvider {
+
+	/**
+	 * returns federationId in direct relation (as recipient or as author) of a sharedSecret
+	 * the federationId must be the one at the remote end
+	 *
+	 * @param string $sharedSecret
+	 * @param array $payload
+	 *
+	 * @experimental 31.0.0
+	 * @return string
+	 */
+	public function getFederationIdFromSharedSecret(string $sharedSecret, array $payload): string;
+}

lib/unstable/Security/Signature/Model/Signatory.php

@@ -42,7 +42,7 @@ use OCP\AppFramework\Db\Entity;
  * @method void setAccount(string $account)
  * @method string getAccount()
  * @method void setMetadata(array $metadata)
- * @method array getMetadata()
+ * @method ?array getMetadata()
  * @method void setCreation(int $creation)
  * @method int getCreation()
  * @method void setLastUpdated(int $creation)
@@ -59,7 +59,7 @@ class Signatory extends Entity implements JsonSerializable {
 	protected string $account = '';
 	protected int $type = 9;
 	protected int $status = 1;
-	protected array $metadata = [];
+	protected ?array $metadata = null;
 	protected int $creation = 0;
 	protected int $lastUpdated = 0;