From 96384cd9502bef255e4a64ce5429e0805fb1854f Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 29 Jan 2025 19:52:05 +0100 Subject: [PATCH] fix(FediverseAction): Ensure valid fediverse links are generated Harden also for existing values of the profile. Signed-off-by: Ferdinand Thiessen --- build/psalm-baseline.xml | 8 + .../Profile/Actions/FediverseAction.php | 23 ++- .../Profile/Actions/FediverseActionTest.php | 186 ++++++++++++++++++ 3 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 tests/lib/Profile/Actions/FediverseActionTest.php diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 9d03850ac10..184587e572d 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2483,6 +2483,14 @@ + + + + + + + + diff --git a/lib/private/Profile/Actions/FediverseAction.php b/lib/private/Profile/Actions/FediverseAction.php index 1076027629d..b48f1db5c50 100644 --- a/lib/private/Profile/Actions/FediverseAction.php +++ b/lib/private/Profile/Actions/FediverseAction.php @@ -10,6 +10,7 @@ declare(strict_types=1); namespace OC\Profile\Actions; use OCP\Accounts\IAccountManager; +use OCP\Accounts\PropertyDoesNotExistException; use OCP\IURLGenerator; use OCP\IUser; use OCP\L10N\IFactory; @@ -27,8 +28,13 @@ class FediverseAction implements ILinkAction { } public function preload(IUser $targetUser): void { - $account = $this->accountManager->getAccount($targetUser); - $this->value = $account->getProperty(IAccountManager::PROPERTY_FEDIVERSE)->getValue(); + try { + $account = $this->accountManager->getAccount($targetUser); + $this->value = $account->getProperty(IAccountManager::PROPERTY_FEDIVERSE)->getValue(); + } catch (PropertyDoesNotExistException) { + // `getTarget` will return null to skip this action + $this->value = ''; + } } public function getAppId(): string { @@ -57,11 +63,18 @@ class FediverseAction implements ILinkAction { } public function getTarget(): ?string { - if (empty($this->value)) { + if ($this->value === '') { + return null; + } + + $handle = $this->value[0] === '@' ? substr($this->value, 1) : $this->value; + [$username, $instance] = [...explode('@', $handle, 2), '']; + + if (($username === '') || ($instance === '')) { + return null; + } elseif (str_contains($username, '/') || str_contains($instance, '/')) { return null; } - $username = $this->value[0] === '@' ? substr($this->value, 1) : $this->value; - [$username, $instance] = explode('@', $username); return 'https://' . $instance . '/@' . $username; } } diff --git a/tests/lib/Profile/Actions/FediverseActionTest.php b/tests/lib/Profile/Actions/FediverseActionTest.php new file mode 100644 index 00000000000..c0e2e6e6309 --- /dev/null +++ b/tests/lib/Profile/Actions/FediverseActionTest.php @@ -0,0 +1,186 @@ +accountManager = $this->createMock(IAccountManager::class); + $this->l10n = $this->createMock(IL10N::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + + $factory = $this->createMock(IFactory::class); + $factory->method('get') + ->with('lib') + ->willReturn($this->l10n); + + $this->action = new FediverseAction( + $this->accountManager, + $factory, + $this->urlGenerator, + ); + } + + public function testGetActionId(): void { + self::assertEquals( + IAccountManager::PROPERTY_FEDIVERSE, + $this->action->getId(), + ); + } + + public function testGetAppId(): void { + self::assertEquals( + 'core', + $this->action->getAppId(), + ); + } + + public function testGetDisplayId(): void { + $this->l10n->expects(self::once()) + ->method('t') + ->with('Fediverse') + ->willReturn('Translated fediverse'); + + self::assertEquals( + 'Translated fediverse', + $this->action->getDisplayId(), + ); + } + + public function testGetPriority(): void { + self::assertEquals( + 50, + $this->action->getPriority(), + ); + } + + public function testGetIcon(): void { + $this->urlGenerator->expects(self::once()) + ->method('getAbsoluteURL') + ->with('the-image-path') + ->willReturn('resolved-image-path'); + + $this->urlGenerator->expects(self::once()) + ->method('imagePath') + ->with('core', 'actions/mastodon.svg') + ->willReturn('the-image-path'); + + self::assertEquals( + 'resolved-image-path', + $this->action->getIcon(), + ); + } + + public static function dataGetTitle(): array { + return [ + ['the-user@example.com'], + ['@the-user@example.com'], + ]; + } + + /** @dataProvider dataGetTitle */ + public function testGetTitle(string $value): void { + $property = $this->createMock(IAccountProperty::class); + $property->method('getValue') + ->willReturn($value); + + $user = $this->createMock(IUser::class); + + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_FEDIVERSE) + ->willReturn($property); + + $this->accountManager->method('getAccount') + ->with($user) + ->willReturn($account); + + $this->l10n->expects(self::once()) + ->method('t') + ->with(self::anything(), ['@the-user@example.com']) + ->willReturn('Translated title'); + + // Preload and test + $this->action->preload($user); + self::assertEquals( + 'Translated title', + $this->action->getTitle(), + ); + } + + public static function dataGetTarget(): array { + return [ + ['', null], + [null, null], + ['user@example.com', 'https://example.com/@user'], + ['@user@example.com', 'https://example.com/@user'], + ['@user@social.example.com', 'https://social.example.com/@user'], + // invalid stuff + ['@user', null], + ['@example.com', null], + ['@@example.com', null], + // evil stuff + ['user@example.com/evil.exe', null], + ['@user@example.com/evil.exe', null], + ['user/evil.exe@example.com', null], + ['@user/evil.exe@example.com', null], + ]; + } + + /** @dataProvider dataGetTarget */ + public function testGetTarget(?string $value, ?string $expected): void { + $user = $this->createMock(IUser::class); + + $account = $this->createMock(IAccount::class); + $this->accountManager->method('getAccount') + ->with($user) + ->willReturn($account); + + if ($value === null) { + // Property does not exist so throw + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_FEDIVERSE) + ->willThrowException(new PropertyDoesNotExistException(IAccountManager::PROPERTY_FEDIVERSE)); + } else { + $property = $this->createMock(IAccountProperty::class); + $property->method('getValue') + ->willReturn($value); + $account->method('getProperty') + ->willReturn($property); + } + + // Preload and test + $this->action->preload($user); + self::assertEquals( + $expected, + $this->action->getTarget(), + ); + } +}