diff --git a/app/config/collections/platform.php b/app/config/collections/platform.php index a5fedb6461..1a2097732b 100644 --- a/app/config/collections/platform.php +++ b/app/config/collections/platform.php @@ -564,6 +564,17 @@ return [ 'default' => null, 'array' => false, 'filters' => [], + ], + [ + '$id' => ID::custom('scheme'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => 256, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], ] ], 'indexes' => [ diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 67600ee752..2e60d08d43 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1188,8 +1188,8 @@ App::get('/v1/account/sessions/oauth2/:provider') ->label('abuse-limit', 50) ->label('abuse-key', 'ip:{ip}') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('oAuthProviders')), true), 'OAuth2 Provider. Currently, supported providers are: ' . \implode(', ', \array_keys(\array_filter(Config::getParam('oAuthProviders'), fn ($node) => (!$node['mock'])))) . '.') - ->param('success', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) - ->param('failure', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('success', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) + ->param('failure', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) ->param('scopes', [], new ArrayList(new Text(APP_LIMIT_ARRAY_ELEMENT_SIZE), APP_LIMIT_ARRAY_PARAMS_SIZE), 'A list of custom OAuth2 scopes. Check each provider internal docs for a list of supported scopes. Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' scopes are allowed, each ' . APP_LIMIT_ARRAY_ELEMENT_SIZE . ' characters long.', true) ->inject('request') ->inject('response') @@ -1784,8 +1784,8 @@ App::get('/v1/account/tokens/oauth2/:provider') ->label('abuse-limit', 50) ->label('abuse-key', 'ip:{ip}') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('oAuthProviders')), true), 'OAuth2 Provider. Currently, supported providers are: ' . \implode(', ', \array_keys(\array_filter(Config::getParam('oAuthProviders'), fn ($node) => (!$node['mock'])))) . '.') - ->param('success', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) - ->param('failure', '', fn ($clients) => new Redirect($clients), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('success', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect back to your app after a successful login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) + ->param('failure', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect back to your app after a failed login attempt. Only URLs from hostnames in your project\'s platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) ->param('scopes', [], new ArrayList(new Text(APP_LIMIT_ARRAY_ELEMENT_SIZE), APP_LIMIT_ARRAY_PARAMS_SIZE), 'A list of custom OAuth2 scopes. Check each provider internal docs for a list of supported scopes. Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' scopes are allowed, each ' . APP_LIMIT_ARRAY_ELEMENT_SIZE . ' characters long.', true) ->inject('request') ->inject('response') @@ -1864,7 +1864,7 @@ App::post('/v1/account/tokens/magic-url') ->label('abuse-key', ['url:{url},email:{param-email}', 'url:{url},ip:{ip}']) ->param('userId', '', new CustomId(), 'Unique Id. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') ->param('email', '', new Email(), 'User email.') - ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the magic URL login. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) + ->param('url', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect the user back to your app from the magic URL login. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) ->param('phrase', false, new Boolean(), 'Toggle for security phrase. If enabled, email will be send with a randomly generated phrase and the phrase will also be included in the response. Confirming phrases match increases the security of your authentication flow.', true) ->inject('request') ->inject('response') @@ -3157,7 +3157,7 @@ App::post('/v1/account/recovery') ->label('abuse-limit', 10) ->label('abuse-key', ['url:{url},email:{param-email}', 'url:{url},ip:{ip}']) ->param('email', '', new Email(), 'User email.') - ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the recovery email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) + ->param('url', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect the user back to your app from the recovery email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['hostnames', 'schemes']) ->inject('request') ->inject('response') ->inject('user') @@ -3432,7 +3432,7 @@ App::post('/v1/account/verification') )) ->label('abuse-limit', 10) ->label('abuse-key', 'url:{url},userId:{userId}') - ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) // TODO add built-in confirm page + ->param('url', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['hostnames', 'schemes']) // TODO add built-in confirm page ->inject('request') ->inject('response') ->inject('project') diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 48d20cd17f..048f8f48d4 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -1746,6 +1746,7 @@ App::post('/v1/projects/:projectId/platforms') ->param('key', '', new Text(256), 'Package name for Android or bundle ID for iOS or macOS. Max length: 256 chars.', true) ->param('store', '', new Text(256), 'App store or Google Play store ID. Max length: 256 chars.', true) ->param('hostname', '', new Hostname(), 'Platform client hostname. Max length: 256 chars.', true) + ->param('scheme', '', new Text(256), 'Platform client scheme. Max length: 256 chars.', true) ->inject('response') ->inject('dbForPlatform') ->action(function (string $projectId, string $type, string $name, string $key, string $store, string $hostname, Response $response, Database $dbForPlatform) { @@ -1768,7 +1769,8 @@ App::post('/v1/projects/:projectId/platforms') 'name' => $name, 'key' => $key, 'store' => $store, - 'hostname' => $hostname + 'hostname' => $hostname, + 'scheme' => $scheme, ]); $platform = $dbForPlatform->createDocument('platforms', $platform); diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 63a29f279d..17da2a96f6 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -455,7 +455,7 @@ App::post('/v1/teams/:teamId/memberships') } return new ArrayList(new Key(), APP_LIMIT_ARRAY_PARAMS_SIZE); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](https://appwrite.io/docs/permissions). Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' roles are allowed, each 32 characters long.', false, ['project']) - ->param('url', '', fn ($clients) => new Redirect($clients), 'URL to redirect the user back to your app from the invitation email. This parameter is not required when an API key is supplied. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['clients']) // TODO add our own built-in confirm page + ->param('url', '', fn ($hostnames, $schemes) => new Redirect($hostnames, $schemes), 'URL to redirect the user back to your app from the invitation email. This parameter is not required when an API key is supplied. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', true, ['hostnames', 'schemes']) // TODO add our own built-in confirm page ->param('name', '', new Text(128), 'Name of the new team member. Max length: 128 chars.', true) ->inject('response') ->inject('project') diff --git a/app/controllers/api/vcs.php b/app/controllers/api/vcs.php index 2c145febcc..0d45d62dff 100644 --- a/app/controllers/api/vcs.php +++ b/app/controllers/api/vcs.php @@ -288,8 +288,8 @@ App::get('/v1/vcs/github/authorize') type: MethodType::WEBAUTH, hide: true, )) - ->param('success', '', fn ($clients) => new Host($clients), 'URL to redirect back to console after a successful installation attempt.', true, ['clients']) - ->param('failure', '', fn ($clients) => new Host($clients), 'URL to redirect back to console after a failed installation attempt.', true, ['clients']) + ->param('success', '', fn ($hostnames) => new Host($hostnames), 'URL to redirect back to console after a successful installation attempt.', true, ['hostnames']) + ->param('failure', '', fn ($hostnames) => new Host($hostnames), 'URL to redirect back to console after a failed installation attempt.', true, ['hostnames']) ->inject('request') ->inject('response') ->inject('project') diff --git a/app/init.php b/app/init.php index f812ef094c..9c7166c1e6 100644 --- a/app/init.php +++ b/app/init.php @@ -1187,58 +1187,57 @@ App::setResource('queueForCertificates', function (Connection $queue) { App::setResource('queueForMigrations', function (Connection $queue) { return new Migration($queue); }, ['queue']); -App::setResource('clients', function ($request, $console, $project) { - $console->setAttribute('platforms', [ // Always allow current host - '$collection' => ID::custom('platforms'), - 'name' => 'Current Host', - 'type' => Origin::CLIENT_TYPE_WEB, - 'hostname' => $request->getHostname(), - ], Document::SET_TYPE_APPEND); +App::setResource('platforms', function (Document $project, Document $console) { + $platforms = []; + $platforms[] = $project->getAttribute('platforms', []); + $platforms[] = $console->getAttribute('platforms', []); + return $platforms; +}, ['project', 'console']); +App::setResource('hostnames', function (array $platforms) { + // Always allow console hostname + $hostnames = [System::getEnv('_APP_CONSOLE_HOSTNAME', 'console')]; - $hostnames = explode(',', System::getEnv('_APP_CONSOLE_HOSTNAMES', '')); + // Add configured hostnames + $configured = explode(',', System::getEnv('_APP_CONSOLE_HOSTNAMES', '')); $validator = new Hostname(); - foreach ($hostnames as $hostname) { + foreach ($configured as $hostname) { $hostname = trim($hostname); - if (!$validator->isValid($hostname)) { - continue; - } - $console->setAttribute('platforms', [ - '$collection' => ID::custom('platforms'), - 'type' => Origin::CLIENT_TYPE_WEB, - 'name' => $hostname, - 'hostname' => $hostname, - ], Document::SET_TYPE_APPEND); - } - - /** - * Get All verified client URLs for both console and current projects - * + Filter for duplicated entries - */ - $clientsConsole = \array_map( - fn ($node) => $node['hostname'], - \array_filter( - $console->getAttribute('platforms', []), - fn ($node) => (isset($node['type']) && ($node['type'] === Origin::CLIENT_TYPE_WEB) && !empty($node['hostname'])) - ) - ); - - $clients = $clientsConsole; - $platforms = $project->getAttribute('platforms', []); - - foreach ($platforms as $node) { - if ( - isset($node['type']) && - ($node['type'] === Origin::CLIENT_TYPE_WEB || - $node['type'] === Origin::CLIENT_TYPE_FLUTTER_WEB) && - !empty($node['hostname']) - ) { - $clients[] = $node['hostname']; + if ($validator->isValid($hostname)) { + $hostnames[] = $hostname; } } - return \array_unique($clients); -}, ['request', 'console', 'project']); + // Add platforms from DB + foreach ($platforms as $platform) { + if (!empty($platform['hostname']) && in_array($platform['type'], [ + Origin::CLIENT_TYPE_WEB, + Origin::CLIENT_TYPE_FLUTTER_WEB, + ])) { + $hostnames[] = $platform['hostname']; + } + } + return \array_unique($hostnames); +}, ['platforms']); +App::setResource('schemes', function (array $platforms) { + // Allow expo development scheme by default + $schemes = ['exp']; + + foreach ($platforms as $platform) { + if (!empty($platform['scheme']) && in_array($platform['type'], [ + Origin::CLIENT_TYPE_IOS, + Origin::CLIENT_TYPE_ANDROID, + Origin::CLIENT_TYPE_FLUTTER_IOS, + Origin::CLIENT_TYPE_FLUTTER_ANDROID, + Origin::CLIENT_TYPE_FLUTTER_REACT_NATIVE_IOS, + Origin::CLIENT_TYPE_FLUTTER_REACT_NATIVE_ANDROID + ])) { + schemes[] = $platform['scheme']; + } + } + + return \array_unique($schemes); +}, ['platforms']); App::setResource('user', function ($mode, $project, $console, $request, $response, $dbForProject, $dbForPlatform) { /** @var Appwrite\Utopia\Request $request */ /** @var Appwrite\Utopia\Response $response */ diff --git a/src/Appwrite/Network/Validator/Redirect.php b/src/Appwrite/Network/Validator/Redirect.php index a84dea1ee9..1b019d4201 100644 --- a/src/Appwrite/Network/Validator/Redirect.php +++ b/src/Appwrite/Network/Validator/Redirect.php @@ -14,11 +14,13 @@ use Utopia\Validator\Host; class Redirect extends Host { /** - * @param array $whitelist + * @param array $hostnames White list of allowed hostnames + * @param array $schemes White list of allowed schemes */ - public function __construct(array $whitelist) + public function __construct(array $hostnames, array $schemes) { - parent::__construct($whitelist); + $this->schemes = $schemes; + parent::__construct($hostnames); } /** @@ -45,30 +47,17 @@ class Redirect extends Host public function isValid($value): bool { // `parse_url` returns false for URL with only a scheme - // We need to check for this case separately + // We need to handle parsing the scheme manually if (preg_match('/^([a-z][a-z0-9+\.-]*):\/+$/i', $value, $matches)) { $scheme = strtolower($matches[1]); - return $scheme !== 'javascript'; } - // `parse_url` returns false for invalid URLs - $url = \parse_url($value); - if ($url === false || !isset($url["scheme"])) { - return false; + // If the scheme is not http or https, check the hostname + if (\in_array($scheme, ["http", "https"])) { + return parent::isValid($value); } - // If scheme is javascript, it's an XSS vector - $scheme = strtolower($url["scheme"]); - if ($scheme === "javascript") { - return false; - } - - // If scheme is not http or https, we don't need to check the host - // Allow deep links to other user apps. - if (!\in_array($scheme, ["http", "https"])) { - return true; - } - - return parent::isValid($value); + // Otherwise, check the scheme whitelist + return \in_array($scheme, $this->schemes); } } diff --git a/tests/unit/Network/Validators/RedirectTest.php b/tests/unit/Network/Validators/RedirectTest.php index a334c69610..1f73595878 100644 --- a/tests/unit/Network/Validators/RedirectTest.php +++ b/tests/unit/Network/Validators/RedirectTest.php @@ -10,26 +10,18 @@ class RedirectTest extends TestCase public function redirectsProvider(): array { return [ - "custom scheme" => [[], "exp://192.168.0.1", true], - "only scheme with triple slash" => [[], "myapp:///", true], - "only scheme" => [[], "myapp://", true], - "javascript scheme" => [[], "javascript://alert(1)", false], - "invalid url" => [[], "192.168.0.1", false], - "scheme case + invalid host" => [ - ["notexample.com"], - "HTTPS://example.com", - false, - ], - "scheme case + valid host" => [ - ["example.com"], - "HTTPS://example.com", - true, - ], - "javascript scheme with different case" => [[], "JaVaScRiPt://alert(1)", false], - "multiple slashes after scheme" => [[], "myapp:////", true], + "expo scheme" => [[], ["exp"], "exp://192.168.0.1", true], + "custom scheme" => [[], ["myapp"], "myapp://", true], + "custom scheme triple slash" => [[], ["myapp"], "myapp:///", true], "scheme with special chars" => [[], "my-app+custom.123://", true], - "empty string" => [[], "", false], + "url https" => [["example.com"], [], "https://example.com", true], + "url http" => [["example.com"], [], "http://example.com", true], "malformed scheme" => [[], "http:/example.com", false], + "invalid url" => [[], [], "example.com", false], + "invalid host" => [["notexample.com"], [], "https://example.com", false], + "javascript scheme" => [[], "javascript://alert(1)", false], + "javascript scheme with different case" => [[], "JaVaScRiPt://alert(1)", false], + "empty string" => [[], "", false], ]; } @@ -37,11 +29,12 @@ class RedirectTest extends TestCase * @dataProvider redirectsProvider */ public function testIsValid( - array $allowList, + array $hostnames, + array $schemes, string $value, bool $expected ): void { - $validator = new Redirect($allowList); + $validator = new Redirect($hostnames, $schemes); $this->assertEquals($expected, $validator->isValid($value)); }