From db96073459289a03860f72fc7c3f3fb96ceb97e5 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Tue, 7 Jul 2026 12:56:40 +0200 Subject: [PATCH] fix(auth): confine OCM access token to the masked share endpoint Gate the temporary-token bearer auth behind an allowOcmAccessToken flag that only the public, share-masked webdav endpoints set, and drop the BearerAuth backend from the CalDAV/CardDAV entrypoints. The token is now honored only on /public.php/webdav, where per-share masking applies. Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: Micke Nordin --- apps/dav/appinfo/v1/caldav.php | 11 +--- apps/dav/appinfo/v1/carddav.php | 11 +--- apps/dav/appinfo/v1/publicwebdav.php | 1 + apps/dav/appinfo/v2/publicremote.php | 1 + apps/dav/lib/Connector/Sabre/BearerAuth.php | 3 +- .../unit/Connector/Sabre/BearerAuthTest.php | 34 ++++++++++++ lib/private/User/Session.php | 7 ++- tests/lib/User/SessionTest.php | 53 +++++++++++++++++++ 8 files changed, 98 insertions(+), 23 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index 6127c245df7..d1c1381be0a 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -18,7 +18,6 @@ use OCA\DAV\CalDAV\Security\RateLimitingPlugin; use OCA\DAV\CalDAV\Validation\CalDavValidatePlugin; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\Connector\Sabre\Auth; -use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; use OCA\DAV\Connector\Sabre\MaintenancePlugin; use OCA\DAV\Connector\Sabre\Principal; @@ -48,12 +47,6 @@ $authBackend = new Auth( Server::get(IThrottler::class), 'principals/' ); -$bearerAuthBackend = new BearerAuth( - Server::get(IUserSession::class), - Server::get(ISession::class), - Server::get(IRequest::class), - Server::get(IConfig::class), -); $principalBackend = new Principal( Server::get(IUserManager::class), Server::get(IGroupManager::class), @@ -115,9 +108,7 @@ $server->setBaseUri($baseuri); // Add plugins $server->addPlugin(new MaintenancePlugin(Server::get(IConfig::class), $davL10n)); -$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); -$authPlugin->addBackend($bearerAuthBackend); -$server->addPlugin($authPlugin); +$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend)); $server->addPlugin(new \Sabre\CalDAV\Plugin()); $server->addPlugin(new LegacyDAVACL()); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index d2b4681f763..d883bae450b 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -17,7 +17,6 @@ use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin; use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\Connector\Sabre\Auth; -use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; use OCA\DAV\Connector\Sabre\MaintenancePlugin; use OCA\DAV\Connector\Sabre\Principal; @@ -45,12 +44,6 @@ $authBackend = new Auth( Server::get(IThrottler::class), 'principals/' ); -$bearerAuthBackend = new BearerAuth( - Server::get(IUserSession::class), - Server::get(ISession::class), - Server::get(IRequest::class), - Server::get(IConfig::class), -); $principalBackend = new Principal( Server::get(IUserManager::class), Server::get(IGroupManager::class), @@ -97,9 +90,7 @@ $server->httpRequest->setUrl(Server::get(IRequest::class)->getRequestUri()); $server->setBaseUri($baseuri); // Add plugins $server->addPlugin(new MaintenancePlugin(Server::get(IConfig::class), Server::get(IL10nFactory::class)->get('dav'))); -$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); -$authPlugin->addBackend($bearerAuthBackend); -$server->addPlugin($authPlugin); +$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend)); $server->addPlugin(new Plugin()); $server->addPlugin(new LegacyDAVACL()); diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 1bcd4c28c85..f993a810429 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -56,6 +56,7 @@ $bearerAuthBackend = new BearerAuth( Server::get(ISession::class), Server::get(IRequest::class), Server::get(IConfig::class), + allowOcmAccessToken: true, ); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); $authPlugin->addBackend($bearerAuthBackend); diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php index 1e3b08c2599..6fb5cafe6cb 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -74,6 +74,7 @@ $bearerAuthBackend = new BearerAuth( $session, $request, Server::get(IConfig::class), + allowOcmAccessToken: true, ); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); $authPlugin->addBackend($bearerAuthBackend); diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index bb922bae587..4d1530cf980 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -28,6 +28,7 @@ class BearerAuth extends AbstractBearer { private IConfig $config, private string $principalPrefix = 'principals/users/', private string $token = '', + private bool $allowOcmAccessToken = false, ) { // setup realm $defaults = new Defaults(); @@ -57,7 +58,7 @@ class BearerAuth extends AbstractBearer { \OC_User::setIncognitoMode(false); if (!$this->userSession->isLoggedIn()) { - $this->userSession->tryTokenLogin($this->request); + $this->userSession->tryTokenLogin($this->request, $this->allowOcmAccessToken); } if ($this->userSession->isLoggedIn()) { 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 4b8255efa31..cf12927cc97 100644 --- a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php @@ -70,6 +70,40 @@ class BearerAuthTest extends TestCase { $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('Token')); } + public function testValidateBearerTokenDefaultsOcmFlagToFalse(): void { + $this->userSession + ->method('isLoggedIn') + ->willReturnOnConsecutiveCalls(false, false); + $this->userSession + ->expects($this->once()) + ->method('tryTokenLogin') + ->with($this->request, false); + + $this->assertFalse($this->bearerAuth->validateBearerToken('Token')); + } + + public function testValidateBearerTokenPassesOcmFlagWhenAllowed(): void { + $bearerAuth = new BearerAuth( + $this->userSession, + $this->session, + $this->request, + $this->config, + allowOcmAccessToken: true, + ); + $this->userSession + ->method('isLoggedIn') + ->willReturnOnConsecutiveCalls(false, true); + $this->userSession + ->expects($this->once()) + ->method('tryTokenLogin') + ->with($this->request, true); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser')->willReturn($user); + + $this->assertSame('principals/users/admin', $bearerAuth->validateBearerToken('Token')); + } + public function testChallenge(): void { /** @var RequestInterface&MockObject $request */ $request = $this->createMock(RequestInterface::class); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index ac31f22b640..04ee0aa796f 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -816,10 +816,13 @@ class Session implements IUserSession, Emitter { * Tries to login the user with auth token header * * @param IRequest $request + * @param bool $allowOcmAccessToken Whether an OCM access token may log in + * from a Bearer header. Only the masked + * public share endpoints may set this. * @todo check remember me cookie * @return boolean */ - public function tryTokenLogin(IRequest $request) { + public function tryTokenLogin(IRequest $request, bool $allowOcmAccessToken = false) { $authHeader = $request->getHeader('Authorization'); $tokenFromCookie = false; if (str_starts_with($authHeader, 'Bearer ')) { @@ -847,7 +850,7 @@ class Session implements IUserSession, Emitter { if ($dbToken instanceof PublicKeyToken && $dbToken->getType() === IToken::TEMPORARY_TOKEN && !$tokenFromCookie - && $dbToken->getName() !== IToken::OCM_ACCESS_TOKEN_NAME) { + && !($allowOcmAccessToken && $dbToken->getName() === IToken::OCM_ACCESS_TOKEN_NAME)) { // Session token but from Bearer header, not allowed return false; } diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 0bd1f446901..f6db3d22214 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -662,6 +662,59 @@ class SessionTest extends \Test\TestCase { self::assertTrue($loginResult); } + public function testTryTokenLoginOcmAccessTokenRejectedFromBearerByDefault(): void { + $request = $this->createMock(IRequest::class); + $request->method('getHeader')->with('Authorization')->willReturn('Bearer ocm-access-token'); + $dbToken = new PublicKeyToken(); + $dbToken->setId(42); + $dbToken->setUid('alice'); + $dbToken->setLoginName('alice'); + $dbToken->setLastCheck(0); + $dbToken->setType(IToken::TEMPORARY_TOKEN); + $dbToken->setName(IToken::OCM_ACCESS_TOKEN_NAME); + $this->tokenProvider->expects(self::once()) + ->method('getToken') + ->with('ocm-access-token') + ->willReturn($dbToken); + // The guard must reject before any login is attempted. + $this->manager->expects(self::never()) + ->method('get'); + + $loginResult = $this->userSession->tryTokenLogin($request); + + self::assertFalse($loginResult); + } + + public function testTryTokenLoginOcmAccessTokenAllowedFromBearerWhenPermitted(): void { + $request = $this->createMock(IRequest::class); + $request->method('getHeader')->with('Authorization')->willReturn('Bearer ocm-access-token'); + $dbToken = new PublicKeyToken(); + $dbToken->setId(42); + $dbToken->setUid('alice'); + $dbToken->setLoginName('alice'); + $dbToken->setLastCheck(0); + $dbToken->setType(IToken::TEMPORARY_TOKEN); + $dbToken->setName(IToken::OCM_ACCESS_TOKEN_NAME); + $this->tokenProvider->method('getToken') + ->with('ocm-access-token') + ->willReturn($dbToken); + $this->session->method('set') + ->willReturnCallback(function ($key, $value): void { + if ($key === 'app_password') { + throw new ExpectationFailedException('app_password should not be set in session'); + } + }); + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(true); + $this->manager->method('get') + ->with('alice') + ->willReturn($user); + + $loginResult = $this->userSession->tryTokenLogin($request, true); + + self::assertTrue($loginResult); + } + public function testRememberLoginValidToken(): void { $session = $this->createMock(Memory::class); $managerMethods = get_class_methods(Manager::class);