From ebc7ae9872afd20c3e29f35ffda6420685eeeda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:04 +0200 Subject: [PATCH 1/9] feat(auth): support permanent OCM refresh tokens and bearer login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- .../Authentication/Token/PublicKeyToken.php | 5 +++++ lib/private/User/Session.php | 5 ++++- lib/public/Authentication/Token/IToken.php | 16 ++++++++++++++ .../Token/PublicKeyTokenProviderTest.php | 1 + tests/lib/User/SessionTest.php | 21 +++++++++++++++---- 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index e3d5e7001ff5b..dce7d40101466 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -205,6 +205,11 @@ public function getRemember(): int { return parent::getRemember(); } + #[\Override] + public function getType(): int { + return $this->getter('type'); + } + #[\Override] public function setToken(string $token): void { parent::setToken($token); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 2f3e94c198bac..96d9a16c39ccc 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -839,7 +839,10 @@ public function tryTokenLogin(IRequest $request) { return false; } - if ($dbToken instanceof PublicKeyToken && $dbToken->getType() === IToken::TEMPORARY_TOKEN && !$tokenFromCookie) { + if ($dbToken instanceof PublicKeyToken + && $dbToken->getType() === IToken::TEMPORARY_TOKEN + && !$tokenFromCookie + && $dbToken->getName() !== IToken::OCM_ACCESS_TOKEN_NAME) { // Session token but from Bearer header, not allowed return false; } diff --git a/lib/public/Authentication/Token/IToken.php b/lib/public/Authentication/Token/IToken.php index c4496fe1f50cf..138fe1dd10abd 100644 --- a/lib/public/Authentication/Token/IToken.php +++ b/lib/public/Authentication/Token/IToken.php @@ -48,6 +48,15 @@ interface IToken extends JsonSerializable { */ public const SCOPE_SKIP_PASSWORD_VALIDATION = 'password-unconfirmable'; + /** + * Token name used for OCM access tokens issued via the + * federated token endpoint. Marks a TEMPORARY_TOKEN as legitimate + * Bearer-header auth (vs. a browser session id leaked into a header). + * + * @since 33.0.0 + */ + public const OCM_ACCESS_TOKEN_NAME = 'OCM Access Token'; + /** * Get the token ID * @since 28.0.0 @@ -131,4 +140,11 @@ public function setPassword(string $password): void; * @since 28.0.0 */ public function setExpires(?int $expires): void; + + /** + * Get the type of the token + * @return int One of IToken::TEMPORARY_TOKEN, IToken::PERMANENT_TOKEN, or IToken::WIPE_TOKEN + * @since 32.0.0 + */ + public function getType(): int; } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 1c4bfba202d29..92c9166afad06 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -448,6 +448,7 @@ public function testRenewSessionTokenWithPassword(): void { public function testGetToken(): void { $token = new PublicKeyToken(); + $token->setType(IToken::TEMPORARY_TOKEN); $this->config->method('getSystemValue') ->with('secret') diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 721d93e122fb0..6219da38a452c 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -331,11 +331,17 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void { ->getMock(); $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); - $session->expects($this->never()) - ->method('set'); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('foo'); + $user->method('isEnabled')->willReturn(true); + $manager->method('get') + ->with('foo') + ->willReturn($user); + $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); + $token->setId(1); $token->setLoginName('foo'); $token->setLastCheck(0); // Never $token->setUid('foo'); @@ -369,11 +375,17 @@ public function testLoginLastCheckUpdate(): void { ->getMock(); $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); - $session->expects($this->never()) - ->method('set'); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('foo'); + $user->method('isEnabled')->willReturn(true); + $manager->method('get') + ->with('foo') + ->willReturn($user); + $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); + $token->setId(1); $token->setLoginName('foo'); $token->setLastCheck(0); // Never $token->setUid('foo'); @@ -1323,4 +1335,5 @@ public function testLogClientInThrottlerEmail(): void { $this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-A-PASSWORD', $request, $this->throttler)); } + } From 1f0044f212ce4045aa6918a4835d31eea46710f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:21 +0200 Subject: [PATCH 2/9] feat(dav): accept bearer access tokens on webdav endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- apps/dav/appinfo/v1/caldav.php | 11 ++++++++++- apps/dav/appinfo/v1/carddav.php | 11 ++++++++++- apps/dav/appinfo/v1/publicwebdav.php | 16 ++++++++++++++-- apps/dav/appinfo/v2/publicremote.php | 16 ++++++++++++++-- apps/dav/lib/Connector/Sabre/BearerAuth.php | 19 +++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index d1c1381be0a6e..6127c245df7c2 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -18,6 +18,7 @@ use OCA\DAV\CalDAV\Validation\CalDavValidatePlugin; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\Connector\Sabre\Auth; +use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; use OCA\DAV\Connector\Sabre\MaintenancePlugin; use OCA\DAV\Connector\Sabre\Principal; @@ -47,6 +48,12 @@ Server::get(IThrottler::class), 'principals/' ); +$bearerAuthBackend = new BearerAuth( + Server::get(IUserSession::class), + Server::get(ISession::class), + Server::get(IRequest::class), + Server::get(IConfig::class), +); $principalBackend = new Principal( Server::get(IUserManager::class), Server::get(IGroupManager::class), @@ -108,7 +115,9 @@ // Add plugins $server->addPlugin(new MaintenancePlugin(Server::get(IConfig::class), $davL10n)); -$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend)); +$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); +$authPlugin->addBackend($bearerAuthBackend); +$server->addPlugin($authPlugin); $server->addPlugin(new \Sabre\CalDAV\Plugin()); $server->addPlugin(new LegacyDAVACL()); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index d883bae450bbb..d2b4681f763b4 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -17,6 +17,7 @@ use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\Connector\Sabre\Auth; +use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; use OCA\DAV\Connector\Sabre\MaintenancePlugin; use OCA\DAV\Connector\Sabre\Principal; @@ -44,6 +45,12 @@ Server::get(IThrottler::class), 'principals/' ); +$bearerAuthBackend = new BearerAuth( + Server::get(IUserSession::class), + Server::get(ISession::class), + Server::get(IRequest::class), + Server::get(IConfig::class), +); $principalBackend = new Principal( Server::get(IUserManager::class), Server::get(IGroupManager::class), @@ -90,7 +97,9 @@ $server->setBaseUri($baseuri); // Add plugins $server->addPlugin(new MaintenancePlugin(Server::get(IConfig::class), Server::get(IL10nFactory::class)->get('dav'))); -$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend)); +$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); +$authPlugin->addBackend($bearerAuthBackend); +$server->addPlugin($authPlugin); $server->addPlugin(new Plugin()); $server->addPlugin(new LegacyDAVACL()); diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 316a84d9767ee..d0b33c86c1c5a 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -9,6 +9,7 @@ use OC\Files\Storage\Wrapper\DirPermissionsMask; use OC\Files\View; use OCA\DAV\Connector\LegacyPublicAuth; +use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\ServerFactory; use OCA\DAV\Files\Sharing\FilesDropPlugin; use OCA\DAV\Files\Sharing\PublicLinkCheckPlugin; @@ -49,7 +50,14 @@ Server::get(ISession::class), Server::get(IThrottler::class) ); +$bearerAuthBackend = new BearerAuth( + Server::get(IUserSession::class), + Server::get(ISession::class), + Server::get(IRequest::class), + Server::get(IConfig::class), +); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); +$authPlugin->addBackend($bearerAuthBackend); /** @var IEventDispatcher $eventDispatcher */ $eventDispatcher = Server::get(IEventDispatcher::class); @@ -80,6 +88,7 @@ $authPlugin, function (\Sabre\DAV\Server $server) use ( $authBackend, + $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin ) { @@ -90,8 +99,11 @@ function (\Sabre\DAV\Server $server) use ( // this is what is thrown when trying to access a non-existing share throw new \Sabre\DAV\Exception\NotAuthenticated(); } - - $share = $authBackend->getShare(); + try { + $share = $authBackend->getShare(); + } catch (AssertionError $e) { + $share = $bearerAuthBackend->getShare(); + } $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php index 027314e6edc16..0d9dcdfe02f76 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -9,6 +9,7 @@ use OC\Files\Storage\Wrapper\DirPermissionsMask; use OC\Files\Storage\Wrapper\PermissionsMask; use OC\Files\View; +use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\PublicAuth; use OCA\DAV\Connector\Sabre\ServerFactory; use OCA\DAV\Files\Sharing\FilesDropPlugin; @@ -67,7 +68,14 @@ Server::get(LoggerInterface::class), Server::get(IURLGenerator::class), ); +$bearerAuthBackend = new BearerAuth( + Server::get(IUserSession::class), + $session, + $request, + Server::get(IConfig::class), +); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); +$authPlugin->addBackend($bearerAuthBackend); $l10nFactory = Server::get(IFactory::class); $serverFactory = new ServerFactory( @@ -87,7 +95,7 @@ $filesDropPlugin = new FilesDropPlugin(); /** @var string $baseuri defined in public.php */ -$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $linkCheckPlugin, $filesDropPlugin) { +$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin) { // GET must be allowed for e.g. showing images and allowing Zip downloads if ($server->httpRequest->getMethod() !== 'GET') { // If this is *not* a GET request we only allow access to public DAV from AJAX or when Server2Server is allowed @@ -99,7 +107,11 @@ } } - $share = $authBackend->getShare(); + try { + $share = $authBackend->getShare(); + } catch (NotFound $e) { + $share = $bearerAuthBackend->getShare(); + } $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index 0e58c697a92d7..bb922bae587c5 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -13,6 +13,9 @@ use OCP\IRequest; use OCP\ISession; use OCP\IUserSession; +use OCP\Server; +use OCP\Share\IManager; +use OCP\Share\IShare; use Sabre\DAV\Auth\Backend\AbstractBearer; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; @@ -24,6 +27,7 @@ public function __construct( private IRequest $request, private IConfig $config, private string $principalPrefix = 'principals/users/', + private string $token = '', ) { // setup realm $defaults = new Defaults(); @@ -42,6 +46,15 @@ private function setupUserFs($userId) { #[\Override] public function validateBearerToken($bearerToken) { \OC_Util::setupFS(); + $this->token = $bearerToken; + + // public.php sets incognito mode for anonymous share access, which makes + // Session::getUser() return null and consequently Session::isLoggedIn() + // return false even after a successful token login. Disable it here so + // the logged-in user is visible for the rest of the request. If the + // bearer token is invalid and Sabre falls back to one of the public + // auth backends, that backend will re-enable incognito mode itself. + \OC_User::setIncognitoMode(false); if (!$this->userSession->isLoggedIn()) { $this->userSession->tryTokenLogin($this->request); @@ -53,6 +66,12 @@ public function validateBearerToken($bearerToken) { return false; } + public function getShare(): IShare { + $shareManager = Server::get(IManager::class); + $share = $shareManager->getShareByToken($this->token); + return $share; + } + /** * \Sabre\DAV\Auth\Backend\AbstractBearer::challenge sets an WWW-Authenticate * header which some DAV clients can't handle. Thus we override this function From be3f8e65dcfc395fc6f408cf07cdc94b5e819233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:22 +0200 Subject: [PATCH 3/9] feat(cloud_federation_api): add token exchange endpoint issuing JWT access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- apps/cloud_federation_api/appinfo/info.xml | 4 + .../composer/composer/autoload_classmap.php | 5 + .../composer/composer/autoload_static.php | 5 + .../CleanupExpiredOcmTokensJob.php | 37 ++ .../lib/Controller/TokenController.php | 274 +++++++++++++ .../lib/Db/OcmTokenMap.php | 41 ++ .../lib/Db/OcmTokenMapMapper.php | 58 +++ .../Version1017Date20260306120000.php | 54 +++ apps/cloud_federation_api/openapi.json | 20 +- .../CleanupExpiredOcmTokensJobTest.php | 45 +++ .../tests/Controller/TokenControllerTest.php | 377 ++++++++++++++++++ 11 files changed, 913 insertions(+), 7 deletions(-) create mode 100644 apps/cloud_federation_api/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php create mode 100644 apps/cloud_federation_api/lib/Controller/TokenController.php create mode 100644 apps/cloud_federation_api/lib/Db/OcmTokenMap.php create mode 100644 apps/cloud_federation_api/lib/Db/OcmTokenMapMapper.php create mode 100644 apps/cloud_federation_api/lib/Migration/Version1017Date20260306120000.php create mode 100644 apps/cloud_federation_api/tests/BackgroundJob/CleanupExpiredOcmTokensJobTest.php create mode 100644 apps/cloud_federation_api/tests/Controller/TokenControllerTest.php diff --git a/apps/cloud_federation_api/appinfo/info.xml b/apps/cloud_federation_api/appinfo/info.xml index 77a543f8fe9d7..7991f4f062d06 100644 --- a/apps/cloud_federation_api/appinfo/info.xml +++ b/apps/cloud_federation_api/appinfo/info.xml @@ -21,4 +21,8 @@ + + + OCA\CloudFederationAPI\BackgroundJob\CleanupExpiredOcmTokensJob + diff --git a/apps/cloud_federation_api/composer/composer/autoload_classmap.php b/apps/cloud_federation_api/composer/composer/autoload_classmap.php index 5441bfc832535..038e4d6873d24 100644 --- a/apps/cloud_federation_api/composer/composer/autoload_classmap.php +++ b/apps/cloud_federation_api/composer/composer/autoload_classmap.php @@ -8,13 +8,18 @@ return array( 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCA\\CloudFederationAPI\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php', + 'OCA\\CloudFederationAPI\\BackgroundJob\\CleanupExpiredOcmTokensJob' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredOcmTokensJob.php', 'OCA\\CloudFederationAPI\\Capabilities' => $baseDir . '/../lib/Capabilities.php', 'OCA\\CloudFederationAPI\\Config' => $baseDir . '/../lib/Config.php', 'OCA\\CloudFederationAPI\\Controller\\OCMRequestController' => $baseDir . '/../lib/Controller/OCMRequestController.php', 'OCA\\CloudFederationAPI\\Controller\\RequestHandlerController' => $baseDir . '/../lib/Controller/RequestHandlerController.php', + 'OCA\\CloudFederationAPI\\Controller\\TokenController' => $baseDir . '/../lib/Controller/TokenController.php', 'OCA\\CloudFederationAPI\\Db\\FederatedInvite' => $baseDir . '/../lib/Db/FederatedInvite.php', 'OCA\\CloudFederationAPI\\Db\\FederatedInviteMapper' => $baseDir . '/../lib/Db/FederatedInviteMapper.php', + 'OCA\\CloudFederationAPI\\Db\\OcmTokenMap' => $baseDir . '/../lib/Db/OcmTokenMap.php', + 'OCA\\CloudFederationAPI\\Db\\OcmTokenMapMapper' => $baseDir . '/../lib/Db/OcmTokenMapMapper.php', 'OCA\\CloudFederationAPI\\Events\\FederatedInviteAcceptedEvent' => $baseDir . '/../lib/Events/FederatedInviteAcceptedEvent.php', 'OCA\\CloudFederationAPI\\Migration\\Version1016Date202502262004' => $baseDir . '/../lib/Migration/Version1016Date202502262004.php', + 'OCA\\CloudFederationAPI\\Migration\\Version1017Date20260306120000' => $baseDir . '/../lib/Migration/Version1017Date20260306120000.php', 'OCA\\CloudFederationAPI\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', ); diff --git a/apps/cloud_federation_api/composer/composer/autoload_static.php b/apps/cloud_federation_api/composer/composer/autoload_static.php index 6d192f98e5138..f68f6714afa27 100644 --- a/apps/cloud_federation_api/composer/composer/autoload_static.php +++ b/apps/cloud_federation_api/composer/composer/autoload_static.php @@ -23,14 +23,19 @@ class ComposerStaticInitCloudFederationAPI public static $classMap = array ( 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCA\\CloudFederationAPI\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php', + 'OCA\\CloudFederationAPI\\BackgroundJob\\CleanupExpiredOcmTokensJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredOcmTokensJob.php', 'OCA\\CloudFederationAPI\\Capabilities' => __DIR__ . '/..' . '/../lib/Capabilities.php', 'OCA\\CloudFederationAPI\\Config' => __DIR__ . '/..' . '/../lib/Config.php', 'OCA\\CloudFederationAPI\\Controller\\OCMRequestController' => __DIR__ . '/..' . '/../lib/Controller/OCMRequestController.php', 'OCA\\CloudFederationAPI\\Controller\\RequestHandlerController' => __DIR__ . '/..' . '/../lib/Controller/RequestHandlerController.php', + 'OCA\\CloudFederationAPI\\Controller\\TokenController' => __DIR__ . '/..' . '/../lib/Controller/TokenController.php', 'OCA\\CloudFederationAPI\\Db\\FederatedInvite' => __DIR__ . '/..' . '/../lib/Db/FederatedInvite.php', 'OCA\\CloudFederationAPI\\Db\\FederatedInviteMapper' => __DIR__ . '/..' . '/../lib/Db/FederatedInviteMapper.php', + 'OCA\\CloudFederationAPI\\Db\\OcmTokenMap' => __DIR__ . '/..' . '/../lib/Db/OcmTokenMap.php', + 'OCA\\CloudFederationAPI\\Db\\OcmTokenMapMapper' => __DIR__ . '/..' . '/../lib/Db/OcmTokenMapMapper.php', 'OCA\\CloudFederationAPI\\Events\\FederatedInviteAcceptedEvent' => __DIR__ . '/..' . '/../lib/Events/FederatedInviteAcceptedEvent.php', 'OCA\\CloudFederationAPI\\Migration\\Version1016Date202502262004' => __DIR__ . '/..' . '/../lib/Migration/Version1016Date202502262004.php', + 'OCA\\CloudFederationAPI\\Migration\\Version1017Date20260306120000' => __DIR__ . '/..' . '/../lib/Migration/Version1017Date20260306120000.php', 'OCA\\CloudFederationAPI\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', ); diff --git a/apps/cloud_federation_api/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php b/apps/cloud_federation_api/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php new file mode 100644 index 0000000000000..7419439555204 --- /dev/null +++ b/apps/cloud_federation_api/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php @@ -0,0 +1,37 @@ +setInterval(6 * 60 * 60); // run every 6 hours + $this->setTimeSensitivity(self::TIME_INSENSITIVE); + } + + #[\Override] + protected function run($argument): void { + $this->mapper->deleteExpired($this->time->getTime()); + } +} diff --git a/apps/cloud_federation_api/lib/Controller/TokenController.php b/apps/cloud_federation_api/lib/Controller/TokenController.php new file mode 100644 index 0000000000000..9e2ae8a886f53 --- /dev/null +++ b/apps/cloud_federation_api/lib/Controller/TokenController.php @@ -0,0 +1,274 @@ +signatureManager->getIncomingSignedRequest($this->signatoryManager); + $this->logger->debug('Token request signature verified', [ + 'origin' => $signedRequest->getOrigin() + ]); + return $signedRequest; + } catch (SignatureNotFoundException|SignatoryNotFoundException $e) { + $this->logger->debug('Token request not signed', ['exception' => $e]); + + if ($this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, lazy: true)) { + $this->logger->notice('Rejected unsigned token request', ['exception' => $e]); + throw new IncomingRequestException('Unsigned request not allowed'); + } + return null; + } catch (SignatureException $e) { + $this->logger->warning('Invalid token request signature', ['exception' => $e]); + throw new IncomingRequestException('Invalid signature'); + } + } + + /** + * @return array{0: string, 1: string} [JWS algorithm, key material accepted by firebase/php-jwt] + * @throws \RuntimeException if the key cannot be parsed or its type is unsupported + */ + private function resolveJwtSigningKey(string $privateKeyPem): array { + $key = openssl_pkey_get_private($privateKeyPem); + if ($key === false) { + throw new \RuntimeException('Cannot parse signatory private key'); + } + $details = openssl_pkey_get_details($key); + + if (isset($details['rsa'])) { + $algorithm = $details['bits'] >= 4096 ? 'RS512' : 'RS256'; + return [$algorithm, $privateKeyPem]; + } + if (isset($details['ec'])) { + $algorithm = match ($details['ec']['curve_name'] ?? '') { + 'prime256v1' => 'ES256', + 'secp384r1' => 'ES384', + default => throw new \RuntimeException('Unsupported EC curve for JWT access token: ' . ($details['ec']['curve_name'] ?? 'unknown')), + }; + return [$algorithm, $privateKeyPem]; + } + + throw new \RuntimeException('Unsupported signatory key type for JWT access token'); + } + + /** + * Exchange a refresh token for a short-lived access token + * + * @param string $grant_type OAuth grant type, must be `authorization_code` + * @param string $code The refresh token to exchange for an access token + * @return DataResponse|DataResponse + * + * 200: Access token successfully generated + * 400: Bad request - missing refresh token or invalid request format + * 401: Unauthorized - invalid or expired refresh token, or invalid signature + * 500: Internal server error + */ + #[PublicPage] + #[NoCSRFRequired] + #[FrontpageRoute(verb: 'POST', url: '/api/v1/access-token')] + public function accessToken(string $grant_type = '', string $code = ''): DataResponse { + try { + $signedRequest = $this->verifySignedRequest(); + } catch (IncomingRequestException $e) { + $this->logger->warning('Token request signature verification failed', [ + 'exception' => $e + ]); + return new DataResponse( + ['error' => 'invalid_request'], + Http::STATUS_UNAUTHORIZED + ); + } + + if ($grant_type !== 'authorization_code') { + return new DataResponse( + ['error' => 'unsupported_grant_type'], + Http::STATUS_BAD_REQUEST + ); + } + + if ($code === '') { + return new DataResponse( + ['error' => 'refresh_token is required'], + Http::STATUS_BAD_REQUEST + ); + } + + $refreshToken = $code; + + try { + $token = $this->tokenProvider->getToken($refreshToken); + + if ($token->getType() !== IToken::PERMANENT_TOKEN) { + $this->logger->warning('Attempted to use non-permanent token as refresh token', [ + 'tokenId' => $token->getId(), + ]); + return new DataResponse( + ['error' => 'invalid_grant'], + Http::STATUS_UNAUTHORIZED + ); + } + + // After the first exchange the refresh token must only be usable to + // obtain further access tokens, never as a direct filesystem/WebDAV + // credential. Lock down its filesystem scope so a leaked refresh token + // cannot be replayed as a bearer against the WebDAV endpoints. + $scope = $token->getScopeAsArray(); + if (($scope[IToken::SCOPE_FILESYSTEM] ?? true) !== false) { + $scope[IToken::SCOPE_FILESYSTEM] = false; + $token->setScope($scope); + $this->tokenProvider->updateToken($token); + } + + // Revoke the previous access token for this refresh token, if any. + $existingMapping = $this->ocmTokenMapMapper->findByRefreshToken($refreshToken); + if ($existingMapping !== null) { + try { + $this->tokenProvider->invalidateTokenById( + $token->getUID(), + $existingMapping->getAccessTokenId() + ); + } catch (\Exception) { + // Token may already be gone; ignore. + } + $this->ocmTokenMapMapper->delete($existingMapping); + } + + $share = $this->shareManager->getShareByToken($refreshToken); + // access_token TTL from the refresh-token scope; default 3600, clamped 300..86400. + $ttl = (int)($token->getScopeAsArray()['ocm_access_token_ttl'] ?? 3600); + $expiresIn = max(300, min(86400, $ttl)); + $issuedAt = $this->timeFactory->getTime(); + $expiresAt = $issuedAt + $expiresIn; + + $signatory = $this->signatoryManager->getLocalJwksSignatory(); + if ($signatory === null) { + throw new \RuntimeException('No JWKS-published OCM signatory available to sign the access token'); + } + $keyId = $signatory->getKeyId(); + $issuer = parse_url($keyId, PHP_URL_SCHEME) . '://' . Signatory::extractIdentityFromUri($keyId); + + [$jwtAlgorithm, $jwtKey] = $this->resolveJwtSigningKey($signatory->getPrivateKey()); + + $payload = [ + 'iss' => $issuer, + 'sub' => $share->getShareOwner(), + 'aud' => $share->getSharedWith(), + 'client_id' => (string)$token->getId(), + 'iat' => $issuedAt, + 'exp' => $expiresAt, + 'jti' => $this->random->generate(16, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS), + ]; + + $accessTokenString = JWT::encode($payload, $jwtKey, $jwtAlgorithm, $keyId, ['typ' => 'at+jwt']); + + $accessToken = $this->tokenProvider->generateToken( + $accessTokenString, + $token->getUID(), + $token->getLoginName(), + null, // No password for access tokens + IToken::OCM_ACCESS_TOKEN_NAME, + IToken::TEMPORARY_TOKEN, + IToken::DO_NOT_REMEMBER + ); + + $accessToken->setExpires($expiresAt); + $this->tokenProvider->updateToken($accessToken); + + $mapping = new OcmTokenMap(); + $mapping->setAccessTokenId($accessToken->getId()); + $mapping->setRefreshToken($refreshToken); + $mapping->setExpires($expiresAt); + $this->ocmTokenMapMapper->insert($mapping); + + return new DataResponse([ + 'access_token' => $accessTokenString, + 'token_type' => 'Bearer', + 'expires_in' => $expiresIn, + ], Http::STATUS_OK); + } catch (InvalidTokenException $e) { + $this->logger->info('Invalid refresh token provided', [ + 'exception' => $e, + ]); + return new DataResponse( + ['error' => 'invalid_grant'], + Http::STATUS_UNAUTHORIZED + ); + } catch (ExpiredTokenException $e) { + $this->logger->info('Expired refresh token provided', [ + 'exception' => $e, + ]); + return new DataResponse( + ['error' => 'invalid_grant'], + Http::STATUS_UNAUTHORIZED + ); + } catch (\Exception $e) { + $this->logger->error('Error generating access token', [ + 'exception' => $e, + ]); + return new DataResponse( + ['error' => 'server_error'], + Http::STATUS_INTERNAL_SERVER_ERROR + ); + } + } +} diff --git a/apps/cloud_federation_api/lib/Db/OcmTokenMap.php b/apps/cloud_federation_api/lib/Db/OcmTokenMap.php new file mode 100644 index 0000000000000..931ca08f6ebd9 --- /dev/null +++ b/apps/cloud_federation_api/lib/Db/OcmTokenMap.php @@ -0,0 +1,41 @@ +addType('accessTokenId', Types::INTEGER); + $this->addType('refreshToken', Types::STRING); + $this->addType('expires', Types::INTEGER); + } +} diff --git a/apps/cloud_federation_api/lib/Db/OcmTokenMapMapper.php b/apps/cloud_federation_api/lib/Db/OcmTokenMapMapper.php new file mode 100644 index 0000000000000..c54e4e106be03 --- /dev/null +++ b/apps/cloud_federation_api/lib/Db/OcmTokenMapMapper.php @@ -0,0 +1,58 @@ + + */ +class OcmTokenMapMapper extends QBMapper { + public function __construct(IDBConnection $db) { + parent::__construct($db, 'ocm_token_map', OcmTokenMap::class); + } + + /** + * @throws DoesNotExistException + */ + public function getByAccessTokenId(int $accessTokenId): OcmTokenMap { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('access_token_id', $qb->createNamedParameter($accessTokenId))); + + return $this->findEntity($qb); + } + + /** + * Find the current mapping for a given refresh token, if any. + */ + public function findByRefreshToken(string $refreshToken): ?OcmTokenMap { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('refresh_token', $qb->createNamedParameter($refreshToken))); + + try { + return $this->findEntity($qb); + } catch (DoesNotExistException) { + return null; + } + } + + public function deleteExpired(int $time): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where($qb->expr()->lt('expires', $qb->createNamedParameter($time))); + $qb->executeStatement(); + } +} diff --git a/apps/cloud_federation_api/lib/Migration/Version1017Date20260306120000.php b/apps/cloud_federation_api/lib/Migration/Version1017Date20260306120000.php new file mode 100644 index 0000000000000..4211ada182ce3 --- /dev/null +++ b/apps/cloud_federation_api/lib/Migration/Version1017Date20260306120000.php @@ -0,0 +1,54 @@ +hasTable('ocm_token_map')) { + return null; + } + + $table = $schema->createTable('ocm_token_map'); + $table->addColumn('id', Types::INTEGER, [ + 'autoincrement' => true, + 'notnull' => true, + 'unsigned' => true, + ]); + $table->addColumn('access_token_id', Types::INTEGER, [ + 'notnull' => true, + 'unsigned' => true, + ]); + $table->addColumn('refresh_token', Types::STRING, [ + 'notnull' => true, + 'length' => 512, + ]); + $table->addColumn('expires', Types::INTEGER, [ + 'notnull' => true, + ]); + + $table->setPrimaryKey(['id']); + $table->addIndex(['access_token_id'], 'ocm_tkmap_atid'); + $table->addIndex(['expires'], 'ocm_tkmap_exp'); + + return $schema; + } +} diff --git a/apps/cloud_federation_api/openapi.json b/apps/cloud_federation_api/openapi.json index 21f669a2c5f79..695fa5fb56008 100644 --- a/apps/cloud_federation_api/openapi.json +++ b/apps/cloud_federation_api/openapi.json @@ -161,23 +161,25 @@ }, "protocol": { "type": "object", - "description": "e,.g. ['name' => 'webdav', 'options' => ['username' => 'john', 'permissions' => 31]]", + "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]] or Multi format: ['name' => 'multi', 'webdav' => [...]]", "required": [ - "name", - "options" + "name" ], "properties": { "name": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" }, "options": { "type": "object", "additionalProperties": { "type": "object" } + }, + "webdav": { + "type": "object", + "additionalProperties": { + "type": "object" + } } } }, @@ -494,6 +496,10 @@ { "name": "request_handler", "description": "Open-Cloud-Mesh-API" + }, + { + "name": "token", + "description": "Controller for the /token endpoint Exchanges long-lived refresh tokens for short-lived access tokens" } ] } diff --git a/apps/cloud_federation_api/tests/BackgroundJob/CleanupExpiredOcmTokensJobTest.php b/apps/cloud_federation_api/tests/BackgroundJob/CleanupExpiredOcmTokensJobTest.php new file mode 100644 index 0000000000000..c81f734579f31 --- /dev/null +++ b/apps/cloud_federation_api/tests/BackgroundJob/CleanupExpiredOcmTokensJobTest.php @@ -0,0 +1,45 @@ +timeFactory = $this->createMock(ITimeFactory::class); + $this->mapper = $this->createMock(OcmTokenMapMapper::class); + + $this->job = new CleanupExpiredOcmTokensJob($this->timeFactory, $this->mapper); + } + + public function testRunDeletesExpiredTokens(): void { + $now = 1700000000; + $this->timeFactory->expects($this->once()) + ->method('getTime') + ->willReturn($now); + + $this->mapper->expects($this->once()) + ->method('deleteExpired') + ->with($now); + + $method = new \ReflectionMethod(CleanupExpiredOcmTokensJob::class, 'run'); + $method->invoke($this->job, []); + } +} diff --git a/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php b/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php new file mode 100644 index 0000000000000..e454b1c40175f --- /dev/null +++ b/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php @@ -0,0 +1,377 @@ +request = $this->createMock(IRequest::class); + $this->tokenProvider = $this->createMock(IProvider::class); + $this->random = $this->createMock(ISecureRandom::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->signatureManager = $this->createMock(ISignatureManager::class); + $this->signatoryManager = $this->createMock(OCMSignatoryManager::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->ocmTokenMapMapper = $this->createMock(OcmTokenMapMapper::class); + $this->shareManager = $this->createMock(IShareManager::class); + + $this->controller = new TokenController( + $this->request, + $this->tokenProvider, + $this->random, + $this->timeFactory, + $this->logger, + $this->signatureManager, + $this->signatoryManager, + $this->appConfig, + $this->ocmTokenMapMapper, + $this->shareManager, + ); + } + + protected function tearDown(): void { + JWT::$timestamp = null; + parent::tearDown(); + } + + /** + * Configure the collaborators so that exchanging $refreshToken issues a JWT + * access token. Returns the refresh token mock for further expectations. + */ + private function configureHappyPath( + string $refreshToken, + int $tokenId, + string $uid, + string $shareOwner, + string $sharedWith, + string $jti, + array $scope = [IToken::SCOPE_FILESYSTEM => true], + ): IToken&MockObject { + $privateKey = openssl_pkey_new([ + 'private_key_bits' => 2048, + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + ]); + openssl_pkey_export($privateKey, $privateKeyPem); + $this->publicKeyPem = openssl_pkey_get_details($privateKey)['key']; + + $refreshTokenMock = $this->createMock(IToken::class); + $refreshTokenMock->method('getType')->willReturn(IToken::PERMANENT_TOKEN); + $refreshTokenMock->method('getId')->willReturn($tokenId); + $refreshTokenMock->method('getUID')->willReturn($uid); + $refreshTokenMock->method('getLoginName')->willReturn($uid); + $refreshTokenMock->method('getScopeAsArray')->willReturn($scope); + $this->tokenProvider->method('getToken') + ->with($refreshToken) + ->willReturn($refreshTokenMock); + + $this->ocmTokenMapMapper->method('findByRefreshToken') + ->with($refreshToken) + ->willReturn(null); + + $share = $this->createMock(IShare::class); + $share->method('getShareOwner')->willReturn($shareOwner); + $share->method('getSharedWith')->willReturn($sharedWith); + $this->shareManager->method('getShareByToken') + ->with($refreshToken) + ->willReturn($share); + + $signatory = new Signatory(); + $signatory->setKeyId('https://local.example.com/index.php/ocm#signature'); + $signatory->setPrivateKey($privateKeyPem); + $this->signatoryManager->method('getLocalJwksSignatory')->willReturn($signatory); + + $this->random->method('generate')->willReturn($jti); + $this->timeFactory->method('getTime')->willReturn(1000000); + + $accessToken = $this->createMock(IToken::class); + $accessToken->method('getId')->willReturn(456); + $this->tokenProvider->method('generateToken')->willReturn($accessToken); + + return $refreshTokenMock; + } + + public function testAccessTokenSuccess(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->with($this->signatoryManager) + ->willReturn($signedRequest); + + $this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@remote.example.com', 'fixedjtivalue00'); + + $this->ocmTokenMapMapper->expects($this->once()) + ->method('insert') + ->with($this->callback(function ($mapping) { + return $mapping->getAccessTokenId() === 456 + && $mapping->getRefreshToken() === 'valid-refresh-token' + && $mapping->getExpires() === 1000000 + 3600; + })); + + $result = $this->controller->accessToken('authorization_code', 'valid-refresh-token'); + + $this->assertInstanceOf(DataResponse::class, $result); + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + + $data = $result->getData(); + $this->assertSame('Bearer', $data['token_type']); + $this->assertSame(3600, $data['expires_in']); + $this->assertNotEmpty($data['access_token']); + + // Evaluate token validity at the mocked issue time, not the real clock. + JWT::$timestamp = 1000000; + $decoded = JWT::decode($data['access_token'], new Key($this->publicKeyPem, 'RS256')); + $this->assertSame('https://local.example.com', $decoded->iss); + $this->assertSame('owner', $decoded->sub); + $this->assertSame('sharee@remote.example.com', $decoded->aud); + $this->assertSame('123', $decoded->client_id); + $this->assertSame('fixedjtivalue00', $decoded->jti); + $this->assertSame(1000000, $decoded->iat); + $this->assertSame(1000000 + 3600, $decoded->exp); + } + + public function testAccessTokenLocksRefreshTokenToExchangeOnly(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->with($this->signatoryManager) + ->willReturn($signedRequest); + + $refreshTokenMock = $this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@remote.example.com', 'fixedjtivalue00'); + + // The refresh token must be downgraded so it can no longer mount the + // filesystem, only be replayed against the token endpoint. + $refreshTokenMock->expects($this->once()) + ->method('setScope') + ->with($this->callback(fn (array $scope): bool => ($scope[IToken::SCOPE_FILESYSTEM] ?? null) === false)); + + $result = $this->controller->accessToken('authorization_code', 'valid-refresh-token'); + + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + } + + public function testAccessTokenDoesNotRelockAlreadyLockedRefreshToken(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->with($this->signatoryManager) + ->willReturn($signedRequest); + + $refreshTokenMock = $this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@remote.example.com', 'fixedjtivalue00', [IToken::SCOPE_FILESYSTEM => false]); + + // Already locked from a previous exchange: do not rewrite the scope. + $refreshTokenMock->expects($this->never())->method('setScope'); + + $result = $this->controller->accessToken('authorization_code', 'valid-refresh-token'); + + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + } + + public function testAccessTokenWithoutSignatureEnforcementDisabled(): void { + $this->signatureManager->method('getIncomingSignedRequest') + ->willThrowException(new SignatureNotFoundException()); + + $this->appConfig->method('getValueBool') + ->with('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, false, true) + ->willReturn(false); + + $this->configureHappyPath('refresh-token', 123, 'testuser', 'owner', 'sharee', 'fixedjtivalue00'); + + $result = $this->controller->accessToken('authorization_code', 'refresh-token'); + + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + } + + public function testAccessTokenWithoutSignatureEnforcementEnabled(): void { + $this->signatureManager->method('getIncomingSignedRequest') + ->willThrowException(new SignatureNotFoundException()); + + $this->appConfig->method('getValueBool') + ->with('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, false, true) + ->willReturn(true); + + $result = $this->controller->accessToken('authorization_code', 'refresh-token'); + + $this->assertEquals(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertEquals(['error' => 'invalid_request'], $result->getData()); + } + + public function testAccessTokenInvalidSignature(): void { + $this->signatureManager->method('getIncomingSignedRequest') + ->willThrowException(new SignatureException('Invalid signature')); + + $result = $this->controller->accessToken('authorization_code', 'refresh-token'); + + $this->assertEquals(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertEquals(['error' => 'invalid_request'], $result->getData()); + } + + public function testAccessTokenUnsupportedGrantType(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $result = $this->controller->accessToken('password', 'refresh-token'); + + $this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertEquals(['error' => 'unsupported_grant_type'], $result->getData()); + } + + public function testAccessTokenMissingGrantType(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $result = $this->controller->accessToken('', 'refresh-token'); + + $this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertEquals(['error' => 'unsupported_grant_type'], $result->getData()); + } + + public function testAccessTokenMissingRefreshToken(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $result = $this->controller->accessToken('authorization_code', ''); + + $this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertEquals(['error' => 'refresh_token is required'], $result->getData()); + } + + public function testAccessTokenNonPermanentToken(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $refreshToken = $this->createMock(IToken::class); + $refreshToken->method('getType')->willReturn(IToken::TEMPORARY_TOKEN); + $refreshToken->method('getId')->willReturn(123); + + $this->tokenProvider->method('getToken') + ->with('non-permanent-token') + ->willReturn($refreshToken); + + $result = $this->controller->accessToken('authorization_code', 'non-permanent-token'); + + $this->assertEquals(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertEquals(['error' => 'invalid_grant'], $result->getData()); + } + + public function testAccessTokenInvalidToken(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $this->tokenProvider->method('getToken') + ->with('invalid-token') + ->willThrowException(new InvalidTokenException()); + + $result = $this->controller->accessToken('authorization_code', 'invalid-token'); + + $this->assertEquals(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertEquals(['error' => 'invalid_grant'], $result->getData()); + } + + public function testAccessTokenExpiredToken(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $this->tokenProvider->method('getToken') + ->with('expired-token') + ->willThrowException(new ExpiredTokenException($this->createMock(IToken::class))); + + $result = $this->controller->accessToken('authorization_code', 'expired-token'); + + $this->assertEquals(Http::STATUS_UNAUTHORIZED, $result->getStatus()); + $this->assertEquals(['error' => 'invalid_grant'], $result->getData()); + } + + public function testAccessTokenServerError(): void { + $signedRequest = $this->createMock(IIncomingSignedRequest::class); + $signedRequest->method('getOrigin')->willReturn('remote.example.com'); + $this->signatureManager->method('getIncomingSignedRequest') + ->willReturn($signedRequest); + + $this->tokenProvider->method('getToken') + ->willThrowException(new \RuntimeException('Database connection failed')); + + $result = $this->controller->accessToken('authorization_code', 'some-token'); + + $this->assertEquals(Http::STATUS_INTERNAL_SERVER_ERROR, $result->getStatus()); + $this->assertEquals(['error' => 'server_error'], $result->getData()); + } + + public function testAccessTokenWithSignatoryNotFoundException(): void { + $this->signatureManager->method('getIncomingSignedRequest') + ->willThrowException(new SignatoryNotFoundException()); + + $this->appConfig->method('getValueBool') + ->with('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, false, true) + ->willReturn(false); + + $this->configureHappyPath('refresh-token', 123, 'testuser', 'owner', 'sharee', 'fixedjtivalue00'); + + $result = $this->controller->accessToken('authorization_code', 'refresh-token'); + + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + } +} From 6db2a516e6d7e3611062e89347a93fdc1c62ac94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:23 +0200 Subject: [PATCH 4/9] feat(cloud_federation_api): accept new protocol envelope and delegate validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept both the legacy options.sharedSecret envelope and the new protocol[name].sharedSecret form. Preserve the original cloud ID so the factory can discover capabilities, then reset shareWith to the local username for user lookup. Delegate per-protocol validation to providers via the new IValidationAwareCloudFederationProvider interface, with split exception handling: BadRequestException -> 400, ProviderCouldNotAddShareException -> the exception's own HTTP status (501 fallback). In the notification handler, fall back to looking up the refresh token via OcmTokenMapMapper when the access token cannot identify the federation. Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- .../Controller/RequestHandlerController.php | 52 ++++++++++++++++--- apps/cloud_federation_api/openapi.json | 2 +- openapi.json | 2 +- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index f89b2ba5f9697..932976cd681f7 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -7,9 +7,11 @@ namespace OCA\CloudFederationAPI\Controller; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\OCM\OCMSignatoryManager; use OCA\CloudFederationAPI\Config; use OCA\CloudFederationAPI\Db\FederatedInviteMapper; +use OCA\CloudFederationAPI\Db\OcmTokenMapMapper; use OCA\CloudFederationAPI\Events\FederatedInviteAcceptedEvent; use OCA\CloudFederationAPI\ResponseDefinitions; use OCP\AppFramework\Controller; @@ -31,6 +33,7 @@ use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudIdManager; use OCP\Federation\ISignedCloudFederationProvider; +use OCP\Federation\IValidationAwareCloudFederationProvider; use OCP\IAppConfig; use OCP\IGroupManager; use OCP\IRequest; @@ -39,6 +42,7 @@ use OCP\OCM\IOCMDiscoveryService; use OCP\Security\Signature\Exceptions\IncomingRequestException; use OCP\Security\Signature\IIncomingSignedRequest; +use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; use OCP\Util; use Psr\Log\LoggerInterface; @@ -85,7 +89,7 @@ public function __construct( * @param string|null $ownerDisplayName Display name of the user who shared the item * @param string|null $sharedBy Provider specific UID of the user who shared the resource * @param string|null $sharedByDisplayName Display name of the user who shared the resource - * @param array{name: list, options: array} $protocol e,.g. ['name' => 'webdav', 'options' => ['username' => 'john', 'permissions' => 31]] + * @param array{name: string, options?: array, webdav?: array} $protocol Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]] * @param string $shareType 'group' or 'user' share * @param string $resourceType 'file', 'calendar',... * @@ -94,6 +98,10 @@ public function __construct( * 201: The notification was successfully received. The display name of the recipient might be returned in the body * 400: Bad request due to invalid parameters, e.g. when `shareWith` is not found or required properties are missing * 501: Share type or the resource type is not supported + * + * @psalm-suppress InvalidReturnType + * @psalm-suppress InvalidReturnStatement + * @psalm-suppress LessSpecificReturnStatement */ #[PublicPage] #[NoCSRFRequired] @@ -120,9 +128,6 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ || $shareType === null || !is_array($protocol) || !isset($protocol['name']) - || !isset($protocol['options']) - || !is_array($protocol['options']) - || !isset($protocol['options']['sharedSecret']) ) { return new JSONResponse( [ @@ -133,6 +138,20 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ ); } + $protocolName = $protocol['name']; + $hasOldFormat = isset($protocol['options']) && is_array($protocol['options']) && isset($protocol['options']['sharedSecret']); + $hasNewFormat = isset($protocol[$protocolName]) && is_array($protocol[$protocolName]) && isset($protocol[$protocolName]['sharedSecret']); + + if (!$hasOldFormat && !$hasNewFormat) { + return new JSONResponse( + [ + 'message' => 'Missing sharedSecret in protocol', + 'validationErrors' => [], + ], + Http::STATUS_BAD_REQUEST + ); + } + $supportedShareTypes = $this->config->getSupportedShareTypes($resourceType); if (!in_array($shareType, $supportedShareTypes)) { return new JSONResponse( @@ -142,6 +161,7 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ } $cloudId = $this->cloudIdManager->resolveCloudId($shareWith); + $shareWithCloudId = $shareWith; // preserve full cloud ID for factory capability discovery $shareWith = $cloudId->getUser(); if ($shareType === 'user') { @@ -186,14 +206,28 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ try { $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); - $share = $this->factory->getCloudFederationShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, '', $shareType, $resourceType); + // Pass the original cloud ID so the factory can discover capabilities without warning. + // Then reset shareWith to the local username that shareReceived() needs for user lookup. + $share = $this->factory->getCloudFederationShare($shareWithCloudId, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, '', $shareType, $resourceType); + $share->setShareWith($shareWith); $share->setProtocol($protocol); + if ($provider instanceof IValidationAwareCloudFederationProvider) { + $provider->validateShare($share); + } $provider->shareReceived($share); - } catch (ProviderDoesNotExistsException|ProviderCouldNotAddShareException $e) { + } catch (BadRequestException $e) { + return new JSONResponse($e->getReturnMessage(), Http::STATUS_BAD_REQUEST); + } catch (ProviderDoesNotExistsException $e) { return new JSONResponse( ['message' => $e->getMessage()], Http::STATUS_NOT_IMPLEMENTED ); + } catch (ProviderCouldNotAddShareException $e) { + $status = $e->getCode() ?: Http::STATUS_NOT_IMPLEMENTED; + return new JSONResponse( + ['message' => $e->getMessage()], + $status + ); } catch (\Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse( @@ -477,6 +511,12 @@ private function confirmNotificationIdentity( $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); if ($provider instanceof ISignedCloudFederationProvider || $provider instanceof \NCU\Federation\ISignedCloudFederationProvider) { $identity = $provider->getFederationIdFromSharedSecret($sharedSecret, $notification); + if ($identity === '') { + $tokenProvider = Server::get(PublicKeyTokenProvider::class); + $accessTokenDb = $tokenProvider->getToken($sharedSecret); + $mapping = Server::get(OcmTokenMapMapper::class)->getByAccessTokenId($accessTokenDb->getId()); + $identity = $provider->getFederationIdFromSharedSecret($mapping->getRefreshToken(), $notification); + } } else { $this->logger->debug('cloud federation provider {provider} does not implements ISignedCloudFederationProvider', ['provider' => $provider::class]); return; diff --git a/apps/cloud_federation_api/openapi.json b/apps/cloud_federation_api/openapi.json index 695fa5fb56008..b4656447709db 100644 --- a/apps/cloud_federation_api/openapi.json +++ b/apps/cloud_federation_api/openapi.json @@ -161,7 +161,7 @@ }, "protocol": { "type": "object", - "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]] or Multi format: ['name' => 'multi', 'webdav' => [...]]", + "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]]", "required": [ "name" ], diff --git a/openapi.json b/openapi.json index ba4ee767a4b16..4e0308001f228 100644 --- a/openapi.json +++ b/openapi.json @@ -18127,7 +18127,7 @@ }, "protocol": { "type": "object", - "description": "e,.g. ['name' => 'webdav', 'options' => ['username' => 'john', 'permissions' => 31]]", + "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]]", "required": [ "name", "options" From 2180005e7acf0da60592ea703fdcb638633a5f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:36 +0200 Subject: [PATCH 5/9] feat(ocm): advertise exchange-token capability and token endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- .../Federation/CalendarFederationProvider.php | 84 ++++++++------ .../CalendarFederationProviderTest.php | 4 +- .../Federation/CloudFederationFactory.php | 56 ++++++++- .../Federation/CloudFederationShare.php | 46 ++++++-- lib/private/OCM/Model/OCMProvider.php | 47 ++++++-- lib/private/OCM/OCMDiscoveryService.php | 7 +- lib/private/Server.php | 6 +- ...ValidationAwareCloudFederationProvider.php | 33 ++++++ .../OCM/ICapabilityAwareOCMProvider.php | 19 +++ lib/public/OCM/IOCMProvider.php | 24 +++- lib/public/OCM/OCMCapabilities.php | 108 ++++++++++++++++++ tests/lib/OCM/DiscoveryServiceTest.php | 4 +- 12 files changed, 376 insertions(+), 62 deletions(-) create mode 100644 lib/public/Federation/IValidationAwareCloudFederationProvider.php create mode 100644 lib/public/OCM/OCMCapabilities.php diff --git a/apps/dav/lib/CalDAV/Federation/CalendarFederationProvider.php b/apps/dav/lib/CalDAV/Federation/CalendarFederationProvider.php index 348852fb9310d..27fcc8039d7e1 100644 --- a/apps/dav/lib/CalDAV/Federation/CalendarFederationProvider.php +++ b/apps/dav/lib/CalDAV/Federation/CalendarFederationProvider.php @@ -22,10 +22,11 @@ use OCP\Federation\ICloudFederationProvider; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; +use OCP\Federation\IValidationAwareCloudFederationProvider; use OCP\Share\Exceptions\ShareNotFound; use Psr\Log\LoggerInterface; -class CalendarFederationProvider implements ICloudFederationProvider { +class CalendarFederationProvider implements ICloudFederationProvider, IValidationAwareCloudFederationProvider { public const PROVIDER_ID = 'calendar'; public const CALENDAR_RESOURCE = 'calendar'; public const USER_SHARE_TYPE = 'user'; @@ -45,7 +46,7 @@ public function getShareType(): string { } #[\Override] - public function shareReceived(ICloudFederationShare $share): string { + public function validateShare(ICloudFederationShare $share): void { if (!$this->calendarFederationConfig->isFederationEnabled()) { $this->logger->debug('Received a federation invite but federation is disabled'); throw new ProviderCouldNotAddShareException( @@ -65,38 +66,16 @@ public function shareReceived(ICloudFederationShare $share): string { // TODO: Implement group shares } - $rawProtocol = $share->getProtocol(); - if (!isset($rawProtocol[ICalendarFederationProtocol::PROP_VERSION])) { + $parsed = $this->parseShare($share); + if ($parsed === null) { throw new ProviderCouldNotAddShareException( - 'No protocol version', + 'Invalid or unsupported protocol payload', '', Http::STATUS_BAD_REQUEST, ); } - switch ($rawProtocol[ICalendarFederationProtocol::PROP_VERSION]) { - case CalendarFederationProtocolV1::VERSION: - try { - $protocol = CalendarFederationProtocolV1::parse($rawProtocol); - } catch (CalendarProtocolParseException $e) { - throw new ProviderCouldNotAddShareException( - 'Invalid protocol data (v1)', - '', - Http::STATUS_BAD_REQUEST, - ); - } - $calendarUrl = $protocol->getUrl(); - $displayName = $protocol->getDisplayName(); - $color = $protocol->getColor(); - $access = $protocol->getAccess(); - $components = $protocol->getComponents(); - break; - default: - throw new ProviderCouldNotAddShareException( - 'Unknown protocol version', - '', - Http::STATUS_BAD_REQUEST, - ); - } + + [, $calendarUrl, $displayName, , $access, ] = $parsed; if (!$calendarUrl || !$displayName) { throw new ProviderCouldNotAddShareException( @@ -106,15 +85,24 @@ public function shareReceived(ICloudFederationShare $share): string { ); } + if (!in_array($access, [DavSharingBackend::ACCESS_READ, DavSharingBackend::ACCESS_READ_WRITE], true)) { + throw new ProviderCouldNotAddShareException( + "Unsupported access value: $access", + '', + Http::STATUS_BAD_REQUEST, + ); + } + } + + #[\Override] + public function shareReceived(ICloudFederationShare $share): string { + $this->validateShare($share); + [, $calendarUrl, $displayName, $color, $access, $components] = $this->parseShare($share); + // convert access to permissions $permissions = match ($access) { DavSharingBackend::ACCESS_READ => Constants::PERMISSION_READ, DavSharingBackend::ACCESS_READ_WRITE => Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, - default => throw new ProviderCouldNotAddShareException( - "Unsupported access value: $access", - '', - Http::STATUS_BAD_REQUEST, - ), }; // The calendar uri is the local name of the calendar. As such it must not contain slashes. @@ -220,4 +208,32 @@ private function handleSyncCalendarNotification(array $notification): array { return []; } + + /** + * @return array{0: CalendarFederationProtocolV1, 1: string, 2: string, 3: ?string, 4: int, 5: string}|null + * [parsed protocol, calendarUrl, displayName, color, access, components], or null when + * the envelope cannot be parsed (missing/unsupported version, parse error). + */ + private function parseShare(ICloudFederationShare $share): ?array { + $rawProtocol = $share->getProtocol(); + if (!isset($rawProtocol[ICalendarFederationProtocol::PROP_VERSION])) { + return null; + } + if ($rawProtocol[ICalendarFederationProtocol::PROP_VERSION] !== CalendarFederationProtocolV1::VERSION) { + return null; + } + try { + $protocol = CalendarFederationProtocolV1::parse($rawProtocol); + } catch (CalendarProtocolParseException $e) { + return null; + } + return [ + $protocol, + $protocol->getUrl(), + $protocol->getDisplayName(), + $protocol->getColor(), + $protocol->getAccess(), + $protocol->getComponents(), + ]; + } } diff --git a/apps/dav/tests/unit/CalDAV/Federation/CalendarFederationProviderTest.php b/apps/dav/tests/unit/CalDAV/Federation/CalendarFederationProviderTest.php index 6e6577b24fa13..8237453a89eb6 100644 --- a/apps/dav/tests/unit/CalDAV/Federation/CalendarFederationProviderTest.php +++ b/apps/dav/tests/unit/CalDAV/Federation/CalendarFederationProviderTest.php @@ -210,7 +210,7 @@ public function testShareReceivedWithInvalidProtocolVersion(): void { ->method('add'); $this->expectException(ProviderCouldNotAddShareException::class); - $this->expectExceptionMessage('Unknown protocol version'); + $this->expectExceptionMessage('Invalid or unsupported protocol payload'); $this->expectExceptionCode(400); $this->assertEquals(10, $this->calendarFederationProvider->shareReceived($share)); } @@ -238,7 +238,7 @@ public function testShareReceivedWithoutProtocolVersion(): void { ->method('add'); $this->expectException(ProviderCouldNotAddShareException::class); - $this->expectExceptionMessage('No protocol version'); + $this->expectExceptionMessage('Invalid or unsupported protocol payload'); $this->expectExceptionCode(400); $this->assertEquals(10, $this->calendarFederationProvider->shareReceived($share)); } diff --git a/lib/private/Federation/CloudFederationFactory.php b/lib/private/Federation/CloudFederationFactory.php index ffe86305295b8..46e03a1a019f9 100644 --- a/lib/private/Federation/CloudFederationFactory.php +++ b/lib/private/Federation/CloudFederationFactory.php @@ -10,8 +10,18 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationNotification; use OCP\Federation\ICloudFederationShare; +use OCP\Federation\ICloudIdManager; +use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; +use Psr\Log\LoggerInterface; class CloudFederationFactory implements ICloudFederationFactory { + public function __construct( + private IOCMDiscoveryService $ocmDiscoveryService, + private ICloudIdManager $cloudIdManager, + private LoggerInterface $logger, + ) { + } /** * get a CloudFederationShare Object to prepare a share you want to send * @@ -32,7 +42,51 @@ class CloudFederationFactory implements ICloudFederationFactory { */ #[\Override] public function getCloudFederationShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, $sharedSecret, $shareType, $resourceType) { - return new CloudFederationShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, $shareType, $resourceType, $sharedSecret); + $useExchangeToken = false; + $remoteDomain = null; + + try { + $cloudId = $this->cloudIdManager->resolveCloudId($shareWith); + $remoteDomain = $cloudId->getRemote(); + + try { + $remoteProvider = $this->ocmDiscoveryService->discover($remoteDomain); + $capabilities = $remoteProvider->getCapabilities(); + $useExchangeToken = $capabilities->hasExchangeToken(); + + $this->logger->debug('OCM provider capabilities discovered', [ + 'remote' => $remoteDomain, + 'capabilities' => $capabilities->toArray(), + 'useExchangeToken' => $useExchangeToken, + ]); + } catch (OCMProviderException $e) { + $this->logger->warning('Failed to discover OCM provider, using legacy share method', [ + 'remote' => $remoteDomain, + 'exception' => $e->getMessage(), + ]); + } + } catch (\InvalidArgumentException $e) { + $this->logger->warning('Invalid cloud ID format, using legacy share method', [ + 'shareWith' => $shareWith, + 'exception' => $e->getMessage(), + ]); + } + + return new CloudFederationShare( + $shareWith, + $name, + $description, + $providerId, + $owner, + $ownerDisplayName, + $sharedBy, + $sharedByDisplayName, + $shareType, + $resourceType, + $sharedSecret, + $useExchangeToken, + $remoteDomain + ); } /** diff --git a/lib/private/Federation/CloudFederationShare.php b/lib/private/Federation/CloudFederationShare.php index bcc9e7a87d3f5..2da089f92596b 100644 --- a/lib/private/Federation/CloudFederationShare.php +++ b/lib/private/Federation/CloudFederationShare.php @@ -41,6 +41,8 @@ class CloudFederationShare implements ICloudFederationShare { * @param string $shareType ('group' or 'user' share) * @param string $resourceType ('file', 'calendar',...) * @param string $sharedSecret + * @param bool $useExchangeToken whether to use exchange-token protocol (new way) or sharedSecret (old way) + * @param string|null $remoteDomain remote domain for constructing webdav URI */ public function __construct($shareWith = '', $name = '', @@ -53,6 +55,8 @@ public function __construct($shareWith = '', $shareType = '', $resourceType = '', $sharedSecret = '', + $useExchangeToken = false, + $remoteDomain = null, ) { $this->setShareWith($shareWith); $this->setResourceName($name); @@ -62,13 +66,27 @@ public function __construct($shareWith = '', $this->setOwnerDisplayName($ownerDisplayName); $this->setSharedBy($sharedBy); $this->setSharedByDisplayName($sharedByDisplayName); - $this->setProtocol([ - 'name' => 'webdav', - 'options' => [ - 'sharedSecret' => $sharedSecret, - 'permissions' => '{http://open-cloud-mesh.org/ns}share-permissions' - ] - ]); + + if ($useExchangeToken) { + $webdavUri = $remoteDomain ? 'https://' . $remoteDomain . '/public.php/webdav/' : ''; + $this->setProtocol([ + 'name' => 'webdav', + 'webdav' => [ + 'uri' => $webdavUri, + 'sharedSecret' => $sharedSecret, + 'permissions' => ['{http://open-cloud-mesh.org/ns}share-permissions'] + ] + ]); + } else { + $this->setProtocol([ + 'name' => 'webdav', + 'options' => [ + 'sharedSecret' => $sharedSecret, + 'permissions' => '{http://open-cloud-mesh.org/ns}share-permissions' + ] + ]); + } + $this->setShareType($shareType); $this->setResourceType($resourceType); } @@ -352,7 +370,19 @@ public function getShareType() { */ #[\Override] public function getShareSecret() { - return $this->share['protocol']['options']['sharedSecret']; + $protocol = $this->share['protocol']; + if (isset($protocol['options']['sharedSecret'])) { + return $protocol['options']['sharedSecret']; + } + + if (isset($protocol['name'])) { + $protocolName = $protocol['name']; + if (isset($protocol[$protocolName]['sharedSecret'])) { + return $protocol[$protocolName]['sharedSecret']; + } + } + + return ''; } /** diff --git a/lib/private/OCM/Model/OCMProvider.php b/lib/private/OCM/Model/OCMProvider.php index 6c1591013a7a3..1a4c2bec2f8a0 100644 --- a/lib/private/OCM/Model/OCMProvider.php +++ b/lib/private/OCM/Model/OCMProvider.php @@ -13,6 +13,7 @@ use OCP\OCM\Exceptions\OCMProviderException; use OCP\OCM\IOCMProvider; use OCP\OCM\IOCMResource; +use OCP\OCM\OCMCapabilities; use OCP\Security\Signature\Model\Signatory; /** @@ -24,6 +25,7 @@ class OCMProvider implements IOCMProvider { private string $inviteAcceptDialog = ''; private array $capabilities = []; private string $endPoint = ''; + private string $tokenEndPoint = ''; /** @var IOCMResource[] */ private array $resourceTypes = []; private ?Signatory $signatory = null; @@ -119,6 +121,29 @@ public function getEndPoint(): string { return $this->endPoint; } + /** + * @param string $tokenEndPoint + * + * @return $this + */ + #[\Override] + public function setTokenEndPoint(string $endPoint): static { + $this->tokenEndPoint = $endPoint; + + return $this; + } + + /** + * @return string + */ + #[\Override] + public function getTokenEndPoint(): string { + if (in_array('exchange-token', $this->capabilities)) { + return $this->tokenEndPoint; + } + return ''; + } + /** * @return string */ @@ -141,12 +166,9 @@ public function setCapabilities(array $capabilities): static { return $this; } - /** - * @return array - */ #[\Override] - public function getCapabilities(): array { - return $this->capabilities; + public function getCapabilities(): OCMCapabilities { + return new OCMCapabilities($this->capabilities); } /** @@ -268,6 +290,12 @@ public function import(array $data): static { $this->setSignatory($signatory); } } + if (isset($data['capabilities'])) { + $this->setCapabilities($data['capabilities']); + } + if (isset($data['tokenEndPoint'])) { + $this->setTokenEndPoint($data['tokenEndPoint']); + } if (!$this->looksValid()) { throw new OCMProviderException('remote provider does not look valid'); @@ -303,9 +331,12 @@ public function jsonSerialize(): array { 'resourceTypes' => $resourceTypes ]; - $capabilities = $this->getCapabilities(); - if ($capabilities) { - $response['capabilities'] = $capabilities; + if ($this->capabilities !== []) { + $response['capabilities'] = $this->capabilities; + } + $tokenEndpoint = $this->getTokenEndPoint(); + if ($tokenEndpoint) { + $response['tokenEndPoint'] = $tokenEndpoint; } $inviteAcceptDialog = $this->getInviteAcceptDialog(); if ($inviteAcceptDialog !== '') { diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index fc9cba7369789..d25f0d4cdcf60 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -50,7 +50,7 @@ #[Consumable(since: '28.0.0')] final class OCMDiscoveryService implements IOCMDiscoveryService { private ICache $cache; - public const API_VERSION = '1.1.0'; + public const API_VERSION = '1.1.2'; private ?IOCMProvider $localProvider = null; /** @var array */ private array $remoteProviders = []; @@ -93,6 +93,7 @@ public function discover(string $remote, bool $skipCache = false): IOCMProvider } if (array_key_exists($remote, $this->remoteProviders)) { + return $this->remoteProviders[$remote]; } @@ -193,6 +194,7 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { } $url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare'); + $tokenUrl = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.Token.accessToken'); $pos = strrpos($url, '/'); if ($pos === false) { $this->logger->debug('generated route should contain a slash character'); @@ -204,7 +206,8 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { $provider->setEnabled(true); $provider->setApiVersion(self::API_VERSION); $provider->setEndPoint(substr($url, 0, $pos)); - $provider->setCapabilities(['invite-accepted', 'notifications', 'shares']); + $provider->setCapabilities(['invite-accepted', 'notifications', 'shares', 'exchange-token']); + $provider->setTokenEndPoint($tokenUrl); if ($signingEnabled) { $provider->setCapabilities(['http-sig']); } diff --git a/lib/private/Server.php b/lib/private/Server.php index de5d98f48eb51..b78f0a6a4263b 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1202,7 +1202,11 @@ public function __construct( $this->registerAlias(\OCP\GlobalScale\IConfig::class, \OC\GlobalScale\Config::class); $this->registerAlias(ICloudFederationProviderManager::class, CloudFederationProviderManager::class); $this->registerService(ICloudFederationFactory::class, function (Server $c) { - return new CloudFederationFactory(); + return new CloudFederationFactory( + $c->get(\OCP\OCM\IOCMDiscoveryService::class), + $c->get(\OCP\Federation\ICloudIdManager::class), + $c->get(\Psr\Log\LoggerInterface::class) + ); }); $this->registerAlias(IControllerMethodReflector::class, ControllerMethodReflector::class); diff --git a/lib/public/Federation/IValidationAwareCloudFederationProvider.php b/lib/public/Federation/IValidationAwareCloudFederationProvider.php new file mode 100644 index 0000000000000..286105a6d83a4 --- /dev/null +++ b/lib/public/Federation/IValidationAwareCloudFederationProvider.php @@ -0,0 +1,33 @@ + normalized (lowercased, no leading slash) capability strings */ + private readonly array $capabilities; + + /** + * @param list $capabilities + * @since 33.0.0 + */ + public function __construct(array $capabilities = []) { + $normalized = array_map( + static fn (string $c): string => strtolower(ltrim($c, '/')), + $capabilities, + ); + $this->capabilities = array_values(array_unique($normalized)); + } + + /** + * Whether a specific capability is advertised. Convenience for callers that + * deal with capabilities not covered by a named `has*()` method. + * + * @since 33.0.0 + */ + public function has(string $capability): bool { + return in_array(strtolower(ltrim($capability, '/')), $this->capabilities, true); + } + + /** + * Whether the peer accepts OCM share notifications. + * + * @since 33.0.0 + */ + public function hasNotifications(): bool { + return $this->has('notifications'); + } + + /** + * Whether the peer accepts incoming federated shares. + * + * @since 33.0.0 + */ + public function hasShares(): bool { + return $this->has('shares'); + } + + /** + * Whether the peer accepts OCM invitation flow `invite-accepted` callbacks. + * + * @since 33.0.0 + */ + public function hasInviteAccepted(): bool { + return $this->has('invite-accepted'); + } + + /** + * Whether the peer implements the OCM token-exchange flow + * (refresh token → short-lived access token). + * + * @since 33.0.0 + */ + public function hasExchangeToken(): bool { + return $this->has('exchange-token'); + } + + /** + * Raw list of advertised capability strings (normalized). + * + * @return list + * @since 33.0.0 + */ + public function toArray(): array { + return $this->capabilities; + } + + /** + * @return list + * @since 33.0.0 + */ + #[\Override] + public function jsonSerialize(): array { + return $this->capabilities; + } +} diff --git a/tests/lib/OCM/DiscoveryServiceTest.php b/tests/lib/OCM/DiscoveryServiceTest.php index 8ab77e28b97a9..d218b2dec93bb 100644 --- a/tests/lib/OCM/DiscoveryServiceTest.php +++ b/tests/lib/OCM/DiscoveryServiceTest.php @@ -125,7 +125,7 @@ public function testOCMRequest(string $path, int $expectedStatus, ?array $expect public function testLocalBaseCapability(): void { $local = $this->discoveryService->getLocalOCMProvider(); - $this->assertEmpty(array_diff(['notifications', 'shares'], $local->getCapabilities())); + $this->assertEmpty(array_diff(['notifications', 'shares'], $local->getCapabilities()->toArray())); } public function testLocalCapabilitiesAdvertiseHttpSigByDefault(): void { @@ -140,7 +140,7 @@ public function testLocalAddedCapability(): void { $this->context->for('ocm-capability-app')->registerEventListener(LocalOCMDiscoveryEvent::class, LocalOCMDiscoveryTestEvent::class); $this->context->delegateEventListenerRegistrations($this->dispatcher); $local = $this->discoveryService->getLocalOCMProvider(); - $this->assertEmpty(array_diff(['notifications', 'shares', 'ocm-capability-test'], $local->getCapabilities())); + $this->assertEmpty(array_diff(['notifications', 'shares', 'ocm-capability-test'], $local->getCapabilities()->toArray())); } } From e651f3a97755b8f4a46c1b2307baeb709d830df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:33:37 +0200 Subject: [PATCH 6/9] feat(federatedfilesharing): create refresh tokens and sign token exchange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../Controller/RequestHandlerController.php | 2 +- .../lib/FederatedShareProvider.php | 36 +- .../Version1012Date20260306120000.php | 119 +++++++ .../lib/OCM/CloudFederationProviderFiles.php | 133 +++++++- ...ederatedShareProviderReshareRemoteTest.php | 4 +- .../tests/FederatedShareProviderTest.php | 44 ++- .../OCM/CloudFederationProviderFilesTest.php | 312 ++++++++++++++++++ .../features/bootstrap/Sharing.php | 13 +- .../federation_features/federated.feature | 6 +- 11 files changed, 644 insertions(+), 27 deletions(-) create mode 100644 apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php create mode 100644 apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php diff --git a/apps/federatedfilesharing/composer/composer/autoload_classmap.php b/apps/federatedfilesharing/composer/composer/autoload_classmap.php index a5ec2ecd82288..27a3710e120e0 100644 --- a/apps/federatedfilesharing/composer/composer/autoload_classmap.php +++ b/apps/federatedfilesharing/composer/composer/autoload_classmap.php @@ -17,6 +17,7 @@ 'OCA\\FederatedFileSharing\\Listeners\\LoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/LoadAdditionalScriptsListener.php', 'OCA\\FederatedFileSharing\\Migration\\Version1010Date20200630191755' => $baseDir . '/../lib/Migration/Version1010Date20200630191755.php', 'OCA\\FederatedFileSharing\\Migration\\Version1011Date20201120125158' => $baseDir . '/../lib/Migration/Version1011Date20201120125158.php', + 'OCA\\FederatedFileSharing\\Migration\\Version1012Date20260306120000' => $baseDir . '/../lib/Migration/Version1012Date20260306120000.php', 'OCA\\FederatedFileSharing\\Notifications' => $baseDir . '/../lib/Notifications.php', 'OCA\\FederatedFileSharing\\Notifier' => $baseDir . '/../lib/Notifier.php', 'OCA\\FederatedFileSharing\\OCM\\CloudFederationProviderFiles' => $baseDir . '/../lib/OCM/CloudFederationProviderFiles.php', diff --git a/apps/federatedfilesharing/composer/composer/autoload_static.php b/apps/federatedfilesharing/composer/composer/autoload_static.php index c415c51b5929f..77ce59fe0054f 100644 --- a/apps/federatedfilesharing/composer/composer/autoload_static.php +++ b/apps/federatedfilesharing/composer/composer/autoload_static.php @@ -32,6 +32,7 @@ class ComposerStaticInitFederatedFileSharing 'OCA\\FederatedFileSharing\\Listeners\\LoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadAdditionalScriptsListener.php', 'OCA\\FederatedFileSharing\\Migration\\Version1010Date20200630191755' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630191755.php', 'OCA\\FederatedFileSharing\\Migration\\Version1011Date20201120125158' => __DIR__ . '/..' . '/../lib/Migration/Version1011Date20201120125158.php', + 'OCA\\FederatedFileSharing\\Migration\\Version1012Date20260306120000' => __DIR__ . '/..' . '/../lib/Migration/Version1012Date20260306120000.php', 'OCA\\FederatedFileSharing\\Notifications' => __DIR__ . '/..' . '/../lib/Notifications.php', 'OCA\\FederatedFileSharing\\Notifier' => __DIR__ . '/..' . '/../lib/Notifier.php', 'OCA\\FederatedFileSharing\\OCM\\CloudFederationProviderFiles' => __DIR__ . '/..' . '/../lib/OCM/CloudFederationProviderFiles.php', diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index 3ecbee61080f1..219aabaecb599 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -398,7 +398,7 @@ public function move(int $id, ?string $token = null, ?string $remote = null, ?st ->set('owner', $qb->createNamedParameter($cloudId->getUser())) ->set('remote_id', $qb->createNamedParameter($newRemoteId)) ->where($qb->expr()->eq('remote_id', $qb->createNamedParameter($id))) - ->andWhere($qb->expr()->eq('share_token', $qb->createNamedParameter($token))); + ->andWhere($qb->expr()->eq('refresh_token', $qb->createNamedParameter($token))); $affected = $query->executeStatement(); if ($affected > 0) { diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 0957b1c0ef214..63751fe26b06d 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -8,8 +8,12 @@ namespace OCA\FederatedFileSharing; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; use OC\Share20\Share; +use OCA\CloudFederationAPI\Db\OcmTokenMapMapper; +use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\Authentication\Token\IToken; use OCP\Constants; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Federation\ICloudFederationProviderManager; @@ -23,6 +27,8 @@ use OCP\IDBConnection; use OCP\IL10N; use OCP\IUserManager; +use OCP\Security\ISecureRandom; +use OCP\Server; use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IShare; @@ -137,7 +143,7 @@ public function create(IShare $share): IShare { $ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']); $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate); [$token, $remoteId] = $this->notifications->requestReShare( - $remoteShare['share_token'], + $remoteShare['refresh_token'], $remoteShare['remote_id'], $shareId, $remoteShare['remote'], @@ -170,7 +176,15 @@ public function create(IShare $share): IShare { * @throws \Exception */ protected function createFederatedShare(IShare $share): string { - $token = $this->tokenHandler->generateToken(); + + $provider = Server::get(PublicKeyTokenProvider::class); + $token = Server::get(ISecureRandom::class)->generate(32, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS); + $uid = $share->getSharedBy(); + $user = $this->userManager->get($uid); + $name = $user?->getDisplayName() ?? $uid; + $pass = $share->getPassword(); + + $dbToken = $provider->generateToken($token, $uid, $uid, $pass, $name, type: IToken::PERMANENT_TOKEN); $shareId = $this->addShareToDB( $share->getNodeId(), $share->getNodeType(), @@ -721,6 +735,24 @@ public function getShareByToken(string $token): IShare { $data = $cursor->fetchAssociative(); + if ($data === false) { + // Token not found as refresh token, try looking it up as access token + try { + $accessTokenDb = Server::get(PublicKeyTokenProvider::class)->getToken($token); + $mapping = Server::get(OcmTokenMapMapper::class)->getByAccessTokenId($accessTokenDb->getId()); + + $qb2 = $this->dbConnection->getQueryBuilder(); + $cursor = $qb2->select('*') + ->from('share') + ->where($qb2->expr()->in('share_type', $qb2->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($qb2->expr()->eq('token', $qb2->createNamedParameter($mapping->getRefreshToken()))) + ->executeQuery(); + + $data = $cursor->fetch(); + } catch (InvalidTokenException|\OCP\AppFramework\Db\DoesNotExistException) { + // Token is not a valid access token or has no mapping, share not found + } + } if ($data === false) { throw new ShareNotFound('Share not found', $this->l->t('Could not find share')); } diff --git a/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php new file mode 100644 index 0000000000000..d34f05427ae04 --- /dev/null +++ b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php @@ -0,0 +1,119 @@ +getQueryBuilder(); + $result = $qb->select('id', 'token', 'uid_initiator') + ->from('share') + ->where($qb->expr()->in( + 'share_type', + $qb->createNamedParameter( + [IShare::TYPE_REMOTE, IShare::TYPE_REMOTE_GROUP], + IQueryBuilder::PARAM_INT_ARRAY + ) + )) + ->executeQuery(); + + $registered = 0; + $skipped = 0; + + while ($row = $result->fetchAssociative()) { + $shareId = (int)$row['id']; + $token = (string)$row['token']; + $uid = (string)$row['uid_initiator']; + + if (strlen($token) < PublicKeyTokenProvider::TOKEN_MIN_LENGTH) { + // Old short token from TokenHandler — leave it as-is. + // Replacing it would invalidate the token stored on the receiving instance, + // breaking Basic-auth access to those shares. These shares keep working via + // Basic auth and are simply not eligible for the OCM token exchange flow. + $skipped++; + continue; + } + + // Long token — check if it's already in oc_authtoken. + try { + $tokenProvider->getToken($token); + $skipped++; + continue; + } catch (InvalidTokenException) { + // Not registered yet — fall through to create it. + } + + $user = $userManager->get($uid); + $name = $user?->getDisplayName() ?? $uid; + + try { + $tokenProvider->generateToken( + $token, + $uid, + $uid, + null, + $name, + IToken::PERMANENT_TOKEN, + ); + $registered++; + } catch (\Exception $e) { + $output->warning(sprintf( + 'Could not register auth token for share %d (uid=%s): %s', + $shareId, + $uid, + $e->getMessage() + )); + } + } + + $result->closeCursor(); + + $output->info(sprintf( + 'Federated share token migration: %d registered, %d skipped (already up-to-date or legacy short token).', + $registered, + $skipped + )); + } +} diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 419d9ebd7c631..f0b6843c6499a 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -9,6 +9,8 @@ use OC\AppFramework\Http; use OC\Files\Filesystem; +use OC\OCM\OCMSignatoryManager; +use OC\OCM\Rfc9421SignatoryManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Federation\TrustedServers; @@ -34,12 +36,16 @@ use OCP\Files\ISetupManager; use OCP\Files\NotFoundException; use OCP\HintException; +use OCP\Http\Client\IClientService; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; +use OCP\OCM\IOCMDiscoveryService; +use OCP\Security\Signature\ISignatureManager; use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; @@ -71,6 +77,11 @@ public function __construct( private readonly IProviderFactory $shareProviderFactory, private readonly ISetupManager $setupManager, private readonly ExternalShareMapper $externalShareMapper, + private readonly IOCMDiscoveryService $discoveryService, + private readonly IClientService $clientService, + private readonly ISignatureManager $signatureManager, + private readonly OCMSignatoryManager $signatoryManager, + private readonly IAppConfig $appConfig, ) { } @@ -107,6 +118,30 @@ public function shareReceived(ICloudFederationShare $share): string { $ownerFederatedId = $share->getOwner(); $shareType = $this->mapShareTypeToNextcloud($share->getShareType()); + // Check for must-exchange-token requirement + $requirements = $protocol['webdav']['requirements'] ?? $protocol['options']['requirements'] ?? []; + $mustExchangeToken = in_array('must-exchange-token', $requirements); + $accessToken = ''; + + if ($mustExchangeToken) { + // Exchange the sharedSecret for an access token (required) + $accessToken = $this->exchangeToken($remote, $token); + if ($accessToken === null) { + throw new ProviderCouldNotAddShareException('Failed to exchange token as required by must-exchange-token', '', Http::STATUS_BAD_REQUEST); + } + } else { + // Check if remote has exchange-token capability and try to exchange (optional) + try { + $ocmProvider = $this->discoveryService->discover(rtrim($remote, '/')); + if ($ocmProvider->getCapabilities()->hasExchangeToken()) { + $accessToken = $this->exchangeToken($remote, $token) ?? ''; + $this->logger->debug('Exchanged token for remote with exchange-token capability', ['remote' => $remote, 'success' => !empty($accessToken)]); + } + } catch (\Exception $e) { + $this->logger->debug('Could not discover remote capabilities for token exchange', ['remote' => $remote, 'exception' => $e]); + } + } + // if no explicit information about the person who created the share was sent // we assume that the share comes from the owner if ($sharedByFederatedId === null) { @@ -147,8 +182,8 @@ public function shareReceived(ICloudFederationShare $share): string { $externalShare->generateId(); $externalShare->setRemote($remote); $externalShare->setRemoteId($remoteId); - $externalShare->setShareToken($token); - $externalShare->setPassword(''); + $externalShare->setRefreshToken($token); // refresh token (sharedSecret) + $externalShare->setAccessToken($accessToken ?: null); $externalShare->setName($name); $externalShare->setOwner($owner); $externalShare->setShareType($shareType); @@ -685,4 +720,98 @@ public function getFederationIdFromSharedSecret( return $share->getShareOwner(); } } + + /** + * Exchange a sharedSecret (refresh token) for an access token via the remote server's token endpoint + * + * @param string $remote The remote server URL + * @param string $sharedSecret The shared secret to exchange + * @return string|null The access token, or null on failure + */ + private function exchangeToken(string $remote, #[SensitiveParameter] string $sharedSecret): ?string { + try { + $ocmProvider = $this->discoveryService->discover(rtrim($remote, '/')); + $tokenEndpoint = $ocmProvider->getTokenEndPoint(); + + if ($tokenEndpoint === '') { + $this->logger->warning('Remote server does not expose tokenEndPoint', ['remote' => $remote]); + return null; + } + + $client = $this->clientService->newClient(); + $clientId = parse_url($this->urlGenerator->getAbsoluteURL('/'), PHP_URL_HOST); + + $payload = [ + 'grant_type' => 'authorization_code', + 'client_id' => $clientId, + 'code' => $sharedSecret, + ]; + + $options = [ + 'body' => http_build_query($payload), + 'headers' => [ + 'Content-Type' => 'application/x-www-form-urlencoded', + ], + 'timeout' => 10, + 'connect_timeout' => 10, + ]; + + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + new Rfc9421SignatoryManager($this->signatoryManager), + $options, + 'post', + $tokenEndpoint + ); + $this->logger->debug('Token request signed successfully', ['remote' => $remote]); + } catch (\Exception $e) { + $this->logger->error('Failed to sign token request', [ + 'remote' => $remote, + 'exception' => $e, + 'endpoint' => $tokenEndpoint, + ]); + return null; + } + + $response = $client->post($tokenEndpoint, $options); + + $statusCode = $response->getStatusCode(); + if ($statusCode !== 200) { + $this->logger->warning('Token exchange returned unexpected HTTP status', [ + 'remote' => $remote, + 'status' => $statusCode, + ]); + return null; + } + + $data = json_decode($response->getBody(), true); + + if (!is_array($data)) { + $this->logger->warning('Token exchange response is not valid JSON', ['remote' => $remote]); + return null; + } + + $accessToken = $data['access_token'] ?? null; + $tokenType = $data['token_type'] ?? null; + + if (!is_string($accessToken) || $accessToken === '') { + $this->logger->warning('Token exchange response missing or invalid access_token', ['remote' => $remote]); + return null; + } + + if (!is_string($tokenType) || strtolower($tokenType) !== 'bearer') { + $this->logger->warning('Token exchange response has unexpected token_type', [ + 'remote' => $remote, + 'token_type' => $tokenType, + ]); + return null; + } + + $this->logger->debug('Successfully exchanged token for access token', ['remote' => $remote]); + return $accessToken; + } catch (\Exception $e) { + $this->logger->warning('Failed to exchange token', ['remote' => $remote, 'exception' => $e]); + return null; + } + } } diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderReshareRemoteTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderReshareRemoteTest.php index 2aa6f8790cde8..fff9fb23d4801 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderReshareRemoteTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderReshareRemoteTest.php @@ -133,7 +133,7 @@ public function testCreateRemoteOwner(): void { 'share_type' => 0, 'remote' => 'https://origin.test/', 'remote_id' => '10', - 'share_token' => 'share_token1', + 'refresh_token' => 'share_token1', 'password' => '', 'name' => '/Share1', 'owner' => 'jane', // owner in share_external is the user on the remote instance @@ -167,7 +167,7 @@ public function testCreateRemoteOwner(): void { 'share_type' => 0, 'remote' => 'https://origin.test/', 'remote_id' => '10', - 'share_token' => 'share_token2', + 'refresh_token' => 'share_token2', 'password' => '', 'name' => '/Share1', 'owner' => 'jane', // owner in share_external is the user on the remote instance diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index ddf16bb4e06ae..a4d0dd5695508 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -10,11 +10,13 @@ namespace OCA\FederatedFileSharing\Tests; use LogicException; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Federation\CloudIdManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\FederatedFileSharing\Notifications; use OCA\FederatedFileSharing\TokenHandler; +use OCP\Authentication\Token\IToken; use OCP\Constants; use OCP\Contacts\IManager as IContactsManager; use OCP\EventDispatcher\IEventDispatcher; @@ -28,6 +30,7 @@ use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserManager; +use OCP\Security\ISecureRandom; use OCP\Server; use OCP\Share\IManager; use OCP\Share\IShare; @@ -88,6 +91,23 @@ protected function setUp(): void { $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); + // Mock ISecureRandom to return predictable tokens (must be 32+ chars) + $secureRandom = $this->createMock(ISecureRandom::class); + $tokenCounter = 0; + $secureRandom->method('generate') + ->willReturnCallback(function () use (&$tokenCounter) { + $tokenCounter++; + return 'token' . $tokenCounter . 'token' . $tokenCounter . 'token' . $tokenCounter . 'token' . $tokenCounter . 'token' . $tokenCounter . 'ab'; + }); + $this->overwriteService(ISecureRandom::class, $secureRandom); + + // Mock PublicKeyTokenProvider to avoid database token creation + $tokenProvider = $this->createMock(PublicKeyTokenProvider::class); + $mockToken = $this->createMock(IToken::class); + $tokenProvider->method('generateToken') + ->willReturn($mockToken); + $this->overwriteService(PublicKeyTokenProvider::class, $tokenProvider); + $this->provider = new FederatedShareProvider( $this->connection, $this->addressHandler, @@ -147,7 +167,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -185,7 +205,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate 'file_source' => 42, 'permissions' => 19, 'accepted' => 0, - 'token' => 'token', + 'token' => 'token1token1token1token1token1ab', 'expiration' => $expectedDataDate, ]; foreach (array_keys($expected) as $key) { @@ -200,7 +220,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate $this->assertEquals('file', $share->getNodeType()); $this->assertEquals(42, $share->getNodeId()); $this->assertEquals(19, $share->getPermissions()); - $this->assertEquals('token', $share->getToken()); + $this->assertEquals('token1token1token1token1token1ab', $share->getToken()); $this->assertEquals($expirationDate, $share->getExpirationDate()); } @@ -230,7 +250,7 @@ public function testCreateCouldNotFindServer(): void { $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->matchesRegularExpression('/^[A-Za-z0-9]{32}$/'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -284,7 +304,7 @@ public function testCreateException(): void { $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->matchesRegularExpression('/^[A-Za-z0-9]{32}$/'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -373,7 +393,7 @@ public function testCreateAlreadyShared(): void { $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -445,7 +465,7 @@ public function testUpdate(string $owner, string $sharedBy, ?\DateTime $expirati $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -883,9 +903,9 @@ public function testGetAccessList(): void { $folder1 = $rootFolder->getUserFolder($u1->getUID())->newFolder('foo'); $file1 = $folder1->newFile('bar1'); - $this->tokenHandler->expects($this->exactly(2)) - ->method('generateToken') - ->willReturnOnConsecutiveCalls('token1', 'token2'); + // Token generation now uses ISecureRandom instead of tokenHandler + $this->tokenHandler->expects($this->never()) + ->method('generateToken'); $this->notifications->expects($this->atLeastOnce()) ->method('sendRemoteShare') ->willReturn(true); @@ -926,11 +946,11 @@ public function testGetAccessList(): void { $result = $this->provider->getAccessList([$file1], true); $this->assertEquals(['remote' => [ 'user@server.com' => [ - 'token' => 'token1', + 'token' => 'token1token1token1token1token1ab', 'node_id' => $file1->getId(), ], 'foobar@localhost' => [ - 'token' => 'token2', + 'token' => 'token2token2token2token2token2ab', 'node_id' => $file1->getId(), ], ]], $result); diff --git a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php new file mode 100644 index 0000000000000..15cd8078eadf7 --- /dev/null +++ b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php @@ -0,0 +1,312 @@ +appManager = $this->createMock(IAppManager::class); + $this->federatedShareProvider = $this->createMock(FederatedShareProvider::class); + $this->addressHandler = $this->createMock(AddressHandler::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->shareManager = $this->createMock(IManager::class); + $this->cloudIdManager = $this->createMock(ICloudIdManager::class); + $this->activityManager = $this->createMock(IActivityManager::class); + $this->notificationManager = $this->createMock(INotificationManager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->cloudFederationFactory = $this->createMock(ICloudFederationFactory::class); + $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->config = $this->createMock(IConfig::class); + $this->externalShareManager = $this->createMock(Manager::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->filenameValidator = $this->createMock(IFilenameValidator::class); + $this->shareProviderFactory = $this->createMock(IProviderFactory::class); + $this->setupManager = $this->createMock(ISetupManager::class); + $this->externalShareMapper = $this->createMock(ExternalShareMapper::class); + $this->discoveryService = $this->createMock(IOCMDiscoveryService::class); + $this->clientService = $this->createMock(IClientService::class); + $this->signatureManager = $this->createMock(ISignatureManager::class); + $this->signatoryManager = $this->createMock(OCMSignatoryManager::class); + $this->appConfig = $this->createMock(IAppConfig::class); + + $this->provider = new CloudFederationProviderFiles( + $this->appManager, + $this->federatedShareProvider, + $this->addressHandler, + $this->userManager, + $this->shareManager, + $this->cloudIdManager, + $this->activityManager, + $this->notificationManager, + $this->urlGenerator, + $this->cloudFederationFactory, + $this->cloudFederationProviderManager, + $this->groupManager, + $this->config, + $this->externalShareManager, + $this->logger, + $this->filenameValidator, + $this->shareProviderFactory, + $this->setupManager, + $this->externalShareMapper, + $this->discoveryService, + $this->clientService, + $this->signatureManager, + $this->signatoryManager, + $this->appConfig, + ); + } + + private function enableS2S(): void { + $this->appManager->method('isEnabledForUser') + ->with('files_sharing') + ->willReturn(true); + $this->federatedShareProvider->method('isIncomingServer2serverShareEnabled') + ->willReturn(true); + } + + private function buildShare(array $requirements = []): ICloudFederationShare&MockObject { + $share = $this->createMock(ICloudFederationShare::class); + $share->method('getProtocol')->willReturn([ + 'name' => 'webdav', + 'webdav' => ['requirements' => $requirements], + ]); + $share->method('getOwner')->willReturn('owner@example.com'); + $share->method('getOwnerDisplayName')->willReturn('Owner Name'); + $share->method('getShareSecret')->willReturn('refresh-token-abc'); + $share->method('getResourceName')->willReturn('/SharedFolder'); + $share->method('getShareWith')->willReturn('localuser'); + $share->method('getProviderId')->willReturn('42'); + $share->method('getSharedBy')->willReturn('owner@example.com'); + $share->method('getShareType')->willReturn('user'); + return $share; + } + + /** + * When must-exchange-token is required but the remote has no token endpoint, + * shareReceived must throw rather than silently accept the share. + */ + public function testShareReceivedMustExchangeTokenThrowsWhenExchangeFails(): void { + $this->enableS2S(); + + $this->addressHandler->method('splitUserRemote') + ->with('owner@example.com') + ->willReturn(['owner', 'https://example.com/']); + + $share = $this->buildShare(['must-exchange-token']); + + $ocmProvider = $this->createMock(IOCMProvider::class); + $ocmProvider->method('getTokenEndPoint')->willReturn(''); + + $this->discoveryService->method('discover') + ->willReturn($ocmProvider); + + $this->expectException(ProviderCouldNotAddShareException::class); + + $this->provider->shareReceived($share); + } + + /** + * When must-exchange-token is required and the token exchange succeeds, + * the access token is stored on the share (we drive through share creation + * up to the "user does not exist" guard to avoid a full integration setup). + */ + public function testShareReceivedMustExchangeTokenStoresAccessToken(): void { + $this->enableS2S(); + + $this->addressHandler->method('splitUserRemote') + ->with('owner@example.com') + ->willReturn(['owner', 'https://example.com/']); + + $share = $this->buildShare(['must-exchange-token']); + + $tokenEndpoint = 'https://example.com/index.php/ocm/token'; + + $ocmProvider = $this->createMock(IOCMProvider::class); + $ocmProvider->method('getTokenEndPoint')->willReturn($tokenEndpoint); + $ocmProvider->method('getCapabilities')->willReturn(new OCMCapabilities([])); + + $this->discoveryService->method('discover')->willReturn($ocmProvider); + + $this->urlGenerator->method('getAbsoluteURL')->willReturn('https://local.example/'); + + $signedOptions = [ + 'body' => 'grant_type=authorization_code&client_id=local.example&code=refresh-token-abc', + 'headers' => ['Content-Type' => 'application/x-www-form-urlencoded', 'Signature' => 'sig'], + 'timeout' => 10, + 'connect_timeout' => 10, + ]; + $this->signatureManager->method('signOutgoingRequestIClientPayload') + ->willReturn($signedOptions); + + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode')->willReturn(200); + $response->method('getBody')->willReturn(json_encode([ + 'access_token' => 'access-token-xyz', + 'token_type' => 'Bearer', + ])); + + $httpClient = $this->createMock(\OCP\Http\Client\IClient::class); + $httpClient->method('post')->willReturn($response); + $this->clientService->method('newClient')->willReturn($httpClient); + + // Exchange succeeds → share creation continues; we stop it at the user + // lookup stage to avoid a full integration setup. + $this->userManager->method('get')->with('localuser')->willReturn(null); + $this->filenameValidator->method('isFilenameValid')->willReturn(true); + + $this->expectException(ProviderCouldNotAddShareException::class); + $this->expectExceptionMessage('User does not exists'); + + $this->provider->shareReceived($share); + } + + /** + * When exchange-token capability is present but the discovery service throws, + * shareReceived must not propagate the exception — the token exchange is optional. + */ + public function testShareReceivedOptionalExchangeGracefulOnDiscoveryFailure(): void { + $this->enableS2S(); + + $this->addressHandler->method('splitUserRemote') + ->with('owner@example.com') + ->willReturn(['owner', 'https://example.com/']); + + // Build a share with no must-exchange-token requirement + $share = $this->buildShare(); + + $this->discoveryService->method('discover') + ->willThrowException(new \Exception('network error')); + + // Discovery failure is caught and logged; share creation continues. + // We stop it at the user lookup stage. + $this->userManager->method('get')->with('localuser')->willReturn(null); + $this->filenameValidator->method('isFilenameValid')->willReturn(true); + + $this->expectException(ProviderCouldNotAddShareException::class); + $this->expectExceptionMessage('User does not exists'); + + $this->provider->shareReceived($share); + } + + /** + * When exchange-token capability is present and the exchange succeeds, + * the access token is set (we stop at user-not-found to avoid full setup). + */ + public function testShareReceivedOptionalExchangeStoresAccessTokenOnSuccess(): void { + $this->enableS2S(); + + $this->addressHandler->method('splitUserRemote') + ->with('owner@example.com') + ->willReturn(['owner', 'https://example.com/']); + + $share = $this->buildShare(); + + $tokenEndpoint = 'https://example.com/index.php/ocm/token'; + + $ocmProvider = $this->createMock(IOCMProvider::class); + $ocmProvider->method('getTokenEndPoint')->willReturn($tokenEndpoint); + $ocmProvider->method('getCapabilities')->willReturn(new OCMCapabilities(['exchange-token'])); + + $this->discoveryService->method('discover')->willReturn($ocmProvider); + + $this->urlGenerator->method('getAbsoluteURL')->willReturn('https://local.example/'); + + $signedOptions = [ + 'body' => 'grant_type=authorization_code&client_id=local.example&code=refresh-token-abc', + 'headers' => ['Content-Type' => 'application/x-www-form-urlencoded', 'Signature' => 'sig'], + 'timeout' => 10, + 'connect_timeout' => 10, + ]; + $this->signatureManager->method('signOutgoingRequestIClientPayload') + ->willReturn($signedOptions); + + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode')->willReturn(200); + $response->method('getBody')->willReturn(json_encode([ + 'access_token' => 'access-token-xyz', + 'token_type' => 'Bearer', + ])); + + $httpClient = $this->createMock(\OCP\Http\Client\IClient::class); + $httpClient->method('post')->willReturn($response); + $this->clientService->method('newClient')->willReturn($httpClient); + + $this->userManager->method('get')->with('localuser')->willReturn(null); + $this->filenameValidator->method('isFilenameValid')->willReturn(true); + + $this->expectException(ProviderCouldNotAddShareException::class); + $this->expectExceptionMessage('User does not exists'); + + $this->provider->shareReceived($share); + } +} diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index c0a41208d5a75..6420b2218fce3 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -333,7 +333,8 @@ public function isFieldInResponse($field, $contentExpected) { if (count($data->element) > 0) { foreach ($data as $element) { if ($contentExpected == 'A_TOKEN') { - return (strlen((string)$element->$field) == 15); + $tokenLength = strlen((string)$element->$field); + return $tokenLength == 15 || $tokenLength == 32; } elseif ($contentExpected == 'A_NUMBER') { return is_numeric((string)$element->$field); } elseif ($contentExpected == 'AN_URL') { @@ -348,7 +349,8 @@ public function isFieldInResponse($field, $contentExpected) { return false; } else { if ($contentExpected == 'A_TOKEN') { - return (strlen((string)$data->$field) == 15); + $tokenLength = strlen((string)$data->$field); + return $tokenLength == 15 || $tokenLength == 32; } elseif ($contentExpected == 'A_NUMBER') { return is_numeric((string)$data->$field); } elseif ($contentExpected == 'AN_URL') { @@ -640,9 +642,10 @@ private function assertFieldIsInReturnedShare(string $field, string $contentExpe if ($contentExpected === 'A_NUMBER') { Assert::assertTrue(is_numeric((string)$returnedShare->$field), "Field '$field' is not a number: " . $returnedShare->$field); } elseif ($contentExpected === 'A_TOKEN') { - // A token is composed by 15 characters from - // ISecureRandom::CHAR_HUMAN_READABLE. - Assert::assertMatchesRegularExpression('/^[abcdefgijkmnopqrstwxyzABCDEFGHJKLMNPQRSTWXYZ23456789]{15}$/', (string)$returnedShare->$field, "Field '$field' is not a token"); + // A token is either: + // - 15 characters from ISecureRandom::CHAR_HUMAN_READABLE (legacy), or + // - 32 characters from ISecureRandom::CHAR_ALPHANUMERIC (new OCM tokens) + Assert::assertMatchesRegularExpression('/^([a-zA-Z0-9]{15}|[a-zA-Z0-9]{32})$/', (string)$returnedShare->$field, "Field '$field' is not a token"); } elseif (strpos($contentExpected, 'REGEXP ') === 0) { Assert::assertMatchesRegularExpression(substr($contentExpected, strlen('REGEXP ')), (string)$returnedShare->$field, "Field '$field' does not match"); } else { diff --git a/build/integration/federation_features/federated.feature b/build/integration/federation_features/federated.feature index d3a414cb8046c..e697922187482 100644 --- a/build/integration/federation_features/federated.feature +++ b/build/integration/federation_features/federated.feature @@ -111,7 +111,7 @@ Feature: federated | id | A_NUMBER | | remote | LOCAL | | remote_id | A_NUMBER | - | share_token | A_TOKEN | + | refresh_token | A_TOKEN | | name | /textfile0.txt | | owner | user0 | | user | user1 | @@ -140,7 +140,7 @@ Feature: federated | id | A_NUMBER | | remote | LOCAL | | remote_id | A_NUMBER | - | share_token | A_TOKEN | + | refresh_token | A_TOKEN | | name | /textfile0.txt | | owner | gs-user0 | | user | group1 | @@ -154,7 +154,7 @@ Feature: federated | id | A_NUMBER | | remote | LOCAL | | remote_id | A_NUMBER | - | share_token | A_TOKEN | + | refresh_token | A_TOKEN | | name | /textfile0.txt | | owner | gs-user0 | | user | group1 | From b0a9129a16fddfd213293baed3da84b846493ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:34:01 +0200 Subject: [PATCH 7/9] feat(files_sharing): store and refresh OCM access tokens for external shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- .../composer/composer/autoload_classmap.php | 2 + .../composer/composer/autoload_static.php | 2 + .../lib/Command/CleanupRemoteStorages.php | 4 +- .../lib/External/ExternalShare.php | 20 +- .../lib/External/ExternalShareMapper.php | 4 +- apps/files_sharing/lib/External/Manager.php | 33 +- .../lib/External/MountProvider.php | 8 +- apps/files_sharing/lib/External/Storage.php | 121 +++++- .../Version33000Date20260306120000.php | 89 +++++ .../Version33000Date20260306150000.php | 39 ++ .../files_sharing/lib/ResponseDefinitions.php | 2 +- apps/files_sharing/openapi.json | 4 +- .../Command/CleanupRemoteStoragesTest.php | 20 +- .../tests/External/ManagerTest.php | 18 +- .../External/ManagerUpdateAccessTokenTest.php | 114 ++++++ lib/private/Files/Storage/DAV.php | 357 ++++++++++++++++-- lib/private/Share20/Manager.php | 15 + .../BearerAuthAwareSabreClientTest.php | 74 ++++ 18 files changed, 855 insertions(+), 71 deletions(-) create mode 100644 apps/files_sharing/lib/Migration/Version33000Date20260306120000.php create mode 100644 apps/files_sharing/lib/Migration/Version33000Date20260306150000.php create mode 100644 apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php create mode 100644 tests/lib/Files/Storage/BearerAuthAwareSabreClientTest.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 8140fc52f1628..21d92a0fce6a7 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -89,6 +89,8 @@ 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => $baseDir . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\Migration\\Version32000Date20251017081948' => $baseDir . '/../lib/Migration/Version32000Date20251017081948.php', 'OCA\\Files_Sharing\\Migration\\Version33000Date20251030081948' => $baseDir . '/../lib/Migration/Version33000Date20251030081948.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20260306120000' => $baseDir . '/../lib/Migration/Version33000Date20260306120000.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20260306150000' => $baseDir . '/../lib/Migration/Version33000Date20260306150000.php', 'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index fd2189f481888..b259b057d147d 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -104,6 +104,8 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => __DIR__ . '/..' . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\Migration\\Version32000Date20251017081948' => __DIR__ . '/..' . '/../lib/Migration/Version32000Date20251017081948.php', 'OCA\\Files_Sharing\\Migration\\Version33000Date20251030081948' => __DIR__ . '/..' . '/../lib/Migration/Version33000Date20251030081948.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20260306120000' => __DIR__ . '/..' . '/../lib/Migration/Version33000Date20260306120000.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20260306150000' => __DIR__ . '/..' . '/../lib/Migration/Version33000Date20260306150000.php', 'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', diff --git a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php index 32946a382ebd4..ac4a863a6557c 100644 --- a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php +++ b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php @@ -161,7 +161,7 @@ private function getRemoteStorages(): array { */ private function getRemoteShareIds(): array { $queryBuilder = $this->connection->getQueryBuilder(); - $queryBuilder->select(['id', 'share_token', 'owner', 'remote']) + $queryBuilder->select(['id', 'refresh_token', 'owner', 'remote']) ->from('share_external'); $result = $queryBuilder->executeQuery(); @@ -170,7 +170,7 @@ private function getRemoteShareIds(): array { while ($row = $result->fetchAssociative()) { $cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']); $remote = $cloudId->getRemote(); - $remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote); + $remoteShareIds[$row['id']] = 'shared::' . md5($row['refresh_token'] . '@' . $remote); } $result->closeCursor(); diff --git a/apps/files_sharing/lib/External/ExternalShare.php b/apps/files_sharing/lib/External/ExternalShare.php index b452b8ce013a3..db9112d6a4358 100644 --- a/apps/files_sharing/lib/External/ExternalShare.php +++ b/apps/files_sharing/lib/External/ExternalShare.php @@ -26,10 +26,14 @@ * @method void setRemote(string $remote) * @method string getRemoteId() * @method void setRemoteId(string $remoteId) - * @method string getShareToken() - * @method void setShareToken(string $shareToken) + * @method string getRefreshToken() + * @method void setRefreshToken(string $refreshToken) * @method string|null getPassword() * @method void setPassword(?string $password) + * @method string|null getAccessToken() + * @method void setAccessToken(?string $accessToken) + * @method int|null getAccessTokenExpires() + * @method void setAccessTokenExpires(?int $accessTokenExpires) * @method string getName() * @method string getOwner() * @method void setOwner(string $owner) @@ -48,8 +52,10 @@ class ExternalShare extends SnowflakeAwareEntity implements \JsonSerializable { protected ?int $shareType = null; protected ?string $remote = null; protected ?string $remoteId = null; - protected ?string $shareToken = null; + protected ?string $refreshToken = null; protected ?string $password = null; + protected ?string $accessToken = null; + protected ?int $accessTokenExpires = null; protected ?string $name = null; protected ?string $owner = null; protected ?string $user = null; @@ -63,8 +69,10 @@ public function __construct() { $this->addType('shareType', Types::INTEGER); $this->addType('remote', Types::STRING); $this->addType('remoteId', Types::STRING); - $this->addType('shareToken', Types::STRING); + $this->addType('refreshToken', Types::STRING); $this->addType('password', Types::STRING); + $this->addType('accessToken', Types::STRING); + $this->addType('accessTokenExpires', Types::INTEGER); $this->addType('name', Types::STRING); $this->addType('owner', Types::STRING); $this->addType('user', Types::STRING); @@ -99,7 +107,7 @@ public function jsonSerialize(): array { 'share_type' => $this->getShareType() ?? IShare::TYPE_USER, // unfortunately nullable on the DB level, but never null. 'remote' => $this->getRemote(), 'remote_id' => $this->getRemoteId(), - 'share_token' => $this->getShareToken(), + 'refresh_token' => $this->getRefreshToken(), 'name' => $this->getName(), 'owner' => $this->getOwner(), 'user' => $this->getUser(), @@ -126,7 +134,7 @@ public function clone(): self { $newShare->setShareType($this->getShareType()); $newShare->setRemote($this->getRemote()); $newShare->setRemoteId($this->getRemoteId()); - $newShare->setShareToken($this->getShareToken()); + $newShare->setRefreshToken($this->getRefreshToken()); $newShare->setPassword($this->getPassword()); $newShare->setName($this->getName()); $newShare->setOwner($this->getOwner()); diff --git a/apps/files_sharing/lib/External/ExternalShareMapper.php b/apps/files_sharing/lib/External/ExternalShareMapper.php index 994c839decc81..9db5b2d95b4b5 100644 --- a/apps/files_sharing/lib/External/ExternalShareMapper.php +++ b/apps/files_sharing/lib/External/ExternalShareMapper.php @@ -55,7 +55,7 @@ public function getShareByToken(string $token): ExternalShare { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from(self::TABLE_NAME) - ->where($qb->expr()->eq('share_token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))) + ->where($qb->expr()->eq('refresh_token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))) ->setMaxResults(1); return $this->findEntity($qb); } @@ -238,7 +238,7 @@ public function getShareByRemoteIdAndToken(string $id, mixed $token): ?ExternalS ->where( $qb->expr()->andX( $qb->expr()->eq('remote_id', $qb->createNamedParameter($id)), - $qb->expr()->eq('share_token', $qb->createNamedParameter($token)) + $qb->expr()->eq('refresh_token', $qb->createNamedParameter($token)) ) ); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index f40e5d4a34927..f139c08218413 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -111,8 +111,10 @@ public function addShare(ExternalShare $externalShare, IUser|IGroup|null $shareW $options = [ 'remote' => $externalShare->getRemote(), - 'token' => $externalShare->getShareToken(), + 'token' => $externalShare->getRefreshToken(), 'password' => $externalShare->getPassword(), + 'access_token' => $externalShare->getAccessToken(), + 'access_token_expires' => $externalShare->getAccessTokenExpires(), 'mountpoint' => $externalShare->getMountpoint(), 'owner' => $externalShare->getOwner(), 'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates'), @@ -190,6 +192,7 @@ private function updateSubShare(ExternalShare $externalShare, IUser $user, ?stri $subShare->generateId(); $subShare->setRemote($externalShare->getRemote()); $subShare->setPassword($externalShare->getPassword()); + $subShare->setAccessToken($externalShare->getAccessToken()); $subShare->setName($externalShare->getName()); $subShare->setOwner($externalShare->getOwner()); $subShare->setUser($user->getUID()); @@ -198,7 +201,7 @@ private function updateSubShare(ExternalShare $externalShare, IUser $user, ?stri $subShare->setRemoteId($externalShare->getRemoteId()); $subShare->setParent((string)$externalShare->getId()); $subShare->setShareType($externalShare->getShareType()); - $subShare->setShareToken($externalShare->getShareToken()); + $subShare->setRefreshToken($externalShare->getRefreshToken()); $this->externalShareMapper->insert($subShare); } } @@ -337,7 +340,7 @@ private function sendFeedbackToRemote(ExternalShare $externalShare, string $feed $endpoint = $federationEndpoints['share'] ?? '/ocs/v2.php/cloud/shares'; $url = rtrim($externalShare->getRemote(), '/') . $endpoint . '/' . $externalShare->getRemoteId() . '/' . $feedback . '?format=json'; - $fields = ['token' => $externalShare->getShareToken()]; + $fields = ['token' => $externalShare->getRefreshToken()]; $client = $this->clientService->newClient(); @@ -373,7 +376,7 @@ protected function tryOCMEndPoint(ExternalShare $externalShare, string $feedback 'file', $externalShare->getRemoteId(), [ - 'sharedSecret' => $externalShare->getShareToken(), + 'sharedSecret' => $externalShare->getRefreshToken(), 'message' => 'Recipient accept the share' ] ); @@ -385,7 +388,7 @@ protected function tryOCMEndPoint(ExternalShare $externalShare, string $feedback 'file', $externalShare->getRemoteId(), [ - 'sharedSecret' => $externalShare->getShareToken(), + 'sharedSecret' => $externalShare->getRefreshToken(), 'message' => 'Recipient declined the share' ] ); @@ -565,4 +568,24 @@ public function getAcceptedShares(): array { return []; } } + + /** + * Update the access token for a share. + * + * @param string $refreshToken The refresh token to identify the share + * @param string $accessToken The new access token to store + */ + public function updateAccessToken(string $refreshToken, string $accessToken, int $expiresAt): void { + try { + $share = $this->externalShareMapper->getShareByToken($refreshToken); + $share->setAccessToken($accessToken); + $share->setAccessTokenExpires($expiresAt); + $this->externalShareMapper->update($share); + $this->logger->debug('Updated access token for share', ['shareId' => $share->getId()]); + } catch (DoesNotExistException $e) { + $this->logger->warning('Could not find share to update access token', ['exception' => $e]); + } catch (Exception $e) { + $this->logger->error('Failed to update access token', ['exception' => $e]); + } + } } diff --git a/apps/files_sharing/lib/External/MountProvider.php b/apps/files_sharing/lib/External/MountProvider.php index e0cd7f7ec8aa1..d26ea83d55ee8 100644 --- a/apps/files_sharing/lib/External/MountProvider.php +++ b/apps/files_sharing/lib/External/MountProvider.php @@ -61,7 +61,7 @@ private function getMount(IUser $user, array $data, IStorageFactory $storageFact #[\Override] public function getMountsForUser(IUser $user, IStorageFactory $loader): array { $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'remote', 'share_token', 'password', 'mountpoint', 'owner') + $qb->select('id', 'remote', 'refresh_token', 'password', 'access_token', 'access_token_expires', 'mountpoint', 'owner') ->from('share_external') ->where($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID()))) ->andWhere($qb->expr()->eq('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED, IQueryBuilder::PARAM_INT))); @@ -69,7 +69,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader): array { $mounts = []; while ($row = $result->fetchAssociative()) { $row['manager'] = $this; - $row['token'] = $row['share_token']; + $row['token'] = $row['refresh_token']; $mounts[] = $this->getMount($user, $row, $loader); } $result->closeCursor(); @@ -102,7 +102,7 @@ public function getMountsForPath( } $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'remote', 'share_token', 'password', 'mountpoint', 'owner') + $qb->select('id', 'remote', 'refresh_token', 'password', 'access_token', 'access_token_expires', 'mountpoint', 'owner') ->from('share_external') ->where($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID()))) ->andWhere($qb->expr()->eq('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED, IQueryBuilder::PARAM_INT))); @@ -118,7 +118,7 @@ public function getMountsForPath( $mounts = []; while ($row = $result->fetchAssociative()) { $row['manager'] = $this; - $row['token'] = $row['share_token']; + $row['token'] = $row['refresh_token']; $mount = $this->getMount($user, $row, $loader); $mounts[$mount->getMountPoint()] = $mount; } diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 929d9cbf976ea..d52e9945f0b0a 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -51,11 +51,21 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private bool $updateChecked = false; private ExternalShareManager $manager; private IConfig $config; - private IAppConfig $appConfig; + protected IAppConfig $appConfig; private IShareManager $shareManager; + private bool $tokenRefreshed = false; + /** Unix timestamp until which the current access token is considered valid (0 = unknown/expired) */ + private int $tokenExpiresAt = 0; + /** Number of consecutive token exchange failures (resets on success or DB-reuse) */ + private int $refreshFailureCount = 0; + /** Unix timestamp before which the next exchange attempt must not be made (0 = no wait) */ + private int $refreshBackoffUntil = 0; + + private const REFRESH_MAX_ATTEMPTS = 3; + private const REFRESH_BACKOFF_SECONDS = 5; /** - * @param array{HttpClientService: IClientService, manager: ExternalShareManager, cloudId: ICloudId, mountpoint: string, token: string, password: ?string}|array $options + * @param array{HttpClientService: IClientService, manager: ExternalShareManager, cloudId: ICloudId, mountpoint: string, token: string, access_token: ?string, access_token_expires: ?int}|array $options */ public function __construct($options) { $this->memcacheFactory = Server::get(ICacheFactory::class); @@ -73,14 +83,30 @@ public function __construct($options) { $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; + } + + // Only use Bearer auth when an access token is already stored. + // Shares created before the exchange-token capability was introduced have no + // stored token and must keep using basic auth for backwards compatibility. + if (!empty($options['access_token'])) { + $authType = \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER; } $host = parse_url($remote, PHP_URL_HOST); + // If host extraction fails (e.g., endpoint has no scheme), fall back to cloudId's remote + if ($host === null) { + $host = parse_url($this->cloudId->getRemote(), PHP_URL_HOST); + } $port = parse_url($remote, PHP_URL_PORT); + if ($port === null) { + $port = parse_url($this->cloudId->getRemote(), PHP_URL_PORT); + } $host .= ($port === null) ? '' : ':' . $port; // we add port if available // in case remote NC is on a sub folder and using deprecated ocm provider @@ -91,20 +117,105 @@ public function __construct($options) { $this->mountPoint = $options['mountpoint']; $this->token = $options['token']; + $this->tokenExpiresAt = (int)($options['access_token_expires'] ?? 0); + + // Determine scheme - fall back to cloudId's remote if $remote has no scheme + $scheme = parse_url($remote, PHP_URL_SCHEME) ?? parse_url($this->cloudId->getRemote(), PHP_URL_SCHEME) ?? 'https'; parent::__construct( [ - 'secure' => ((parse_url($remote, PHP_URL_SCHEME) ?? 'https') === 'https'), + 'secure' => ($scheme === 'https'), 'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false), 'host' => $host, 'root' => $webDavEndpoint, 'user' => $options['token'], - 'authType' => \Sabre\DAV\Client::AUTH_BASIC, - 'password' => (string)$options['password'] + 'authType' => $authType, + 'password' => $authType === \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER + ? (string)($options['access_token'] ?? '') + : (string)($options['password'] ?? ''), + 'discoveryService' => $discoveryService, ] ); } + /** + * Refresh the access token. Extends parent to also persist to database. + * + * Uses expiry timestamps instead of a boolean flag so that concurrent + * processes can detect that another process already obtained a fresh token + * and reuse it rather than performing a redundant exchange. + * + * After a failed exchange, a 60-second backoff is applied so that + * subsequent file operations do not hammer the remote token endpoint. + * The DB is still consulted during backoff in case a concurrent process + * succeeded; only the outgoing exchange call is suppressed. + * + * @return string|null the access token (freshly exchanged or reused from + * DB), or null if refresh is currently not possible + */ + #[\Override] + protected function refreshAccessToken(): ?string { + $now = time(); + + // Fast path: in-memory token is still valid (single-process guard). + if ($this->tokenExpiresAt > $now && !empty($this->password)) { + return $this->password; + } + + // Slow path: check DB — a concurrent process may have already refreshed. + $share = $this->manager->getShareByToken($this->token); + if ($share !== false) { + $dbExpiry = $share->getAccessTokenExpires(); + $dbToken = $share->getAccessToken(); + if ($dbExpiry !== null && $dbExpiry > $now && $dbToken !== null) { + // Another process already refreshed — reuse DB token and reset failure state. + $this->password = $dbToken; + $this->bearerToken = $dbToken; + $this->tokenExpiresAt = $dbExpiry; + $this->refreshFailureCount = 0; + $this->refreshBackoffUntil = 0; + $this->logger->debug('Reused access token refreshed by another process', ['app' => 'files_sharing']); + return $dbToken; + } + } + + // Gave up after max attempts: stop trying for the lifetime of this instance. + if ($this->refreshFailureCount >= self::REFRESH_MAX_ATTEMPTS) { + return null; + } + + // Still within the inter-attempt wait: don't hit the endpoint yet. + if ($this->refreshBackoffUntil > $now) { + return null; + } + + // No valid token in DB — perform the exchange ourselves. + try { + $expiresAt = $now + 3600; // access tokens are valid for 1 hour + $newAccessToken = $this->exchangeRefreshToken(); + $this->password = $newAccessToken; + $this->bearerToken = $newAccessToken; + $this->tokenExpiresAt = $expiresAt; + $this->refreshFailureCount = 0; + $this->refreshBackoffUntil = 0; + + $this->manager->updateAccessToken($this->token, $newAccessToken, $expiresAt); + + $this->logger->debug('Successfully refreshed access token', ['app' => 'files_sharing']); + return $newAccessToken; + } catch (\Exception $e) { + $this->refreshFailureCount++; + $this->refreshBackoffUntil = $now + self::REFRESH_BACKOFF_SECONDS; + $this->logger->warning('Failed to refresh access token (attempt {attempt}/{max})', [ + 'app' => 'files_sharing', + 'attempt' => $this->refreshFailureCount, + 'max' => self::REFRESH_MAX_ATTEMPTS, + 'exception' => $e, + ]); + return null; + } + } + #[\Override] public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher { if (!$storage) { diff --git a/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php b/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php new file mode 100644 index 0000000000000..ae419194d95a5 --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php @@ -0,0 +1,89 @@ +getTable('share_external'); + + $changed = false; + + if (!$table->hasColumn('access_token')) { + $table->addColumn('access_token', Types::STRING, [ + 'notnull' => false, + 'default' => null, + 'length' => 4000, + ]); + $changed = true; + } + + if (!$table->hasColumn('access_token_expires')) { + $table->addColumn('access_token_expires', Types::INTEGER, [ + 'notnull' => false, + 'default' => null, + ]); + $changed = true; + } + + if (!$table->hasColumn('refresh_token')) { + $table->addColumn('refresh_token', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + 'default' => null, + ]); + $changed = true; + } + + return $changed ? $schema : null; + } + + #[Override] + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $table = $schema->getTable('share_external'); + if (!$table->hasColumn('share_token') || !$table->hasColumn('refresh_token')) { + return; + } + + $qb = $this->db->getQueryBuilder(); + $qb->update('share_external') + ->set('refresh_token', 'share_token'); + $qb->executeStatement(); + } +} diff --git a/apps/files_sharing/lib/Migration/Version33000Date20260306150000.php b/apps/files_sharing/lib/Migration/Version33000Date20260306150000.php new file mode 100644 index 0000000000000..dc37ccea32ccf --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version33000Date20260306150000.php @@ -0,0 +1,39 @@ +getTable('share_external'); + + if (!$table->hasColumn('share_token')) { + return null; + } + + $table->dropColumn('share_token'); + + return $schema; + } +} diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php index f23b662260b5f..3687d5d66d1e4 100644 --- a/apps/files_sharing/lib/ResponseDefinitions.php +++ b/apps/files_sharing/lib/ResponseDefinitions.php @@ -93,7 +93,7 @@ * permissions: int|null, * remote: string, * remote_id: string, - * share_token: string, + * refresh_token: string, * share_type: int, * type: string|null, * user: string, diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 3ecfe58234afa..c31231f8ee996 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -409,7 +409,7 @@ "permissions", "remote", "remote_id", - "share_token", + "refresh_token", "share_type", "type", "user", @@ -461,7 +461,7 @@ "remote_id": { "type": "string" }, - "share_token": { + "refresh_token": { "type": "string" }, "share_type": { diff --git a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php index a1a3a2fe623f6..6fd9fe39cc719 100644 --- a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php +++ b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php @@ -30,13 +30,13 @@ class CleanupRemoteStoragesTest extends TestCase { private ICloudIdManager&MockObject $cloudIdManager; private $storages = [ - ['id' => 'shared::7b4a322b22f9d0047c38d77d471ce3cf', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e1', 'remote' => 'https://hostname.tld/owncloud1', 'user' => 'user1'], - ['id' => 'shared::efe3b456112c3780da6155d3a9b9141c', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e2', 'remote' => 'https://hostname.tld/owncloud2', 'user' => 'user2'], - ['notExistingId' => 'shared::33323d9f4ca416a9e3525b435354bc6f', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e3', 'remote' => 'https://hostname.tld/owncloud3', 'user' => 'user3'], + ['id' => 'shared::7b4a322b22f9d0047c38d77d471ce3cf', 'refresh_token' => 'f2c69dad1dc0649f26976fd210fc62e1', 'remote' => 'https://hostname.tld/owncloud1', 'user' => 'user1'], + ['id' => 'shared::efe3b456112c3780da6155d3a9b9141c', 'refresh_token' => 'f2c69dad1dc0649f26976fd210fc62e2', 'remote' => 'https://hostname.tld/owncloud2', 'user' => 'user2'], + ['notExistingId' => 'shared::33323d9f4ca416a9e3525b435354bc6f', 'refresh_token' => 'f2c69dad1dc0649f26976fd210fc62e3', 'remote' => 'https://hostname.tld/owncloud3', 'user' => 'user3'], ['id' => 'shared::7fe41a07d3f517a923f4b2b599e72cbb', 'files_count' => 2], - ['id' => 'shared::de4aeb2f378d222b6d2c5fd8f4e42f8e', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e5', 'remote' => 'https://hostname.tld/owncloud5', 'user' => 'user5'], + ['id' => 'shared::de4aeb2f378d222b6d2c5fd8f4e42f8e', 'refresh_token' => 'f2c69dad1dc0649f26976fd210fc62e5', 'remote' => 'https://hostname.tld/owncloud5', 'user' => 'user5'], ['id' => 'shared::af712293ab5eb9e6a1745a13818b99fe', 'files_count' => 3], - ['notExistingId' => 'shared::c34568c143cdac7d2f06e0800b5280f9', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e7', 'remote' => 'https://hostname.tld/owncloud7', 'user' => 'user7'], + ['notExistingId' => 'shared::c34568c143cdac7d2f06e0800b5280f9', 'refresh_token' => 'f2c69dad1dc0649f26976fd210fc62e7', 'remote' => 'https://hostname.tld/owncloud7', 'user' => 'user7'], ]; protected function setUp(): void { @@ -62,10 +62,10 @@ protected function setUp(): void { $storage['numeric_id'] = $storageQuery->getLastInsertId(); } - if (isset($storage['share_token'])) { + if (isset($storage['refresh_token'])) { $externalShare = new ExternalShare(); $externalShare->generateId(); - $externalShare->setShareToken($storage['share_token']); + $externalShare->setRefreshToken($storage['refresh_token']); $externalShare->setRemote($storage['remote']); $externalShare->setName('irrelevant'); $externalShare->setOwner('irrelevant'); @@ -96,7 +96,7 @@ protected function tearDown(): void { $shareExternalQuery = Server::get(IDBConnection::class)->getQueryBuilder(); $shareExternalQuery->delete('share_external') - ->where($shareExternalQuery->expr()->eq('share_token', $shareExternalQuery->createParameter('share_token'))) + ->where($shareExternalQuery->expr()->eq('refresh_token', $shareExternalQuery->createParameter('refresh_token'))) ->andWhere($shareExternalQuery->expr()->eq('remote', $shareExternalQuery->createParameter('remote'))); foreach ($this->storages as $storage) { @@ -105,8 +105,8 @@ protected function tearDown(): void { $storageQuery->executeStatement(); } - if (isset($storage['share_token'])) { - $shareExternalQuery->setParameter('share_token', $storage['share_token']); + if (isset($storage['refresh_token'])) { + $shareExternalQuery->setParameter('refresh_token', $storage['refresh_token']); $shareExternalQuery->setParameter('remote', $storage['remote']); $shareExternalQuery->executeStatement(); } diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index 804f415f522de..bba55a0ebbafa 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -195,7 +195,7 @@ public function testAddUserShare(): void { $userShare = new ExternalShare(); $userShare->generateId(); $userShare->setRemote('http://localhost'); - $userShare->setShareToken('token1'); + $userShare->setRefreshToken('token1'); $userShare->setPassword(''); $userShare->setName('/SharedFolder'); $userShare->setOwner('foobar'); @@ -214,7 +214,7 @@ public function testAddGroupShare(): void { $groupShare->setShareType(IShare::TYPE_GROUP); $groupShare->setAccepted(IShare::STATUS_PENDING); $groupShare->setRemoteId('2342'); - $groupShare->setShareToken('token1'); + $groupShare->setRefreshToken('token1'); $groupShare->setPassword(''); $groupShare->setName('/SharedFolder'); $this->doTestAddShare($groupShare, $this->group1, isGroup: true); @@ -239,10 +239,10 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1->getName() . '}}', $userOrGroup); $shareData2 = $shareData1->clone(); - $shareData2->setShareToken('token2'); + $shareData2->setRefreshToken('token2'); $shareData2->generateId(); $shareData3 = $shareData1->clone(); - $shareData3->setShareToken('token3'); + $shareData3->setRefreshToken('token3'); $shareData3->generateId(); $this->setupMounts(); @@ -445,7 +445,7 @@ private function createTestUserShare(string $userId = 'user1'): ExternalShare { $share = new ExternalShare(); $share->generateId(); $share->setRemote('http://localhost'); - $share->setShareToken('token1'); + $share->setRefreshToken('token1'); $share->setPassword(''); $share->setName('/SharedFolder'); $share->setOwner('foobar'); @@ -465,7 +465,7 @@ private function createTestGroupShare(string $groupId = 'group1'): array { $share = new ExternalShare(); $share->generateId(); $share->setRemote('http://localhost'); - $share->setShareToken('token1'); + $share->setRefreshToken('token1'); $share->setPassword(''); $share->setName('/SharedFolder'); $share->setOwner('foobar'); @@ -651,7 +651,7 @@ public function testDeleteUserShares(): void { $share = new ExternalShare(); $share->generateId(); $share->setRemote('http://localhost'); - $share->setShareToken('token1'); + $share->setRefreshToken('token1'); $share->setPassword(''); $share->setName('/SharedFolder'); $share->setOwner('foobar'); @@ -701,7 +701,7 @@ public function testDeleteGroupShares(): void { $share = new ExternalShare(); $share->generateId(); $share->setRemote('http://localhost'); - $share->setShareToken('token1'); + $share->setRefreshToken('token1'); $share->setPassword(''); $share->setName('/SharedFolder'); $share->setOwner('foobar'); @@ -730,7 +730,7 @@ public function testDeleteGroupShares(): void { protected function assertExternalShareEntry(ExternalShare $expected, ExternalShare $actual, int $share, string $mountPoint, IUser|IGroup $targetEntity): void { $this->assertEquals($expected->getRemote(), $actual->getRemote(), 'Asserting remote of a share #' . $share); - $this->assertEquals($expected->getShareToken(), $actual->getShareToken(), 'Asserting token of a share #' . $share); + $this->assertEquals($expected->getRefreshToken(), $actual->getRefreshToken(), 'Asserting token of a share #' . $share); $this->assertEquals($expected->getName(), $actual->getName(), 'Asserting name of a share #' . $share); $this->assertEquals($expected->getOwner(), $actual->getOwner(), 'Asserting owner of a share #' . $share); $this->assertEquals($expected->getAccepted(), $actual->getAccepted(), 'Asserting accept of a share #' . $share); diff --git a/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php new file mode 100644 index 0000000000000..c4a5fe2db8b3d --- /dev/null +++ b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php @@ -0,0 +1,114 @@ +externalShareMapper = $this->createMock(ExternalShareMapper::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser')->willReturn(null); + + $this->manager = new Manager( + $this->createMock(IDBConnection::class), + $this->createMock(\OC\Files\Mount\Manager::class), + $this->createMock(IStorageFactory::class), + $this->createMock(IClientService::class), + $this->createMock(INotificationManager::class), + $this->createMock(IDiscoveryService::class), + $this->createMock(ICloudFederationProviderManager::class), + $this->createMock(ICloudFederationFactory::class), + $this->createMock(IGroupManager::class), + $userSession, + $this->createMock(IEventDispatcher::class), + $this->logger, + $this->createMock(IRootFolder::class), + $this->createMock(ISetupManager::class), + $this->createMock(ICertificateManager::class), + $this->externalShareMapper, + $this->createMock(IConfig::class), + ); + } + + public function testUpdateAccessTokenUpdatesShareInDb(): void { + $share = new ExternalShare(); + $share->setRefreshToken('refresh-token'); + + $this->externalShareMapper->expects($this->once()) + ->method('getShareByToken') + ->with('refresh-token') + ->willReturn($share); + + $this->externalShareMapper->expects($this->once()) + ->method('update') + ->with($this->callback(function (ExternalShare $s) { + return $s->getAccessToken() === 'new-access-token' + && $s->getAccessTokenExpires() === 9999; + })); + + $this->manager->updateAccessToken('refresh-token', 'new-access-token', 9999); + } + + public function testUpdateAccessTokenLogsWarningWhenShareNotFound(): void { + $this->externalShareMapper->method('getShareByToken') + ->willThrowException(new DoesNotExistException('not found')); + + $this->externalShareMapper->expects($this->never())->method('update'); + + $this->logger->expects($this->once()) + ->method('warning') + ->with($this->stringContains('Could not find share')); + + $this->manager->updateAccessToken('missing-token', 'access', 0); + } + + public function testUpdateAccessTokenLogsErrorOnDbException(): void { + $this->externalShareMapper->method('getShareByToken') + ->willThrowException(new Exception('db error')); + + $this->externalShareMapper->expects($this->never())->method('update'); + + $this->logger->expects($this->once()) + ->method('error') + ->with($this->stringContains('Failed to update access token')); + + $this->manager->updateAccessToken('some-token', 'access', 0); + } +} diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 0a87be81d3124..08ee02eef4473 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -13,6 +13,7 @@ use Icewind\Streams\IteratorDirectory; use OC\Files\Filesystem; use OC\MemCache\ArrayCache; +use OC\OCM\OCMSignatoryManager; use OCP\AppFramework\Http; use OCP\Constants; use OCP\Diagnostics\IEventLogger; @@ -25,10 +26,16 @@ use OCP\Files\StorageNotAvailableException; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; +use OCP\IAppConfig; use OCP\ICertificateManager; use OCP\IConfig; use OCP\ITempManager; +use OCP\IURLGenerator; use OCP\Lock\LockedException; +use OCP\OCM\Exceptions\OCMArgumentException; +use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; +use OCP\Security\Signature\ISignatureManager; use OCP\Server; use OCP\Util; use Psr\Http\Message\ResponseInterface; @@ -38,6 +45,78 @@ use Sabre\HTTP\ClientException; use Sabre\HTTP\ClientHttpException; use Sabre\HTTP\RequestInterface; +use Sabre\HTTP\ResponseInterface as SabreResponseInterface; + +/** + * Sabre HTTP Client extended with Bearer-token authentication and transparent + * refresh-on-401: when a request fails with HTTP 401 the client invokes a + * registered refresh callback once, applies the new token, and replays the + * request. Callers can use the client normally without thinking about token + * expiry. + * + * @package OC\Files\Storage + */ +class BearerAuthAwareSabreClient extends Client { + /** + * Bearer authentication. + */ + public const AUTH_BEARER = 8; + + /** @var (\Closure(): ?string)|null returns a fresh bearer token, or null if it cannot be refreshed */ + private ?\Closure $refreshTokenCallback = null; + + /** Guard against re-entry if the replayed request also returns 401. */ + private bool $retrying = false; + + public function __construct(array $settings) { + parent::__construct($settings); + + if (isset($settings['bearerToken']) && isset($settings['authType']) && ($settings['authType'] & self::AUTH_BEARER)) { + $this->applyBearerToken((string)$settings['bearerToken']); + } + } + + /** + * Register a callback invoked when a request comes back with HTTP 401. The + * callback should return a fresh bearer token, or null to give up. When a + * non-empty token is returned the failing request is replayed once. + * + * @param (callable(): ?string)|null $callback + */ + public function setRefreshTokenCallback(?callable $callback): void { + $this->refreshTokenCallback = $callback === null ? null : \Closure::fromCallable($callback); + } + + #[\Override] + public function send(RequestInterface $request): SabreResponseInterface { + try { + return parent::send($request); + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() !== 401 || $this->retrying || $this->refreshTokenCallback === null) { + throw $e; + } + $this->retrying = true; + try { + $newToken = ($this->refreshTokenCallback)(); + if (!is_string($newToken) || $newToken === '') { + throw $e; + } + $this->applyBearerToken($newToken); + return parent::send($request); + } finally { + $this->retrying = false; + } + } + } + + private function applyBearerToken(string $token): void { + /** @psalm-suppress InvalidArrayOffset */ + $curlType = $this->curlSettings[CURLOPT_HTTPAUTH] ?? 0; + $curlType |= CURLAUTH_BEARER; + $this->addCurlSetting(CURLOPT_HTTPAUTH, $curlType); + $this->addCurlSetting(CURLOPT_XOAUTH2_BEARER, $token); + } +} /** * Class DAV @@ -49,8 +128,7 @@ class DAV extends Common { protected $password; /** @var string */ protected $user; - /** @var string|null */ - protected $authType; + protected ?int $authType = null; /** @var string */ protected $host; /** @var bool */ @@ -62,6 +140,8 @@ class DAV extends Common { protected $certPath; /** @var bool */ protected $ready; + /** The resolved bearer token for AUTH_BEARER (access token or exchanged token) */ + protected string $bearerToken = ''; /** @var Client */ protected $client; /** @var ArrayCache */ @@ -73,6 +153,11 @@ class DAV extends Common { protected LoggerInterface $logger; protected IEventLogger $eventLogger; protected IMimeTypeDetector $mimeTypeDetector; + protected IOCMDiscoveryService $discoveryService; + protected ISignatureManager $signatureManager; + protected OCMSignatoryManager $signatoryManager; + protected IAppConfig $appConfig; + protected IURLGenerator $urlGenerator; /** @var int */ private $timeout; @@ -83,6 +168,7 @@ class DAV extends Common { '{DAV:}getcontenttype', '{http://owncloud.org/ns}permissions', '{http://open-collaboration-services.org/ns}share-permissions', + '{http://open-cloud-mesh.org/ns}share-permissions', '{DAV:}resourcetype', '{DAV:}getetag', '{DAV:}quota-available-bytes', @@ -95,6 +181,11 @@ class DAV extends Common { public function __construct(array $parameters) { $this->statCache = new ArrayCache(); $this->httpClientService = Server::get(IClientService::class); + if (isset($parameters['discoveryService'])) { + $this->discoveryService = $parameters['discoveryService']; + } else { + $this->discoveryService = Server::get(IOCMDiscoveryService::class); + } if (isset($parameters['host']) && isset($parameters['user']) && isset($parameters['password'])) { $host = $parameters['host']; //remove leading http[s], will be generated in createBaseUri() @@ -135,6 +226,10 @@ public function __construct(array $parameters) { // This timeout value will be used for the download and upload of files $this->timeout = Server::get(IConfig::class)->getSystemValueInt('davstorage.request_timeout', IClient::DEFAULT_REQUEST_TIMEOUT); $this->mimeTypeDetector = Server::get(IMimeTypeDetector::class); + $this->signatureManager = Server::get(ISignatureManager::class); + $this->signatoryManager = Server::get(OCMSignatoryManager::class); + $this->appConfig = Server::get(IAppConfig::class); + $this->urlGenerator = Server::get(IURLGenerator::class); } protected function init(): void { @@ -143,6 +238,19 @@ protected function init(): void { } $this->ready = true; + // For Basic auth, the share token is kept as the user name + $token = $this->user; + // If using Bearer auth, use stored access token or exchange refresh token for access token + if ($this->authType !== null && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER)) { + // Check if we already have an access token stored (password field) + if (!empty($this->password)) { + $token = $this->password; + } else { + $token = $this->exchangeRefreshToken(); + } + $this->bearerToken = $token; + } + $settings = [ 'baseUri' => $this->createBaseUri(), 'userName' => $this->user, @@ -151,13 +259,18 @@ protected function init(): void { if ($this->authType !== null) { $settings['authType'] = $this->authType; } + if ($this->isBearerAuth()) { + // The Sabre client must authenticate with the resolved access token + // (JWT), not the share token kept in the user name field. + $settings['bearerToken'] = $this->bearerToken; + } $proxy = Server::get(IConfig::class)->getSystemValueString('proxy', ''); if ($proxy !== '') { $settings['proxy'] = $proxy; } - $this->client = new Client($settings); + $this->client = new BearerAuthAwareSabreClient($settings); $this->client->setThrowExceptions(true); $proxyExclude = Server::get(IConfig::class)->getSystemValue('proxyexclude', []); @@ -165,6 +278,10 @@ protected function init(): void { $this->client->addCurlSetting(CURLOPT_NOPROXY, implode(',', $proxyExclude)); } + if ($this->isBearerAuth()) { + $this->client->setRefreshTokenCallback(fn (): ?string => $this->refreshAccessToken()); + } + if ($this->secure === true) { if ($this->verify === false) { $this->client->addCurlSetting(CURLOPT_SSL_VERIFYPEER, false); @@ -191,6 +308,172 @@ protected function init(): void { }); } + /** + * Exchange refresh token for access token via the remote server's token endpoint + * + * @return string The access token + * @throws StorageNotAvailableException If token exchange fails + */ + protected function exchangeRefreshToken(): string { + try { + $host = ($this->secure ? 'https://' : 'http://') . $this->host; + $ocmProvider = $this->discoveryService->discover($host); + $tokenEndpoint = $ocmProvider->getTokenEndPoint(); + + if ($tokenEndpoint === '') { + $this->logger->error('OCM provider response missing tokenEndPoint', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not discover token endpoint'); + } + + $client = $this->httpClientService->newClient(); + $clientId = parse_url($this->urlGenerator->getAbsoluteURL('/'), PHP_URL_HOST); + $payload = [ + 'grant_type' => 'authorization_code', + 'client_id' => $clientId, + 'code' => $this->user, // refresh token is stored in user field + ]; + + $options = [ + 'body' => http_build_query($payload), + 'headers' => [ + 'Content-Type' => 'application/x-www-form-urlencoded', + ], + 'timeout' => 10, + 'connect_timeout' => 10, + ]; + + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + $this->signatoryManager, + $options, + 'post', + $tokenEndpoint + ); + $this->logger->debug('Token request signed successfully', ['app' => 'dav']); + } catch (\Exception $e) { + $this->logger->error('Failed to sign token request', [ + 'app' => 'dav', + 'exception' => $e, + 'endpoint' => $tokenEndpoint, + ]); + throw new StorageNotAvailableException('Could not sign token request: ' . $e->getMessage()); + } + + $response = $client->post($tokenEndpoint, $options); + + $statusCode = $response->getStatusCode(); + if ($statusCode !== 200) { + $this->logger->error('Token exchange returned unexpected HTTP status', [ + 'app' => 'dav', + 'status' => $statusCode, + ]); + throw new StorageNotAvailableException('Could not obtain access token: unexpected HTTP status ' . $statusCode); + } + + $data = json_decode($response->getBody(), true); + + if (!is_array($data)) { + $this->logger->error('Token exchange response is not valid JSON', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not obtain access token: invalid response format'); + } + + $accessToken = $data['access_token'] ?? null; + $tokenType = $data['token_type'] ?? null; + + if (!is_string($accessToken) || $accessToken === '') { + $this->logger->error('Token exchange response missing or invalid access_token', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not obtain access token: missing access_token field'); + } + + if (!is_string($tokenType) || strtolower($tokenType) !== 'bearer') { + $this->logger->error('Token exchange response has unexpected token_type', [ + 'app' => 'dav', + 'token_type' => $tokenType, + ]); + throw new StorageNotAvailableException('Could not obtain access token: unexpected token_type'); + } + + $this->logger->debug('Successfully exchanged refresh token for access token', ['app' => 'dav']); + return $accessToken; + } catch (OCMProviderException|OCMArgumentException $e) { + $this->logger->error('OCM provider response missing tokenEndPoint', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not discover token endpoint'); + } catch (StorageNotAvailableException $e) { + throw $e; + } catch (\Exception $e) { + $this->logger->error('Error exchanging refresh token for access token: ' . $e->getMessage(), [ + 'app' => 'dav', + 'exception' => $e, + ]); + throw new StorageNotAvailableException('Could not obtain access token: ' . $e->getMessage()); + } + } + + /** + * Check if bearer authentication is being used + */ + protected function isBearerAuth(): bool { + return $this->authType !== null + && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER); + } + + /** Guard against re-entry while a Guzzle-path 401 is being recovered. */ + private bool $retryingAuth = false; + + /** + * Wrap a Guzzle-based operation with retry-on-401 using the bearer-token + * refresh. Sabre-based operations don't need this — {@see BearerAuthAwareSabreClient} + * handles 401 transparently on its own. + * + * @template T + * @param callable(): T $operation + * @return T + * @throws \GuzzleHttp\Exception\ClientException + */ + protected function withAuthRetry(callable $operation): mixed { + try { + return $operation(); + } catch (\GuzzleHttp\Exception\ClientException $e) { + if (!$this->isBearerAuth() + || $this->retryingAuth + || !($e->getResponse() instanceof ResponseInterface) + || $e->getResponse()->getStatusCode() !== 401) { + throw $e; + } + $this->retryingAuth = true; + try { + if ($this->refreshAccessToken() === null) { + throw $e; + } + return $operation(); + } finally { + $this->retryingAuth = false; + } + } + } + + /** + * Exchange the long-lived refresh token for a new short-lived access token + * and update {@see $bearerToken} (and the stored password so subsequent + * init() calls reuse the same token). Used as the refresh callback for the + * Sabre client and by {@see withAuthRetry} for the Guzzle paths. + * + * @return string|null new access token, or null if the exchange failed + */ + protected function refreshAccessToken(): ?string { + $this->logger->debug('Bearer token expired, exchanging for a fresh access token', ['app' => 'dav']); + try { + $this->password = ''; // force a fresh exchange instead of reusing the expired one + $newToken = $this->exchangeRefreshToken(); + } catch (\Exception $e) { + $this->logger->warning('Failed to refresh bearer token: ' . $e->getMessage(), ['app' => 'dav', 'exception' => $e]); + return null; + } + $this->bearerToken = $newToken; + $this->password = $newToken; + return $newToken; + } + /** * Clear the stat cache */ @@ -377,22 +660,31 @@ public function fopen(string $path, string $mode) { case 'r': case 'rb': try { - $response = $this->httpClientService - ->newClient() - ->get($this->createBaseUri() . $this->encodePath($path), [ - 'auth' => [$this->user, $this->password], - 'stream' => true, - // set download timeout for users with slow connections or large files - 'timeout' => $this->timeout, - 'verify' => $this->verify, - ]); + $response = $this->withAuthRetry(function () use ($path) { + if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { + $auth = []; + $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; + } else { + $auth = [$this->user, $this->password]; + $headers = []; + } + return $this->httpClientService + ->newClient() + ->get($this->createBaseUri() . $this->encodePath($path), [ + 'headers' => $headers, + 'auth' => $auth, + 'stream' => true, + // set download timeout for users with slow connections or large files + 'timeout' => $this->timeout, + 'verify' => $this->verify, + ]); + }); } catch (\GuzzleHttp\Exception\ClientException $e) { if ($e->getResponse() instanceof ResponseInterface && $e->getResponse()->getStatusCode() === 404) { return false; - } else { - throw $e; } + throw $e; } if ($response->getStatusCode() !== Http::STATUS_OK) { @@ -529,17 +821,26 @@ protected function uploadFile(string $path, string $target): void { // invalidate $target = $this->cleanPath($target); $this->statCache->remove($target); - $source = fopen($path, 'r'); - - $this->httpClientService - ->newClient() - ->put($this->createBaseUri() . $this->encodePath($target), [ - 'body' => $source, - 'auth' => [$this->user, $this->password], - // set upload timeout for users with slow connections or large files - 'timeout' => $this->timeout, - 'verify' => $this->verify, - ]); + + $this->withAuthRetry(function () use ($path, $target) { + $source = fopen($path, 'r'); + $auth = [$this->user, $this->password]; + $headers = []; + if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { + $auth = []; + $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; + } + $this->httpClientService + ->newClient() + ->put($this->createBaseUri() . $this->encodePath($target), [ + 'body' => $source, + 'headers' => $headers, + 'auth' => $auth, + // set upload timeout for users with slow connections or large files + 'timeout' => $this->timeout, + 'verify' => $this->verify, + ]); + }); $this->removeCachedFile($target); } @@ -570,6 +871,8 @@ public function rename(string $source, string $target): bool { $this->removeCachedFile($source); $this->removeCachedFile($target); return true; + } catch (ClientHttpException $e) { + $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); } @@ -599,6 +902,8 @@ public function copy(string $source, string $target): bool { $this->statCache->set($target, true); $this->removeCachedFile($target); return true; + } catch (ClientHttpException $e) { + $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); } @@ -904,6 +1209,8 @@ public function getDirectoryContent(string $directory): \Traversable { $this->statCache->set($file, $response); yield $this->getMetaFromPropfind($file, $response); } + } catch (ClientHttpException $e) { + $this->convertException($e, $directory); } catch (\Exception $e) { $this->convertException($e, $directory); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 1bf284fcf1784..ea381dc46ec97 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -9,6 +9,8 @@ namespace OC\Share20; use ArrayIterator; +use OC\Authentication\Exceptions\InvalidTokenException; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Core\AppInfo\ConfigLexicon; use OC\Files\Filesystem; use OC\KnownUser\KnownUserService; @@ -45,6 +47,7 @@ use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Security\PasswordContext; +use OCP\Server; use OCP\Share; use OCP\Share\Events\BeforeShareCreatedEvent; use OCP\Share\Events\BeforeShareDeletedEvent; @@ -1443,6 +1446,18 @@ public function getShareByToken(string $token): IShare { } } + // Try to fetch a federated share by access token + if ($share === null) { + try { + $provider = $this->factory->getProviderForType(IShare::TYPE_REMOTE); + $tokenProvider = Server::get(PublicKeyTokenProvider::class); + $accessTokenDb = $tokenProvider->getToken($token); + $refreshToken = $accessTokenDb->getUID(); + $share = $provider->getShareByToken($refreshToken); + } catch (ProviderException|ShareNotFound|InvalidTokenException $e) { + } + } + // If it is not a link share try to fetch a mail share by token if ($share === null && $this->shareProviderExists(IShare::TYPE_EMAIL)) { try { diff --git a/tests/lib/Files/Storage/BearerAuthAwareSabreClientTest.php b/tests/lib/Files/Storage/BearerAuthAwareSabreClientTest.php new file mode 100644 index 0000000000000..dc750333b6ed1 --- /dev/null +++ b/tests/lib/Files/Storage/BearerAuthAwareSabreClientTest.php @@ -0,0 +1,74 @@ +getProperty('curlSettings'); + $property->setAccessible(true); + $settings = $property->getValue($client); + return $settings[$key] ?? null; + } + + public function testBearerTokenIsUsedForAuthentication(): void { + $client = new BearerAuthAwareSabreClient([ + 'baseUri' => 'https://example.com/', + // The user name holds the long-lived share secret, never the bearer. + 'userName' => 'refresh-secret', + 'password' => '', + 'authType' => BearerAuthAwareSabreClient::AUTH_BEARER, + 'bearerToken' => 'the-access-jwt', + ]); + + $this->assertSame('the-access-jwt', $this->getCurlSetting($client, CURLOPT_XOAUTH2_BEARER)); + } + + public function testShareSecretIsNotUsedAsBearer(): void { + $client = new BearerAuthAwareSabreClient([ + 'baseUri' => 'https://example.com/', + 'userName' => 'refresh-secret', + 'password' => '', + 'authType' => BearerAuthAwareSabreClient::AUTH_BEARER, + 'bearerToken' => 'the-access-jwt', + ]); + + $this->assertNotSame('refresh-secret', $this->getCurlSetting($client, CURLOPT_XOAUTH2_BEARER)); + } + + public function testNoBearerAppliedWithoutBearerAuthType(): void { + $client = new BearerAuthAwareSabreClient([ + 'baseUri' => 'https://example.com/', + 'userName' => 'user', + 'password' => 'pass', + 'authType' => \Sabre\DAV\Client::AUTH_BASIC, + 'bearerToken' => 'the-access-jwt', + ]); + + $this->assertNull($this->getCurlSetting($client, CURLOPT_XOAUTH2_BEARER)); + } + + public function testNoBearerAppliedWithoutBearerToken(): void { + $client = new BearerAuthAwareSabreClient([ + 'baseUri' => 'https://example.com/', + 'userName' => 'refresh-secret', + 'password' => '', + 'authType' => BearerAuthAwareSabreClient::AUTH_BEARER, + ]); + + $this->assertNull($this->getCurlSetting($client, CURLOPT_XOAUTH2_BEARER)); + } +} From 1080d15f2b73b2c857a1a9eb6b9d7e4c477e69ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 27 May 2026 21:34:02 +0200 Subject: [PATCH 8/9] chore: regenerate autoloaders and update psalm baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Micke Nordin Signed-off-by: Micke Nordin Signed-off-by: Enrique Pérez Arnaud --- build/files-checker.php | 2 ++ build/psalm-baseline.xml | 1 + lib/composer/composer/autoload_classmap.php | 2 ++ lib/composer/composer/autoload_static.php | 2 ++ openapi.json | 22 +++++++++++++-------- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/build/files-checker.php b/build/files-checker.php index 8b44761bcfe98..1c3f0e6dbc5ca 100644 --- a/build/files-checker.php +++ b/build/files-checker.php @@ -32,8 +32,10 @@ '__mocks__', '__tests__', '3rdparty', + 'AGENTS.md', 'AUTHORS', 'CHANGELOG.md', + 'CLAUDE.md', 'CODE_OF_CONDUCT.md', 'COPYING', 'COPYING-README', diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index fe478aab69d41..b7d879c8d2cdf 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -38,6 +38,7 @@ ['uid' => &$uid] )]]> + diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index fe73bc0443d9a..44eec9ca01530 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -397,6 +397,7 @@ 'OCP\\Federation\\ICloudIdManager' => $baseDir . '/lib/public/Federation/ICloudIdManager.php', 'OCP\\Federation\\ICloudIdResolver' => $baseDir . '/lib/public/Federation/ICloudIdResolver.php', 'OCP\\Federation\\ISignedCloudFederationProvider' => $baseDir . '/lib/public/Federation/ISignedCloudFederationProvider.php', + 'OCP\\Federation\\IValidationAwareCloudFederationProvider' => $baseDir . '/lib/public/Federation/IValidationAwareCloudFederationProvider.php', 'OCP\\Files' => $baseDir . '/lib/public/Files.php', 'OCP\\FilesMetadata\\AMetadataEvent' => $baseDir . '/lib/public/FilesMetadata/AMetadataEvent.php', 'OCP\\FilesMetadata\\Event\\MetadataBackgroundEvent' => $baseDir . '/lib/public/FilesMetadata/Event/MetadataBackgroundEvent.php', @@ -747,6 +748,7 @@ 'OCP\\OCM\\IOCMDiscoveryService' => $baseDir . '/lib/public/OCM/IOCMDiscoveryService.php', 'OCP\\OCM\\IOCMProvider' => $baseDir . '/lib/public/OCM/IOCMProvider.php', 'OCP\\OCM\\IOCMResource' => $baseDir . '/lib/public/OCM/IOCMResource.php', + 'OCP\\OCM\\OCMCapabilities' => $baseDir . '/lib/public/OCM/OCMCapabilities.php', 'OCP\\OCS\\IDiscoveryService' => $baseDir . '/lib/public/OCS/IDiscoveryService.php', 'OCP\\OpenMetrics\\IMetricFamily' => $baseDir . '/lib/public/OpenMetrics/IMetricFamily.php', 'OCP\\OpenMetrics\\Metric' => $baseDir . '/lib/public/OpenMetrics/Metric.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f4d201e709a0c..8bcf42141bd21 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -438,6 +438,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Federation\\ICloudIdManager' => __DIR__ . '/../../..' . '/lib/public/Federation/ICloudIdManager.php', 'OCP\\Federation\\ICloudIdResolver' => __DIR__ . '/../../..' . '/lib/public/Federation/ICloudIdResolver.php', 'OCP\\Federation\\ISignedCloudFederationProvider' => __DIR__ . '/../../..' . '/lib/public/Federation/ISignedCloudFederationProvider.php', + 'OCP\\Federation\\IValidationAwareCloudFederationProvider' => __DIR__ . '/../../..' . '/lib/public/Federation/IValidationAwareCloudFederationProvider.php', 'OCP\\Files' => __DIR__ . '/../../..' . '/lib/public/Files.php', 'OCP\\FilesMetadata\\AMetadataEvent' => __DIR__ . '/../../..' . '/lib/public/FilesMetadata/AMetadataEvent.php', 'OCP\\FilesMetadata\\Event\\MetadataBackgroundEvent' => __DIR__ . '/../../..' . '/lib/public/FilesMetadata/Event/MetadataBackgroundEvent.php', @@ -788,6 +789,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\OCM\\IOCMDiscoveryService' => __DIR__ . '/../../..' . '/lib/public/OCM/IOCMDiscoveryService.php', 'OCP\\OCM\\IOCMProvider' => __DIR__ . '/../../..' . '/lib/public/OCM/IOCMProvider.php', 'OCP\\OCM\\IOCMResource' => __DIR__ . '/../../..' . '/lib/public/OCM/IOCMResource.php', + 'OCP\\OCM\\OCMCapabilities' => __DIR__ . '/../../..' . '/lib/public/OCM/OCMCapabilities.php', 'OCP\\OCS\\IDiscoveryService' => __DIR__ . '/../../..' . '/lib/public/OCS/IDiscoveryService.php', 'OCP\\OpenMetrics\\IMetricFamily' => __DIR__ . '/../../..' . '/lib/public/OpenMetrics/IMetricFamily.php', 'OCP\\OpenMetrics\\Metric' => __DIR__ . '/../../..' . '/lib/public/OpenMetrics/Metric.php', diff --git a/openapi.json b/openapi.json index 4e0308001f228..de54aa42989c2 100644 --- a/openapi.json +++ b/openapi.json @@ -41,6 +41,10 @@ "name": "cloud_federation_api/request_handler", "description": "Open-Cloud-Mesh-API" }, + { + "name": "cloud_federation_api/token", + "description": "Controller for the /token endpoint Exchanges long-lived refresh tokens for short-lived access tokens" + }, { "name": "federatedfilesharing/mount_public_link", "description": "Class MountPublicLinkController convert public links to federated shares" @@ -2540,7 +2544,7 @@ "permissions", "remote", "remote_id", - "share_token", + "refresh_token", "share_type", "type", "user", @@ -2592,7 +2596,7 @@ "remote_id": { "type": "string" }, - "share_token": { + "refresh_token": { "type": "string" }, "share_type": { @@ -18129,21 +18133,23 @@ "type": "object", "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]]", "required": [ - "name", - "options" + "name" ], "properties": { "name": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" }, "options": { "type": "object", "additionalProperties": { "type": "object" } + }, + "webdav": { + "type": "object", + "additionalProperties": { + "type": "object" + } } } }, From 691d00c162dc2f12dda71cf93002d147aa3e0d9d Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Thu, 4 Jun 2026 13:12:49 +0200 Subject: [PATCH 9/9] fix(cloud_federation_api): accept multi-protocol share envelopes Shares using the OCM multi-protocol envelope (name multi, with the secret carried in a sibling protocol entry such as webdav) were rejected with Missing sharedSecret in protocol. Scan every protocol entry for the shared secret during validation, resolve the secret from the matching entry, and let the files provider serve the webdav entry of a multi envelope. Covers the file and folder resource types. Signed-off-by: Micke Nordin --- .../Controller/RequestHandlerController.php | 41 ++++++-- apps/cloud_federation_api/openapi.json | 2 +- .../tests/RequestHandlerControllerTest.php | 94 +++++++++++++++++++ .../lib/OCM/CloudFederationProviderFiles.php | 10 +- .../OCM/CloudFederationProviderFilesTest.php | 46 +++++++++ .../Federation/CloudFederationShare.php | 13 ++- openapi.json | 2 +- 7 files changed, 198 insertions(+), 10 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 932976cd681f7..ba4209e05dd65 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -89,7 +89,7 @@ public function __construct( * @param string|null $ownerDisplayName Display name of the user who shared the item * @param string|null $sharedBy Provider specific UID of the user who shared the resource * @param string|null $sharedByDisplayName Display name of the user who shared the resource - * @param array{name: string, options?: array, webdav?: array} $protocol Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]] + * @param array{name: string, options?: array, webdav?: array} $protocol Legacy format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']], new single-protocol format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]], or multi-protocol envelope per OCM spec: ['name' => 'multi', 'webdav' => ['sharedSecret' => '...', ...], 'webapp' => [...]] * @param string $shareType 'group' or 'user' share * @param string $resourceType 'file', 'calendar',... * @@ -138,11 +138,7 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ ); } - $protocolName = $protocol['name']; - $hasOldFormat = isset($protocol['options']) && is_array($protocol['options']) && isset($protocol['options']['sharedSecret']); - $hasNewFormat = isset($protocol[$protocolName]) && is_array($protocol[$protocolName]) && isset($protocol[$protocolName]['sharedSecret']); - - if (!$hasOldFormat && !$hasNewFormat) { + if (!$this->protocolCarriesSharedSecret($protocol)) { return new JSONResponse( [ 'message' => 'Missing sharedSecret in protocol', @@ -441,6 +437,39 @@ public function receiveNotification($notificationType, $resourceType, $providerI return new JSONResponse($result, Http::STATUS_CREATED); } + /** + * Check that the protocol envelope carries at least one sharedSecret. + * + * Accepts the legacy single-protocol shape ['name' => '', 'options' => ['sharedSecret' => ...]], + * the new single-protocol shape ['name' => '', '' => ['sharedSecret' => ...]] and the + * multi-protocol envelope from the OCM spec ['name' => 'multi', '' => ['sharedSecret' => ...], ...]. + * Because a payload may use 'name' => 'multi' even when it carries a single inner protocol, we do not + * gate on the name value but scan every sibling entry for the first sharedSecret. The full envelope is + * forwarded to the resource provider unchanged; the provider decides which entries it understands. + * + * @param array $protocol + * @see https://github.com/cs3org/OCM-API/ + */ + private function protocolCarriesSharedSecret(array $protocol): bool { + if ( + isset($protocol['options']) + && is_array($protocol['options']) + && isset($protocol['options']['sharedSecret']) + && is_string($protocol['options']['sharedSecret']) + ) { + return true; + } + foreach ($protocol as $key => $value) { + if ($key === 'name' || $key === 'options' || !is_array($value)) { + continue; + } + if (isset($value['sharedSecret']) && is_string($value['sharedSecret'])) { + return true; + } + } + return false; + } + /** * map login name to internal LDAP UID if a LDAP backend is in use * diff --git a/apps/cloud_federation_api/openapi.json b/apps/cloud_federation_api/openapi.json index b4656447709db..85272802c560d 100644 --- a/apps/cloud_federation_api/openapi.json +++ b/apps/cloud_federation_api/openapi.json @@ -161,7 +161,7 @@ }, "protocol": { "type": "object", - "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]]", + "description": "Legacy format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']], new single-protocol format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]], or multi-protocol envelope per OCM spec: ['name' => 'multi', 'webdav' => ['sharedSecret' => '...', ...], 'webapp' => [...]]", "required": [ "name" ], diff --git a/apps/cloud_federation_api/tests/RequestHandlerControllerTest.php b/apps/cloud_federation_api/tests/RequestHandlerControllerTest.php index 326da930d9c6f..76c9f9e5fe98d 100644 --- a/apps/cloud_federation_api/tests/RequestHandlerControllerTest.php +++ b/apps/cloud_federation_api/tests/RequestHandlerControllerTest.php @@ -18,7 +18,10 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; +use OCP\Federation\ICloudFederationProvider; use OCP\Federation\ICloudFederationProviderManager; +use OCP\Federation\ICloudFederationShare; +use OCP\Federation\ICloudId; use OCP\Federation\ICloudIdManager; use OCP\IAppConfig; use OCP\IGroupManager; @@ -126,4 +129,95 @@ public function testInviteAccepted(): void { $this->assertEquals($json, $this->requestHandlerController->inviteAccepted($recipientProvider, $token, $recipientId, $recipientEmail, $recipientName)); } + + public function testAddShareRejectsProtocolWithoutSharedSecret(): void { + // Disable signature verification so we reach the protocol validation. + $this->appConfig->method('getValueBool')->willReturn(true); + + $protocol = [ + 'name' => 'multi', + 'webdav' => ['requirements' => ['must-exchange-token']], + ]; + + $result = $this->requestHandlerController->addShare( + 'bob@https://bob.example.com', 'Jupyter', '', '8', + 'alice@alice.example.com', 'alice', 'alice@alice.example.com', 'alice', + $protocol, 'user', 'file', + ); + + $this->assertInstanceOf(JSONResponse::class, $result); + $this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus()); + $this->assertSame('Missing sharedSecret in protocol', $result->getData()['message']); + } + + public function testAddShareAcceptsMultiProtocolSharedSecret(): void { + // Disable signature verification so we reach the protocol validation. + $this->appConfig->method('getValueBool')->willReturn(true); + // No supported share types: the share passes the sharedSecret gate and is + // rejected later with 501, proving the multi envelope validated. + $this->config->method('getSupportedShareTypes')->willReturn([]); + + $protocol = [ + 'name' => 'multi', + 'webdav' => ['sharedSecret' => 'XHRcgrx1X8uZELY8kxApldZtzoreH8Wj', 'requirements' => ['must-exchange-token']], + 'webapp' => ['sharedSecret' => 'XHRcgrx1X8uZELY8kxApldZtzoreH8Wj', 'uri' => 'https://app.example/open'], + ]; + + $result = $this->requestHandlerController->addShare( + 'bob@https://bob.example.com', 'Jupyter', '', '8', + 'alice@alice.example.com', 'alice', 'alice@alice.example.com', 'alice', + $protocol, 'user', 'file', + ); + + $this->assertInstanceOf(JSONResponse::class, $result); + $this->assertEquals(Http::STATUS_NOT_IMPLEMENTED, $result->getStatus()); + $this->assertNotSame('Missing sharedSecret in protocol', $result->getData()['message'] ?? ''); + } + + public function testAddShareRoutesFolderResourceTypeMultiProtocol(): void { + // Disable signature verification so we reach the share handling. + $this->appConfig->method('getValueBool')->willReturn(true); + // The files provider is registered for both 'file' and 'folder'. + $this->config->method('getSupportedShareTypes')->with('folder')->willReturn(['user']); + + $cloudId = $this->createMock(ICloudId::class); + $cloudId->method('getUser')->willReturn('bob'); + $this->cloudIdManager->method('resolveCloudId')->willReturn($cloudId); + + $this->userManager->method('userExists')->with('bob')->willReturn(true); + + $share = $this->createMock(ICloudFederationShare::class); + $this->cloudFederationFactory->method('getCloudFederationShare')->willReturn($share); + + $provider = $this->createMock(ICloudFederationProvider::class); + $provider->expects($this->once()) + ->method('shareReceived') + ->with($share) + ->willReturn('share-id-1'); + $this->cloudFederationProviderManager->expects($this->once()) + ->method('getCloudFederationProvider') + ->with('folder') + ->willReturn($provider); + + $recipient = $this->createMock(IUser::class); + $recipient->method('getDisplayName')->willReturn('Bob'); + $recipient->method('getUID')->willReturn('bob'); + $this->userManager->method('get')->with('bob')->willReturn($recipient); + + $protocol = [ + 'name' => 'multi', + 'webdav' => ['sharedSecret' => 'XHRcgrx1X8uZELY8kxApldZtzoreH8Wj', 'requirements' => ['must-exchange-token']], + 'webapp' => ['sharedSecret' => 'XHRcgrx1X8uZELY8kxApldZtzoreH8Wj', 'uri' => 'https://app.example/open'], + ]; + + $result = $this->requestHandlerController->addShare( + 'bob@https://bob.example.com', 'Jupyter', '', '8', + 'alice@alice.example.com', 'alice', 'alice@alice.example.com', 'alice', + $protocol, 'user', 'folder', + ); + + $this->assertInstanceOf(JSONResponse::class, $result); + $this->assertEquals(Http::STATUS_CREATED, $result->getStatus()); + $this->assertSame('Bob', $result->getData()['recipientDisplayName']); + } } diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index f0b6843c6499a..ba02696991549 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -97,7 +97,15 @@ public function shareReceived(ICloudFederationShare $share): string { } $protocol = $share->getProtocol(); - if ($protocol['name'] !== 'webdav') { + $protocolName = $protocol['name'] ?? ''; + // The files provider serves the webdav protocol. It is named directly in the + // legacy and new single-protocol formats, and carried as a sibling entry in + // the OCM multi-protocol envelope (name => 'multi'). + if ($protocolName === 'multi') { + if (!isset($protocol['webdav']) || !is_array($protocol['webdav'])) { + throw new ProviderCouldNotAddShareException('Unsupported protocol for data exchange.', '', Http::STATUS_NOT_IMPLEMENTED); + } + } elseif ($protocolName !== 'webdav') { throw new ProviderCouldNotAddShareException('Unsupported protocol for data exchange.', '', Http::STATUS_NOT_IMPLEMENTED); } diff --git a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php index 15cd8078eadf7..be482490e9d10 100644 --- a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php +++ b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php @@ -151,6 +151,52 @@ private function buildShare(array $requirements = []): ICloudFederationShare&Moc return $share; } + private function buildMultiShare(array $requirements = []): ICloudFederationShare&MockObject { + $share = $this->createMock(ICloudFederationShare::class); + // OCM multi-protocol envelope: name => 'multi' with the webdav entry as a + // sibling, alongside other protocols (e.g. webapp) the files provider ignores. + $share->method('getProtocol')->willReturn([ + 'name' => 'multi', + 'webdav' => ['sharedSecret' => 'refresh-token-abc', 'requirements' => $requirements], + 'webapp' => ['sharedSecret' => 'refresh-token-abc', 'uri' => 'https://app.example/open'], + ]); + $share->method('getOwner')->willReturn('owner@example.com'); + $share->method('getOwnerDisplayName')->willReturn('Owner Name'); + $share->method('getShareSecret')->willReturn('refresh-token-abc'); + $share->method('getResourceName')->willReturn('/SharedFolder'); + $share->method('getShareWith')->willReturn('localuser'); + $share->method('getProviderId')->willReturn('42'); + $share->method('getSharedBy')->willReturn('owner@example.com'); + $share->method('getShareType')->willReturn('user'); + return $share; + } + + /** + * A multi-protocol envelope (name => 'multi') must be accepted by serving its + * webdav entry. We drive through to the user lookup guard to prove the protocol + * check passed rather than rejecting the share as unsupported. + */ + public function testShareReceivedAcceptsMultiProtocolEnvelope(): void { + $this->enableS2S(); + + $this->addressHandler->method('splitUserRemote') + ->with('owner@example.com') + ->willReturn(['owner', 'https://example.com/']); + + $share = $this->buildMultiShare(); + + $this->discoveryService->method('discover') + ->willThrowException(new \Exception('network error')); + + $this->userManager->method('get')->with('localuser')->willReturn(null); + $this->filenameValidator->method('isFilenameValid')->willReturn(true); + + $this->expectException(ProviderCouldNotAddShareException::class); + $this->expectExceptionMessage('User does not exists'); + + $this->provider->shareReceived($share); + } + /** * When must-exchange-token is required but the remote has no token endpoint, * shareReceived must throw rather than silently accept the share. diff --git a/lib/private/Federation/CloudFederationShare.php b/lib/private/Federation/CloudFederationShare.php index 2da089f92596b..0ffcc6b2db302 100644 --- a/lib/private/Federation/CloudFederationShare.php +++ b/lib/private/Federation/CloudFederationShare.php @@ -377,9 +377,20 @@ public function getShareSecret() { if (isset($protocol['name'])) { $protocolName = $protocol['name']; - if (isset($protocol[$protocolName]['sharedSecret'])) { + // New single-protocol format: the secret lives under the protocol name. + if (isset($protocol[$protocolName]['sharedSecret']) && is_string($protocol[$protocolName]['sharedSecret'])) { return $protocol[$protocolName]['sharedSecret']; } + // Multi-protocol envelope (name => 'multi'): the secret lives in one of + // the sibling protocol entries, which all carry the same share secret. + foreach ($protocol as $key => $value) { + if ($key === 'name' || $key === 'options' || !is_array($value)) { + continue; + } + if (isset($value['sharedSecret']) && is_string($value['sharedSecret'])) { + return $value['sharedSecret']; + } + } } return ''; diff --git a/openapi.json b/openapi.json index de54aa42989c2..30e23dfd35d82 100644 --- a/openapi.json +++ b/openapi.json @@ -18131,7 +18131,7 @@ }, "protocol": { "type": "object", - "description": "Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]]", + "description": "Legacy format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']], new single-protocol format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]], or multi-protocol envelope per OCM spec: ['name' => 'multi', 'webdav' => ['sharedSecret' => '...', ...], 'webapp' => [...]]", "required": [ "name" ],