Merge pull request #333 from owncloud/fix-absolute-urls

Add command to repair notifications and properly handle mail sending …

appinfo/info.xml

@@ -24,6 +24,7 @@
 
 	<commands>
 		<command>OCA\Notifications\Command\Generate</command>
+		<command>OCA\Notifications\Command\RepairNotifications</command>
 	</commands>
 	<settings>
 		<personal>OCA\Notifications\Panels\Personal\NotificationsPanel</personal>

lib/Command/RepairNotifications.php

@@ -0,0 +1,74 @@
+<?php
+/**
+ * @author Jannik Stehle <[email protected]>
+ * @author Jan Ackermann <[email protected]>
+ *
+ * @copyright Copyright (c) 2021, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Notifications\Command;
+
+use OCA\Notifications\Handler;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class RepairNotifications extends Command {
+
+	/** @var Handler */
+	protected $handler;
+
+	public static $availableSubjects = [
+		'relativeLinks'
+	];
+
+	/**
+	 * @param Handler $handler
+	 */
+	public function __construct(Handler $handler) {
+		parent::__construct();
+		$this->handler = $handler;
+	}
+
+	protected function configure() {
+		$this
+			->setName('notifications:repairNotifications')
+			->setDescription('Repair existing notifications')
+			->addArgument('subject', InputArgument::REQUIRED, 'Subject to repair')
+		;
+	}
+
+	/**
+	 * @param InputInterface $input
+	 * @param OutputInterface $output
+	 * @return int
+	 */
+	protected function execute(InputInterface $input, OutputInterface $output) {
+		$subject = $input->getArgument('subject');
+
+		if (!\in_array($subject, self::$availableSubjects)) {
+			$output->writeln('Invalid subject');
+			return 1;
+		}
+
+		$updatedNotificationsCount = $this->handler->removeBaseUrlFromAbsoluteLinks();
+
+		$output->writeln("$updatedNotificationsCount notifications were updated");
+		return 0;
+	}
+}

lib/Handler.php

@@ -310,4 +310,53 @@ protected function notificationFromRow(array $row) {
 
 		return $notification;
 	}
+
+	/**
+	 * Remove the base url from absolute links in the database.
+	 * This affects the columns 'link' and 'action'.
+	 * e.g: http://owncloud.com/test -> /test
+	 *
+	 * @return int number of updated notifications
+	 */
+	public function removeBaseUrlFromAbsoluteLinks() {
+		$sql = $this->connection->getQueryBuilder();
+		$sql->select(['notification_id', 'link', 'actions'])
+			->from('notifications')
+			->where($sql->expr()->like('link', $sql->createPositionalParameter('http%')))
+			->orWhere($sql->expr()->like('actions', $sql->createPositionalParameter('%"link":"http%')));
+
+		$statement = $sql->execute();
+		$counter = 0;
+
+		while ($row = $statement->fetch()) {
+			$sql = $this->connection->getQueryBuilder();
+			$sql->update('notifications')
+				->where($sql->expr()->eq('notification_id', $sql->createNamedParameter($row['notification_id'])));
+
+			$linkUrlComponents = \parse_url($row['link']);
+			if (isset($linkUrlComponents['scheme'], $linkUrlComponents['path'])) {
+				$sql->set('link', $sql->createNamedParameter($linkUrlComponents['path']));
+			}
+
+			if (\strpos($row['actions'], 'http') !== false) {
+				$actions = \json_decode($row['actions'], true);
+
+				foreach ($actions as $index => $action) {
+					$actionUrlComponents = \parse_url($action['link']);
+					if (isset($actionUrlComponents['scheme'], $actionUrlComponents['path'])) {
+						$actions[$index]['link'] = $actionUrlComponents['path'];
+					}
+				}
+
+				$sql->set('actions', $sql->createNamedParameter(\json_encode($actions)));
+			}
+
+			$counter++;
+			$sql->execute();
+		}
+
+		$statement->closeCursor();
+
+		return $counter;
+	}
 }

lib/Mailer/NotificationMailer.php

@@ -26,6 +26,7 @@
 use OCP\Mail\IMailer;
 use OCP\Template;
 use OCA\Notifications\Configuration\OptionsStorage;
+use OCP\IURLGenerator;
 
 /**
  * The class will focus on sending notifications via email. In addition, some email-related
@@ -41,10 +42,14 @@ class NotificationMailer {
 	/** @var OptionsStorage */
 	private $optionsStorage;
 
-	public function __construct(IManager $manager, IMailer $mailer, OptionsStorage $optionsStorage) {
+	/** @var IURLGenerator */
+	private $urlGenerator;
+
+	public function __construct(IManager $manager, IMailer $mailer, OptionsStorage $optionsStorage, IURLGenerator $urlGenerator) {
 		$this->manager = $manager;
 		$this->mailer = $mailer;
 		$this->optionsStorage = $optionsStorage;
+		$this->urlGenerator = $urlGenerator;
 	}
 
 	/**
@@ -71,8 +76,12 @@ public function sendNotification(INotification $notification, $serverUrl, $email
 		$emailMessage->setTo([$emailAddress]);
 
 		$notificationLink = $notification->getLink();
+		$urlComponents = \parse_url($notificationLink);
+
 		if ($notificationLink === '') {
 			$notificationLink = $serverUrl;
+		} elseif (!isset($urlComponents['host'])) {
+			$notificationLink = $this->urlGenerator->getAbsoluteURL($notificationLink);
 		}
 
 		$parsedSubject = $notification->getParsedSubject();

tests/Unit/Command/RepairNotificationsTest.php

@@ -0,0 +1,64 @@
+<?php
+/**
+ * @author Jannik Stehle <[email protected]>
+ * @author Jan Ackermann <[email protected]>
+ *
+ * @copyright Copyright (c) 2021, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Notifications\Tests\Unit\Command;
+
+use OCA\Notifications\Command\Generate;
+use OCA\Notifications\Command\RepairNotifications;
+use OCA\Notifications\Handler;
+use OCA\Notifications\Tests\Unit\TestCase;
+use Symfony\Component\Console\Tester\CommandTester;
+
+class RepairNotificationsTest extends TestCase {
+
+	/** @var Handler | \PHPUnit\Framework\MockObject\MockObject */
+	protected $handler;
+	/** @var Generate */
+	protected $command;
+	/** @var CommandTester */
+	protected $tester;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->handler = $this->createMock(Handler::class);
+		$this->command = new RepairNotifications($this->handler);
+		$this->tester = new CommandTester($this->command);
+	}
+
+	public function testInvalidSubject() {
+		$options = [];
+		$input = ['subject' => 'test'];
+		$response = $this->tester->execute($input, $options);
+		$this->assertEquals(1, $response);
+	}
+
+	public function testRepairLinks() {
+		$options = [];
+		$input = ['subject' => RepairNotifications::$availableSubjects[0]];
+
+		$this->handler->expects($this->once())->method('removeBaseUrlFromAbsoluteLinks');
+
+		$response = $this->tester->execute($input, $options);
+		$this->assertEquals(0, $response);
+	}
+}

tests/Unit/Mailer/NotificationMailerTest.php

@@ -22,6 +22,7 @@
 namespace OCA\Notifications\Tests\Unit\Mailer;
 
 use OC\Mail\Mailer;
+use OCP\IURLGenerator;
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
 use OCP\L10N\IFactory;
@@ -36,10 +37,10 @@ class NotificationMailerTest extends \Test\TestCase {
 	private $mailer;
 	/** @var OptionsStorage */
 	private $optionsStorage;
-	/** @var IFactory */
-	private $l10nFactory;
 	/** @var NotificationMailer*/
 	private $notificationMailer;
+	/** @var IURLGenerator*/
+	private $urlGenerator;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -57,11 +58,11 @@ protected function setUp(): void {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->l10nFactory = $this->getMockBuilder(IFactory::class)
+		$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->notificationMailer = new NotificationMailer($this->manager, $this->mailer, $this->optionsStorage, $this->l10nFactory);
+		$this->notificationMailer = new NotificationMailer($this->manager, $this->mailer, $this->optionsStorage, $this->urlGenerator);
 	}
 
 	public function emailProvider() {
@@ -106,7 +107,6 @@ public function testSendNotification() {
 				return \vsprintf($text, $params);
 			}));
 
-		$this->l10nFactory->method('get')->willReturn($mockedL10N);
 		$this->mailer->expects($this->once())->method('send');
 
 		$this->optionsStorage->method('getOptions')
@@ -144,7 +144,6 @@ public function testSendNotificationFailedRecipients() {
 				return \vsprintf($text, $params);
 			}));
 
-		$this->l10nFactory->method('get')->willReturn($mockedL10N);
 		$this->mailer->expects($this->once())
 			->method('send')
 			->willReturn(['userTest1']);