From 4ac59275c335ae950fce5ea0ec47d2839d423a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:14:35 +0100 Subject: [PATCH 01/58] feat: Bearer auth aware Sabre HTTP client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/Files/Storage/DAV.php | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 8ccfe30513686..4696505382971 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -38,6 +38,43 @@ use Sabre\HTTP\ClientHttpException; use Sabre\HTTP\RequestInterface; +/* + * Class BearerAuthAwareSabreClient + * + * This is an extension of the Sabre HTTP Client + * to provide it with the ability to make bearer authn requests. + * + * @package OC\Files\Storage + */ +class BearerAuthAwareSabreClient extends Client +{ + /** + * Bearer authentication. + */ + const AUTH_BEARER = 8; + + /** + * Constructor. + * + * See Sabre\DAV\Client + * + */ + public function __construct(array $settings) + { + parent::__construct($settings); + + if (isset($settings['userName']) && isset($settings['authType']) && ($settings['authType'] & self::AUTH_BEARER)) { + $userName = $settings['userName']; + + $curlType = $this->curlSettings[CURLOPT_HTTPAUTH]; + $curlType |= CURLAUTH_BEARER; + + $this->addCurlSetting(CURLOPT_HTTPAUTH, $curlType); + $this->addCurlSetting(CURLOPT_XOAUTH2_BEARER, $userName); + } + } +} + /** * Class DAV * From f0a8e79f3e2613732d0d65fbf277585ac3db3ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:15:39 +0100 Subject: [PATCH 02/58] feat(dav): Add token endpoint to exchange refresh tokens for access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/Controller/TokenController.php | 198 ++++++++++++++++++ lib/private/OCM/Model/OCMProvider.php | 32 +++ lib/private/OCM/OCMDiscoveryService.php | 8 +- 5 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 apps/dav/lib/Controller/TokenController.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 361a57cee407b..4bc194f1e300d 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -263,6 +263,7 @@ 'OCA\\DAV\\Controller\\ExampleContentController' => $baseDir . '/../lib/Controller/ExampleContentController.php', 'OCA\\DAV\\Controller\\InvitationResponseController' => $baseDir . '/../lib/Controller/InvitationResponseController.php', 'OCA\\DAV\\Controller\\OutOfOfficeController' => $baseDir . '/../lib/Controller/OutOfOfficeController.php', + 'OCA\\DAV\\Controller\\TokenController' => $baseDir . '/../lib/Controller/TokenController.php', 'OCA\\DAV\\Controller\\UpcomingEventsController' => $baseDir . '/../lib/Controller/UpcomingEventsController.php', 'OCA\\DAV\\DAV\\CustomPropertiesBackend' => $baseDir . '/../lib/DAV/CustomPropertiesBackend.php', 'OCA\\DAV\\DAV\\GroupPrincipalBackend' => $baseDir . '/../lib/DAV/GroupPrincipalBackend.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 959860e3312a8..e4904517a3895 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -278,6 +278,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Controller\\ExampleContentController' => __DIR__ . '/..' . '/../lib/Controller/ExampleContentController.php', 'OCA\\DAV\\Controller\\InvitationResponseController' => __DIR__ . '/..' . '/../lib/Controller/InvitationResponseController.php', 'OCA\\DAV\\Controller\\OutOfOfficeController' => __DIR__ . '/..' . '/../lib/Controller/OutOfOfficeController.php', + 'OCA\\DAV\\Controller\\TokenController' => __DIR__ . '/..' . '/../lib/Controller/TokenController.php', 'OCA\\DAV\\Controller\\UpcomingEventsController' => __DIR__ . '/..' . '/../lib/Controller/UpcomingEventsController.php', 'OCA\\DAV\\DAV\\CustomPropertiesBackend' => __DIR__ . '/..' . '/../lib/DAV/CustomPropertiesBackend.php', 'OCA\\DAV\\DAV\\GroupPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/GroupPrincipalBackend.php', diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php new file mode 100644 index 0000000000000..24093fdc8fa71 --- /dev/null +++ b/apps/dav/lib/Controller/TokenController.php @@ -0,0 +1,198 @@ +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'); + } + } + + /** + * Exchange a refresh token for a short-lived 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 + */ + #[PublicPage] + #[NoCSRFRequired] + #[FrontpageRoute(verb: 'POST', url: '/api/v1/access-token')] + public function accessToken(): 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 + ); + } + + $body = file_get_contents('php://input'); + $data = json_decode($body, true); + + if (!is_array($data)) { + return new DataResponse( + ['error' => 'invalid_request'], + Http::STATUS_BAD_REQUEST + ); + } + + $refreshToken = $data['code'] ?? ''; + $grantType = $data['grant_type'] ?? ''; + + if ($grantType !== 'authorization_code') { + return new DataResponse( + ['error' => 'unsupported_grant_type'], + Http::STATUS_BAD_REQUEST + ); + } + + if (empty($refreshToken)) { + return new DataResponse( + ['error' => 'refresh_token is required'], + Http::STATUS_BAD_REQUEST + ); + } + + 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 + ); + } + + $accessTokenString = $this->random->generate( + 72, + ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS + ); + + $expiresIn = 3600; // 1 hour in seconds + $expiresAt = $this->timeFactory->getTime() + $expiresIn; + + $accessToken = $this->tokenProvider->generateToken( + $accessTokenString, + $refreshToken, // Keep refresh token with access token as UID + $token->getLoginName(), + null, // No password for access tokens + 'OCM Access Token', + IToken::TEMPORARY_TOKEN, + IToken::DO_NOT_REMEMBER + ); + + $accessToken->setExpires($expiresAt); + $this->tokenProvider->updateToken($accessToken); + + 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/lib/private/OCM/Model/OCMProvider.php b/lib/private/OCM/Model/OCMProvider.php index bbbace0d882c6..928aa74613642 100644 --- a/lib/private/OCM/Model/OCMProvider.php +++ b/lib/private/OCM/Model/OCMProvider.php @@ -24,6 +24,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; @@ -111,6 +112,27 @@ public function getEndPoint(): string { return $this->endPoint; } + /** + * @param string $tokenEndPoint + * + * @return $this + */ + public function setTokenEndPoint(string $endPoint): static { + $this->tokenEndPoint = $endPoint; + + return $this; + } + + /** + * @return string + */ + public function getTokenEndPoint(): string { + if (in_array('exchange-token', $this->capabilities)) { + return $this->tokenEndPoint; + } + return ''; + } + /** * @return string */ @@ -250,6 +272,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'); @@ -289,6 +317,10 @@ public function jsonSerialize(): array { if ($capabilities) { $response['capabilities'] = $capabilities; } + $tokenEndpoint = $this->getTokenEndPoint(); + if ($tokenEndpoint) { + $response['tokenEndPoint'] = $tokenEndpoint; + } $inviteAcceptDialog = $this->getInviteAcceptDialog(); if ($inviteAcceptDialog !== '') { $response['inviteAcceptDialog'] = $inviteAcceptDialog; diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index 17a84c12d5007..00e3842a2807e 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -49,7 +49,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 = []; @@ -91,6 +91,7 @@ public function discover(string $remote, bool $skipCache = false): IOCMProvider } if (array_key_exists($remote, $this->remoteProviders)) { + return $this->remoteProviders[$remote]; } @@ -127,7 +128,6 @@ public function discover(string $remote, bool $skipCache = false): IOCMProvider $remote . '/ocm-provider', ]; - foreach ($urls as $url) { $exception = null; $body = null; @@ -191,6 +191,7 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { } $url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare'); + $tokenUrl = $this->urlGenerator->linkToRouteAbsolute('dav.Token.accessToken'); $pos = strrpos($url, '/'); if ($pos === false) { $this->logger->debug('generated route should contain a slash character'); @@ -200,7 +201,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); // The inviteAcceptDialog is available from the contacts app, if this config value is set $inviteAcceptDialog = $this->appConfig->getValueString('core', ConfigLexicon::OCM_INVITE_ACCEPT_DIALOG); From 640283d2248a7b1305987e7ea2f472f15bff93a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:16:51 +0100 Subject: [PATCH 03/58] feat(dav): Add Bearer auth backend for webdav requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/appinfo/v1/publicwebdav.php | 16 ++++++++++++++-- apps/dav/lib/Connector/Sabre/BearerAuth.php | 12 ++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 7b576496cc89f..6788933ad1473 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -9,6 +9,7 @@ use OC\Files\Storage\Wrapper\PermissionsMask; 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/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index 23453ae8efbab..ba0228a70d64e 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -12,6 +12,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; @@ -23,6 +26,7 @@ public function __construct( private IRequest $request, private IConfig $config, private string $principalPrefix = 'principals/users/', + private string $token = '', ) { // setup realm $defaults = new Defaults(); @@ -40,6 +44,7 @@ private function setupUserFs($userId) { */ public function validateBearerToken($bearerToken) { \OC_Util::setupFS(); + $this->token = $bearerToken; if (!$this->userSession->isLoggedIn()) { $this->userSession->tryTokenLogin($this->request); @@ -51,6 +56,13 @@ public function validateBearerToken($bearerToken) { return false; } + public function getShare(): IShare { + $shareManager = Server::get(IManager::class); + $share = $shareManager->getShareByToken($this->token); + assert($share !== null); + 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 a6f40b6e284eac8368316b77b58ff9b2aadd048c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:17:59 +0100 Subject: [PATCH 04/58] feat(dav): New method doTryTokenLogin to allow to try token login with token rather that request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 3 +++ lib/private/User/Session.php | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index ba0228a70d64e..609a47dbbe6de 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -49,6 +49,9 @@ public function validateBearerToken($bearerToken) { if (!$this->userSession->isLoggedIn()) { $this->userSession->tryTokenLogin($this->request); } + if (!$this->userSession->isLoggedIn()) { + $this->userSession->doTryTokenLogin($bearerToken); + } if ($this->userSession->isLoggedIn()) { return $this->setupUserFs($this->userSession->getUser()->getUID()); } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 811c5ba4bc326..8664be8656812 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -830,6 +830,10 @@ public function tryTokenLogin(IRequest $request) { } else { return false; } + return $this->doTryTokenLogin($token); + } + + public function doTryTokenLogin($token) { if (!$this->loginWithToken($token)) { return false; From 1c15da210b2fa06f7f6c36913dccf2e433fb0d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:18:45 +0100 Subject: [PATCH 05/58] feat(federatedfilesharing): Create permanent refresh token when creating a remote share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/FederatedShareProvider.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 1302f2ca980af..92872431ab68a 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -7,6 +7,8 @@ */ namespace OCA\FederatedFileSharing; +use OC\Authentication\Token\IToken; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; use OC\Share20\Share; use OCP\Constants; @@ -22,6 +24,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; @@ -170,7 +174,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(); + $pass = $share->getPassword(); + + $dbToken = $provider->generateToken($token, $uid, $uid, $pass, $name, type: IToken::PERMANENT_TOKEN); $shareId = $this->addShareToDB( $share->getNodeId(), $share->getNodeType(), From a8c0b0ad2ec13976f5befc7dbd412d800c29a0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:19:42 +0100 Subject: [PATCH 06/58] feat(federatedfilesharing): When a remote requests a share with a token, it may be an access token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../Controller/RequestHandlerController.php | 8 ++++ apps/dav/lib/Controller/TokenController.php | 17 ++++---- .../lib/FederatedShareProvider.php | 14 +++++++ lib/private/Files/Storage/DAV.php | 40 +++++++++---------- lib/private/Share20/Manager.php | 16 ++++++++ lib/private/User/Session.php | 27 +++++++++++-- 6 files changed, 89 insertions(+), 33 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index bfccb2fe20eaf..0567eaa1be118 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -7,6 +7,7 @@ namespace OCA\CloudFederationAPI\Controller; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\OCM\OCMSignatoryManager; use OCA\CloudFederationAPI\Config; use OCA\CloudFederationAPI\Db\FederatedInviteMapper; @@ -43,6 +44,7 @@ use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; use OCP\Security\Signature\IIncomingSignedRequest; use OCP\Security\Signature\ISignatureManager; +use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; use OCP\Util; use Psr\Log\LoggerInterface; @@ -490,6 +492,12 @@ private function confirmNotificationIdentity( $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); if ($provider instanceof ISignedCloudFederationProvider) { $identity = $provider->getFederationIdFromSharedSecret($sharedSecret, $notification); + if ($identity === '') { + $tokenProvider = Server::get(PublicKeyTokenProvider::class); + $accessTokenDb = $tokenProvider->getToken($sharedSecret); + $refreshToken = $accessTokenDb->getUID(); + $identity = $provider->getFederationIdFromSharedSecret($refreshToken, $notification); + } } else { $this->logger->debug('cloud federation provider {provider} does not implements ISignedCloudFederationProvider', ['provider' => $provider::class]); return; diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index 24093fdc8fa71..afc63d8e9ba23 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -7,7 +7,14 @@ namespace OCA\DAV\Controller; +use NCU\Security\Signature\Exceptions\IncomingRequestException; +use NCU\Security\Signature\Exceptions\SignatoryNotFoundException; +use NCU\Security\Signature\Exceptions\SignatureException; +use NCU\Security\Signature\Exceptions\SignatureNotFoundException; +use NCU\Security\Signature\ISignatureManager; use OC\Authentication\Token\IProvider; +use OC\OCM\OCMSignatoryManager; +use OC\Security\Signature\Model\IncomingSignedRequest; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -18,18 +25,10 @@ use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Token\IToken; +use OCP\IAppConfig; use OCP\IRequest; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; -use OCP\IAppConfig; -use OC\OCM\OCMSignatoryManager; -use NCU\Security\Signature\ISignatureManager; -use NCU\Security\Signature\Exceptions\SignatureNotFoundException; -use NCU\Security\Signature\Exceptions\SignatureException; -use NCU\Security\Signature\Exceptions\SignatoryNotFoundException; -use NCU\Security\Signature\Model\IIncomingSignedRequest; -use NCU\Security\Signature\Exceptions\IncomingRequestException; -use OC\Security\Signature\Model\IncomingSignedRequest; /** * Controller for the /token endpoint diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 92872431ab68a..9d8aa9e4ae7c7 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -724,6 +724,20 @@ public function getShareByToken(string $token): IShare { $data = $cursor->fetchAssociative(); + if ($data === false) { + + $provider = Server::get(PublicKeyTokenProvider::class); + $accessTokenDb = $provider->getToken($token); + $refreshToken = $accessTokenDb->getUID(); + + $cursor = $qb->select('*') + ->from('share') + ->where($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($qb->expr()->eq('token', $qb->createNamedParameter($refreshToken))) + ->executeQuery(); + + $data = $cursor->fetch(); + } if ($data === false) { throw new ShareNotFound('Share not found', $this->l->t('Could not find share')); } diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 4696505382971..32d08e5d0e5bd 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -46,33 +46,31 @@ * * @package OC\Files\Storage */ -class BearerAuthAwareSabreClient extends Client -{ - /** - * Bearer authentication. - */ - const AUTH_BEARER = 8; - - /** - * Constructor. +class BearerAuthAwareSabreClient extends Client { + /** + * Bearer authentication. + */ + public const AUTH_BEARER = 8; + + /** + * Constructor. * * See Sabre\DAV\Client * - */ - public function __construct(array $settings) - { - parent::__construct($settings); + */ + public function __construct(array $settings) { + parent::__construct($settings); - if (isset($settings['userName']) && isset($settings['authType']) && ($settings['authType'] & self::AUTH_BEARER)) { - $userName = $settings['userName']; + if (isset($settings['userName']) && isset($settings['authType']) && ($settings['authType'] & self::AUTH_BEARER)) { + $userName = $settings['userName']; - $curlType = $this->curlSettings[CURLOPT_HTTPAUTH]; - $curlType |= CURLAUTH_BEARER; + $curlType = $this->curlSettings[CURLOPT_HTTPAUTH]; + $curlType |= CURLAUTH_BEARER; - $this->addCurlSetting(CURLOPT_HTTPAUTH, $curlType); - $this->addCurlSetting(CURLOPT_XOAUTH2_BEARER, $userName); - } - } + $this->addCurlSetting(CURLOPT_HTTPAUTH, $curlType); + $this->addCurlSetting(CURLOPT_XOAUTH2_BEARER, $userName); + } + } } /** diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index db5fcf5ae94e3..9c696e56f290e 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -8,6 +8,7 @@ namespace OC\Share20; use ArrayIterator; +use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Core\AppInfo\ConfigLexicon; use OC\Files\Filesystem; use OC\Files\Mount\MoveableMount; @@ -44,6 +45,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; @@ -1435,6 +1437,20 @@ 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 $e) { + } catch (ShareNotFound $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/lib/private/User/Session.php b/lib/private/User/Session.php index 8664be8656812..6786e0156c850 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -619,14 +619,35 @@ private function loginWithToken($token) { // Ignore and use empty string instead } - $this->manager->emit('\OC\User', 'preLogin', [$dbToken->getLoginName(), $password]); - $user = $this->manager->get($uid); if (is_null($user)) { + // Maybe this is an access token. We keep the refresh tokens as UID of access tokens + try { + $token = $uid; + $dbToken = $this->tokenProvider->getToken($token); + } catch (InvalidTokenException $ex) { + return false; + } + $uid = $dbToken->getUID(); + + // When logging in with token, the password must be decrypted first before passing to login hook + $password = ''; + try { + $password = $this->tokenProvider->getPassword($dbToken, $token); + } catch (PasswordlessTokenException $ex) { + // Ignore and use empty string instead + } // user does not exist - return false; + $user = $this->manager->get($uid); + if (is_null($user)) { + return false; + } } + $this->manager->emit('\OC\User', 'preLogin', [$dbToken->getLoginName(), $password]); + + // See line 173 in this module, needed for completeLogin + OC_User::setIncognitoMode(false); return $this->completeLogin( $user, [ From d221f9896045b69fb4cb7a6a27972faf612c5226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:20:39 +0100 Subject: [PATCH 07/58] feat(files_sharing): When requesting a remote share with bearer auth, get an access token to use as bearer token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/lib/External/Storage.php | 11 +- lib/private/Files/Storage/DAV.php | 109 +++++++++++++++++++- 2 files changed, 113 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 68c69e9e7c87f..b472bb2ceb26e 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -72,10 +72,16 @@ public function __construct($options) { $ocmProvider = $discoveryService->discover($this->cloudId->getRemote()); $webDavEndpoint = $ocmProvider->extractProtocolEntry('file', 'webdav'); $remote = $ocmProvider->getEndPoint(); + $authType = \Sabre\DAV\Client::AUTH_BASIC; + $capabilities = $ocmProvider->getCapabilities(); + if (in_array('exchange-token', $capabilities)) { + $authType = \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER; + } } 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; } $host = parse_url($remote, PHP_URL_HOST); @@ -98,8 +104,9 @@ public function __construct($options) { 'host' => $host, 'root' => $webDavEndpoint, 'user' => $options['token'], - 'authType' => \Sabre\DAV\Client::AUTH_BASIC, - 'password' => (string)$options['password'] + 'authType' => $authType, + 'password' => (string)$options['password'], + 'discoveryService' => $discoveryService, ] ); } diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 32d08e5d0e5bd..cca02b3be8b35 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -10,8 +10,10 @@ use Exception; use Icewind\Streams\CallbackWrapper; use Icewind\Streams\IteratorDirectory; +use NCU\Security\Signature\ISignatureManager; use OC\Files\Filesystem; use OC\MemCache\ArrayCache; +use OC\OCM\OCMSignatoryManager; use OCP\AppFramework\Http; use OCP\Constants; use OCP\Diagnostics\IEventLogger; @@ -24,10 +26,14 @@ 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\Lock\LockedException; +use OCP\OCM\Exceptions\OCMArgumentException; +use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; use OCP\Server; use OCP\Util; use Psr\Http\Message\ResponseInterface; @@ -38,7 +44,7 @@ use Sabre\HTTP\ClientHttpException; use Sabre\HTTP\RequestInterface; -/* +/** * Class BearerAuthAwareSabreClient * * This is an extension of the Sabre HTTP Client @@ -107,6 +113,10 @@ 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; /** @var int */ private $timeout; @@ -129,6 +139,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() @@ -169,6 +184,9 @@ 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); } protected function init(): void { @@ -177,9 +195,82 @@ protected function init(): void { } $this->ready = true; + // If using Bearer auth, exchange refresh token for access token + $userName = $this->user; + if ($this->authType !== null && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER)) { + try { + $host = 'https://' . $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(); + $payload = [ + 'grant_type' => 'authorization_code', + 'client_id' => 'receiver.example.org', + 'code' => $this->user, + ]; + + $options = [ + 'body' => json_encode($payload), + 'headers' => [ + 'Content-Type' => 'application/json', + ], + 'timeout' => 10, + 'connect_timeout' => 10, + ]; + + // Try signing the request + if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + $this->signatoryManager, + $options, + 'post', + $tokenEndpoint + ); + $this->logger->debug('Token request signed successfully', ['app' => 'dav']); + } catch (\Exception $e) { + $this->logger->warning('Failed to sign token request, continuing without signature', [ + 'app' => 'dav', + 'exception' => $e, + 'endpoint' => $tokenEndpoint, + ]); + } + } + + $response = $client->post($tokenEndpoint, $options); + + $body = $response->getBody(); + $data = json_decode($body, true); + + if (isset($data['access_token'])) { + $userName = $data['access_token']; + $this->user = $userName; + $this->logger->debug('Successfully exchanged refresh token for access token', ['app' => 'dav']); + } else { + $this->logger->error('Failed to get access token from response', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not obtain access token'); + } + } catch (OCMProviderException|OCMArgumentException $e) { + $this->logger->error('OCM provider response missing tokenEndPoint', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not discover token endpoint'); + } 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()); + } + } + $settings = [ 'baseUri' => $this->createBaseUri(), - 'userName' => $this->user, + 'userName' => $userName, 'password' => $this->password, ]; if ($this->authType !== null) { @@ -191,7 +282,7 @@ protected function init(): void { $settings['proxy'] = $proxy; } - $this->client = new Client($settings); + $this->client = new BearerAuthAwareSabreClient($settings); $this->client->setThrowExceptions(true); if ($this->secure === true) { @@ -398,10 +489,14 @@ public function fopen(string $path, string $mode) { case 'r': case 'rb': try { + $auth = [$this->user, $this->password]; + if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { + $auth = [$this->user, '', 'bearer']; + } $response = $this->httpClientService ->newClient() ->get($this->createBaseUri() . $this->encodePath($path), [ - 'auth' => [$this->user, $this->password], + 'auth' => $auth, 'stream' => true, // set download timeout for users with slow connections or large files 'timeout' => $this->timeout, @@ -549,11 +644,15 @@ protected function uploadFile(string $path, string $target): void { $this->statCache->remove($target); $source = fopen($path, 'r'); + $auth = [$this->user, $this->password]; + if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { + $auth = [$this->user, '', 'bearer']; + } $this->httpClientService ->newClient() ->put($this->createBaseUri() . $this->encodePath($target), [ 'body' => $source, - 'auth' => [$this->user, $this->password], + 'auth' => $auth, // set upload timeout for users with slow connections or large files 'timeout' => $this->timeout, 'verify' => $this->verify, From a86a4af286c163ac73f633195d0fcd9f76a5f433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:21:38 +0100 Subject: [PATCH 08/58] feat: adapt to guzzle api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/Files/Storage/DAV.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index cca02b3be8b35..858c4f7d4f6e3 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -490,12 +490,15 @@ public function fopen(string $path, string $mode) { case 'rb': try { $auth = [$this->user, $this->password]; + $headers = []; if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = [$this->user, '', 'bearer']; + $auth = []; + $headers = ['Authorization' => 'Bearer ' . $this->user]; } $response = $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 @@ -645,13 +648,16 @@ protected function uploadFile(string $path, string $target): void { $source = fopen($path, 'r'); $auth = [$this->user, $this->password]; + $headers = []; if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = [$this->user, '', 'bearer']; + $auth = []; + $headers = ['Authorization' => 'Bearer ' . $this->user]; } $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, From ca6ba1982dc16c9654adb91470c0e4b175b74a66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 23 Dec 2025 13:22:30 +0100 Subject: [PATCH 09/58] feat(cloud_federation_api): adapt to new format for share creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../Controller/RequestHandlerController.php | 19 +++++-- .../Federation/CloudFederationFactory.php | 57 ++++++++++++++++++- .../Federation/CloudFederationShare.php | 46 ++++++++++++--- lib/private/Server.php | 6 +- 4 files changed, 114 insertions(+), 14 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 0567eaa1be118..87e2854d9ec80 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -93,7 +93,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',... * @@ -128,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( [ @@ -141,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( diff --git a/lib/private/Federation/CloudFederationFactory.php b/lib/private/Federation/CloudFederationFactory.php index d06de0f2f588e..d7b5d79efcbbd 100644 --- a/lib/private/Federation/CloudFederationFactory.php +++ b/lib/private/Federation/CloudFederationFactory.php @@ -9,8 +9,18 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationNotification; use OCP\Federation\ICloudFederationShare; +use OCP\Federation\ICloudIdManager; +use OCP\OCM\IOCMDiscoveryService; +use OCP\OCM\Exceptions\OCMProviderException; +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 * @@ -30,7 +40,52 @@ class CloudFederationFactory implements ICloudFederationFactory { * @since 14.0.0 */ 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 = in_array('exchange-token', $capabilities, true); + + $this->logger->debug('OCM provider capabilities discovered', [ + 'remote' => $remoteDomain, + 'capabilities' => $capabilities, + '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 6bd35cea763e4..2befc99cb5035 100644 --- a/lib/private/Federation/CloudFederationShare.php +++ b/lib/private/Federation/CloudFederationShare.php @@ -40,6 +40,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 = '', @@ -52,6 +54,8 @@ public function __construct($shareWith = '', $shareType = '', $resourceType = '', $sharedSecret = '', + $useExchangeToken = false, + $remoteDomain = null, ) { $this->setShareWith($shareWith); $this->setResourceName($name); @@ -61,13 +65,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); } @@ -328,7 +346,19 @@ public function getShareType() { * @since 14.0.0 */ 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/Server.php b/lib/private/Server.php index e24d47f3f0611..636ed60f22e98 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1213,7 +1213,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); From 60266b76b96de6fc1a3df493d17d0e18b3b8cfd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 8 Jan 2026 12:25:54 +0100 Subject: [PATCH 10/58] feat(cloud_federation_api): support multi protocol for share creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/Controller/RequestHandlerController.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 87e2854d9ec80..7f323b9f3086c 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -93,7 +93,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 Old format: ['name' => 'webdav', 'options' => ['sharedSecret' => '...', 'permissions' => '...']] or New format: ['name' => 'webdav', 'webdav' => ['uri' => '...', 'sharedSecret' => '...', 'permissions' => [...]]] or Multi format: ['name' => 'multi', 'webdav' => [...]] * @param string $shareType 'group' or 'user' share * @param string $resourceType 'file', 'calendar',... * @@ -142,7 +142,20 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ $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) { + // For multi-protocol, we only consider webdav + $hasMultiFormat = false; + if ($protocolName === 'multi') { + if (isset($protocol['webdav']) && is_array($protocol['webdav']) && isset($protocol['webdav']['sharedSecret'])) { + $hasMultiFormat = true; + $protocol = [ + 'name' => 'webdav', + 'webdav' => $protocol['webdav'] + ]; + $protocolName = 'webdav'; + } + } + + if (!$hasOldFormat && !$hasNewFormat && !$hasMultiFormat) { return new JSONResponse( [ 'message' => 'Missing sharedSecret in protocol', From 64e8b9ae0a23f62212c190f062426f7de29d1540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 8 Jan 2026 12:26:26 +0100 Subject: [PATCH 11/58] fix(dav): data sent to token endpoint must be application/x-www-form-urlencoded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Controller/TokenController.php | 9 +-------- lib/private/Files/Storage/DAV.php | 10 +++++++--- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index afc63d8e9ba23..11c4890c57d41 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -103,14 +103,7 @@ public function accessToken(): DataResponse { } $body = file_get_contents('php://input'); - $data = json_decode($body, true); - - if (!is_array($data)) { - return new DataResponse( - ['error' => 'invalid_request'], - Http::STATUS_BAD_REQUEST - ); - } + parse_str($body, $data); $refreshToken = $data['code'] ?? ''; $grantType = $data['grant_type'] ?? ''; diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 858c4f7d4f6e3..389f52b9517c0 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -30,6 +30,7 @@ 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; @@ -117,6 +118,7 @@ class DAV extends Common { protected ISignatureManager $signatureManager; protected OCMSignatoryManager $signatoryManager; protected IAppConfig $appConfig; + protected IURLGenerator $urlGenerator; /** @var int */ private $timeout; @@ -187,6 +189,7 @@ public function __construct(array $parameters) { $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 { @@ -209,16 +212,17 @@ protected function init(): void { } $client = $this->httpClientService->newClient(); + $clientId = parse_url($this->urlGenerator->getAbsoluteURL('/'), PHP_URL_HOST); $payload = [ 'grant_type' => 'authorization_code', - 'client_id' => 'receiver.example.org', + 'client_id' => $clientId, 'code' => $this->user, ]; $options = [ - 'body' => json_encode($payload), + 'body' => http_build_query($payload), 'headers' => [ - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/x-www-form-urlencoded', ], 'timeout' => 10, 'connect_timeout' => 10, From 2c373e5c7f7662ca4917afc36d1fd50644d0b046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 9 Jan 2026 16:22:24 +0100 Subject: [PATCH 12/58] fix(dav): when receiving a share, account for the must-exchange-token requirement, in addition to the exchaange-token capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Controller/TokenController.php | 2 +- .../lib/FederatedShareProvider.php | 30 ++-- .../lib/OCM/CloudFederationProviderFiles.php | 83 ++++++++- apps/files_sharing/lib/External/Manager.php | 19 +++ apps/files_sharing/lib/External/Storage.php | 68 +++++++- lib/private/Files/Storage/DAV.php | 161 ++++++++++-------- 6 files changed, 275 insertions(+), 88 deletions(-) diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index 11c4890c57d41..b942193e35eb0 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -136,7 +136,7 @@ public function accessToken(): DataResponse { } $accessTokenString = $this->random->generate( - 72, + 64, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS ); diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 9d8aa9e4ae7c7..d2dabc9cffd6e 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -10,6 +10,7 @@ use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; +use OCP\Authentication\Exceptions\InvalidTokenException; use OC\Share20\Share; use OCP\Constants; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -725,18 +726,23 @@ public function getShareByToken(string $token): IShare { $data = $cursor->fetchAssociative(); if ($data === false) { - - $provider = Server::get(PublicKeyTokenProvider::class); - $accessTokenDb = $provider->getToken($token); - $refreshToken = $accessTokenDb->getUID(); - - $cursor = $qb->select('*') - ->from('share') - ->where($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) - ->andWhere($qb->expr()->eq('token', $qb->createNamedParameter($refreshToken))) - ->executeQuery(); - - $data = $cursor->fetch(); + // Token not found as refresh token, try looking it up as access token + try { + $provider = Server::get(PublicKeyTokenProvider::class); + $accessTokenDb = $provider->getToken($token); + $refreshToken = $accessTokenDb->getUID(); + + $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($refreshToken))) + ->executeQuery(); + + $data = $cursor->fetch(); + } catch (InvalidTokenException) { + // Token is not a valid access token, 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/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 3ae6366fabe63..84e9e1de57355 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -41,6 +41,8 @@ use OCP\Notification\IManager as INotificationManager; use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; +use OCP\Http\Client\IClientService; +use OCP\OCM\IOCMDiscoveryService; use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; @@ -70,6 +72,8 @@ public function __construct( private readonly IProviderFactory $shareProviderFactory, private readonly ISetupManager $setupManager, private readonly ExternalShareMapper $externalShareMapper, + private readonly IOCMDiscoveryService $discoveryService, + private readonly IClientService $clientService, ) { } @@ -106,6 +110,31 @@ 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, '/')); + $capabilities = $ocmProvider->getCapabilities(); + if (in_array('exchange-token', $capabilities)) { + $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) { @@ -146,8 +175,8 @@ public function shareReceived(ICloudFederationShare $share): string { $externalShare->generateId(); $externalShare->setRemote($remote); $externalShare->setRemoteId($remoteId); - $externalShare->setShareToken($token); - $externalShare->setPassword(''); + $externalShare->setShareToken($token); // refresh token (sharedSecret) + $externalShare->setPassword($accessToken); // access token (empty if no token exchange) $externalShare->setName($name); $externalShare->setOwner($owner); $externalShare->setShareType($shareType); @@ -684,4 +713,54 @@ 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, + ]; + + $response = $client->post($tokenEndpoint, [ + 'body' => http_build_query($payload), + 'headers' => [ + 'Content-Type' => 'application/x-www-form-urlencoded', + ], + 'timeout' => 10, + 'connect_timeout' => 10, + ]); + + $data = json_decode($response->getBody(), true); + + if (isset($data['access_token'])) { + $this->logger->debug('Successfully exchanged token for access token', ['remote' => $remote]); + return $data['access_token']; + } + + $this->logger->warning('Token exchange response missing access_token', ['remote' => $remote, 'response' => $data]); + return null; + } catch (\Exception $e) { + $this->logger->warning('Failed to exchange token', ['remote' => $remote, 'exception' => $e]); + return null; + } + } } diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 9693e52439b89..8b831fd72e71a 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -566,4 +566,23 @@ public function getAcceptedShares(): array { return []; } } + + /** + * Update the access token for a share. + * + * @param string $shareToken The share token (refresh token) to identify the share + * @param string $accessToken The new access token to store + */ + public function updateAccessToken(string $shareToken, string $accessToken): void { + try { + $share = $this->externalShareMapper->getShareByToken($shareToken); + $share->setPassword($accessToken); + $this->externalShareMapper->update($share); + $this->logger->debug('Updated access token for share', ['shareToken' => substr($shareToken, 0, 8) . '...']); + } catch (DoesNotExistException $e) { + $this->logger->warning('Could not find share to update access token', ['shareToken' => substr($shareToken, 0, 8) . '...']); + } catch (Exception $e) { + $this->logger->error('Failed to update access token', ['exception' => $e]); + } + } } diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index b472bb2ceb26e..3675294c7a63a 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -52,6 +52,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private IConfig $config; private IAppConfig $appConfig; private IShareManager $shareManager; + private bool $tokenRefreshed = false; /** * @param array{HttpClientService: IClientService, manager: ExternalShareManager, cloudId: ICloudId, mountpoint: string, token: string, password: ?string}|array $options @@ -84,6 +85,12 @@ public function __construct($options) { $authType = \Sabre\DAV\Client::AUTH_BASIC; } + // If we have a stored access token (password), use Bearer auth regardless of discovery + // This handles the case where the share was created with must-exchange-token + if (!empty($options['password'])) { + $authType = \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER; + } + $host = parse_url($remote, PHP_URL_HOST); $port = parse_url($remote, PHP_URL_PORT); $host .= ($port === null) ? '' : ':' . $port; // we add port if available @@ -111,6 +118,63 @@ public function __construct($options) { ); } + /** + * Refresh the access token by exchanging the refresh token. + * Updates both the in-memory password and the database. + * + * @return bool True if token was refreshed successfully + */ + protected function refreshAccessToken(): bool { + if ($this->tokenRefreshed) { + // only try to refresh once per request + return false; + } + $this->tokenRefreshed = true; + + try { + $newAccessToken = $this->exchangeRefreshToken(); + $this->password = $newAccessToken; + + $this->manager->updateAccessToken($this->token, $newAccessToken); + + $this->ready = false; + $this->client = null; + + $this->logger->debug('Successfully refreshed access token', ['app' => 'files_sharing']); + return true; + } catch (\Exception $e) { + $this->logger->warning('Failed to refresh access token', [ + 'app' => 'files_sharing', + 'exception' => $e, + ]); + return false; + } + } + + /** + * Execute an operation with automatic token refresh on 401 errors. + * + * @template T + * @param callable(): T $operation The operation to execute + * @return T + * @throws \Exception + */ + protected function withTokenRefresh(callable $operation) { + try { + return $operation(); + } catch (\Sabre\HTTP\ClientHttpException $e) { + if ($e->getHttpStatus() === 401 && $this->refreshAccessToken()) { + return $operation(); + } + throw $e; + } catch (\Sabre\DAV\Exception\NotAuthenticated $e) { + if ($this->refreshAccessToken()) { + return $operation(); + } + throw $e; + } + } + public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher { if (!$storage) { $storage = $this; @@ -172,7 +236,7 @@ public function hasUpdated(string $path, int $time): bool { } $this->updateChecked = true; try { - return parent::hasUpdated('', $time); + return $this->withTokenRefresh(fn () => parent::hasUpdated('', $time)); } catch (StorageInvalidException $e) { // check if it needs to be removed $this->checkStorageAvailability(); @@ -186,7 +250,7 @@ public function hasUpdated(string $path, int $time): bool { public function test(): bool { try { - return parent::test(); + return $this->withTokenRefresh(fn () => parent::test()); } catch (StorageInvalidException $e) { // check if it needs to be removed $this->checkStorageAvailability(); diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 389f52b9517c0..52729c3d60318 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -103,6 +103,8 @@ class DAV extends Common { protected $certPath; /** @var bool */ protected $ready; + /** @var string The resolved bearer token for AUTH_BEARER (access token or exchanged token) */ + protected $bearerToken; /** @var Client */ protected $client; /** @var ArrayCache */ @@ -198,78 +200,16 @@ protected function init(): void { } $this->ready = true; - // If using Bearer auth, exchange refresh token for access token + // If using Bearer auth, use stored access token or exchange refresh token for access token $userName = $this->user; if ($this->authType !== null && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER)) { - try { - $host = 'https://' . $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, - ]; - - $options = [ - 'body' => http_build_query($payload), - 'headers' => [ - 'Content-Type' => 'application/x-www-form-urlencoded', - ], - 'timeout' => 10, - 'connect_timeout' => 10, - ]; - - // Try signing the request - if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { - try { - $options = $this->signatureManager->signOutgoingRequestIClientPayload( - $this->signatoryManager, - $options, - 'post', - $tokenEndpoint - ); - $this->logger->debug('Token request signed successfully', ['app' => 'dav']); - } catch (\Exception $e) { - $this->logger->warning('Failed to sign token request, continuing without signature', [ - 'app' => 'dav', - 'exception' => $e, - 'endpoint' => $tokenEndpoint, - ]); - } - } - - $response = $client->post($tokenEndpoint, $options); - - $body = $response->getBody(); - $data = json_decode($body, true); - - if (isset($data['access_token'])) { - $userName = $data['access_token']; - $this->user = $userName; - $this->logger->debug('Successfully exchanged refresh token for access token', ['app' => 'dav']); - } else { - $this->logger->error('Failed to get access token from response', ['app' => 'dav']); - throw new StorageNotAvailableException('Could not obtain access token'); - } - } catch (OCMProviderException|OCMArgumentException $e) { - $this->logger->error('OCM provider response missing tokenEndPoint', ['app' => 'dav']); - throw new StorageNotAvailableException('Could not discover token endpoint'); - } 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 we already have an access token stored (password field) + if (!empty($this->password)) { + $userName = $this->password; + } else { + $userName = $this->exchangeRefreshToken(); } + $this->bearerToken = $userName; } $settings = [ @@ -315,6 +255,85 @@ 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 = 'https://' . $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 signing the request + if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + $this->signatoryManager, + $options, + 'post', + $tokenEndpoint + ); + $this->logger->debug('Token request signed successfully', ['app' => 'dav']); + } catch (\Exception $e) { + $this->logger->warning('Failed to sign token request, continuing without signature', [ + 'app' => 'dav', + 'exception' => $e, + 'endpoint' => $tokenEndpoint, + ]); + } + } + + $response = $client->post($tokenEndpoint, $options); + + $body = $response->getBody(); + $data = json_decode($body, true); + + if (isset($data['access_token'])) { + $this->logger->debug('Successfully exchanged refresh token for access token', ['app' => 'dav']); + return $data['access_token']; + } else { + $this->logger->error('Failed to get access token from response', ['app' => 'dav']); + throw new StorageNotAvailableException('Could not obtain access token'); + } + } 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()); + } + } + /** * Clear the stat cache */ @@ -497,7 +516,7 @@ public function fopen(string $path, string $mode) { $headers = []; if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->user]; + $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; } $response = $this->httpClientService ->newClient() @@ -655,7 +674,7 @@ protected function uploadFile(string $path, string $target): void { $headers = []; if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->user]; + $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; } $this->httpClientService ->newClient() From 1d5ae83667d5109193fe590333b1e683cf5e95d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 12 Jan 2026 11:00:01 +0100 Subject: [PATCH 13/58] fix(federatedfilesharing): POSTs to token endpoint should be signed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/OCM/CloudFederationProviderFiles.php | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 84e9e1de57355..4e86b8dad38a8 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -7,6 +7,7 @@ namespace OCA\FederatedFileSharing\OCM; use OC\AppFramework\Http; +use OC\OCM\OCMSignatoryManager; use OC\Files\Filesystem; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; @@ -33,6 +34,7 @@ use OCP\Files\ISetupManager; use OCP\Files\NotFoundException; use OCP\HintException; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IURLGenerator; @@ -46,6 +48,7 @@ use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; +use OCP\Security\Signature\ISignatureManager; use OCP\Util; use Override; use Psr\Log\LoggerInterface; @@ -74,6 +77,9 @@ public function __construct( private readonly ExternalShareMapper $externalShareMapper, private readonly IOCMDiscoveryService $discoveryService, private readonly IClientService $clientService, + private readonly ISignatureManager $signatureManager, + private readonly OCMSignatoryManager $signatoryManager, + private readonly IAppConfig $appConfig, ) { } @@ -740,14 +746,35 @@ private function exchangeToken(string $remote, #[SensitiveParameter] string $sha 'code' => $sharedSecret, ]; - $response = $client->post($tokenEndpoint, [ + $options = [ 'body' => http_build_query($payload), 'headers' => [ 'Content-Type' => 'application/x-www-form-urlencoded', ], 'timeout' => 10, 'connect_timeout' => 10, - ]); + ]; + + // Try signing the request + if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + $this->signatoryManager, + $options, + 'post', + $tokenEndpoint + ); + $this->logger->debug('Token request signed successfully', ['remote' => $remote]); + } catch (\Exception $e) { + $this->logger->warning('Failed to sign token request, continuing without signature', [ + 'remote' => $remote, + 'exception' => $e, + 'endpoint' => $tokenEndpoint, + ]); + } + } + + $response = $client->post($tokenEndpoint, $options); $data = json_decode($response->getBody(), true); From 5a118bbaa9996f8679b998c10b5efc31114acbc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 12 Jan 2026 11:12:11 +0100 Subject: [PATCH 14/58] fix(federatedfilesharing): POSTs to token endpoint MUST be signed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/OCM/CloudFederationProviderFiles.php | 32 +++++++++---------- lib/private/Files/Storage/DAV.php | 32 +++++++++---------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 4e86b8dad38a8..da32560d00e8d 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -755,23 +755,21 @@ private function exchangeToken(string $remote, #[SensitiveParameter] string $sha 'connect_timeout' => 10, ]; - // Try signing the request - if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { - try { - $options = $this->signatureManager->signOutgoingRequestIClientPayload( - $this->signatoryManager, - $options, - 'post', - $tokenEndpoint - ); - $this->logger->debug('Token request signed successfully', ['remote' => $remote]); - } catch (\Exception $e) { - $this->logger->warning('Failed to sign token request, continuing without signature', [ - 'remote' => $remote, - 'exception' => $e, - 'endpoint' => $tokenEndpoint, - ]); - } + try { + $options = $this->signatureManager->signOutgoingRequestIClientPayload( + $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); diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 52729c3d60318..3fe4ca7261b37 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -289,23 +289,21 @@ protected function exchangeRefreshToken(): string { 'connect_timeout' => 10, ]; - // Try signing the request - if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { - try { - $options = $this->signatureManager->signOutgoingRequestIClientPayload( - $this->signatoryManager, - $options, - 'post', - $tokenEndpoint - ); - $this->logger->debug('Token request signed successfully', ['app' => 'dav']); - } catch (\Exception $e) { - $this->logger->warning('Failed to sign token request, continuing without signature', [ - 'app' => 'dav', - 'exception' => $e, - 'endpoint' => $tokenEndpoint, - ]); - } + 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); From 8980ba0c5de634f0f4d9abb91e37bb6a831a3037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 8 Jan 2026 12:32:53 +0100 Subject: [PATCH 15/58] fix: federated share provider tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 9 ++++++--- apps/federatedfilesharing/lib/FederatedShareProvider.php | 2 +- .../tests/FederatedShareProviderTest.php | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index 609a47dbbe6de..be36b05519040 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -46,13 +46,16 @@ public function validateBearerToken($bearerToken) { \OC_Util::setupFS(); $this->token = $bearerToken; - if (!$this->userSession->isLoggedIn()) { + $loggedIn = $this->userSession->isLoggedIn(); + if (!$loggedIn) { $this->userSession->tryTokenLogin($this->request); + $loggedIn = $this->userSession->isLoggedIn(); } - if (!$this->userSession->isLoggedIn()) { + if (!$loggedIn) { $this->userSession->doTryTokenLogin($bearerToken); + $loggedIn = $this->userSession->isLoggedIn(); } - if ($this->userSession->isLoggedIn()) { + if ($loggedIn) { return $this->setupUserFs($this->userSession->getUser()->getUID()); } diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index d2dabc9cffd6e..eef2b6c5f1cc0 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -180,7 +180,7 @@ protected function createFederatedShare(IShare $share): string { $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(); + $name = $user?->getDisplayName() ?? $uid; $pass = $share->getPassword(); $dbToken = $provider->generateToken($token, $uid, $uid, $pass, $name, type: IToken::PERMANENT_TOKEN); diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index da37617c121dd..11fd6cc4e645d 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -229,7 +229,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(), @@ -283,7 +283,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(), From 8d6fa5b8f186ca9ce9feb908c5068c8afb31db03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 8 Jan 2026 12:33:33 +0100 Subject: [PATCH 16/58] fix: share manager test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/Share20/Manager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 9c696e56f290e..91e627e36cf90 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -8,6 +8,7 @@ namespace OC\Share20; use ArrayIterator; +use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Core\AppInfo\ConfigLexicon; use OC\Files\Filesystem; @@ -1445,8 +1446,7 @@ public function getShareByToken(string $token): IShare { $accessTokenDb = $tokenProvider->getToken($token); $refreshToken = $accessTokenDb->getUID(); $share = $provider->getShareByToken($refreshToken); - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound|InvalidTokenException $e) { } } From 99c4a247eb618c39debd263ab71d551d384f8814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 10:52:48 +0100 Subject: [PATCH 17/58] fix(federatedfilesharing): fix federated share provider tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../tests/FederatedShareProviderTest.php | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 11fd6cc4e645d..292a287ea2b09 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -9,11 +9,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; @@ -27,6 +29,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; @@ -87,6 +90,19 @@ protected function setUp(): void { $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); + // Mock ISecureRandom to return a predictable token (must be 32+ chars) + $secureRandom = $this->createMock(ISecureRandom::class); + $secureRandom->method('generate') + ->willReturn('tokentokentokentokentokentokenab'); + $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, @@ -146,7 +162,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('tokentokentokentokentokentokenab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -184,7 +200,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate 'file_source' => 42, 'permissions' => 19, 'accepted' => 0, - 'token' => 'token', + 'token' => 'tokentokentokentokentokentokenab', 'expiration' => $expectedDataDate, ]; foreach (array_keys($expected) as $key) { @@ -199,7 +215,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('tokentokentokentokentokentokenab', $share->getToken()); $this->assertEquals($expirationDate, $share->getExpirationDate()); } @@ -373,7 +389,7 @@ public function testCreateAlreadyShared(): void { $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('tokentokentokentokentokentokenab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -445,7 +461,7 @@ public function testUpdate(string $owner, string $sharedBy, ?\DateTime $expirati $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('token'), + $this->equalTo('tokentokentokentokentokentokenab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), From a1d5959ab390937dfdf3ea23d437e78baabfbb0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 11:20:14 +0100 Subject: [PATCH 18/58] fix(federatedfilesharing): fixing federated share provider tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../tests/FederatedShareProviderTest.php | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 292a287ea2b09..38267819412ee 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -90,10 +90,14 @@ protected function setUp(): void { $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); - // Mock ISecureRandom to return a predictable token (must be 32+ chars) + // Mock ISecureRandom to return predictable tokens (must be 32+ chars) $secureRandom = $this->createMock(ISecureRandom::class); + $tokenCounter = 0; $secureRandom->method('generate') - ->willReturn('tokentokentokentokentokentokenab'); + ->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 @@ -162,7 +166,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('tokentokentokentokentokentokenab'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -200,7 +204,7 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate 'file_source' => 42, 'permissions' => 19, 'accepted' => 0, - 'token' => 'tokentokentokentokentokentokenab', + 'token' => 'token1token1token1token1token1ab', 'expiration' => $expectedDataDate, ]; foreach (array_keys($expected) as $key) { @@ -215,7 +219,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('tokentokentokentokentokentokenab', $share->getToken()); + $this->assertEquals('token1token1token1token1token1ab', $share->getToken()); $this->assertEquals($expirationDate, $share->getExpirationDate()); } @@ -389,7 +393,7 @@ public function testCreateAlreadyShared(): void { $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('tokentokentokentokentokentokenab'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -461,7 +465,7 @@ public function testUpdate(string $owner, string $sharedBy, ?\DateTime $expirati $this->notifications->expects($this->once()) ->method('sendRemoteShare') ->with( - $this->equalTo('tokentokentokentokentokentokenab'), + $this->equalTo('token1token1token1token1token1ab'), $this->equalTo('user@server.com'), $this->equalTo('myFile'), $this->anything(), @@ -943,11 +947,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); From f22ee84dfd526b3f0c1b5027a9f72a590b5aab6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 11:39:24 +0100 Subject: [PATCH 19/58] fix(federatedfilesharing): fixing federated share provider tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../tests/FederatedShareProviderTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 38267819412ee..e39f2e03eb152 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -904,9 +904,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); From f09566df76208d82b51dbad0964084a2b5daf03f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 12:37:26 +0100 Subject: [PATCH 20/58] fix: fixing code style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/federatedfilesharing/lib/FederatedShareProvider.php | 2 +- .../lib/OCM/CloudFederationProviderFiles.php | 7 ++++--- lib/private/Federation/CloudFederationFactory.php | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index eef2b6c5f1cc0..6a00cf06c2293 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -10,8 +10,8 @@ use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; -use OCP\Authentication\Exceptions\InvalidTokenException; use OC\Share20\Share; +use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Constants; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Federation\ICloudFederationProviderManager; diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index da32560d00e8d..529b465f8df3f 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -7,8 +7,9 @@ namespace OCA\FederatedFileSharing\OCM; use OC\AppFramework\Http; -use OC\OCM\OCMSignatoryManager; use OC\Files\Filesystem; +use OC\Files\SetupManager; +use OC\OCM\OCMSignatoryManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Federation\TrustedServers; @@ -34,6 +35,7 @@ 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; @@ -41,10 +43,9 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; +use OCP\OCM\IOCMDiscoveryService; use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; -use OCP\Http\Client\IClientService; -use OCP\OCM\IOCMDiscoveryService; use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; diff --git a/lib/private/Federation/CloudFederationFactory.php b/lib/private/Federation/CloudFederationFactory.php index d7b5d79efcbbd..cf39852a891f8 100644 --- a/lib/private/Federation/CloudFederationFactory.php +++ b/lib/private/Federation/CloudFederationFactory.php @@ -10,8 +10,8 @@ use OCP\Federation\ICloudFederationNotification; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; -use OCP\OCM\IOCMDiscoveryService; use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; use Psr\Log\LoggerInterface; class CloudFederationFactory implements ICloudFederationFactory { From 7b8449044e414d034536de2aa1a07fc770479fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 12:56:32 +0100 Subject: [PATCH 21/58] fix: fixing openapi specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/cloud_federation_api/openapi.json | 16 +++++++++------- apps/dav/openapi.json | 7 ++++++- openapi.json | 20 +++++++++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/apps/cloud_federation_api/openapi.json b/apps/cloud_federation_api/openapi.json index 21f669a2c5f79..a284ee1fcce1b 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" + } } } }, diff --git a/apps/dav/openapi.json b/apps/dav/openapi.json index 344d37815318c..8e7f6b548e069 100644 --- a/apps/dav/openapi.json +++ b/apps/dav/openapi.json @@ -1165,5 +1165,10 @@ } } }, - "tags": [] + "tags": [ + { + "name": "token", + "description": "Controller for the /token endpoint Exchanges long-lived refresh tokens for short-lived access tokens" + } + ] } diff --git a/openapi.json b/openapi.json index 50c44e2bde168..85a472bf2bea7 100644 --- a/openapi.json +++ b/openapi.json @@ -41,6 +41,10 @@ "name": "cloud_federation_api/request_handler", "description": "Open-Cloud-Mesh-API" }, + { + "name": "dav/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" @@ -16937,23 +16941,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" + } } } }, From 8366d01054069cfc9dd264709bb1301858cc81fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 13:41:10 +0100 Subject: [PATCH 22/58] fix: fix psalm issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 1 - apps/dav/lib/Controller/TokenController.php | 9 +++++---- .../lib/FederatedShareProvider.php | 2 +- .../Authentication/Token/PublicKeyToken.php | 4 ++++ lib/private/Files/Storage/DAV.php | 3 ++- lib/private/User/Session.php | 3 +-- lib/public/Authentication/Token/IToken.php | 7 +++++++ lib/public/IUserSession.php | 9 +++++++++ lib/public/OCM/ICapabilityAwareOCMProvider.php | 17 +++++++++++++++++ 9 files changed, 46 insertions(+), 9 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index be36b05519040..e4eae2143cb65 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -65,7 +65,6 @@ public function validateBearerToken($bearerToken) { public function getShare(): IShare { $shareManager = Server::get(IManager::class); $share = $shareManager->getShareByToken($this->token); - assert($share !== null); return $share; } diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index b942193e35eb0..4f5e10ab0b4ac 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -12,9 +12,9 @@ use NCU\Security\Signature\Exceptions\SignatureException; use NCU\Security\Signature\Exceptions\SignatureNotFoundException; use NCU\Security\Signature\ISignatureManager; +use NCU\Security\Signature\Model\IIncomingSignedRequest; use OC\Authentication\Token\IProvider; use OC\OCM\OCMSignatoryManager; -use OC\Security\Signature\Model\IncomingSignedRequest; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -53,10 +53,10 @@ public function __construct( /** * Verify the signature of incoming request if available * - * @return IncomingSignedRequest|null null if remote does not support signed requests + * @return IIncomingSignedRequest|null null if remote does not support signed requests * @throws IncomingRequestException if signature is required but invalid */ - private function verifySignedRequest(): ?IncomingSignedRequest { + private function verifySignedRequest(): ?IIncomingSignedRequest { try { $signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager); $this->logger->debug('Token request signature verified', [ @@ -80,11 +80,12 @@ private function verifySignedRequest(): ?IncomingSignedRequest { /** * Exchange a refresh token for a short-lived access token * - * @return DataResponse|DataResponse + * @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] diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 6a00cf06c2293..6307d997118cf 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -7,8 +7,8 @@ */ namespace OCA\FederatedFileSharing; -use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyTokenProvider; +use OCP\Authentication\Token\IToken; use OC\Share20\Exception\InvalidShare; use OC\Share20\Share; use OCP\Authentication\Exceptions\InvalidTokenException; diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index cf3a8b16141bc..eca54e41ab9f9 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -191,6 +191,10 @@ public function getRemember(): int { return parent::getRemember(); } + public function getType(): int { + return parent::getType(); + } + public function setToken(string $token): void { parent::setToken($token); } diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 3fe4ca7261b37..cfd4fe289c828 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -71,6 +71,7 @@ public function __construct(array $settings) { if (isset($settings['userName']) && isset($settings['authType']) && ($settings['authType'] & self::AUTH_BEARER)) { $userName = $settings['userName']; + /** @psalm-suppress InvalidArrayOffset */ $curlType = $this->curlSettings[CURLOPT_HTTPAUTH]; $curlType |= CURLAUTH_BEARER; @@ -90,7 +91,7 @@ class DAV extends Common { protected $password; /** @var string */ protected $user; - /** @var string|null */ + /** @var int|null */ protected $authType; /** @var string */ protected $host; diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 6786e0156c850..48b1a60f71ce6 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -854,8 +854,7 @@ public function tryTokenLogin(IRequest $request) { return $this->doTryTokenLogin($token); } - public function doTryTokenLogin($token) { - + public function doTryTokenLogin(string $token): bool { if (!$this->loginWithToken($token)) { return false; } diff --git a/lib/public/Authentication/Token/IToken.php b/lib/public/Authentication/Token/IToken.php index 546e6a4225550..8ad8f298f1942 100644 --- a/lib/public/Authentication/Token/IToken.php +++ b/lib/public/Authentication/Token/IToken.php @@ -130,4 +130,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/lib/public/IUserSession.php b/lib/public/IUserSession.php index 42cc437aabae1..9d80bb95735ab 100644 --- a/lib/public/IUserSession.php +++ b/lib/public/IUserSession.php @@ -92,4 +92,13 @@ public function getImpersonatingUserID(): ?string; * @since 18.0.0 */ public function setImpersonatingUserID(bool $useCurrentUser = true): void; + + /** + * Try to login with the given token + * + * @param string $token + * @return bool true if successful + * @since 32.0.0 + */ + public function doTryTokenLogin(string $token): bool; } diff --git a/lib/public/OCM/ICapabilityAwareOCMProvider.php b/lib/public/OCM/ICapabilityAwareOCMProvider.php index faf44067d1255..0bfc9aaad38a1 100644 --- a/lib/public/OCM/ICapabilityAwareOCMProvider.php +++ b/lib/public/OCM/ICapabilityAwareOCMProvider.php @@ -15,4 +15,21 @@ * @deprecated 33.0.0 {@see IOCMProvider} */ interface ICapabilityAwareOCMProvider extends IOCMProvider { + /** + * get the token endpoint URL + * + * @return string + * @since 32.0.0 + */ + public function getTokenEndPoint(): string; + + /** + * set the token endpoint URL + * + * @param string $endPoint + * + * @return $this + * @since 32.0.0 + */ + public function setTokenEndPoint(string $endPoint): static; } From 6a950ae9b1d5cd42afaceae034e7216c4648db2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 13:44:05 +0100 Subject: [PATCH 23/58] fix: reorder import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/federatedfilesharing/lib/FederatedShareProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 6307d997118cf..be0700febe794 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -8,10 +8,10 @@ namespace OCA\FederatedFileSharing; use OC\Authentication\Token\PublicKeyTokenProvider; -use OCP\Authentication\Token\IToken; use OC\Share20\Exception\InvalidShare; use OC\Share20\Share; use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\Authentication\Token\IToken; use OCP\Constants; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Federation\ICloudFederationProviderManager; From 26ee360ee4d43e610a02f27df0d1fda3deed00de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 19 Jan 2026 15:57:20 +0100 Subject: [PATCH 24/58] fix(dav): do not import from NCU ns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- 3rdparty | 2 +- apps/dav/lib/Controller/TokenController.php | 12 ++++++------ lib/private/Files/Storage/DAV.php | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/3rdparty b/3rdparty index e1dc48ae9d9b7..8f97d8cef37b3 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit e1dc48ae9d9b7b1e8c7b59d1c8cc3b6a9c50f83a +Subproject commit 8f97d8cef37b32d25e36c16fc2f7be36f1a46901 diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index 4f5e10ab0b4ac..fbad90dd629a8 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -7,12 +7,6 @@ namespace OCA\DAV\Controller; -use NCU\Security\Signature\Exceptions\IncomingRequestException; -use NCU\Security\Signature\Exceptions\SignatoryNotFoundException; -use NCU\Security\Signature\Exceptions\SignatureException; -use NCU\Security\Signature\Exceptions\SignatureNotFoundException; -use NCU\Security\Signature\ISignatureManager; -use NCU\Security\Signature\Model\IIncomingSignedRequest; use OC\Authentication\Token\IProvider; use OC\OCM\OCMSignatoryManager; use OCP\AppFramework\ApiController; @@ -28,6 +22,12 @@ use OCP\IAppConfig; use OCP\IRequest; use OCP\Security\ISecureRandom; +use OCP\Security\Signature\Exceptions\IncomingRequestException; +use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; +use OCP\Security\Signature\Exceptions\SignatureException; +use OCP\Security\Signature\Exceptions\SignatureNotFoundException; +use OCP\Security\Signature\IIncomingSignedRequest; +use OCP\Security\Signature\ISignatureManager; use Psr\Log\LoggerInterface; /** diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index cfd4fe289c828..f4d0773ee47f1 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -10,7 +10,6 @@ use Exception; use Icewind\Streams\CallbackWrapper; use Icewind\Streams\IteratorDirectory; -use NCU\Security\Signature\ISignatureManager; use OC\Files\Filesystem; use OC\MemCache\ArrayCache; use OC\OCM\OCMSignatoryManager; @@ -35,6 +34,7 @@ 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; From 5a142f4bfe853cf12d4b42fbadcc917b468e57a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 23 Jan 2026 14:16:38 +0100 Subject: [PATCH 25/58] fix: fix sqlite integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- build/integration/features/bootstrap/Sharing.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index aba3d3090beca..414f7d61579d5 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -316,7 +316,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') { @@ -331,7 +332,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') { @@ -617,9 +619,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,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 { From f383940804f98bf85e080f3040b76a7b950c2b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 23 Jan 2026 14:26:31 +0100 Subject: [PATCH 26/58] fix(federatedfilesharing): order of imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/OCM/CloudFederationProviderFiles.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 529b465f8df3f..c2213dac41db5 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -44,12 +44,12 @@ 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; use OCP\Share\IProviderFactory; use OCP\Share\IShare; -use OCP\Security\Signature\ISignatureManager; use OCP\Util; use Override; use Psr\Log\LoggerInterface; From dd24e15aac1e6051f29bb30df2e64ba763b5cbf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 23 Jan 2026 14:36:51 +0100 Subject: [PATCH 27/58] fix: fix session tests using Session::loginWithToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- tests/lib/User/SessionTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 84d5bc898a057..d435bc63dc327 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -331,6 +331,13 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void { ->getMock(); $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $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->never()) ->method('set'); $session->expects($this->once()) @@ -369,6 +376,13 @@ public function testLoginLastCheckUpdate(): void { ->getMock(); $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $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->never()) ->method('set'); $session->expects($this->once()) From 22a27348bb867865b0051051e8ebb478ff8a54bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 23 Jan 2026 14:49:45 +0100 Subject: [PATCH 28/58] fix: fix public key token provider test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 51915fc1d4b5a..3af393f3a3bd4 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -451,6 +451,7 @@ public function testRenewSessionTokenWithPassword(): void { public function testGetToken(): void { $token = new PublicKeyToken(); + $token->setType(IToken::TEMPORARY_TOKEN); $this->config->method('getSystemValue') ->with('secret') From 73af9386dd98409d724a512813e99c475c326a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 26 Jan 2026 11:44:08 +0100 Subject: [PATCH 29/58] fix: Fixed undefined $request variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/User/Session.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 48b1a60f71ce6..6b1eaad18a4c8 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -874,6 +874,7 @@ public function doTryTokenLogin(string $token): bool { $this->session->set('app_password', $token); } elseif ($dbToken instanceof PublicKeyToken && $dbToken->getType() === IToken::ONETIME_TOKEN) { $this->tokenProvider->invalidateTokenById($dbToken->getUID(), $dbToken->getId()); + $request = \OCP\Server::get(IRequest::class); if ($request->getPathInfo() !== '/core/getapppassword-onetime') { return false; } From 8f345c7dbad1f23f9ad7952afad4591eb502b3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 26 Jan 2026 11:45:55 +0100 Subject: [PATCH 30/58] fix(files_external): Added missing doTryTokenLogin() method to implement the IUserSession interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_external/lib/Migration/DummyUserSession.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_external/lib/Migration/DummyUserSession.php b/apps/files_external/lib/Migration/DummyUserSession.php index 1ebf0e1ec4f85..3946d8fad381c 100644 --- a/apps/files_external/lib/Migration/DummyUserSession.php +++ b/apps/files_external/lib/Migration/DummyUserSession.php @@ -54,4 +54,8 @@ public function getImpersonatingUserID() : ?string { public function setImpersonatingUserID(bool $useCurrentUser = true): void { //no OP } + + public function doTryTokenLogin(string $token): bool { + return false; + } } From 962bea61550856c2679fc20c62b1e586ad753299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 26 Jan 2026 11:47:27 +0100 Subject: [PATCH 31/58] fix: Fixed parent::getType() to use ->getter('type') to avoid Psalm magic method detection issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/Authentication/Token/PublicKeyToken.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index eca54e41ab9f9..29348e5d2d435 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -192,7 +192,7 @@ public function getRemember(): int { } public function getType(): int { - return parent::getType(); + return $this->getter('type'); } public function setToken(string $token): void { From 6b56dc7cdfa4936f245171f43b95ca86d6b6219c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 26 Jan 2026 11:48:22 +0100 Subject: [PATCH 32/58] fix: Added getTokenEndPoint() and setTokenEndPoint() methods that should have been moved from ICapabilityAwareOCMProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/public/OCM/IOCMProvider.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/public/OCM/IOCMProvider.php b/lib/public/OCM/IOCMProvider.php index 83f2871d5c5d1..12b620bcbc8f8 100644 --- a/lib/public/OCM/IOCMProvider.php +++ b/lib/public/OCM/IOCMProvider.php @@ -158,8 +158,25 @@ public function setCapabilities(array $capabilities): static; * @return $this * @since 33.0.0 */ - public function setInviteAcceptDialog(string $inviteAcceptDialog): static; + + /** + * get the token endpoint URL + * + * @return string + * @since 33.0.0 + */ + public function getTokenEndPoint(): string; + + /** + * set the token endpoint URL + * + * @param string $endPoint + * + * @return $this + * @since 33.0.0 + */ + public function setTokenEndPoint(string $endPoint): static; /** * extract a specific string value from the listing of protocols, based on resource-name and protocol-name * From cb20040a3beaae36c39cc45a2b143568f0deee53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 28 Jan 2026 18:18:53 +0100 Subject: [PATCH 33/58] fix: fix session tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- tests/lib/User/SessionTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index d435bc63dc327..9383ce7b4d0dd 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -338,8 +338,6 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void { ->with('foo') ->willReturn($user); - $session->expects($this->never()) - ->method('set'); $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); @@ -383,8 +381,6 @@ public function testLoginLastCheckUpdate(): void { ->with('foo') ->willReturn($user); - $session->expects($this->never()) - ->method('set'); $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); From 6d82d2489e93981098e68417a0fb2fb03918fa25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 29 Jan 2026 11:52:46 +0100 Subject: [PATCH 34/58] fix(federatedfilesharing): remove unused import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/OCM/CloudFederationProviderFiles.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index c2213dac41db5..4d1330c939dc6 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -8,7 +8,6 @@ use OC\AppFramework\Http; use OC\Files\Filesystem; -use OC\Files\SetupManager; use OC\OCM\OCMSignatoryManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; From 1cf1cecb144d0a6e466e60f684e0ee64997df4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 29 Jan 2026 12:14:52 +0100 Subject: [PATCH 35/58] fix: fix user session tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- tests/lib/User/SessionTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 9383ce7b4d0dd..3d05913f5c6f5 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -341,6 +341,7 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void { $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); + $token->setId(1); $token->setLoginName('foo'); $token->setLastCheck(0); // Never $token->setUid('foo'); @@ -384,6 +385,7 @@ public function testLoginLastCheckUpdate(): void { $session->expects($this->once()) ->method('regenerateId'); $token = new PublicKeyToken(); + $token->setId(1); $token->setLoginName('foo'); $token->setLastCheck(0); // Never $token->setUid('foo'); From ffc4154470a8a0dab5a4887da0dfc1ca1e8821c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 29 Jan 2026 15:37:49 +0100 Subject: [PATCH 36/58] test: test token controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../unit/Controller/TokenControllerTest.php | 377 ++++++++++++++++++ 1 file changed, 377 insertions(+) create mode 100644 apps/dav/tests/unit/Controller/TokenControllerTest.php diff --git a/apps/dav/tests/unit/Controller/TokenControllerTest.php b/apps/dav/tests/unit/Controller/TokenControllerTest.php new file mode 100644 index 0000000000000..82de3c864e2c3 --- /dev/null +++ b/apps/dav/tests/unit/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->controller = new TokenController( + $this->request, + $this->tokenProvider, + $this->random, + $this->timeFactory, + $this->logger, + $this->signatureManager, + $this->signatoryManager, + $this->appConfig, + ); + } + + 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); + + $refreshToken = $this->createMock(IToken::class); + $refreshToken->method('getType')->willReturn(IToken::PERMANENT_TOKEN); + $refreshToken->method('getId')->willReturn(123); + $refreshToken->method('getLoginName')->willReturn('testuser'); + + $this->tokenProvider->method('getToken') + ->with('valid-refresh-token') + ->willReturn($refreshToken); + + $this->random->method('generate') + ->with(64, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS) + ->willReturn('generated-access-token'); + + $this->timeFactory->method('getTime')->willReturn(1000000); + + $accessToken = $this->createMock(IToken::class); + $this->tokenProvider->method('generateToken') + ->with( + 'generated-access-token', + 'valid-refresh-token', + 'testuser', + null, + 'OCM Access Token', + IToken::TEMPORARY_TOKEN, + IToken::DO_NOT_REMEMBER + ) + ->willReturn($accessToken); + + $accessToken->expects($this->once()) + ->method('setExpires') + ->with(1000000 + 3600); + + $this->tokenProvider->expects($this->once()) + ->method('updateToken') + ->with($accessToken); + + // Simulate POST body + $this->simulatePostBody('grant_type=authorization_code&code=valid-refresh-token'); + + $result = $this->controller->accessToken(); + + $this->assertInstanceOf(DataResponse::class, $result); + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + $this->assertEquals([ + 'access_token' => 'generated-access-token', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + ], $result->getData()); + } + + 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); + + $refreshToken = $this->createMock(IToken::class); + $refreshToken->method('getType')->willReturn(IToken::PERMANENT_TOKEN); + $refreshToken->method('getLoginName')->willReturn('testuser'); + + $this->tokenProvider->method('getToken') + ->willReturn($refreshToken); + + $this->random->method('generate')->willReturn('generated-access-token'); + $this->timeFactory->method('getTime')->willReturn(1000000); + + $accessToken = $this->createMock(IToken::class); + $this->tokenProvider->method('generateToken')->willReturn($accessToken); + + $this->simulatePostBody('grant_type=authorization_code&code=refresh-token'); + + $result = $this->controller->accessToken(); + + $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); + + $this->simulatePostBody('grant_type=authorization_code&code=refresh-token'); + + $result = $this->controller->accessToken(); + + $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')); + + $this->simulatePostBody('grant_type=authorization_code&code=refresh-token'); + + $result = $this->controller->accessToken(); + + $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); + + $this->simulatePostBody('grant_type=password&code=refresh-token'); + + $result = $this->controller->accessToken(); + + $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); + + $this->simulatePostBody('code=refresh-token'); + + $result = $this->controller->accessToken(); + + $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); + + $this->simulatePostBody('grant_type=authorization_code'); + + $result = $this->controller->accessToken(); + + $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); + + $this->simulatePostBody('grant_type=authorization_code&code=non-permanent-token'); + + $result = $this->controller->accessToken(); + + $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()); + + $this->simulatePostBody('grant_type=authorization_code&code=invalid-token'); + + $result = $this->controller->accessToken(); + + $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))); + + $this->simulatePostBody('grant_type=authorization_code&code=expired-token'); + + $result = $this->controller->accessToken(); + + $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')); + + $this->simulatePostBody('grant_type=authorization_code&code=some-token'); + + $result = $this->controller->accessToken(); + + $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); + + $refreshToken = $this->createMock(IToken::class); + $refreshToken->method('getType')->willReturn(IToken::PERMANENT_TOKEN); + $refreshToken->method('getLoginName')->willReturn('testuser'); + + $this->tokenProvider->method('getToken')->willReturn($refreshToken); + $this->random->method('generate')->willReturn('generated-access-token'); + $this->timeFactory->method('getTime')->willReturn(1000000); + + $accessToken = $this->createMock(IToken::class); + $this->tokenProvider->method('generateToken')->willReturn($accessToken); + + $this->simulatePostBody('grant_type=authorization_code&code=refresh-token'); + + $result = $this->controller->accessToken(); + + $this->assertEquals(Http::STATUS_OK, $result->getStatus()); + } + + private function simulatePostBody(string $body): void { + // We need to use a stream wrapper to simulate php://input + stream_wrapper_unregister('php'); + stream_wrapper_register('php', TestPhpInputStream::class); + TestPhpInputStream::$body = $body; + } + + protected function tearDown(): void { + // Restore the original php stream wrapper + stream_wrapper_restore('php'); + parent::tearDown(); + } +} + +/** + * Helper class to simulate php://input + */ +class TestPhpInputStream { + public static string $body = ''; + private int $position = 0; + public mixed $context = null; + + public function stream_open(string $path, string $mode, int $options, ?string &$opened_path): bool { + if ($path === 'php://input') { + $this->position = 0; + return true; + } + return false; + } + + public function stream_read(int $count): string { + $result = substr(self::$body, $this->position, $count); + $this->position += strlen($result); + return $result; + } + + public function stream_eof(): bool { + return $this->position >= strlen(self::$body); + } + + public function stream_stat(): array { + return []; + } +} From cce71cc68dea6b4551dd8ec4e1e3cca187f5d244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 29 Jan 2026 15:38:30 +0100 Subject: [PATCH 37/58] test: test doTryTokenLogin method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- tests/lib/User/SessionTest.php | 132 +++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 3d05913f5c6f5..cc6c42fa88c20 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -1336,4 +1336,136 @@ public function testLogClientInThrottlerEmail(): void { $this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-A-PASSWORD', $request, $this->throttler)); } + + public function testDoTryTokenLoginSuccess(): void { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testuser'); + $user->method('isEnabled')->willReturn(true); + + $manager->method('get') + ->with('testuser') + ->willReturn($user); + + $token = $this->createMock(PublicKeyToken::class); + $token->method('getUID')->willReturn('testuser'); + $token->method('getLoginName')->willReturn('testuser'); + $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::PERMANENT_TOKEN); + $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); + + $this->tokenProvider->method('getToken') + ->with('valid-token') + ->willReturn($token); + + $appPasswordSet = false; + $session->expects($this->atLeastOnce()) + ->method('set') + ->willReturnCallback(function ($key, $value) use (&$appPasswordSet) { + // We expect app_password to be set for permanent tokens + if ($key === 'app_password') { + $appPasswordSet = true; + $this->assertEquals('valid-token', $value); + } + return true; + }); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->onlyMethods(['setMagicInCookie']) + ->getMock(); + + $this->assertTrue($userSession->doTryTokenLogin('valid-token')); + $this->assertTrue($appPasswordSet, 'app_password should be set for permanent tokens'); + } + + public function testDoTryTokenLoginInvalidToken(): void { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + + $this->tokenProvider->method('getToken') + ->with('invalid-token') + ->willThrowException(new InvalidTokenException()); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->onlyMethods(['setMagicInCookie']) + ->getMock(); + + $this->assertFalse($userSession->doTryTokenLogin('invalid-token')); + } + + public function testDoTryTokenLoginTemporaryToken(): void { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testuser'); + $user->method('isEnabled')->willReturn(true); + + $manager->method('get') + ->with('testuser') + ->willReturn($user); + + $token = $this->createMock(PublicKeyToken::class); + $token->method('getUID')->willReturn('testuser'); + $token->method('getLoginName')->willReturn('testuser'); + $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::TEMPORARY_TOKEN); + $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); + + $this->tokenProvider->method('getToken') + ->with('temp-token') + ->willReturn($token); + + // app_password should NOT be set for temporary tokens + $session->expects($this->atLeastOnce()) + ->method('set') + ->willReturnCallback(function ($key, $value) { + $this->assertNotEquals('app_password', $key, 'app_password should not be set for temporary tokens'); + return true; + }); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->onlyMethods(['setMagicInCookie']) + ->getMock(); + + $this->assertTrue($userSession->doTryTokenLogin('temp-token')); + } + + public function testDoTryTokenLoginDisabledUser(): void { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testuser'); + $user->method('isEnabled')->willReturn(false); + + $manager->method('get') + ->with('testuser') + ->willReturn($user); + + $token = $this->createMock(PublicKeyToken::class); + $token->method('getUID')->willReturn('testuser'); + $token->method('getLoginName')->willReturn('testuser'); + $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::PERMANENT_TOKEN); + $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); + + $this->tokenProvider->method('getToken') + ->with('valid-token') + ->willReturn($token); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->onlyMethods(['setMagicInCookie']) + ->getMock(); + + $this->expectException(LoginException::class); + $userSession->doTryTokenLogin('valid-token'); + } } From f63b7a3f10a7fa7bfbfef989e85ee5e5a50d92ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 2 Feb 2026 11:30:46 +0100 Subject: [PATCH 38/58] fix(dav): remove unused import in TokenController test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/tests/unit/Controller/TokenControllerTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/dav/tests/unit/Controller/TokenControllerTest.php b/apps/dav/tests/unit/Controller/TokenControllerTest.php index 82de3c864e2c3..17e4038955d53 100644 --- a/apps/dav/tests/unit/Controller/TokenControllerTest.php +++ b/apps/dav/tests/unit/Controller/TokenControllerTest.php @@ -21,7 +21,6 @@ use OCP\IAppConfig; use OCP\IRequest; use OCP\Security\ISecureRandom; -use OCP\Security\Signature\Exceptions\IncomingRequestException; use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; use OCP\Security\Signature\Exceptions\SignatureException; use OCP\Security\Signature\Exceptions\SignatureNotFoundException; From dca4352bf04c47e048ab271df091c21e9bcd933b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Mon, 2 Feb 2026 17:39:38 +0100 Subject: [PATCH 39/58] feat(dav): refresh expired tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- lib/private/Files/Storage/DAV.php | 104 ++++++++++++++++++++++++------ 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index f4d0773ee47f1..5f9d2db9431cf 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -333,6 +333,25 @@ protected function exchangeRefreshToken(): string { } } + /** + * Check if bearer authentication is being used + */ + protected function isBearerAuth(): bool { + return $this->authType !== null && + ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER); + } + + /** + * Reinitialize the client with a fresh access token + * Used when the current bearer token has expired (401 response) + */ + protected function reinitWithFreshToken(): void { + $this->logger->debug('Bearer token expired, refreshing token', ['app' => 'dav']); + $this->ready = false; + $this->password = ''; // Clear to force token exchange in init() + $this->init(); + } + /** * Clear the stat cache */ @@ -425,12 +444,13 @@ public function getPropfindPropertyValue(string $path, string $propertyName): mi * If not, request it from the server then store to cache. * * @param string $path path to propfind + * @param bool $retryOnUnauthorized whether to retry on 401 response (used to prevent infinite loops) * * @return array|false propfind response or false if the entry was not found * * @throws ClientHttpException */ - protected function propfind(string $path): array|false { + protected function propfind(string $path, bool $retryOnUnauthorized = true): array|false { $path = $this->cleanPath($path); $cachedResponse = $this->statCache->get($path); // we either don't know it, or we know it exists but need more details @@ -447,6 +467,9 @@ protected function propfind(string $path): array|false { if ($e->getHttpStatus() === 404 || $e->getHttpStatus() === 405) { $this->statCache->clear($path . '/'); $this->statCache->set($path, false); + } elseif ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->propfind($path, false); } else { $this->convertException($e, $path); } @@ -504,7 +527,7 @@ public function unlink(string $path): bool { return $result; } - public function fopen(string $path, string $mode) { + public function fopen(string $path, string $mode, bool $retryOnUnauthorized = true) { $this->init(); $path = $this->cleanPath($path); switch ($mode) { @@ -531,6 +554,11 @@ public function fopen(string $path, string $mode) { if ($e->getResponse() instanceof ResponseInterface && $e->getResponse()->getStatusCode() === 404) { return false; + } elseif ($e->getResponse() instanceof ResponseInterface + && $e->getResponse()->getStatusCode() === 401 + && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->fopen($path, $mode, false); } else { throw $e; } @@ -617,7 +645,7 @@ public function free_space(string $path): int|float|false { } } - public function touch(string $path, ?int $mtime = null): bool { + public function touch(string $path, ?int $mtime = null, bool $retryOnUnauthorized = true): bool { $this->init(); if (is_null($mtime)) { $mtime = time(); @@ -642,6 +670,10 @@ public function touch(string $path, ?int $mtime = null): bool { if ($e->getHttpStatus() === 501) { return false; } + if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->touch($path, $mtime, false); + } $this->convertException($e, $path); return false; } catch (\Exception $e) { @@ -661,7 +693,7 @@ public function file_put_contents(string $path, mixed $data): int|float|false { return $result; } - protected function uploadFile(string $path, string $target): void { + protected function uploadFile(string $path, string $target, bool $retryOnUnauthorized = true): void { $this->init(); // invalidate @@ -675,21 +707,32 @@ protected function uploadFile(string $path, string $target): void { $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, - ]); + try { + $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, + ]); + } catch (\GuzzleHttp\Exception\ClientException $e) { + if ($e->getResponse() instanceof ResponseInterface + && $e->getResponse()->getStatusCode() === 401 + && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + $this->uploadFile($path, $target, false); + return; + } + throw $e; + } $this->removeCachedFile($target); } - public function rename(string $source, string $target): bool { + public function rename(string $source, string $target, bool $retryOnUnauthorized = true): bool { $this->init(); $source = $this->cleanPath($source); $target = $this->cleanPath($target); @@ -714,13 +757,19 @@ public function rename(string $source, string $target): bool { $this->removeCachedFile($source); $this->removeCachedFile($target); return true; + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->rename($source, $target, false); + } + $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); } return false; } - public function copy(string $source, string $target): bool { + public function copy(string $source, string $target, bool $retryOnUnauthorized = true): bool { $this->init(); $source = $this->cleanPath($source); $target = $this->cleanPath($target); @@ -742,6 +791,12 @@ public function copy(string $source, string $target): bool { $this->statCache->set($target, true); $this->removeCachedFile($target); return true; + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->copy($source, $target, false); + } + $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); } @@ -842,11 +897,12 @@ protected function encodePath(string $path): string { } /** + * @param bool $retryOnUnauthorized whether to retry on 401 response (used to prevent infinite loops) * @return bool * @throws StorageInvalidException * @throws StorageNotAvailableException */ - protected function simpleResponse(string $method, string $path, ?string $body, int $expected): bool { + protected function simpleResponse(string $method, string $path, ?string $body, int $expected, bool $retryOnUnauthorized = true): bool { $path = $this->cleanPath($path); try { $response = $this->client->request($method, $this->encodePath($path), $body); @@ -858,6 +914,11 @@ protected function simpleResponse(string $method, string $path, ?string $body, i return false; } + if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + return $this->simpleResponse($method, $path, $body, $expected, false); + } + $this->convertException($e, $path); } catch (\Exception $e) { $this->convertException($e, $path); @@ -1014,7 +1075,7 @@ protected function convertException(Exception $e, string $path = ''): void { // TODO: only log for now, but in the future need to wrap/rethrow exception } - public function getDirectoryContent(string $directory): \Traversable { + public function getDirectoryContent(string $directory, bool $retryOnUnauthorized = true): \Traversable { $this->init(); $directory = $this->cleanPath($directory); try { @@ -1036,6 +1097,13 @@ public function getDirectoryContent(string $directory): \Traversable { $this->statCache->set($file, $response); yield $this->getMetaFromPropfind($file, $response); } + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { + $this->reinitWithFreshToken(); + yield from $this->getDirectoryContent($directory, false); + return; + } + $this->convertException($e, $directory); } catch (\Exception $e) { $this->convertException($e, $directory); } From 200bc4262951834afbb7766a176a8cda43bb9e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 4 Feb 2026 16:07:26 +0100 Subject: [PATCH 40/58] fix(files_sharing): refactor refreshing access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/lib/External/Storage.php | 34 +--- lib/private/Files/Storage/DAV.php | 198 +++++++++++--------- 2 files changed, 111 insertions(+), 121 deletions(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 3675294c7a63a..a7ea67d85d67f 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -119,12 +119,11 @@ public function __construct($options) { } /** - * Refresh the access token by exchanging the refresh token. - * Updates both the in-memory password and the database. + * Refresh the bearer token. Extends parent to also persist to database. * * @return bool True if token was refreshed successfully */ - protected function refreshAccessToken(): bool { + protected function refreshBearerToken(): bool { if ($this->tokenRefreshed) { // only try to refresh once per request return false; @@ -139,6 +138,7 @@ protected function refreshAccessToken(): bool { $this->ready = false; $this->client = null; + $this->init(); $this->logger->debug('Successfully refreshed access token', ['app' => 'files_sharing']); return true; @@ -151,30 +151,6 @@ protected function refreshAccessToken(): bool { } } - /** - * Execute an operation with automatic token refresh on 401 errors. - * - * @template T - * @param callable(): T $operation The operation to execute - * @return T - * @throws \Exception - */ - protected function withTokenRefresh(callable $operation) { - try { - return $operation(); - } catch (\Sabre\HTTP\ClientHttpException $e) { - if ($e->getHttpStatus() === 401 && $this->refreshAccessToken()) { - return $operation(); - } - throw $e; - } catch (\Sabre\DAV\Exception\NotAuthenticated $e) { - if ($this->refreshAccessToken()) { - return $operation(); - } - throw $e; - } - } - public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher { if (!$storage) { $storage = $this; @@ -236,7 +212,7 @@ public function hasUpdated(string $path, int $time): bool { } $this->updateChecked = true; try { - return $this->withTokenRefresh(fn () => parent::hasUpdated('', $time)); + return parent::hasUpdated('', $time); } catch (StorageInvalidException $e) { // check if it needs to be removed $this->checkStorageAvailability(); @@ -250,7 +226,7 @@ public function hasUpdated(string $path, int $time): bool { public function test(): bool { try { - return $this->withTokenRefresh(fn () => parent::test()); + return parent::test(); } catch (StorageInvalidException $e) { // check if it needs to be removed $this->checkStorageAvailability(); diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 5f9d2db9431cf..68bd428a4ae49 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -337,19 +337,73 @@ protected function exchangeRefreshToken(): string { * Check if bearer authentication is being used */ protected function isBearerAuth(): bool { - return $this->authType !== null && - ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER); + return $this->authType !== null + && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER); } /** - * Reinitialize the client with a fresh access token - * Used when the current bearer token has expired (401 response) + * @var bool Flag to prevent infinite retry loops during token refresh */ - protected function reinitWithFreshToken(): void { + private bool $retryingAuth = false; + + /** + * Execute an operation with automatic retry on 401 Unauthorized when using Bearer auth. + * Handles both Sabre ClientHttpException and Guzzle ClientException. + * + * @template T + * @param callable(): T $operation The operation to execute + * @return T The result of the operation + * @throws ClientHttpException + * @throws \GuzzleHttp\Exception\ClientException + */ + protected function withAuthRetry(callable $operation): mixed { + try { + return $operation(); + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() === 401 && !$this->retryingAuth && $this->isBearerAuth()) { + return $this->retryWithFreshToken($operation); + } + throw $e; + } catch (\GuzzleHttp\Exception\ClientException $e) { + if ($e->getResponse() instanceof ResponseInterface + && $e->getResponse()->getStatusCode() === 401 + && !$this->retryingAuth && $this->isBearerAuth()) { + return $this->retryWithFreshToken($operation); + } + throw $e; + } + } + + /** + * Refresh the bearer token and retry the operation. + * + * @template T + * @param callable(): T $operation The operation to retry + * @return T The result of the operation + */ + private function retryWithFreshToken(callable $operation): mixed { + $this->retryingAuth = true; + try { + if (!$this->refreshBearerToken()) { + throw new StorageNotAvailableException('Failed to refresh bearer token'); + } + return $operation(); + } finally { + $this->retryingAuth = false; + } + } + + /** + * Refresh the bearer token. Override in subclasses to add persistence logic. + * + * @return bool True if token was refreshed successfully + */ + protected function refreshBearerToken(): bool { $this->logger->debug('Bearer token expired, refreshing token', ['app' => 'dav']); $this->ready = false; $this->password = ''; // Clear to force token exchange in init() $this->init(); + return true; } /** @@ -444,13 +498,12 @@ public function getPropfindPropertyValue(string $path, string $propertyName): mi * If not, request it from the server then store to cache. * * @param string $path path to propfind - * @param bool $retryOnUnauthorized whether to retry on 401 response (used to prevent infinite loops) * * @return array|false propfind response or false if the entry was not found * * @throws ClientHttpException */ - protected function propfind(string $path, bool $retryOnUnauthorized = true): array|false { + protected function propfind(string $path): array|false { $path = $this->cleanPath($path); $cachedResponse = $this->statCache->get($path); // we either don't know it, or we know it exists but need more details @@ -458,18 +511,15 @@ protected function propfind(string $path, bool $retryOnUnauthorized = true): arr $this->init(); $response = false; try { - $response = $this->client->propFind( + $response = $this->withAuthRetry(fn () => $this->client->propFind( $this->encodePath($path), $this->getPropfindProperties() - ); + )); $this->statCache->set($path, $response); } catch (ClientHttpException $e) { if ($e->getHttpStatus() === 404 || $e->getHttpStatus() === 405) { $this->statCache->clear($path . '/'); $this->statCache->set($path, false); - } elseif ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->propfind($path, false); } else { $this->convertException($e, $path); } @@ -527,41 +577,37 @@ public function unlink(string $path): bool { return $result; } - public function fopen(string $path, string $mode, bool $retryOnUnauthorized = true) { + public function fopen(string $path, string $mode) { $this->init(); $path = $this->cleanPath($path); switch ($mode) { case 'r': case 'rb': try { - $auth = [$this->user, $this->password]; - $headers = []; - if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; - } - $response = $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, - ]); + $response = $this->withAuthRetry(function () use ($path) { + $auth = [$this->user, $this->password]; + $headers = []; + if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { + $auth = []; + $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; + } + 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; - } elseif ($e->getResponse() instanceof ResponseInterface - && $e->getResponse()->getStatusCode() === 401 - && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->fopen($path, $mode, false); - } else { - throw $e; } + throw $e; } if ($response->getStatusCode() !== Http::STATUS_OK) { @@ -645,7 +691,7 @@ public function free_space(string $path): int|float|false { } } - public function touch(string $path, ?int $mtime = null, bool $retryOnUnauthorized = true): bool { + public function touch(string $path, ?int $mtime = null): bool { $this->init(); if (is_null($mtime)) { $mtime = time(); @@ -656,9 +702,9 @@ public function touch(string $path, ?int $mtime = null, bool $retryOnUnauthorize if ($this->file_exists($path)) { try { $this->statCache->remove($path); - $this->client->proppatch($this->encodePath($path), ['{DAV:}lastmodified' => $mtime]); + $this->withAuthRetry(fn () => $this->client->proppatch($this->encodePath($path), ['{DAV:}lastmodified' => $mtime])); // non-owncloud clients might not have accepted the property, need to recheck it - $response = $this->client->propfind($this->encodePath($path), ['{DAV:}getlastmodified'], 0); + $response = $this->withAuthRetry(fn () => $this->client->propfind($this->encodePath($path), ['{DAV:}getlastmodified'], 0)); if (isset($response['{DAV:}getlastmodified'])) { $remoteMtime = strtotime($response['{DAV:}getlastmodified']); if ($remoteMtime !== $mtime) { @@ -670,10 +716,6 @@ public function touch(string $path, ?int $mtime = null, bool $retryOnUnauthorize if ($e->getHttpStatus() === 501) { return false; } - if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->touch($path, $mtime, false); - } $this->convertException($e, $path); return false; } catch (\Exception $e) { @@ -693,21 +735,21 @@ public function file_put_contents(string $path, mixed $data): int|float|false { return $result; } - protected function uploadFile(string $path, string $target, bool $retryOnUnauthorized = true): void { + protected function uploadFile(string $path, string $target): void { $this->init(); // invalidate $target = $this->cleanPath($target); $this->statCache->remove($target); - $source = fopen($path, 'r'); - $auth = [$this->user, $this->password]; - $headers = []; - if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; - } - try { + $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), [ @@ -718,21 +760,12 @@ protected function uploadFile(string $path, string $target, bool $retryOnUnautho 'timeout' => $this->timeout, 'verify' => $this->verify, ]); - } catch (\GuzzleHttp\Exception\ClientException $e) { - if ($e->getResponse() instanceof ResponseInterface - && $e->getResponse()->getStatusCode() === 401 - && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - $this->uploadFile($path, $target, false); - return; - } - throw $e; - } + }); $this->removeCachedFile($target); } - public function rename(string $source, string $target, bool $retryOnUnauthorized = true): bool { + public function rename(string $source, string $target): bool { $this->init(); $source = $this->cleanPath($source); $target = $this->cleanPath($target); @@ -742,14 +775,14 @@ public function rename(string $source, string $target, bool $retryOnUnauthorized // needs trailing slash in destination $target = rtrim($target, '/') . '/'; } - $this->client->request( + $this->withAuthRetry(fn () => $this->client->request( 'MOVE', $this->encodePath($source), null, [ 'Destination' => $this->createBaseUri() . $this->encodePath($target), ] - ); + )); $this->statCache->clear($source . '/'); $this->statCache->clear($target . '/'); $this->statCache->set($source, false); @@ -758,10 +791,6 @@ public function rename(string $source, string $target, bool $retryOnUnauthorized $this->removeCachedFile($target); return true; } catch (ClientHttpException $e) { - if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->rename($source, $target, false); - } $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); @@ -769,7 +798,7 @@ public function rename(string $source, string $target, bool $retryOnUnauthorized return false; } - public function copy(string $source, string $target, bool $retryOnUnauthorized = true): bool { + public function copy(string $source, string $target): bool { $this->init(); $source = $this->cleanPath($source); $target = $this->cleanPath($target); @@ -779,23 +808,19 @@ public function copy(string $source, string $target, bool $retryOnUnauthorized = // needs trailing slash in destination $target = rtrim($target, '/') . '/'; } - $this->client->request( + $this->withAuthRetry(fn () => $this->client->request( 'COPY', $this->encodePath($source), null, [ 'Destination' => $this->createBaseUri() . $this->encodePath($target), ] - ); + )); $this->statCache->clear($target . '/'); $this->statCache->set($target, true); $this->removeCachedFile($target); return true; } catch (ClientHttpException $e) { - if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->copy($source, $target, false); - } $this->convertException($e); } catch (\Exception $e) { $this->convertException($e); @@ -897,15 +922,14 @@ protected function encodePath(string $path): string { } /** - * @param bool $retryOnUnauthorized whether to retry on 401 response (used to prevent infinite loops) * @return bool * @throws StorageInvalidException * @throws StorageNotAvailableException */ - protected function simpleResponse(string $method, string $path, ?string $body, int $expected, bool $retryOnUnauthorized = true): bool { + protected function simpleResponse(string $method, string $path, ?string $body, int $expected): bool { $path = $this->cleanPath($path); try { - $response = $this->client->request($method, $this->encodePath($path), $body); + $response = $this->withAuthRetry(fn () => $this->client->request($method, $this->encodePath($path), $body)); return $response['statusCode'] === $expected; } catch (ClientHttpException $e) { if ($e->getHttpStatus() === 404 && $method === 'DELETE') { @@ -914,11 +938,6 @@ protected function simpleResponse(string $method, string $path, ?string $body, i return false; } - if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - return $this->simpleResponse($method, $path, $body, $expected, false); - } - $this->convertException($e, $path); } catch (\Exception $e) { $this->convertException($e, $path); @@ -1075,15 +1094,15 @@ protected function convertException(Exception $e, string $path = ''): void { // TODO: only log for now, but in the future need to wrap/rethrow exception } - public function getDirectoryContent(string $directory, bool $retryOnUnauthorized = true): \Traversable { + public function getDirectoryContent(string $directory): \Traversable { $this->init(); $directory = $this->cleanPath($directory); try { - $responses = $this->client->propFind( + $responses = $this->withAuthRetry(fn () => $this->client->propFind( $this->encodePath($directory), $this->getPropfindProperties(), 1 - ); + )); array_shift($responses); //the first entry is the current directory if (!$this->statCache->hasKey($directory)) { @@ -1098,11 +1117,6 @@ public function getDirectoryContent(string $directory, bool $retryOnUnauthorized yield $this->getMetaFromPropfind($file, $response); } } catch (ClientHttpException $e) { - if ($e->getHttpStatus() === 401 && $retryOnUnauthorized && $this->isBearerAuth()) { - $this->reinitWithFreshToken(); - yield from $this->getDirectoryContent($directory, false); - return; - } $this->convertException($e, $directory); } catch (\Exception $e) { $this->convertException($e, $directory); From c65ada4f2df7aae2e6f2bf2bce3874d7ea6c3d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 12:49:12 +0100 Subject: [PATCH 41/58] fix(dav): keep refresh tokens in its own db table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../Controller/RequestHandlerController.php | 5 +- .../composer/composer/autoload_classmap.php | 3 ++ .../dav/composer/composer/autoload_static.php | 3 ++ apps/dav/lib/Controller/TokenController.php | 11 +++- apps/dav/lib/Db/OcmTokenMap.php | 41 ++++++++++++++ apps/dav/lib/Db/OcmTokenMapMapper.php | 42 +++++++++++++++ .../Version1037Date20260306120000.php | 53 +++++++++++++++++++ .../unit/Controller/TokenControllerTest.php | 26 ++++++--- 8 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 apps/dav/lib/Db/OcmTokenMap.php create mode 100644 apps/dav/lib/Db/OcmTokenMapMapper.php create mode 100644 apps/dav/lib/Migration/Version1037Date20260306120000.php diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 7f323b9f3086c..79e985d4eb717 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -8,6 +8,7 @@ namespace OCA\CloudFederationAPI\Controller; use OC\Authentication\Token\PublicKeyTokenProvider; +use OCA\DAV\Db\OcmTokenMapMapper; use OC\OCM\OCMSignatoryManager; use OCA\CloudFederationAPI\Config; use OCA\CloudFederationAPI\Db\FederatedInviteMapper; @@ -519,8 +520,8 @@ private function confirmNotificationIdentity( if ($identity === '') { $tokenProvider = Server::get(PublicKeyTokenProvider::class); $accessTokenDb = $tokenProvider->getToken($sharedSecret); - $refreshToken = $accessTokenDb->getUID(); - $identity = $provider->getFederationIdFromSharedSecret($refreshToken, $notification); + $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]); diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 4bc194f1e300d..8f4965cfa6980 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -282,6 +282,8 @@ 'OCA\\DAV\\Db\\AbsenceMapper' => $baseDir . '/../lib/Db/AbsenceMapper.php', 'OCA\\DAV\\Db\\Direct' => $baseDir . '/../lib/Db/Direct.php', 'OCA\\DAV\\Db\\DirectMapper' => $baseDir . '/../lib/Db/DirectMapper.php', + 'OCA\\DAV\\Db\\OcmTokenMap' => $baseDir . '/../lib/Db/OcmTokenMap.php', + 'OCA\\DAV\\Db\\OcmTokenMapMapper' => $baseDir . '/../lib/Db/OcmTokenMapMapper.php', 'OCA\\DAV\\Db\\Property' => $baseDir . '/../lib/Db/Property.php', 'OCA\\DAV\\Db\\PropertyMapper' => $baseDir . '/../lib/Db/PropertyMapper.php', 'OCA\\DAV\\Direct\\DirectFile' => $baseDir . '/../lib/Direct/DirectFile.php', @@ -392,6 +394,7 @@ 'OCA\\DAV\\Migration\\Version1034Date20250813093701' => $baseDir . '/../lib/Migration/Version1034Date20250813093701.php', 'OCA\\DAV\\Migration\\Version1036Date20251202000000' => $baseDir . '/../lib/Migration/Version1036Date20251202000000.php', 'OCA\\DAV\\Migration\\Version1038Date20260302000000' => $baseDir . '/../lib/Migration/Version1038Date20260302000000.php', + 'OCA\\DAV\\Migration\\Version1037Date20260306120000' => $baseDir . '/../lib/Migration/Version1037Date20260306120000.php', 'OCA\\DAV\\Model\\ExampleEvent' => $baseDir . '/../lib/Model/ExampleEvent.php', 'OCA\\DAV\\Paginate\\LimitedCopyIterator' => $baseDir . '/../lib/Paginate/LimitedCopyIterator.php', 'OCA\\DAV\\Paginate\\PaginateCache' => $baseDir . '/../lib/Paginate/PaginateCache.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index e4904517a3895..a3d4b06d2d3f0 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -297,6 +297,8 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Db\\AbsenceMapper' => __DIR__ . '/..' . '/../lib/Db/AbsenceMapper.php', 'OCA\\DAV\\Db\\Direct' => __DIR__ . '/..' . '/../lib/Db/Direct.php', 'OCA\\DAV\\Db\\DirectMapper' => __DIR__ . '/..' . '/../lib/Db/DirectMapper.php', + 'OCA\\DAV\\Db\\OcmTokenMap' => __DIR__ . '/..' . '/../lib/Db/OcmTokenMap.php', + 'OCA\\DAV\\Db\\OcmTokenMapMapper' => __DIR__ . '/..' . '/../lib/Db/OcmTokenMapMapper.php', 'OCA\\DAV\\Db\\Property' => __DIR__ . '/..' . '/../lib/Db/Property.php', 'OCA\\DAV\\Db\\PropertyMapper' => __DIR__ . '/..' . '/../lib/Db/PropertyMapper.php', 'OCA\\DAV\\Direct\\DirectFile' => __DIR__ . '/..' . '/../lib/Direct/DirectFile.php', @@ -407,6 +409,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\Version1034Date20250813093701' => __DIR__ . '/..' . '/../lib/Migration/Version1034Date20250813093701.php', 'OCA\\DAV\\Migration\\Version1036Date20251202000000' => __DIR__ . '/..' . '/../lib/Migration/Version1036Date20251202000000.php', 'OCA\\DAV\\Migration\\Version1038Date20260302000000' => __DIR__ . '/..' . '/../lib/Migration/Version1038Date20260302000000.php', + 'OCA\\DAV\\Migration\\Version1037Date20260306120000' => __DIR__ . '/..' . '/../lib/Migration/Version1037Date20260306120000.php', 'OCA\\DAV\\Model\\ExampleEvent' => __DIR__ . '/..' . '/../lib/Model/ExampleEvent.php', 'OCA\\DAV\\Paginate\\LimitedCopyIterator' => __DIR__ . '/..' . '/../lib/Paginate/LimitedCopyIterator.php', 'OCA\\DAV\\Paginate\\PaginateCache' => __DIR__ . '/..' . '/../lib/Paginate/PaginateCache.php', diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index fbad90dd629a8..b0d448283be75 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -9,6 +9,8 @@ use OC\Authentication\Token\IProvider; use OC\OCM\OCMSignatoryManager; +use OCA\DAV\Db\OcmTokenMap; +use OCA\DAV\Db\OcmTokenMapMapper; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -46,6 +48,7 @@ public function __construct( private readonly ISignatureManager $signatureManager, private readonly OCMSignatoryManager $signatoryManager, private readonly IAppConfig $appConfig, + private readonly OcmTokenMapMapper $ocmTokenMapMapper, ) { parent::__construct('dav', $request); } @@ -146,7 +149,7 @@ public function accessToken(): DataResponse { $accessToken = $this->tokenProvider->generateToken( $accessTokenString, - $refreshToken, // Keep refresh token with access token as UID + $token->getUID(), $token->getLoginName(), null, // No password for access tokens 'OCM Access Token', @@ -157,6 +160,12 @@ public function accessToken(): DataResponse { $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', diff --git a/apps/dav/lib/Db/OcmTokenMap.php b/apps/dav/lib/Db/OcmTokenMap.php new file mode 100644 index 0000000000000..afd91ffc225a3 --- /dev/null +++ b/apps/dav/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/dav/lib/Db/OcmTokenMapMapper.php b/apps/dav/lib/Db/OcmTokenMapMapper.php new file mode 100644 index 0000000000000..469353d8a935d --- /dev/null +++ b/apps/dav/lib/Db/OcmTokenMapMapper.php @@ -0,0 +1,42 @@ + + */ +class OcmTokenMapMapper extends QBMapper { + public function __construct(IDBConnection $db) { + parent::__construct($db, 'dav_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); + } + + 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/dav/lib/Migration/Version1037Date20260306120000.php b/apps/dav/lib/Migration/Version1037Date20260306120000.php new file mode 100644 index 0000000000000..04fe9000da0de --- /dev/null +++ b/apps/dav/lib/Migration/Version1037Date20260306120000.php @@ -0,0 +1,53 @@ +hasTable('dav_ocm_token_map')) { + return null; + } + + $table = $schema->createTable('dav_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'], 'dav_ocm_tkmap_atid'); + $table->addIndex(['expires'], 'dav_ocm_tkmap_exp'); + + return $schema; + } +} diff --git a/apps/dav/tests/unit/Controller/TokenControllerTest.php b/apps/dav/tests/unit/Controller/TokenControllerTest.php index 17e4038955d53..50e04aac47ac8 100644 --- a/apps/dav/tests/unit/Controller/TokenControllerTest.php +++ b/apps/dav/tests/unit/Controller/TokenControllerTest.php @@ -12,6 +12,7 @@ use OC\Authentication\Token\IProvider; use OC\OCM\OCMSignatoryManager; use OCA\DAV\Controller\TokenController; +use OCA\DAV\Db\OcmTokenMapMapper; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Utility\ITimeFactory; @@ -39,6 +40,7 @@ class TokenControllerTest extends TestCase { private ISignatureManager&MockObject $signatureManager; private OCMSignatoryManager&MockObject $signatoryManager; private IAppConfig&MockObject $appConfig; + private OcmTokenMapMapper&MockObject $ocmTokenMapMapper; private TokenController $controller; @@ -53,6 +55,7 @@ protected function setUp(): void { $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->controller = new TokenController( $this->request, @@ -63,6 +66,7 @@ protected function setUp(): void { $this->signatureManager, $this->signatoryManager, $this->appConfig, + $this->ocmTokenMapMapper, ); } @@ -74,14 +78,15 @@ public function testAccessTokenSuccess(): void { ->with($this->signatoryManager) ->willReturn($signedRequest); - $refreshToken = $this->createMock(IToken::class); - $refreshToken->method('getType')->willReturn(IToken::PERMANENT_TOKEN); - $refreshToken->method('getId')->willReturn(123); - $refreshToken->method('getLoginName')->willReturn('testuser'); + $refreshTokenMock = $this->createMock(IToken::class); + $refreshTokenMock->method('getType')->willReturn(IToken::PERMANENT_TOKEN); + $refreshTokenMock->method('getId')->willReturn(123); + $refreshTokenMock->method('getUID')->willReturn('testuser'); + $refreshTokenMock->method('getLoginName')->willReturn('testuser'); $this->tokenProvider->method('getToken') ->with('valid-refresh-token') - ->willReturn($refreshToken); + ->willReturn($refreshTokenMock); $this->random->method('generate') ->with(64, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS) @@ -90,10 +95,11 @@ public function testAccessTokenSuccess(): void { $this->timeFactory->method('getTime')->willReturn(1000000); $accessToken = $this->createMock(IToken::class); + $accessToken->method('getId')->willReturn(456); $this->tokenProvider->method('generateToken') ->with( 'generated-access-token', - 'valid-refresh-token', + 'testuser', 'testuser', null, 'OCM Access Token', @@ -110,6 +116,14 @@ public function testAccessTokenSuccess(): void { ->method('updateToken') ->with($accessToken); + $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; + })); + // Simulate POST body $this->simulatePostBody('grant_type=authorization_code&code=valid-refresh-token'); From 3a2bf28614a741a89eeacf9dc99a0c1ccdecfc2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 13:06:52 +0100 Subject: [PATCH 42/58] fix(dav): validate token exchange response before using it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/OCM/CloudFederationProviderFiles.php | 35 +++++++++++++--- lib/private/Files/Storage/DAV.php | 40 +++++++++++++++---- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 4d1330c939dc6..93ac8142a2cd1 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -774,15 +774,40 @@ private function exchangeToken(string $remote, #[SensitiveParameter] string $sha $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 (isset($data['access_token'])) { - $this->logger->debug('Successfully exchanged token for access token', ['remote' => $remote]); - return $data['access_token']; + if (!is_array($data)) { + $this->logger->warning('Token exchange response is not valid JSON', ['remote' => $remote]); + return null; } - $this->logger->warning('Token exchange response missing access_token', ['remote' => $remote, 'response' => $data]); - 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/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 68bd428a4ae49..731f772df1d39 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -309,16 +309,40 @@ protected function exchangeRefreshToken(): string { $response = $client->post($tokenEndpoint, $options); - $body = $response->getBody(); - $data = json_decode($body, true); + $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); + } - if (isset($data['access_token'])) { - $this->logger->debug('Successfully exchanged refresh token for access token', ['app' => 'dav']); - return $data['access_token']; - } else { - $this->logger->error('Failed to get access token from response', ['app' => 'dav']); - throw new StorageNotAvailableException('Could not obtain access token'); + $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'); From 9dc12f76f187eaba82266830ba1d2d41265c7908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 13:25:43 +0100 Subject: [PATCH 43/58] fix(dav): keep refresh tokens in its own db table, add migration for old shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/FederatedShareProvider.php | 12 +- .../Version1012Date20260306120000.php | 137 ++++++++++++++++++ 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.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/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index be0700febe794..0c2ec33a31129 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -9,6 +9,7 @@ use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; +use OCA\DAV\Db\OcmTokenMapMapper; use OC\Share20\Share; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Token\IToken; @@ -728,20 +729,19 @@ public function getShareByToken(string $token): IShare { if ($data === false) { // Token not found as refresh token, try looking it up as access token try { - $provider = Server::get(PublicKeyTokenProvider::class); - $accessTokenDb = $provider->getToken($token); - $refreshToken = $accessTokenDb->getUID(); + $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($refreshToken))) + ->andWhere($qb2->expr()->eq('token', $qb2->createNamedParameter($mapping->getRefreshToken()))) ->executeQuery(); $data = $cursor->fetch(); - } catch (InvalidTokenException) { - // Token is not a valid access token, share not found + } catch (InvalidTokenException|\OCP\AppFramework\Db\DoesNotExistException) { + // Token is not a valid access token or has no mapping, share not found } } if ($data === false) { diff --git a/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php new file mode 100644 index 0000000000000..c057fc3740728 --- /dev/null +++ b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php @@ -0,0 +1,137 @@ +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(); + + $replaced = 0; + $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 — cannot register in oc_authtoken. + // Generate a new 32-char token and update oc_share. + $newToken = $random->generate( + 32, + ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS + ); + + $updateQb = $db->getQueryBuilder(); + $updateQb->update('share') + ->set('token', $updateQb->createNamedParameter($newToken)) + ->where($updateQb->expr()->eq('id', $updateQb->createNamedParameter($shareId, IQueryBuilder::PARAM_INT))); + $updateQb->executeStatement(); + + $token = $newToken; + $replaced++; + } else { + // 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 replaced (short tokens), %d registered, %d already up-to-date.', + $replaced, + $registered, + $skipped + )); + + if ($replaced > 0) { + $output->warning(sprintf( + '%d federated share(s) had their token replaced. The remote side\'s copy of the ' + . 'old token is now stale — those shares will need to be re-created.', + $replaced + )); + } + } +} From 57c8ec5e6b90028ef0dbabf6a17b61ff43f78f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 13:49:57 +0100 Subject: [PATCH 44/58] fix(files_sharing): keep access tokens in its own field in the external shares table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/External/ExternalShare.php | 4 ++ apps/files_sharing/lib/External/Manager.php | 4 +- apps/files_sharing/lib/External/Storage.php | 12 +++--- .../Version33000Date20260306120000.php | 41 +++++++++++++++++++ 6 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 apps/files_sharing/lib/Migration/Version33000Date20260306120000.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 5962eab0d5f50..3b9462d87e0a5 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -87,6 +87,7 @@ '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\\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 51895e2be730a..155aeec7d2b2a 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -102,6 +102,7 @@ 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\\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/External/ExternalShare.php b/apps/files_sharing/lib/External/ExternalShare.php index 07a9a119fa051..ff8dd363ba00e 100644 --- a/apps/files_sharing/lib/External/ExternalShare.php +++ b/apps/files_sharing/lib/External/ExternalShare.php @@ -30,6 +30,8 @@ * @method void setShareToken(string $shareToken) * @method string|null getPassword() * @method void setPassword(?string $password) + * @method string|null getAccessToken() + * @method void setAccessToken(?string $accessToken) * @method string getName() * @method string getOwner() * @method void setOwner(string $owner) @@ -50,6 +52,7 @@ class ExternalShare extends SnowflakeAwareEntity implements \JsonSerializable { protected ?string $remoteId = null; protected ?string $shareToken = null; protected ?string $password = null; + protected ?string $accessToken = null; protected ?string $name = null; protected ?string $owner = null; protected ?string $user = null; @@ -65,6 +68,7 @@ public function __construct() { $this->addType('remoteId', Types::STRING); $this->addType('shareToken', Types::STRING); $this->addType('password', Types::STRING); + $this->addType('accessToken', Types::STRING); $this->addType('name', Types::STRING); $this->addType('owner', Types::STRING); $this->addType('user', Types::STRING); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 8b831fd72e71a..2fc97f8e6213d 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -113,6 +113,7 @@ public function addShare(ExternalShare $externalShare, IUser|IGroup|null $shareW 'remote' => $externalShare->getRemote(), 'token' => $externalShare->getShareToken(), 'password' => $externalShare->getPassword(), + 'access_token' => $externalShare->getAccessToken(), 'mountpoint' => $externalShare->getMountpoint(), 'owner' => $externalShare->getOwner(), 'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates'), @@ -190,6 +191,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()); @@ -576,7 +578,7 @@ public function getAcceptedShares(): array { public function updateAccessToken(string $shareToken, string $accessToken): void { try { $share = $this->externalShareMapper->getShareByToken($shareToken); - $share->setPassword($accessToken); + $share->setAccessToken($accessToken); $this->externalShareMapper->update($share); $this->logger->debug('Updated access token for share', ['shareToken' => substr($shareToken, 0, 8) . '...']); } catch (DoesNotExistException $e) { diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index a7ea67d85d67f..85f388b746077 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -55,7 +55,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private bool $tokenRefreshed = false; /** - * @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}|array $options */ public function __construct($options) { $this->memcacheFactory = Server::get(ICacheFactory::class); @@ -85,9 +85,9 @@ public function __construct($options) { $authType = \Sabre\DAV\Client::AUTH_BASIC; } - // If we have a stored access token (password), use Bearer auth regardless of discovery - // This handles the case where the share was created with must-exchange-token - if (!empty($options['password'])) { + // If we have a stored access token, use Bearer auth regardless of discovery. + // This handles the case where the share was created with must-exchange-token. + if (!empty($options['access_token'])) { $authType = \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER; } @@ -112,7 +112,9 @@ public function __construct($options) { 'root' => $webDavEndpoint, 'user' => $options['token'], 'authType' => $authType, - 'password' => (string)$options['password'], + 'password' => $authType === \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER + ? (string)($options['access_token'] ?? '') + : (string)($options['password'] ?? ''), 'discoveryService' => $discoveryService, ] ); diff --git a/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php b/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php new file mode 100644 index 0000000000000..4171add5b2705 --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version33000Date20260306120000.php @@ -0,0 +1,41 @@ +getTable('share_external'); + + if ($table->hasColumn('access_token')) { + return null; + } + + $table->addColumn('access_token', Types::STRING, [ + 'notnull' => false, + 'default' => null, + 'length' => 512, + ]); + + return $schema; + } +} From 374173ee800bfec6ba8e43fda1b4f2e14a51212b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 14:13:01 +0100 Subject: [PATCH 45/58] fix(files_sharing): prevent concurrent requests to refresh token independently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/External/ExternalShare.php | 4 ++ apps/files_sharing/lib/External/Manager.php | 4 +- apps/files_sharing/lib/External/Storage.php | 40 ++++++++++++++++--- .../Version33000Date20260306130000.php | 40 +++++++++++++++++++ 4 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 apps/files_sharing/lib/Migration/Version33000Date20260306130000.php diff --git a/apps/files_sharing/lib/External/ExternalShare.php b/apps/files_sharing/lib/External/ExternalShare.php index ff8dd363ba00e..5b3614c261e93 100644 --- a/apps/files_sharing/lib/External/ExternalShare.php +++ b/apps/files_sharing/lib/External/ExternalShare.php @@ -32,6 +32,8 @@ * @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) @@ -53,6 +55,7 @@ class ExternalShare extends SnowflakeAwareEntity implements \JsonSerializable { protected ?string $shareToken = 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; @@ -69,6 +72,7 @@ public function __construct() { $this->addType('shareToken', 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); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 2fc97f8e6213d..ee532defff023 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -114,6 +114,7 @@ public function addShare(ExternalShare $externalShare, IUser|IGroup|null $shareW 'token' => $externalShare->getShareToken(), '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'), @@ -575,10 +576,11 @@ public function getAcceptedShares(): array { * @param string $shareToken The share token (refresh token) to identify the share * @param string $accessToken The new access token to store */ - public function updateAccessToken(string $shareToken, string $accessToken): void { + public function updateAccessToken(string $shareToken, string $accessToken, int $expiresAt): void { try { $share = $this->externalShareMapper->getShareByToken($shareToken); $share->setAccessToken($accessToken); + $share->setAccessTokenExpires($expiresAt); $this->externalShareMapper->update($share); $this->logger->debug('Updated access token for share', ['shareToken' => substr($shareToken, 0, 8) . '...']); } catch (DoesNotExistException $e) { diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 85f388b746077..cdb952844bc0d 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -53,9 +53,11 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private 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; /** - * @param array{HttpClientService: IClientService, manager: ExternalShareManager, cloudId: ICloudId, mountpoint: string, token: string, access_token: ?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); @@ -103,6 +105,7 @@ public function __construct($options) { $this->mountPoint = $options['mountpoint']; $this->token = $options['token']; + $this->tokenExpiresAt = (int)($options['access_token_expires'] ?? 0); parent::__construct( [ @@ -123,20 +126,45 @@ public function __construct($options) { /** * Refresh the bearer token. Extends parent to also persist to database. * - * @return bool True if token was refreshed successfully + * 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. + * + * @return bool True if token was refreshed (or reused from DB) successfully */ protected function refreshBearerToken(): bool { - if ($this->tokenRefreshed) { - // only try to refresh once per request + $now = time(); + + // Fast path: in-memory token is still valid (single-process guard). + if ($this->tokenExpiresAt > $now) { return false; } - $this->tokenRefreshed = true; + // 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. + $this->password = $dbToken; + $this->tokenExpiresAt = $dbExpiry; + $this->ready = false; + $this->client = null; + $this->init(); + $this->logger->debug('Reused access token refreshed by another process', ['app' => 'files_sharing']); + return true; + } + } + + // 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->tokenExpiresAt = $expiresAt; - $this->manager->updateAccessToken($this->token, $newAccessToken); + $this->manager->updateAccessToken($this->token, $newAccessToken, $expiresAt); $this->ready = false; $this->client = null; diff --git a/apps/files_sharing/lib/Migration/Version33000Date20260306130000.php b/apps/files_sharing/lib/Migration/Version33000Date20260306130000.php new file mode 100644 index 0000000000000..ec1fe15d42f42 --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version33000Date20260306130000.php @@ -0,0 +1,40 @@ +getTable('share_external'); + + if ($table->hasColumn('access_token_expires')) { + return null; + } + + $table->addColumn('access_token_expires', Types::INTEGER, [ + 'notnull' => false, + 'default' => null, + ]); + + return $schema; + } +} From 4c90a7c5a8bc831d58f20cb0beeba78110aa3dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 14:43:07 +0100 Subject: [PATCH 46/58] fix(dav): cleanup expired access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../CleanupExpiredOcmTokensJob.php | 36 +++++++++++++++++++ apps/dav/lib/Controller/TokenController.php | 14 ++++++++ apps/dav/lib/Db/OcmTokenMapMapper.php | 16 +++++++++ 6 files changed, 69 insertions(+) create mode 100644 apps/dav/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 6f17473623d6d..1a040c251b26e 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -31,6 +31,7 @@ OCA\DAV\BackgroundJob\CalendarRetentionJob OCA\DAV\BackgroundJob\PruneOutdatedSyncTokensJob OCA\DAV\BackgroundJob\FederatedCalendarPeriodicSyncJob + OCA\DAV\BackgroundJob\CleanupExpiredOcmTokensJob diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 8f4965cfa6980..d9678a504cc3a 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -15,6 +15,7 @@ 'OCA\\DAV\\BackgroundJob\\BuildReminderIndexBackgroundJob' => $baseDir . '/../lib/BackgroundJob/BuildReminderIndexBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\CalendarRetentionJob' => $baseDir . '/../lib/BackgroundJob/CalendarRetentionJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupDirectLinksJob' => $baseDir . '/../lib/BackgroundJob/CleanupDirectLinksJob.php', + 'OCA\\DAV\\BackgroundJob\\CleanupExpiredOcmTokensJob' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredOcmTokensJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupInvitationTokenJob' => $baseDir . '/../lib/BackgroundJob/CleanupInvitationTokenJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupOrphanedChildrenJob' => $baseDir . '/../lib/BackgroundJob/CleanupOrphanedChildrenJob.php', 'OCA\\DAV\\BackgroundJob\\DeleteOutdatedSchedulingObjects' => $baseDir . '/../lib/BackgroundJob/DeleteOutdatedSchedulingObjects.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index a3d4b06d2d3f0..fedfcb98076dc 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -30,6 +30,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\BackgroundJob\\BuildReminderIndexBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/BuildReminderIndexBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\CalendarRetentionJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CalendarRetentionJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupDirectLinksJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupDirectLinksJob.php', + 'OCA\\DAV\\BackgroundJob\\CleanupExpiredOcmTokensJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredOcmTokensJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupInvitationTokenJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupInvitationTokenJob.php', 'OCA\\DAV\\BackgroundJob\\CleanupOrphanedChildrenJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupOrphanedChildrenJob.php', 'OCA\\DAV\\BackgroundJob\\DeleteOutdatedSchedulingObjects' => __DIR__ . '/..' . '/../lib/BackgroundJob/DeleteOutdatedSchedulingObjects.php', diff --git a/apps/dav/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php b/apps/dav/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php new file mode 100644 index 0000000000000..32f7c06c43738 --- /dev/null +++ b/apps/dav/lib/BackgroundJob/CleanupExpiredOcmTokensJob.php @@ -0,0 +1,36 @@ +setInterval(6 * 60 * 60); // run every 6 hours + $this->setTimeSensitivity(self::TIME_INSENSITIVE); + } + + protected function run($argument): void { + $this->mapper->deleteExpired($this->time->getTime()); + } +} diff --git a/apps/dav/lib/Controller/TokenController.php b/apps/dav/lib/Controller/TokenController.php index b0d448283be75..76265543f2359 100644 --- a/apps/dav/lib/Controller/TokenController.php +++ b/apps/dav/lib/Controller/TokenController.php @@ -139,6 +139,20 @@ public function accessToken(): DataResponse { ); } + // 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); + } + $accessTokenString = $this->random->generate( 64, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS diff --git a/apps/dav/lib/Db/OcmTokenMapMapper.php b/apps/dav/lib/Db/OcmTokenMapMapper.php index 469353d8a935d..535e80b0b131a 100644 --- a/apps/dav/lib/Db/OcmTokenMapMapper.php +++ b/apps/dav/lib/Db/OcmTokenMapMapper.php @@ -33,6 +33,22 @@ public function getByAccessTokenId(int $accessTokenId): OcmTokenMap { 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()) From f69865f235797bbdff3a78d869dccbc802c3c9bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 6 Mar 2026 14:59:42 +0100 Subject: [PATCH 47/58] fix(files_sharing): prevent infinite loop trying unsuccessfully to refresh access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/lib/External/Storage.php | 34 +++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index cdb952844bc0d..c133e66b55914 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -55,6 +55,13 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, 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, access_token: ?string, access_token_expires: ?int}|array $options @@ -130,6 +137,11 @@ public function __construct($options) { * 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 bool True if token was refreshed (or reused from DB) successfully */ protected function refreshBearerToken(): bool { @@ -146,9 +158,11 @@ protected function refreshBearerToken(): bool { $dbExpiry = $share->getAccessTokenExpires(); $dbToken = $share->getAccessToken(); if ($dbExpiry !== null && $dbExpiry > $now && $dbToken !== null) { - // Another process already refreshed — reuse DB token. + // Another process already refreshed — reuse DB token and reset failure state. $this->password = $dbToken; $this->tokenExpiresAt = $dbExpiry; + $this->refreshFailureCount = 0; + $this->refreshBackoffUntil = 0; $this->ready = false; $this->client = null; $this->init(); @@ -157,12 +171,24 @@ protected function refreshBearerToken(): bool { } } + // Gave up after max attempts: stop trying for the lifetime of this instance. + if ($this->refreshFailureCount >= self::REFRESH_MAX_ATTEMPTS) { + return false; + } + + // Still within the inter-attempt wait: don't hit the endpoint yet. + if ($this->refreshBackoffUntil > $now) { + return false; + } + // 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->tokenExpiresAt = $expiresAt; + $this->refreshFailureCount = 0; + $this->refreshBackoffUntil = 0; $this->manager->updateAccessToken($this->token, $newAccessToken, $expiresAt); @@ -173,8 +199,12 @@ protected function refreshBearerToken(): bool { $this->logger->debug('Successfully refreshed access token', ['app' => 'files_sharing']); return true; } catch (\Exception $e) { - $this->logger->warning('Failed to refresh access token', [ + $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 false; From f9de426c5360ae93ec2fa695bbd3497acb94c746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Tue, 10 Mar 2026 14:27:12 +0100 Subject: [PATCH 48/58] fix(files_sharing): missing autoloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/composer/composer/autoload_classmap.php | 1 + apps/files_sharing/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 3b9462d87e0a5..34f96f36bee0b 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -88,6 +88,7 @@ '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\\Version33000Date20260306130000' => $baseDir . '/../lib/Migration/Version33000Date20260306130000.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 155aeec7d2b2a..fae99831318ec 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -103,6 +103,7 @@ class ComposerStaticInitFiles_Sharing '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\\Version33000Date20260306130000' => __DIR__ . '/..' . '/../lib/Migration/Version33000Date20260306130000.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', From 61bade05087b6acc7c5b490603bb1568f826aad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 20 Mar 2026 13:32:12 +0100 Subject: [PATCH 49/58] test(dav): test bearer token login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../unit/Connector/Sabre/BearerAuthTest.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php index e85158db083b2..0041e0e5aa7af 100644 --- a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php @@ -69,6 +69,36 @@ public function testValidateBearerToken(): void { $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('Token')); } + public function testValidateBearerTokenFallbackToDoTryTokenLogin(): void { + // First two isLoggedIn() calls return false (tryTokenLogin did not log in), + // then doTryTokenLogin succeeds and the third call returns true. + $this->userSession + ->expects($this->exactly(3)) + ->method('isLoggedIn') + ->willReturnOnConsecutiveCalls(false, false, true); + + $this->userSession + ->expects($this->once()) + ->method('tryTokenLogin') + ->with($this->request); + + $this->userSession + ->expects($this->once()) + ->method('doTryTokenLogin') + ->with('BearerToken'); + + $user = $this->createMock(IUser::class); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + + $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('BearerToken')); + } + public function testChallenge(): void { /** @var RequestInterface&MockObject $request */ $request = $this->createMock(RequestInterface::class); From 9291b2c8da2e419005d9ce885a4585877ed076e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 20 Mar 2026 13:33:03 +0100 Subject: [PATCH 50/58] test(dav): test cleanup of expired access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../CleanupExpiredOcmTokensJobTest.php | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 apps/dav/tests/unit/BackgroundJob/CleanupExpiredOcmTokensJobTest.php diff --git a/apps/dav/tests/unit/BackgroundJob/CleanupExpiredOcmTokensJobTest.php b/apps/dav/tests/unit/BackgroundJob/CleanupExpiredOcmTokensJobTest.php new file mode 100644 index 0000000000000..aeda2171fb58e --- /dev/null +++ b/apps/dav/tests/unit/BackgroundJob/CleanupExpiredOcmTokensJobTest.php @@ -0,0 +1,44 @@ +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, []); + } +} From b29ebfa1f03aae9fcd932a6cc77beab1a816a0c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 20 Mar 2026 13:38:16 +0100 Subject: [PATCH 51/58] test(federatedfilesharing): test federated shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../OCM/CloudFederationProviderFilesTest.php | 310 ++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100644 apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php diff --git a/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php new file mode 100644 index 0000000000000..2593bf496aa0d --- /dev/null +++ b/apps/federatedfilesharing/tests/OCM/CloudFederationProviderFilesTest.php @@ -0,0 +1,310 @@ +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([]); + + $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(['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); + } +} From 582c9ac9676437d69489c83ab05f9ad0abf999e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 20 Mar 2026 13:39:37 +0100 Subject: [PATCH 52/58] test(files_sharing): test access tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../External/ManagerUpdateAccessTokenTest.php | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php diff --git a/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php new file mode 100644 index 0000000000000..2886c52fc5a6a --- /dev/null +++ b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php @@ -0,0 +1,113 @@ +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->setShareToken('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); + } +} From e171623960c648be726e97f1cd804d26c6c2cecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Fri, 20 Mar 2026 14:11:55 +0100 Subject: [PATCH 53/58] fix(files_sharing): correct access level for appConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/lib/External/Storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index c133e66b55914..596c4f7c864f8 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -50,7 +50,7 @@ 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) */ From 4aabb13d4034f2de3c7c670ec74ea1e29f42f652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 25 Mar 2026 16:17:32 +0100 Subject: [PATCH 54/58] fix: backwards compatibility for shares from instances before upgrading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/Controller/RequestHandlerController.php | 6 +++++- .../lib/OCM/CloudFederationProviderFiles.php | 2 +- apps/files_sharing/lib/External/MountProvider.php | 4 ++-- apps/files_sharing/lib/External/Storage.php | 9 +++------ 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 79e985d4eb717..0c5660faaa533 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -175,6 +175,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') { @@ -219,7 +220,10 @@ 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); $provider->shareReceived($share); } catch (ProviderDoesNotExistsException|ProviderCouldNotAddShareException $e) { diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 93ac8142a2cd1..4bf6d1e0b0b10 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -182,7 +182,7 @@ public function shareReceived(ICloudFederationShare $share): string { $externalShare->setRemote($remote); $externalShare->setRemoteId($remoteId); $externalShare->setShareToken($token); // refresh token (sharedSecret) - $externalShare->setPassword($accessToken); // access token (empty if no token exchange) + $externalShare->setAccessToken($accessToken ?: null); $externalShare->setName($name); $externalShare->setOwner($owner); $externalShare->setShareType($shareType); diff --git a/apps/files_sharing/lib/External/MountProvider.php b/apps/files_sharing/lib/External/MountProvider.php index 1f3620c5644be..df4d5ecc0a11d 100644 --- a/apps/files_sharing/lib/External/MountProvider.php +++ b/apps/files_sharing/lib/External/MountProvider.php @@ -59,7 +59,7 @@ private function getMount(IUser $user, array $data, IStorageFactory $storageFact 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', 'share_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))); @@ -99,7 +99,7 @@ public function getMountsForPath( } $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'remote', 'share_token', 'password', 'mountpoint', 'owner') + $qb->select('id', 'remote', 'share_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))); diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 596c4f7c864f8..11f939d0734e8 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -83,10 +83,6 @@ public function __construct($options) { $webDavEndpoint = $ocmProvider->extractProtocolEntry('file', 'webdav'); $remote = $ocmProvider->getEndPoint(); $authType = \Sabre\DAV\Client::AUTH_BASIC; - $capabilities = $ocmProvider->getCapabilities(); - if (in_array('exchange-token', $capabilities)) { - $authType = \OC\Files\Storage\BearerAuthAwareSabreClient::AUTH_BEARER; - } } catch (OCMProviderException|OCMArgumentException $e) { $this->logger->notice('exception while retrieving webdav endpoint', ['exception' => $e]); $webDavEndpoint = '/public.php/webdav'; @@ -94,8 +90,9 @@ public function __construct($options) { $authType = \Sabre\DAV\Client::AUTH_BASIC; } - // If we have a stored access token, use Bearer auth regardless of discovery. - // This handles the case where the share was created with must-exchange-token. + // 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; } From 68b530e0205358201f407c087439c46f7d80e9d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Wed, 25 Mar 2026 17:29:48 +0100 Subject: [PATCH 55/58] fix: backwards compatibility for shares for instances before upgrading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../Version1012Date20260306120000.php | 58 ++++++------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php index c057fc3740728..4837dbae63d9b 100644 --- a/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php +++ b/apps/federatedfilesharing/lib/Migration/Version1012Date20260306120000.php @@ -19,7 +19,6 @@ use OCP\IUserManager; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; -use OCP\Security\ISecureRandom; use OCP\Server; use OCP\Share\IShare; @@ -28,9 +27,10 @@ * as permanent tokens, which is required for the OCM token exchange flow. * * Shares created before this fork used TokenHandler (15-char tokens) and never - * registered in oc_authtoken. Those tokens are replaced with new 32-char tokens. - * Note: the remote's copy of a replaced token becomes stale; affected shares will - * need to be re-created. + * registered in oc_authtoken. Those legacy short tokens are left untouched so + * that the receiving instance can continue to authenticate via Basic auth with + * the original token. They will never participate in the token exchange flow, + * but they will keep working until the share is re-created with a new token. * * Shares created by this fork (32-char tokens) that are somehow missing from * oc_authtoken are silently repaired. @@ -43,7 +43,6 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { $db = Server::get(IDBConnection::class); $tokenProvider = Server::get(PublicKeyTokenProvider::class); - $random = Server::get(ISecureRandom::class); $userManager = Server::get(IUserManager::class); $qb = $db->getQueryBuilder(); @@ -58,7 +57,6 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array )) ->executeQuery(); - $replaced = 0; $registered = 0; $skipped = 0; @@ -68,30 +66,21 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $uid = (string)$row['uid_initiator']; if (strlen($token) < PublicKeyTokenProvider::TOKEN_MIN_LENGTH) { - // Old short token from TokenHandler — cannot register in oc_authtoken. - // Generate a new 32-char token and update oc_share. - $newToken = $random->generate( - 32, - ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS - ); - - $updateQb = $db->getQueryBuilder(); - $updateQb->update('share') - ->set('token', $updateQb->createNamedParameter($newToken)) - ->where($updateQb->expr()->eq('id', $updateQb->createNamedParameter($shareId, IQueryBuilder::PARAM_INT))); - $updateQb->executeStatement(); + // 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; + } - $token = $newToken; - $replaced++; - } else { - // 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. - } + // 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); @@ -120,18 +109,9 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $result->closeCursor(); $output->info(sprintf( - 'Federated share token migration: %d replaced (short tokens), %d registered, %d already up-to-date.', - $replaced, + 'Federated share token migration: %d registered, %d skipped (already up-to-date or legacy short token).', $registered, $skipped )); - - if ($replaced > 0) { - $output->warning(sprintf( - '%d federated share(s) had their token replaced. The remote side\'s copy of the ' - . 'old token is now stale — those shares will need to be re-created.', - $replaced - )); - } } } From a4b120e339fd3c495cdaa2a75b63642a71053f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 26 Mar 2026 09:33:17 +0100 Subject: [PATCH 56/58] fix: backwards compatibility for shares for instances before upgrading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/files_sharing/lib/External/Storage.php | 12 +++++++++++- lib/private/Files/Storage/DAV.php | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 11f939d0734e8..c88c4fdd58d2a 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -98,7 +98,14 @@ public function __construct($options) { } $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 @@ -111,9 +118,12 @@ public function __construct($options) { $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, diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 731f772df1d39..2b855067424fe 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -132,6 +132,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', From 05ae2bf996a00513f769e611c1d6a1fe21277d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 26 Mar 2026 09:33:46 +0100 Subject: [PATCH 57/58] fix: re-order imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- .../lib/Controller/RequestHandlerController.php | 2 +- apps/federatedfilesharing/lib/FederatedShareProvider.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 0c5660faaa533..9ee30927383fd 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -8,12 +8,12 @@ namespace OCA\CloudFederationAPI\Controller; use OC\Authentication\Token\PublicKeyTokenProvider; -use OCA\DAV\Db\OcmTokenMapMapper; use OC\OCM\OCMSignatoryManager; use OCA\CloudFederationAPI\Config; use OCA\CloudFederationAPI\Db\FederatedInviteMapper; use OCA\CloudFederationAPI\Events\FederatedInviteAcceptedEvent; use OCA\CloudFederationAPI\ResponseDefinitions; +use OCA\DAV\Db\OcmTokenMapMapper; use OCA\FederatedFileSharing\AddressHandler; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 0c2ec33a31129..fcc2e078e6a09 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -9,8 +9,8 @@ use OC\Authentication\Token\PublicKeyTokenProvider; use OC\Share20\Exception\InvalidShare; -use OCA\DAV\Db\OcmTokenMapMapper; use OC\Share20\Share; +use OCA\DAV\Db\OcmTokenMapMapper; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Token\IToken; use OCP\Constants; From 22bb002d99f40f937d5393b82a2fcb9a4d70b779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20P=C3=A9rez=20Arnaud?= Date: Thu, 26 Mar 2026 11:46:10 +0100 Subject: [PATCH 58/58] fix: avoid changing the IUserSession interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Enrique Pérez Arnaud --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 4 - .../unit/Connector/Sabre/BearerAuthTest.php | 30 ---- .../lib/Migration/DummyUserSession.php | 4 - lib/private/User/Session.php | 2 +- lib/public/IUserSession.php | 9 -- tests/lib/User/SessionTest.php | 131 ------------------ 6 files changed, 1 insertion(+), 179 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index e4eae2143cb65..c4bb0121ce98e 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -51,10 +51,6 @@ public function validateBearerToken($bearerToken) { $this->userSession->tryTokenLogin($this->request); $loggedIn = $this->userSession->isLoggedIn(); } - if (!$loggedIn) { - $this->userSession->doTryTokenLogin($bearerToken); - $loggedIn = $this->userSession->isLoggedIn(); - } if ($loggedIn) { return $this->setupUserFs($this->userSession->getUser()->getUID()); } diff --git a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php index 0041e0e5aa7af..e85158db083b2 100644 --- a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php @@ -69,36 +69,6 @@ public function testValidateBearerToken(): void { $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('Token')); } - public function testValidateBearerTokenFallbackToDoTryTokenLogin(): void { - // First two isLoggedIn() calls return false (tryTokenLogin did not log in), - // then doTryTokenLogin succeeds and the third call returns true. - $this->userSession - ->expects($this->exactly(3)) - ->method('isLoggedIn') - ->willReturnOnConsecutiveCalls(false, false, true); - - $this->userSession - ->expects($this->once()) - ->method('tryTokenLogin') - ->with($this->request); - - $this->userSession - ->expects($this->once()) - ->method('doTryTokenLogin') - ->with('BearerToken'); - - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getUID') - ->willReturn('admin'); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->willReturn($user); - - $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('BearerToken')); - } - public function testChallenge(): void { /** @var RequestInterface&MockObject $request */ $request = $this->createMock(RequestInterface::class); diff --git a/apps/files_external/lib/Migration/DummyUserSession.php b/apps/files_external/lib/Migration/DummyUserSession.php index 3946d8fad381c..1ebf0e1ec4f85 100644 --- a/apps/files_external/lib/Migration/DummyUserSession.php +++ b/apps/files_external/lib/Migration/DummyUserSession.php @@ -54,8 +54,4 @@ public function getImpersonatingUserID() : ?string { public function setImpersonatingUserID(bool $useCurrentUser = true): void { //no OP } - - public function doTryTokenLogin(string $token): bool { - return false; - } } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 6b1eaad18a4c8..c5744e90aa9b4 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -854,7 +854,7 @@ public function tryTokenLogin(IRequest $request) { return $this->doTryTokenLogin($token); } - public function doTryTokenLogin(string $token): bool { + private function doTryTokenLogin(string $token): bool { if (!$this->loginWithToken($token)) { return false; } diff --git a/lib/public/IUserSession.php b/lib/public/IUserSession.php index 9d80bb95735ab..42cc437aabae1 100644 --- a/lib/public/IUserSession.php +++ b/lib/public/IUserSession.php @@ -92,13 +92,4 @@ public function getImpersonatingUserID(): ?string; * @since 18.0.0 */ public function setImpersonatingUserID(bool $useCurrentUser = true): void; - - /** - * Try to login with the given token - * - * @param string $token - * @return bool true if successful - * @since 32.0.0 - */ - public function doTryTokenLogin(string $token): bool; } diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index cc6c42fa88c20..9817f7fd734e9 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -1337,135 +1337,4 @@ public function testLogClientInThrottlerEmail(): void { $this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-A-PASSWORD', $request, $this->throttler)); } - public function testDoTryTokenLoginSuccess(): void { - $manager = $this->createMock(Manager::class); - $session = $this->createMock(ISession::class); - - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('testuser'); - $user->method('isEnabled')->willReturn(true); - - $manager->method('get') - ->with('testuser') - ->willReturn($user); - - $token = $this->createMock(PublicKeyToken::class); - $token->method('getUID')->willReturn('testuser'); - $token->method('getLoginName')->willReturn('testuser'); - $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::PERMANENT_TOKEN); - $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); - - $this->tokenProvider->method('getToken') - ->with('valid-token') - ->willReturn($token); - - $appPasswordSet = false; - $session->expects($this->atLeastOnce()) - ->method('set') - ->willReturnCallback(function ($key, $value) use (&$appPasswordSet) { - // We expect app_password to be set for permanent tokens - if ($key === 'app_password') { - $appPasswordSet = true; - $this->assertEquals('valid-token', $value); - } - return true; - }); - - /** @var Session $userSession */ - $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) - ->onlyMethods(['setMagicInCookie']) - ->getMock(); - - $this->assertTrue($userSession->doTryTokenLogin('valid-token')); - $this->assertTrue($appPasswordSet, 'app_password should be set for permanent tokens'); - } - - public function testDoTryTokenLoginInvalidToken(): void { - $manager = $this->createMock(Manager::class); - $session = $this->createMock(ISession::class); - - $this->tokenProvider->method('getToken') - ->with('invalid-token') - ->willThrowException(new InvalidTokenException()); - - /** @var Session $userSession */ - $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) - ->onlyMethods(['setMagicInCookie']) - ->getMock(); - - $this->assertFalse($userSession->doTryTokenLogin('invalid-token')); - } - - public function testDoTryTokenLoginTemporaryToken(): void { - $manager = $this->createMock(Manager::class); - $session = $this->createMock(ISession::class); - - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('testuser'); - $user->method('isEnabled')->willReturn(true); - - $manager->method('get') - ->with('testuser') - ->willReturn($user); - - $token = $this->createMock(PublicKeyToken::class); - $token->method('getUID')->willReturn('testuser'); - $token->method('getLoginName')->willReturn('testuser'); - $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::TEMPORARY_TOKEN); - $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); - - $this->tokenProvider->method('getToken') - ->with('temp-token') - ->willReturn($token); - - // app_password should NOT be set for temporary tokens - $session->expects($this->atLeastOnce()) - ->method('set') - ->willReturnCallback(function ($key, $value) { - $this->assertNotEquals('app_password', $key, 'app_password should not be set for temporary tokens'); - return true; - }); - - /** @var Session $userSession */ - $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) - ->onlyMethods(['setMagicInCookie']) - ->getMock(); - - $this->assertTrue($userSession->doTryTokenLogin('temp-token')); - } - - public function testDoTryTokenLoginDisabledUser(): void { - $manager = $this->createMock(Manager::class); - $session = $this->createMock(ISession::class); - - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('testuser'); - $user->method('isEnabled')->willReturn(false); - - $manager->method('get') - ->with('testuser') - ->willReturn($user); - - $token = $this->createMock(PublicKeyToken::class); - $token->method('getUID')->willReturn('testuser'); - $token->method('getLoginName')->willReturn('testuser'); - $token->method('getType')->willReturn(\OCP\Authentication\Token\IToken::PERMANENT_TOKEN); - $token->method('getLastCheck')->willReturn($this->timeFactory->getTime()); - - $this->tokenProvider->method('getToken') - ->with('valid-token') - ->willReturn($token); - - /** @var Session $userSession */ - $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) - ->onlyMethods(['setMagicInCookie']) - ->getMock(); - - $this->expectException(LoginException::class); - $userSession->doTryTokenLogin('valid-token'); - } }