From 79c79cd96e077f5f3ae22bb6a56520b5764b2c58 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Wed, 16 Feb 2022 16:16:37 +0100 Subject: [PATCH] fix: url scheme validation --- app/controllers/api/avatars.php | 4 ++-- app/controllers/api/projects.php | 8 +++---- src/Appwrite/Network/Validator/URL.php | 19 +++++++++++++++- tests/e2e/Services/Avatars/AvatarsBase.php | 8 +++++++ .../Projects/ProjectsConsoleClientTest.php | 22 +++++++++++++++++++ tests/unit/Network/Validators/URLTest.php | 18 ++++++++++----- 6 files changed, 66 insertions(+), 13 deletions(-) diff --git a/app/controllers/api/avatars.php b/app/controllers/api/avatars.php index 161e87b13..e72911671 100644 --- a/app/controllers/api/avatars.php +++ b/app/controllers/api/avatars.php @@ -144,7 +144,7 @@ App::get('/v1/avatars/image') ->label('sdk.description', '/docs/references/avatars/get-image.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_IMAGE) - ->param('url', '', new URL(), 'Image URL which you want to crop.') + ->param('url', '', new URL(['http', 'https']), 'Image URL which you want to crop.') ->param('width', 400, new Range(0, 2000), 'Resize preview image width, Pass an integer between 0 to 2000.', true) ->param('height', 400, new Range(0, 2000), 'Resize preview image height, Pass an integer between 0 to 2000.', true) ->inject('response') @@ -213,7 +213,7 @@ App::get('/v1/avatars/favicon') ->label('sdk.description', '/docs/references/avatars/get-favicon.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_IMAGE) - ->param('url', '', new URL(), 'Website URL which you want to fetch the favicon from.') + ->param('url', '', new URL(['http', 'https']), 'Website URL which you want to fetch the favicon from.') ->inject('response') ->action(function ($url, $response) { /** @var Appwrite\Utopia\Response $response */ diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 3104f51d7..e9c15ceed 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -48,7 +48,7 @@ App::post('/v1/projects') ->param('teamId', '', new UID(), 'Team unique ID.') ->param('description', '', new Text(256), 'Project description. Max length: 256 chars.', true) ->param('logo', '', new Text(1024), 'Project logo.', true) - ->param('url', '', new URL(), 'Project URL.', true) + ->param('url', '', new URL(['http', 'https']), 'Project URL.', true) ->param('legalName', '', new Text(256), 'Project legal Name. Max length: 256 chars.', true) ->param('legalCountry', '', new Text(256), 'Project legal Country. Max length: 256 chars.', true) ->param('legalState', '', new Text(256), 'Project legal State. Max length: 256 chars.', true) @@ -345,7 +345,7 @@ App::patch('/v1/projects/:projectId') ->param('name', null, new Text(128), 'Project name. Max length: 128 chars.') ->param('description', '', new Text(256), 'Project description. Max length: 256 chars.', true) ->param('logo', '', new Text(1024), 'Project logo.', true) - ->param('url', '', new URL(), 'Project URL.', true) + ->param('url', '', new URL(['http', 'https']), 'Project URL.', true) ->param('legalName', '', new Text(256), 'Project legal name. Max length: 256 chars.', true) ->param('legalCountry', '', new Text(256), 'Project legal country. Max length: 256 chars.', true) ->param('legalState', '', new Text(256), 'Project legal state. Max length: 256 chars.', true) @@ -582,7 +582,7 @@ App::post('/v1/projects/:projectId/webhooks') ->param('projectId', null, new UID(), 'Project unique ID.') ->param('name', null, new Text(128), 'Webhook name. Max length: 128 chars.') ->param('events', null, new ArrayList(new WhiteList(array_keys(Config::getParam('events'), true), true)), 'Events list.') - ->param('url', null, new URL(), 'Webhook URL.') + ->param('url', null, new URL(['http', 'https']), 'Webhook URL.') ->param('security', false, new Boolean(true), 'Certificate verification, false for disabled or true for enabled.') ->param('httpUser', '', new Text(256), 'Webhook HTTP user. Max length: 256 chars.', true) ->param('httpPass', '', new Text(256), 'Webhook HTTP password. Max length: 256 chars.', true) @@ -704,7 +704,7 @@ App::put('/v1/projects/:projectId/webhooks/:webhookId') ->param('webhookId', null, new UID(), 'Webhook unique ID.') ->param('name', null, new Text(128), 'Webhook name. Max length: 128 chars.') ->param('events', null, new ArrayList(new WhiteList(array_keys(Config::getParam('events'), true), true)), 'Events list.') - ->param('url', null, new URL(), 'Webhook URL.') + ->param('url', null, new URL(['http', 'https']), 'Webhook URL.') ->param('security', false, new Boolean(true), 'Certificate verification, false for disabled or true for enabled.') ->param('httpUser', '', new Text(256), 'Webhook HTTP user. Max length: 256 chars.', true) ->param('httpPass', '', new Text(256), 'Webhook HTTP password. Max length: 256 chars.', true) diff --git a/src/Appwrite/Network/Validator/URL.php b/src/Appwrite/Network/Validator/URL.php index 61d1941e0..5933ec83a 100644 --- a/src/Appwrite/Network/Validator/URL.php +++ b/src/Appwrite/Network/Validator/URL.php @@ -9,10 +9,19 @@ use Utopia\Validator; * * Validate that an variable is a valid URL * - * @package Utopia\Validator + * @package Appwrite\Network\Validator */ class URL extends Validator { + protected array $allowedSchemes; + + /** + * @param array $allowedSchemes + */ + public function __construct(array $allowedSchemes = []) + { + $this->allowedSchemes = $allowedSchemes; + } /** * Get Description * @@ -22,6 +31,10 @@ class URL extends Validator */ public function getDescription(): string { + if (!empty($this->allowedSchemes)) { + return 'Value must be a valid URL with following schemes ('. \implode(', ', $this->allowedSchemes) .')'; + } + return 'Value must be a valid URL'; } @@ -39,6 +52,10 @@ class URL extends Validator return false; } + if (!empty($this->allowedSchemes) && !\in_array(\parse_url($value, PHP_URL_SCHEME), $this->allowedSchemes)) { + return false; + } + return true; } diff --git a/tests/e2e/Services/Avatars/AvatarsBase.php b/tests/e2e/Services/Avatars/AvatarsBase.php index 62722fbed..82c6ab99b 100644 --- a/tests/e2e/Services/Avatars/AvatarsBase.php +++ b/tests/e2e/Services/Avatars/AvatarsBase.php @@ -259,6 +259,14 @@ trait AvatarsBase $this->assertEquals(400, $response['headers']['status-code']); + $response = $this->client->call(Client::METHOD_GET, '/avatars/image', [ + 'x-appwrite-project' => $this->getProject()['$id'], + ], [ + 'url' => 'invalid://appwrite.io/images/apple.png' + ]); + + $this->assertEquals(400, $response['headers']['status-code']); + // TODO Add test for non-image file (PDF, WORD) return []; diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 836d00ee1..db0278336 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -834,6 +834,17 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); + $response = $this->client->call(Client::METHOD_POST, '/projects/'.$id.'/webhooks', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'name' => 'Webhook Test', + 'events' => ['account.create', 'account.update.email'], + 'url' => 'invalid://appwrite.io', + ]); + + $this->assertEquals(400, $response['headers']['status-code']); + return $data; } @@ -979,6 +990,17 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); + $response = $this->client->call(Client::METHOD_PUT, '/projects/'.$id.'/webhooks/'.$webhookId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'name' => 'Webhook Test Update', + 'events' => ['account.delete', 'account.sessions.delete', 'storage.files.create'], + 'url' => 'invalid://appwrite.io/new', + ]); + + $this->assertEquals(400, $response['headers']['status-code']); + return $data; } diff --git a/tests/unit/Network/Validators/URLTest.php b/tests/unit/Network/Validators/URLTest.php index 458637da6..84e3fb2e6 100755 --- a/tests/unit/Network/Validators/URLTest.php +++ b/tests/unit/Network/Validators/URLTest.php @@ -17,10 +17,7 @@ use PHPUnit\Framework\TestCase; class URLTest extends TestCase { - /** - * @var Domain - */ - protected $url = null; + protected ?URL $url; public function setUp():void { @@ -32,9 +29,9 @@ class URLTest extends TestCase $this->url = null; } - public function testIsValid() + public function testIsValid(): void { - // Assertions + $this->assertEquals('Value must be a valid URL', $this->url->getDescription()); $this->assertEquals(true, $this->url->isValid('http://example.com')); $this->assertEquals(true, $this->url->isValid('https://example.com')); $this->assertEquals(true, $this->url->isValid('htts://example.com')); // does not validate protocol @@ -45,4 +42,13 @@ class URLTest extends TestCase $this->assertEquals(true, $this->url->isValid('http://www.example.com/foo%2\u00c2\u00a9zbar')); $this->assertEquals(true, $this->url->isValid('http://www.example.com/?q=%3Casdf%3E')); } + + public function testIsValidAllowedSchemes(): void + { + $this->url = new URL(['http', 'https']); + $this->assertEquals('Value must be a valid URL with following schemes (http, https)', $this->url->getDescription()); + $this->assertEquals(true, $this->url->isValid('http://example.com')); + $this->assertEquals(true, $this->url->isValid('https://example.com')); + $this->assertEquals(false, $this->url->isValid('gopher://www.example.com')); + } }