OCM: Send local webdav url instead of the remote

[mickenordin]

Summary

During compatibility testing with CERNBox, this issue came up. The problem is that we are currently mixing up the local and remote webdav urls when sending a share that uses token exchange, so that needs to be fixed.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[MahdiBaghbani]

Thanks for this @micke, the absolute sender-side uri is exactly what reva receivers need.

One thing I noticed while tracing it: the new extractProtocolEntry('file', 'webdav') call on the local provider sits outside the factory's try blocks, and OCMArgumentException extends \Exception directly, so neither existing catch (OCMProviderException, \InvalidArgumentException) stops it.

It goes to FederatedShareProvider::createFederatedShare, which deletes the share and shows "Sharing X failed, could not find Y, maybe the server is currently unreachable or uses a self-signed certificate."

This happens whenever the remote advertises exchange-token but the local provider has no file/webdav entry:

1. occ config:app:set core ocm_discovery_enabled --value=false --type=boolean (then getLocalOCMProvider returns an empty provider), or a LocalOCMDiscoveryEvent listener drops the entry.
2. Share a file with a user on a server that advertises the exchange-token capability.

I have some snippets from the code to show the process:

• Factory call:

  server/lib/private/Federation/CloudFederationFactory.php

  Lines 77 to 82 in c31ac25

  	$webDavUri = '';
  	if ($useExchangeToken) {
  	$webDavUri = $this->urlGenerator->getAbsoluteURL(
  	$this->ocmDiscoveryService->getLocalOCMProvider(false)->extractProtocolEntry('file', 'webdav'),
  	);
  	}

• Share removal:

  server/apps/federatedfilesharing/lib/FederatedShareProvider.php

  Lines 210 to 222 in c31ac25

  	} catch (\Exception $e) {
  	$this->logger->error('Failed to notify remote server of federated share, removing share.', [
  	'app' => 'federatedfilesharing',
  	'exception' => $e,
  	]);
  	$failure = true;
  	}
  	if ($failure) {
  	$this->removeShareFromTable($shareId);
  	$message_t = $this->l->t('Sharing %1$s failed, could not find %2$s, maybe the server is currently unreachable or uses a self-signed certificate.',
  	[$share->getNode()->getName(), $share->getSharedWith()]);
  	throw new \Exception($message_t);

The receiver side already handles the same call by falling back (

server/apps/files_sharing/lib/External/Storage.php

Lines 93 to 104 in c31ac25

	// use default path to webdav if not found on discovery
	try {
	$ocmProvider = $discoveryService->discover($this->cloudId->getRemote());
	$webDavEndpoint = $ocmProvider->extractProtocolEntry('file', 'webdav');
	$remote = $ocmProvider->getEndPoint();
	$authType = \Sabre\DAV\Client::AUTH_BASIC;
	} catch (OCMProviderException|OCMArgumentException $e) {
	$this->logger->notice('exception while retrieving webdav endpoint', ['exception' => $e]);
	$webDavEndpoint = '/public.php/webdav';
	$remote = $this->cloudId->getRemote();
	$authType = \Sabre\DAV\Client::AUTH_BASIC;
	}

).

Could the sender do the same and drop to the legacy share method?

$webDavUri = '';
if ($useExchangeToken) {
	try {
		$webDavUri = $this->urlGenerator->getAbsoluteURL(
			$this->ocmDiscoveryService->getLocalOCMProvider(false)->extractProtocolEntry('file', 'webdav'),
		);
	} catch (OCMArgumentException $e) {
		$this->logger->warning('No local OCM webdav entry, using legacy share method', [
			'exception' => $e->getMessage(),
		]);
		$useExchangeToken = false;
	}
}

(plus use OCP\OCM\Exceptions\OCMArgumentException;)

Separately, the exchange-token envelope still sends {http://open-cloud-mesh.org/ns}share-permissions as the permission value, which reva rejects at ingest.

I'll open a small PR on top of this branch (Thanks to you for reminding me yesterday that I was already in this rabbit hole before 😄) that sends the mapped OCM permissions instead, so the two can be reviewed together.

[provokateurin]

Suggested change

	$webDavUri = '';
	$webDavUri = $remoteDomain;

Why is it not like this to have the previous behaviour, when not using token exchange?

[provokateurin]

Ah because in that case the parameter isn't even used. Please change it, to avoid this problem.

[mickenordin]

Can you clarify what what to change?

[mickenordin]

Thanks for this @micke, the absolute sender-side uri is exactly what reva receivers need.

One thing I noticed while tracing it: the new extractProtocolEntry('file', 'webdav') call on the local provider sits outside the factory's try blocks, and OCMArgumentException extends \Exception directly, so neither existing catch (OCMProviderException, \InvalidArgumentException) stops it.

It goes to FederatedShareProvider::createFederatedShare, which deletes the share and shows "Sharing X failed, could not find Y, maybe the server is currently unreachable or uses a self-signed certificate."

This happens whenever the remote advertises exchange-token but the local provider has no file/webdav entry:

1. `occ config:app:set core ocm_discovery_enabled --value=false --type=boolean` (then `getLocalOCMProvider` returns an empty provider), or a `LocalOCMDiscoveryEvent` listener drops the entry.

2. Share a file with a user on a server that advertises the `exchange-token` capability.

I have some snippets from the code to show the process:

* Factory call: https://github.com/nextcloud/server/blob/c31ac25801fed130179d5928539422273f42b6c9/lib/private/Federation/CloudFederationFactory.php#L77-L82

* Share removal: https://github.com/nextcloud/server/blob/c31ac25801fed130179d5928539422273f42b6c9/apps/federatedfilesharing/lib/FederatedShareProvider.php#L210-L222

The receiver side already handles the same call by falling back (

server/apps/files_sharing/lib/External/Storage.php

Lines 93 to 104 in c31ac25

	// use default path to webdav if not found on discovery
	try {
	$ocmProvider = $discoveryService->discover($this->cloudId->getRemote());
	$webDavEndpoint = $ocmProvider->extractProtocolEntry('file', 'webdav');
	$remote = $ocmProvider->getEndPoint();
	$authType = \Sabre\DAV\Client::AUTH_BASIC;
	} catch (OCMProviderException|OCMArgumentException $e) {
	$this->logger->notice('exception while retrieving webdav endpoint', ['exception' => $e]);
	$webDavEndpoint = '/public.php/webdav';
	$remote = $this->cloudId->getRemote();
	$authType = \Sabre\DAV\Client::AUTH_BASIC;
	}

).

Could the sender do the same and drop to the legacy share method?

$webDavUri = '';
if ($useExchangeToken) {
	try {
		$webDavUri = $this->urlGenerator->getAbsoluteURL(
			$this->ocmDiscoveryService->getLocalOCMProvider(false)->extractProtocolEntry('file', 'webdav'),
		);
	} catch (OCMArgumentException $e) {
		$this->logger->warning('No local OCM webdav entry, using legacy share method', [
			'exception' => $e->getMessage(),
		]);
		$useExchangeToken = false;
	}
}

(plus use OCP\OCM\Exceptions\OCMArgumentException;)

Separately, the exchange-token envelope still sends {http://open-cloud-mesh.org/ns}share-permissions as the permission value, which reva rejects at ingest.

I'll open a small PR on top of this branch (Thanks to you for reminding me yesterday that I was already in this rabbit hole before 😄) that sends the mapped OCM permissions instead, so the two can be reviewed together.

@MahdiBaghbani, I pushed another commit that ported over the general idea of what you had done in the PR to Enriques branch. Can you take another look on what you think?

[mickenordin]

I also noticed that we were looking for the wrong capability to use the new format, we were using token-exchange as a proxy for the newer format, but there is an actual capability for protocol-object so I changed to that too.

[MahdiBaghbani]

Fantastic this seems almost pretty much in line of what I intended to do, thanks @mickenordin !
I'll check it tomorrow afternoon again.

[come-nc]

It’s a code smell to have an app call methods in OC\. Could you find a home for this method in a public API if it makes sense?

[mickenordin]

I moved it to a separate public api now