From 937b3a609ebc3498c1bf9472d0d4c215cc2a6852 Mon Sep 17 00:00:00 2001 From: TimedIn Date: Fri, 14 Mar 2025 21:47:01 +0100 Subject: [PATCH 1/3] feat (contacts): format/clean emails of carddav in plugin Signed-off-by: TimedIn --- .../Validation/CardDavValidatePlugin.php | 39 +++++++++++ .../Validation/CardDavValidatePluginTest.php | 64 +++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php index a5fd80ec124..76d8a7a3221 100644 --- a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php +++ b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php @@ -10,11 +10,14 @@ namespace OCA\DAV\CardDAV\Validation; use OCA\DAV\AppInfo\Application; use OCP\IAppConfig; +use OCP\Mail\IMailer; +use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Server; use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; +use Sabre\VObject; class CardDavValidatePlugin extends ServerPlugin { @@ -25,6 +28,7 @@ class CardDavValidatePlugin extends ServerPlugin { public function initialize(Server $server): void { $server->on('beforeMethod:PUT', [$this, 'beforePut']); + $server->on('beforeMethod:POST', [$this, 'beforePost']); } public function beforePut(RequestInterface $request, ResponseInterface $response): bool { @@ -33,8 +37,43 @@ class CardDavValidatePlugin extends ServerPlugin { if ((int)$request->getRawServerValue('CONTENT_LENGTH') > $cardSizeLimit) { throw new Forbidden("VCard object exceeds $cardSizeLimit bytes"); } + + $this->validateEmail($request, $response); + // all tests passed return true return true; } + public function beforePost(RequestInterface $request, ResponseInterface $response): bool { + $this->validateEmail($request, $response); + return true; + } + + public function validateEmail(RequestInterface $request, ResponseInterface $response) { + // Get and Parse VCard + $cardData = $request->getBodyAsString(); + if (!$cardData) { + return true; + } + + $vCard = VObject\Reader::read($cardData); + + // Get IMailer + $mailer = \OC::$server->get(IMailer::class); + + // Loop through all emails, validate. If needed trim email + foreach ($vCard->EMAIL as $email) { + $trimedEmail = trim((string)$email); + if ($trimedEmail !== '' && !$mailer->validateMailAddress($trimedEmail)) { + throw new BadRequest('Invalid email in vCard'); + } + if ($trimedEmail !== $email) { + $vCard->remove($email); + $vCard->add('EMAIL', $trimedEmail, ['type' => $email['TYPE']]); + } + } + + // Pass parsed vcard + $request->setBody($vCard->serialize()); + } } diff --git a/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php b/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php index 39155aace8b..64fb705a581 100644 --- a/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php +++ b/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php @@ -12,6 +12,7 @@ namespace OCA\DAV\Tests\unit\CardDAV\Validation; use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin; use OCP\IAppConfig; use PHPUnit\Framework\MockObject\MockObject; +use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; @@ -70,4 +71,67 @@ class CardDavValidatePluginTest extends TestCase { } + public function testPutNoEmail(): void { + $vcard = $this->getVCardWithEmail(); + + $this->request + ->method('getBodyAsString') + ->willReturn($vcard); + $this->assertTrue( + $this->plugin->beforePut($this->request, $this->response), + ); + } + public function testPutValidEmail(): void { + $vcard = $this->getVCardWithEmail('example@nextcloud.com'); + + $this->request + ->method('getBodyAsString') + ->willReturn($vcard); + $this->assertTrue( + $this->plugin->beforePut($this->request, $this->response), + ); + } + public function testPutInvalidEmail(): void { + $vcard = $this->getVCardWithEmail('example-nextcloud'); + + $this->request + ->method('getBodyAsString') + ->willReturn($vcard); + $this->expectException(BadRequest::class); + $this->assertTrue( + $this->plugin->beforePut($this->request, $this->response), + ); + } + + public function testPutTrimmingEmail(): void { + $vcard = $this->getVCardWithEmail(' example@nextcloud.com '); + $vcardTrimmed = $this->getVCardWithEmail('example@nextcloud.com'); + + $this->request + ->method('getBodyAsString') + ->willReturn($vcard); + + $this->request + ->expects($this->once()) + ->method('setBody') + ->with($this->equalTo($vcardTrimmed)); + + $this->assertTrue( + $this->plugin->beforePut($this->request, $this->response), + ); + } + + private function getVCardWithEmail($email) { + return << Date: Wed, 19 Mar 2025 13:00:17 +0100 Subject: [PATCH 2/3] fix: fix comparison for email trimming Signed-off-by: TimedIn --- apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php index 76d8a7a3221..dc445949a82 100644 --- a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php +++ b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php @@ -63,11 +63,11 @@ class CardDavValidatePlugin extends ServerPlugin { // Loop through all emails, validate. If needed trim email foreach ($vCard->EMAIL as $email) { - $trimedEmail = trim((string)$email); + $trimedEmail = trim((string) $email); if ($trimedEmail !== '' && !$mailer->validateMailAddress($trimedEmail)) { throw new BadRequest('Invalid email in vCard'); } - if ($trimedEmail !== $email) { + if ($trimedEmail !== (string) $email) { $vCard->remove($email); $vCard->add('EMAIL', $trimedEmail, ['type' => $email['TYPE']]); } From 1ccb0fbefcb02770d75d5f05da01168743313d89 Mon Sep 17 00:00:00 2001 From: TimedIn Date: Sun, 30 Mar 2025 14:04:07 +0200 Subject: [PATCH 3/3] Fix: linting, carddav test Signed-off-by: TimedIn --- apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php | 4 ++-- .../unit/CardDAV/Validation/CardDavValidatePluginTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php index dc445949a82..e9172b6d207 100644 --- a/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php +++ b/apps/dav/lib/CardDAV/Validation/CardDavValidatePlugin.php @@ -63,11 +63,11 @@ class CardDavValidatePlugin extends ServerPlugin { // Loop through all emails, validate. If needed trim email foreach ($vCard->EMAIL as $email) { - $trimedEmail = trim((string) $email); + $trimedEmail = trim((string)$email); if ($trimedEmail !== '' && !$mailer->validateMailAddress($trimedEmail)) { throw new BadRequest('Invalid email in vCard'); } - if ($trimedEmail !== (string) $email) { + if ($trimedEmail !== (string)$email) { $vCard->remove($email); $vCard->add('EMAIL', $trimedEmail, ['type' => $email['TYPE']]); } diff --git a/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php b/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php index 64fb705a581..cdbf2d40060 100644 --- a/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php +++ b/apps/dav/tests/unit/CardDAV/Validation/CardDavValidatePluginTest.php @@ -72,7 +72,7 @@ class CardDavValidatePluginTest extends TestCase { } public function testPutNoEmail(): void { - $vcard = $this->getVCardWithEmail(); + $vcard = $this->getVCardWithEmail(''); $this->request ->method('getBodyAsString')