Merge pull request #54385 from nextcloud/fix/51946/split-discovery-capacities

feat(ocm): split ocm discovery and capacities

apps/cloud_federation_api/lib/Capabilities.php

@@ -9,26 +9,14 @@ declare(strict_types=1);
 
 namespace OCA\CloudFederationAPI;
 
-use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
-use NCU\Security\Signature\Exceptions\SignatoryException;
-use OC\OCM\OCMSignatoryManager;
+use OC\OCM\OCMDiscoveryService;
 use OCP\Capabilities\ICapability;
 use OCP\Capabilities\IInitialStateExcludedCapability;
-use OCP\IAppConfig;
-use OCP\IURLGenerator;
 use OCP\OCM\Exceptions\OCMArgumentException;
-use OCP\OCM\ICapabilityAwareOCMProvider;
-use Psr\Log\LoggerInterface;
 
 class Capabilities implements ICapability, IInitialStateExcludedCapability {
-	public const API_VERSION = '1.1.0';
-
 	public function __construct(
-		private IURLGenerator $urlGenerator,
-		private IAppConfig $appConfig,
-		private ICapabilityAwareOCMProvider $provider,
-		private readonly OCMSignatoryManager $ocmSignatoryManager,
-		private readonly LoggerInterface $logger,
+		private readonly OCMDiscoveryService $ocmDiscoveryService,
 	) {
 	}
 
@@ -39,40 +27,7 @@ class Capabilities implements ICapability, IInitialStateExcludedCapability {
 	 * @throws OCMArgumentException
 	 */
 	public function getCapabilities() {
-		$url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare');
-		$pos = strrpos($url, '/');
-		if ($pos === false) {
-			throw new OCMArgumentException('generated route should contain a slash character');
-		}
-
-		$this->provider->setEnabled(true);
-		$this->provider->setApiVersion(self::API_VERSION);
-		$this->provider->setCapabilities(['/invite-accepted', '/notifications', '/shares']);
-
-		$this->provider->setEndPoint(substr($url, 0, $pos));
-
-		$resource = $this->provider->createNewResourceType();
-		$resource->setName('file')
-			->setShareTypes(['user', 'group'])
-			->setProtocols(['webdav' => '/public.php/webdav/']);
-
-		$this->provider->addResourceType($resource);
-
-		// Adding a public key to the ocm discovery
-		try {
-			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
-				/**
-				 * @experimental 31.0.0
-				 * @psalm-suppress UndefinedInterfaceMethod
-				 */
-				$this->provider->setSignatory($this->ocmSignatoryManager->getLocalSignatory());
-			} else {
-				$this->logger->debug('ocm public key feature disabled');
-			}
-		} catch (SignatoryException|IdentityNotFoundException $e) {
-			$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
-		}
-
-		return ['ocm' => $this->provider->jsonSerialize()];
+		$provider = $this->ocmDiscoveryService->getLocalOCMProvider(false);
+		return ['ocm' => $provider->jsonSerialize()];
 	}
 }

core/AppInfo/ConfigLexicon.php

@@ -27,6 +27,7 @@ class ConfigLexicon implements ILexicon {
 	public const SHARE_LINK_EXPIRE_DATE_DEFAULT = 'shareapi_default_expire_date';
 	public const SHARE_LINK_EXPIRE_DATE_ENFORCED = 'shareapi_enforce_expire_date';
 	public const USER_LANGUAGE = 'lang';
+	public const OCM_DISCOVERY_ENABLED = 'ocm_discovery_enabled';
 	public const USER_LOCALE = 'locale';
 	public const USER_TIMEZONE = 'timezone';
 
@@ -85,6 +86,7 @@ class ConfigLexicon implements ILexicon {
 				definition: 'Enforce expiration date for shares via link or mail'
 			),
 			new Entry(self::LASTCRON_TIMESTAMP, ValueType::INT, 0, 'timestamp of last cron execution'),
+			new Entry(self::OCM_DISCOVERY_ENABLED, ValueType::BOOL, true, 'enable/disable OCM', lazy: true),
 		];
 	}
 

core/Controller/OCMController.php

@@ -10,7 +10,7 @@ declare(strict_types=1);
 namespace OC\Core\Controller;
 
 use Exception;
-use OCA\CloudFederationAPI\Capabilities;
+use OC\OCM\OCMDiscoveryService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\FrontpageRoute;
@@ -18,11 +18,9 @@ use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
-use OCP\Capabilities\ICapability;
 use OCP\IAppConfig;
 use OCP\IRequest;
 use OCP\Server;
-use Psr\Container\ContainerExceptionInterface;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -34,6 +32,7 @@ class OCMController extends Controller {
 	public function __construct(
 		IRequest $request,
 		private readonly IAppConfig $appConfig,
+		private readonly OCMDiscoveryService $ocmDiscoveryService,
 		private LoggerInterface $logger,
 	) {
 		parent::__construct('core', $request);
@@ -56,29 +55,16 @@ class OCMController extends Controller {
 	#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
 	public function discovery(): DataResponse {
 		try {
-			$cap = Server::get(
-				$this->appConfig->getValueString(
-					'core', 'ocm_providers',
-					Capabilities::class,
-					lazy: true
-				)
-			);
-
-			if (!($cap instanceof ICapability)) {
-				throw new Exception('loaded class does not implements OCP\Capabilities\ICapability');
-			}
-
 			return new DataResponse(
-				$cap->getCapabilities()['ocm'] ?? ['enabled' => false],
+				$this->ocmDiscoveryService->getLocalOCMProvider()->jsonSerialize(),
 				Http::STATUS_OK,
 				[
 					'X-NEXTCLOUD-OCM-PROVIDERS' => true,
 					'Content-Type' => 'application/json'
 				]
 			);
-		} catch (ContainerExceptionInterface|Exception $e) {
+		} catch (Exception $e) {
 			$this->logger->error('issue during OCM discovery request', ['exception' => $e]);
-
 			return new DataResponse(
 				['message' => '/ocm-provider/ not supported'],
 				Http::STATUS_INTERNAL_SERVER_ERROR

lib/private/OCM/Model/OCMProvider.php

@@ -10,9 +10,6 @@ declare(strict_types=1);
 namespace OC\OCM\Model;
 
 use NCU\Security\Signature\Model\Signatory;
-use OCP\EventDispatcher\IEventDispatcher;
-use OCP\IConfig;
-use OCP\OCM\Events\ResourceTypeRegisterEvent;
 use OCP\OCM\Exceptions\OCMArgumentException;
 use OCP\OCM\Exceptions\OCMProviderException;
 use OCP\OCM\ICapabilityAwareOCMProvider;
@@ -22,7 +19,6 @@ use OCP\OCM\IOCMResource;
  * @since 28.0.0
  */
 class OCMProvider implements ICapabilityAwareOCMProvider {
-	private string $provider;
 	private bool $enabled = false;
 	private string $apiVersion = '';
 	private string $inviteAcceptDialog = '';
@@ -31,13 +27,10 @@ class OCMProvider implements ICapabilityAwareOCMProvider {
 	/** @var IOCMResource[] */
 	private array $resourceTypes = [];
 	private ?Signatory $signatory = null;
-	private bool $emittedEvent = false;
 
 	public function __construct(
-		protected IEventDispatcher $dispatcher,
-		protected IConfig $config,
+		private readonly string $provider = '',
 	) {
-		$this->provider = 'Nextcloud ' . $config->getSystemValue('version');
 	}
 
 	/**
@@ -180,12 +173,6 @@ class OCMProvider implements ICapabilityAwareOCMProvider {
 	 * @return IOCMResource[]
 	 */
 	public function getResourceTypes(): array {
-		if (!$this->emittedEvent) {
-			$this->emittedEvent = true;
-			$event = new ResourceTypeRegisterEvent($this);
-			$this->dispatcher->dispatchTyped($event);
-		}
-
 		return $this->resourceTypes;
 	}
 

lib/private/OCM/OCMDiscoveryService.php

@@ -11,11 +11,19 @@ namespace OC\OCM;
 
 use GuzzleHttp\Exception\ConnectException;
 use JsonException;
+use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
+use NCU\Security\Signature\Exceptions\SignatoryException;
+use OC\Core\AppInfo\ConfigLexicon;
+use OC\OCM\Model\OCMProvider;
 use OCP\AppFramework\Http;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Http\Client\IClientService;
+use OCP\IAppConfig;
 use OCP\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
+use OCP\IURLGenerator;
+use OCP\OCM\Events\ResourceTypeRegisterEvent;
 use OCP\OCM\Exceptions\OCMProviderException;
 use OCP\OCM\ICapabilityAwareOCMProvider;
 use OCP\OCM\IOCMDiscoveryService;
@@ -26,18 +34,25 @@ use Psr\Log\LoggerInterface;
  */
 class OCMDiscoveryService implements IOCMDiscoveryService {
 	private ICache $cache;
+	public const API_VERSION = '1.1.0';
+
+	private ?ICapabilityAwareOCMProvider $localProvider = null;
+	/** @var array<string, ICapabilityAwareOCMProvider> */
+	private array $remoteProviders = [];
 
 	public function __construct(
 		ICacheFactory $cacheFactory,
 		private IClientService $clientService,
-		private IConfig $config,
-		private ICapabilityAwareOCMProvider $provider,
+		private IEventDispatcher $eventDispatcher,
+		protected IConfig $config,
+		private IAppConfig $appConfig,
+		private IURLGenerator $urlGenerator,
+		private OCMSignatoryManager $ocmSignatoryManager,
 		private LoggerInterface $logger,
 	) {
 		$this->cache = $cacheFactory->createDistributed('ocm-discovery');
 	}
 
-
 	/**
 	 * @param string $remote
 	 * @param bool $skipCache
@@ -56,6 +71,12 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 			}
 		}
 
+		if (array_key_exists($remote, $this->remoteProviders)) {
+			return $this->remoteProviders[$remote];
+		}
+
+		$provider = new OCMProvider();
+
 		if (!$skipCache) {
 			try {
 				$cached = $this->cache->get($remote);
@@ -63,10 +84,11 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 					throw new OCMProviderException('Previous discovery failed.');
 				}
 
-				$this->provider->import(json_decode($cached ?? '', true, 8, JSON_THROW_ON_ERROR) ?? []);
-				return $this->provider;
+				$provider->import(json_decode($cached ?? '', true, 8, JSON_THROW_ON_ERROR) ?? []);
+				$this->remoteProviders[$remote] = $provider;
+				return $provider;
 			} catch (JsonException|OCMProviderException $e) {
-				// we ignore cache on issues
+				$this->logger->warning('cache issue on ocm discovery', ['exception' => $e]);
 			}
 		}
 
@@ -81,15 +103,20 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 			}
 			$response = $client->get($remote . '/ocm-provider/', $options);
 
+			$body = null;
 			if ($response->getStatusCode() === Http::STATUS_OK) {
 				$body = $response->getBody();
 				// update provider with data returned by the request
-				$this->provider->import(json_decode($body, true, 8, JSON_THROW_ON_ERROR) ?? []);
+				$provider->import(json_decode($body, true, 8, JSON_THROW_ON_ERROR) ?? []);
 				$this->cache->set($remote, $body, 60 * 60 * 24);
+				$this->remoteProviders[$remote] = $provider;
+				return $provider;
 			}
-		} catch (JsonException|OCMProviderException $e) {
+
+			throw new OCMProviderException('invalid remote ocm endpoint');
+		} catch (JsonException|OCMProviderException) {
 			$this->cache->set($remote, false, 5 * 60);
-			throw new OCMProviderException('data returned by remote seems invalid - ' . ($body ?? ''));
+			throw new OCMProviderException('data returned by remote seems invalid - status:' . $response->getStatusCode() . ' - ' . ($body ?? ''));
 		} catch (\Exception $e) {
 			$this->cache->set($remote, false, 5 * 60);
 			$this->logger->warning('error while discovering ocm provider', [
@@ -98,7 +125,61 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 			]);
 			throw new OCMProviderException('error while requesting remote ocm provider');
 		}
+	}
 
-		return $this->provider;
+	/**
+	 * @return ICapabilityAwareOCMProvider
+	 */
+	public function getLocalOCMProvider(bool $fullDetails = true): ICapabilityAwareOCMProvider {
+		if ($this->localProvider !== null) {
+			return $this->localProvider;
+		}
+
+		$provider = new OCMProvider('Nextcloud ' . $this->config->getSystemValue('version'));
+		if (!$this->appConfig->getValueBool('core', ConfigLexicon::OCM_DISCOVERY_ENABLED)) {
+			return $provider;
+		}
+
+		$url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare');
+		$pos = strrpos($url, '/');
+		if ($pos === false) {
+			$this->logger->debug('generated route should contain a slash character');
+			return $provider;
+		}
+
+		$provider->setEnabled(true);
+		$provider->setApiVersion(self::API_VERSION);
+		$provider->setEndPoint(substr($url, 0, $pos));
+		$provider->setCapabilities(['/invite-accepted', '/notifications', '/shares']);
+
+		$resource = $provider->createNewResourceType();
+		$resource->setName('file')
+			->setShareTypes(['user', 'group'])
+			->setProtocols(['webdav' => '/public.php/webdav/']);
+		$provider->addResourceType($resource);
+
+		if ($fullDetails) {
+			// Adding a public key to the ocm discovery
+			try {
+				if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
+					/**
+					 * @experimental 31.0.0
+					 * @psalm-suppress UndefinedInterfaceMethod
+					 */
+					$provider->setSignatory($this->ocmSignatoryManager->getLocalSignatory());
+				} else {
+					$this->logger->debug('ocm public key feature disabled');
+				}
+			} catch (SignatoryException|IdentityNotFoundException $e) {
+				$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
+			}
+		}
+
+		$event = new ResourceTypeRegisterEvent($provider);
+		$this->eventDispatcher->dispatchTyped($event);
+
+		$this->localProvider = $provider;
+		return $provider;
 	}
+
 }

lib/private/OCM/OCMSignatoryManager.php

@@ -20,6 +20,9 @@ use OC\Security\IdentityProof\Manager;
 use OCP\IAppConfig;
 use OCP\IURLGenerator;
 use OCP\OCM\Exceptions\OCMProviderException;
+use OCP\Server;
+use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\NotFoundExceptionInterface;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -41,7 +44,6 @@ class OCMSignatoryManager implements ISignatoryManager {
 		private readonly ISignatureManager $signatureManager,
 		private readonly IURLGenerator $urlGenerator,
 		private readonly Manager $identityProofManager,
-		private readonly OCMDiscoveryService $ocmDiscoveryService,
 		private readonly LoggerInterface $logger,
 	) {
 	}
@@ -144,7 +146,7 @@ class OCMSignatoryManager implements ISignatoryManager {
 	 */
 	public function getRemoteSignatory(string $remote): ?Signatory {
 		try {
-			$ocmProvider = $this->ocmDiscoveryService->discover($remote, true);
+			$ocmProvider = Server::get(OCMDiscoveryService::class)->discover($remote, true);
 			/**
 			 * @experimental 31.0.0
 			 * @psalm-suppress UndefinedInterfaceMethod
@@ -152,7 +154,7 @@ class OCMSignatoryManager implements ISignatoryManager {
 			$signatory = $ocmProvider->getSignatory();
 			$signatory?->setSignatoryType(SignatoryType::TRUSTED);
 			return $signatory;
-		} catch (OCMProviderException $e) {
+		} catch (NotFoundExceptionInterface|ContainerExceptionInterface|OCMProviderException $e) {
 			$this->logger->warning('fail to get remote signatory', ['exception' => $e, 'remote' => $remote]);
 			return null;
 		}

lib/private/Server.php

@@ -1227,7 +1227,8 @@ class Server extends ServerContainer implements IServerContainer {
 
 		$this->registerAlias(IPhoneNumberUtil::class, PhoneNumberUtil::class);
 
-		$this->registerAlias(ICapabilityAwareOCMProvider::class, OCMProvider::class);
+		// there is no reason for having OCMProvider as a Service
+		$this->registerDeprecatedAlias(ICapabilityAwareOCMProvider::class, OCMProvider::class);
 		$this->registerDeprecatedAlias(IOCMProvider::class, OCMProvider::class);
 
 		$this->registerAlias(ISetupCheckManager::class, SetupCheckManager::class);

lib/public/OCM/IOCMDiscoveryService.php

@@ -23,9 +23,10 @@ interface IOCMDiscoveryService {
 	 * @param string $remote address of the remote provider
 	 * @param bool $skipCache ignore cache, refresh data
 	 *
-	 * @return IOCMProvider
+	 * @return ICapabilityAwareOCMProvider
 	 * @throws OCMProviderException if no valid discovery data can be returned
 	 * @since 28.0.0
+	 * @since 32.0.0 returns ICapabilityAwareOCMProvider instead of IOCMProvider
 	 */
-	public function discover(string $remote, bool $skipCache = false): IOCMProvider;
+	public function discover(string $remote, bool $skipCache = false): ICapabilityAwareOCMProvider;
 }