diff --git a/apps/dav/lib/Connector/Sabre/DavAclPlugin.php b/apps/dav/lib/Connector/Sabre/DavAclPlugin.php index 913ded880a05d..db79f5b3637b7 100644 --- a/apps/dav/lib/Connector/Sabre/DavAclPlugin.php +++ b/apps/dav/lib/Connector/Sabre/DavAclPlugin.php @@ -55,16 +55,22 @@ public function checkPrivileges($uri, $privileges, $recursion = self::R_PARENT, if ($this->getCurrentUserPrincipal() === $node->getOwner()) { throw new Forbidden('Access denied'); - } else { - throw new NotFound( - sprintf( - "%s with name '%s' could not be found", - $type, - $node->getName() - ) - ); } + // RFC 3744 section 3 only allows masking the 403 as a 404 when the missing + // privilege also hides that the resource exists. If the user may read the node + // its existence is no secret, so surface the real 403 instead of a 404. + if (parent::checkPrivileges($uri, '{DAV:}read', $recursion, false)) { + return parent::checkPrivileges($uri, $privileges, $recursion, true); + } + + throw new NotFound( + sprintf( + "%s with name '%s' could not be found", + $type, + $node->getName() + ) + ); } return $access; diff --git a/apps/dav/tests/unit/Connector/Sabre/DavAclPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/DavAclPluginTest.php new file mode 100644 index 0000000000000..18d9567e7ccdb --- /dev/null +++ b/apps/dav/tests/unit/Connector/Sabre/DavAclPluginTest.php @@ -0,0 +1,118 @@ +tree = $this->createMock(Tree::class); + } + + /** + * Run every case against a real Calendar and a real AddressBook node: the single ACL + * override guards CalDAV and CardDAV alike, and real nodes make get_class() hit both + * arms of the type switch. + * + * @return array flavour => [flavour, node uri] + */ + public static function sharedNodeProvider(): array { + return [ + 'calendar' => ['calendar', 'personal_shared_by_alice'], + 'addressbook' => ['addressbook', 'contacts_shared_by_alice'], + ]; + } + + #[DataProvider('sharedNodeProvider')] + public function testReadableNodeWithoutWriteReturns403(string $flavour, string $uri): void { + $this->tree->method('getNodeForPath') + ->willReturn($this->sharedNode($flavour, $uri, 'principals/users/alice')); + $plugin = $this->pluginFor('principals/users/bob', ['{DAV:}read']); + + $this->expectException(NeedPrivileges::class); + $plugin->checkPrivileges("calendars/bob/$uri/object", '{DAV:}write'); + } + + #[DataProvider('sharedNodeProvider')] + public function testUnreadableNodeReturns404(string $flavour, string $uri): void { + $this->tree->method('getNodeForPath') + ->willReturn($this->sharedNode($flavour, $uri, 'principals/users/alice')); + $plugin = $this->pluginFor('principals/users/bob', []); + + $type = $flavour === 'calendar' ? 'Calendar' : 'Addressbook'; + $this->expectException(NotFound::class); + $this->expectExceptionMessage("$type with name '$uri' could not be found"); + $plugin->checkPrivileges("calendars/bob/$uri/object", '{DAV:}write'); + } + + #[DataProvider('sharedNodeProvider')] + public function testOwnerWithoutPrivilegeReturnsForbidden(string $flavour, string $uri): void { + $this->tree->method('getNodeForPath') + ->willReturn($this->sharedNode($flavour, $uri, 'principals/users/alice')); + $plugin = $this->pluginFor('principals/users/alice', []); + + $this->expectException(Forbidden::class); + $this->expectExceptionMessage('Access denied'); + $plugin->checkPrivileges("calendars/alice/$uri/object", '{DAV:}write'); + } + + private function pluginFor(string $currentPrincipal, array $privilegeSet): DavAclPlugin&MockObject { + $plugin = $this->getMockBuilder(DavAclPlugin::class) + ->onlyMethods(['getCurrentUserPrincipal', 'getCurrentUserPrivilegeSet']) + ->getMock(); + $plugin->method('getCurrentUserPrincipal')->willReturn($currentPrincipal); + $plugin->method('getCurrentUserPrivilegeSet')->willReturn($privilegeSet); + $plugin->initialize(new Server($this->tree)); + return $plugin; + } + + private function sharedNode(string $flavour, string $uri, string $owner): INode { + $info = [ + 'id' => 1, + 'uri' => $uri, + 'principaluri' => 'principals/users/bob', + '{DAV:}displayname' => 'Shared', + '{http://owncloud.org/ns}owner-principal' => $owner, + ]; + if ($flavour === 'calendar') { + return new Calendar( + $this->createMock(CalDavBackend::class), + $info, + $this->createMock(IL10N::class), + $this->createMock(IConfig::class), + $this->createMock(LoggerInterface::class), + ); + } + return new AddressBook( + $this->createMock(CardDavBackend::class), + $info, + $this->createMock(IL10N::class), + ); + } +}