From 2c1218826d2de8cea0ef698a133bd1b903d669ee Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 23 Mar 2021 14:52:04 +0100 Subject: [PATCH] Validate the website field input to be a valid URL Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 3 ++ lib/private/Accounts/AccountManager.php | 30 +++++++++++++++++++ tests/lib/Accounts/AccountManagerTest.php | 26 ++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index c59c16666f..331ea49b8f 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -491,6 +491,9 @@ class UsersController extends Controller { if ($e->getMessage() === IAccountManager::PROPERTY_PHONE) { throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); } + if ($e->getMessage() === IAccountManager::PROPERTY_WEBSITE) { + throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid website')); + } throw new \InvalidArgumentException($this->l10n->t('Some account data was invalid')); } } diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index ea8f99e021..eff025e511 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -120,6 +120,25 @@ class AccountManager implements IAccountManager { throw new \InvalidArgumentException(self::PROPERTY_PHONE); } + /** + * + * @param string $input + * @return string + * @throws \InvalidArgumentException When the website did not have http(s) as protocol or the host name was empty + */ + protected function parseWebsite(string $input): string { + $parts = parse_url($input); + if (!isset($parts['scheme']) || ($parts['scheme'] !== 'https' && $parts['scheme'] !== 'http')) { + throw new \InvalidArgumentException(self::PROPERTY_WEBSITE); + } + + if (!isset($parts['host']) || $parts['host'] === '') { + throw new \InvalidArgumentException(self::PROPERTY_WEBSITE); + } + + return $input; + } + /** * update user record * @@ -155,6 +174,17 @@ class AccountManager implements IAccountManager { } } + if (isset($data[self::PROPERTY_WEBSITE]) && $data[self::PROPERTY_WEBSITE]['value'] !== '') { + try { + $data[self::PROPERTY_WEBSITE]['value'] = $this->parseWebsite($data[self::PROPERTY_WEBSITE]['value']); + } catch (\InvalidArgumentException $e) { + if ($throwOnData) { + throw $e; + } + $data[self::PROPERTY_WEBSITE]['value'] = ''; + } + } + $allowedScopes = [ self::SCOPE_PRIVATE, self::SCOPE_LOCAL, diff --git a/tests/lib/Accounts/AccountManagerTest.php b/tests/lib/Accounts/AccountManagerTest.php index 27ebed6979..687ae29ff7 100644 --- a/tests/lib/Accounts/AccountManagerTest.php +++ b/tests/lib/Accounts/AccountManagerTest.php @@ -455,4 +455,30 @@ class AccountManagerTest extends TestCase { self::assertEquals($phoneNumber, self::invokePrivate($instance, 'parsePhoneNumber', [$phoneInput])); } } + + public function dataParseWebsite(): array { + return [ + ['https://nextcloud.com', 'https://nextcloud.com'], + ['http://nextcloud.com', 'http://nextcloud.com'], + ['ftp://nextcloud.com', null], + ['//nextcloud.com/', null], + ['https:///?query', null], + ]; + } + + /** + * @dataProvider dataParseWebsite + * @param string $websiteInput + * @param string|null $websiteOutput + */ + public function testParseWebsite(string $websiteInput, ?string $websiteOutput): void { + $instance = $this->getInstance(); + + if ($websiteOutput === null) { + $this->expectException(\InvalidArgumentException::class); + self::invokePrivate($instance, 'parseWebsite', [$websiteInput]); + } else { + self::assertEquals($websiteOutput, self::invokePrivate($instance, 'parseWebsite', [$websiteInput])); + } + } }