upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-13 21:17:20 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

Merge pull request #59615 from nextcloud/backport/59606/stable33

Browse source 
[stable33] fix(notifications): Require absolute links for support of desktop and mobile clients
This commit is contained in:
Joas Schilling 2026-04-13 20:55:48 +02:00 committed by GitHub
commit fba14e4459
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 87 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/private/Notification/Action.php
View file

@ -76,6 +76,12 @@ class Action implements IAction {
if ($link === '' || isset($link[256])) {
throw new InvalidValueException('link');
}
// Only allow absolute URLs for support of desktop and mobile clients
if (!str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
throw new InvalidValueException('link');
}
if (!in_array($requestType, [
self::TYPE_GET,
self::TYPE_POST,

17
lib/private/Notification/Manager.php
View file

@ -372,23 +372,6 @@ class Manager implements IManager {
throw new IncompleteParsedNotificationException();
}
$link = $notification->getLink();
if ($link !== '' && !str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
$this->logger->warning('Link of notification is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
}
$icon = $notification->getIcon();
if ($icon !== '' && !str_starts_with($icon, 'http://') && !str_starts_with($icon, 'https://')) {
$this->logger->warning('Icon of notification is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
}
foreach ($notification->getParsedActions() as $action) {
$link = $action->getLink();
if ($link !== '' && !str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
$this->logger->warning('Link of action is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
}
}
return $notification;
}

12
lib/private/Notification/Notification.php
View file

@ -310,6 +310,12 @@ class Notification implements INotification {
if ($link === '' || isset($link[4000])) {
throw new InvalidValueException('link');
}
// Only allow absolute URLs for support of desktop and mobile clients
if (!str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
throw new InvalidValueException('link');
}
$this->link = $link;
return $this;
}
@ -328,6 +334,12 @@ class Notification implements INotification {
if ($icon === '' || isset($icon[4000])) {
throw new InvalidValueException('icon');
}
// Only allow absolute URLs for support of desktop and mobile clients
if (!str_starts_with($icon, 'http://') && !str_starts_with($icon, 'https://')) {
throw new InvalidValueException('icon');
}
$this->icon = $icon;
return $this;
}

4
lib/public/Notification/IAction.php
View file

@ -77,6 +77,10 @@ interface IAction {
public function isPrimary(): bool;
/**
* Set the target endpoint for this action
*
* All links should always be relative to support desktop and mobile clients.
*
* @param string $link
* @param string $requestType
* @return $this

4
lib/public/Notification/INotification.php
View file

@ -232,6 +232,10 @@ interface INotification {
public function getRichMessageParameters(): array;
/**
* Set the target endpoint for this action
*
* All links should always be relative to support desktop and mobile clients.
*
* @param string $link
* @return $this
* @throws InvalidValueException if the link is invalid

38
tests/lib/Notification/ActionTest.php
View file

@ -94,10 +94,13 @@ class ActionTest extends TestCase {
public static function dataSetLink(): array {
return [
['test1', 'GET'],
['test2', 'POST'],
[str_repeat('a', 1), 'PUT'],
[str_repeat('a', 256), 'DELETE'],
['http://example.tld/', 'GET'],
['https://example.tld/api/v1/resource', 'POST'],
['https://example.tld/?q=1&r=2', 'PUT'],
['https://example.tld/path#frag', 'DELETE'],
['https://example.tld/web', 'WEB'],
// Maximum length (256 chars total, including the scheme)
['https://' . str_repeat('a', 256 - 8), 'GET'],
];
}
@ -115,12 +118,27 @@ class ActionTest extends TestCase {
public static function dataSetLinkInvalid(): array {
return [
// Invalid link
// Invalid link — empty / too long
['', 'GET'],
[str_repeat('a', 257), 'GET'],
['https://' . str_repeat('a', 257 - 8), 'GET'],
// Invalid type
['url', 'notGET'],
// Disallowed schemes
['javascript:alert(1)', 'WEB'],
['JavaScript:alert(1)', 'WEB'],
['javascript:alert(1)', 'GET'],
['data:text/html,<script>alert(1)</script>', 'WEB'],
['vbscript:msgbox("xss")', 'WEB'],
['file:///etc/passwd', 'GET'],
['mailto:test@example.tld', 'WEB'],
['ftp://example.tld/', 'GET'],
// Relative urls
['/relative/path', 'WEB'],
['//protocol-relative.tld/', 'GET'],
['url', 'GET'],
// Invalid type (with a valid http(s) link, so the type check is what trips)
['https://example.tld/', 'notGET'],
];
}
@ -159,7 +177,7 @@ class ActionTest extends TestCase {
$this->action->setLabel('label');
$this->assertFalse($this->action->isValid());
$this->assertFalse($this->action->isValidParsed());
$this->action->setLink('link', 'GET');
$this->action->setLink('https://example.tld/', 'GET');
$this->assertTrue($this->action->isValid());
$this->assertFalse($this->action->isValidParsed());
}
@ -170,7 +188,7 @@ class ActionTest extends TestCase {
$this->action->setParsedLabel('label');
$this->assertFalse($this->action->isValid());
$this->assertFalse($this->action->isValidParsed());
$this->action->setLink('link', 'GET');
$this->action->setLink('https://example.tld/', 'GET');
$this->assertFalse($this->action->isValid());
$this->assertTrue($this->action->isValidParsed());
}

55
tests/lib/Notification/NotificationTest.php
View file

@ -327,7 +327,13 @@ class NotificationTest extends TestCase {
}
public static function dataSetLink(): array {
return self::dataValidString(4000);
return [
['http://example.tld/'],
['https://example.tld/'],
['https://example.tld/path/to/resource?query=1&other=2#fragment'],
// Maximum length (4000 chars total, including the scheme)
['https://' . str_repeat('a', 4000 - 8)],
];
}
/**
@ -341,29 +347,38 @@ class NotificationTest extends TestCase {
}
public static function dataSetLinkInvalid(): array {
return self::dataInvalidString(4000);
return [
// Empty / too long
[''],
['https://' . str_repeat('a', 4001 - 8)],
// Disallowed schemes
['javascript:alert(1)'],
['JavaScript:alert(1)'],
['data:text/html,<script>alert(1)</script>'],
['vbscript:msgbox("xss")'],
['file:///etc/passwd'],
['mailto:test@example.tld'],
['ftp://example.tld/'],
['ws://example.tld/'],
// Relative urls
['/relative/path'],
['//protocol-relative.tld/'],
['example.tld/path'],
['test1'],
];
}
/**
* @param mixed $link
*
*/
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetLinkInvalid')]
public function testSetLinkInvalid($link): void {
public function testSetLinkInvalid(string $link): void {
$this->expectException(\InvalidArgumentException::class);
$this->notification->setLink($link);
}
public static function dataSetIcon(): array {
return self::dataValidString(4000);
}
/**
* @param string $icon
*/
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetIcon')]
public function testSetIcon($icon): void {
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetLink')]
public function testSetIcon(string $icon): void {
$this->assertSame('', $this->notification->getIcon());
$this->assertSame($this->notification, $this->notification->setIcon($icon));
$this->assertSame($icon, $this->notification->getIcon());
@ -373,12 +388,8 @@ class NotificationTest extends TestCase {
return self::dataInvalidString(4000);
}
/**
* @param mixed $icon
*
*/
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetIconInvalid')]
public function testSetIconInvalid($icon): void {
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetLinkInvalid')]
public function testSetIconInvalid(string $icon): void {
$this->expectException(\InvalidArgumentException::class);
$this->notification->setIcon($icon);