From 53086fe97550c37f31e204a0e47a0b46a5f31aaf Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Wed, 9 Jul 2025 12:13:26 -0700 Subject: [PATCH 1/4] fix: success validation in oauth2 redirect We switched to using the Redirect class for validating redirect URLs to cover additional cases like react native expo scheme, but we missed this validation. --- app/controllers/api/account.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 915335f1c2..99269f1f18 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1316,15 +1316,17 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ->inject('request') ->inject('response') ->inject('project') + ->inject('platforms') + ->inject('devKey') ->inject('user') ->inject('dbForProject') ->inject('geodb') ->inject('queueForEvents') - ->action(function (string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response, Document $project, Document $user, Database $dbForProject, Reader $geodb, Event $queueForEvents) use ($oauthDefaultSuccess) { + ->action(function (string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response, Document $project, array $platforms, Document $devKey, Document $user, Database $dbForProject, Reader $geodb, Event $queueForEvents) use ($oauthDefaultSuccess) { $protocol = $request->getProtocol(); $callback = $protocol . '://' . $request->getHostname() . '/v1/account/sessions/oauth2/callback/' . $provider . '/' . $project->getId(); $defaultState = ['success' => $project->getAttribute('url', ''), 'failure' => '']; - $validateURL = new URL(); + $redirect = new Redirect($platforms); $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; $providerEnabled = $project->getAttribute('oAuthProviders', [])[$provider . 'Enabled'] ?? false; @@ -1351,11 +1353,11 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $state = $defaultState; } - if (!$validateURL->isValid($state['success'])) { + if ($devKey->isEmpty() && !$redirect->isValid($state['success'])) { throw new Exception(Exception::PROJECT_INVALID_SUCCESS_URL); } - if (!empty($state['failure']) && !$validateURL->isValid($state['failure'])) { + if ($devKey->isEmpty() && !empty($state['failure']) && !$redirect->isValid($state['failure'])) { throw new Exception(Exception::PROJECT_INVALID_FAILURE_URL); } $failure = []; From f202685450edf34409c2cc76d143436923583a12 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Wed, 9 Jul 2025 21:55:38 -0700 Subject: [PATCH 2/4] fix: handle parsing URL that is only a scheme In react native, a redirect URL may only be a url scheme such as appwrite-callback-myproject://. Since parse_url() fails on this type of URL, we need to add this workaround. --- src/Appwrite/URL/URL.php | 15 ++++++++++++++- tests/unit/URL/URLTest.php | 9 +++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Appwrite/URL/URL.php b/src/Appwrite/URL/URL.php index 98250ab429..a511c8dc13 100644 --- a/src/Appwrite/URL/URL.php +++ b/src/Appwrite/URL/URL.php @@ -26,7 +26,20 @@ class URL 'fragment' => '', ]; - return \array_merge($default, \parse_url($url)); + $parsed = \parse_url($url); + if (is_array($parsed)) { + return \array_merge($default, $parsed); + } + + // see if $url is just a scheme + if (preg_match('/^([a-z][a-z0-9+.-]*):/i', $url, $matches)) { + $scheme = $matches[1]; + return \array_merge($default, [ + 'scheme' => $scheme + ]); + } + + throw new \InvalidArgumentException('Invalid URL: ' . $url); } /** diff --git a/tests/unit/URL/URLTest.php b/tests/unit/URL/URLTest.php index fecaf25bca..93415ee20b 100644 --- a/tests/unit/URL/URLTest.php +++ b/tests/unit/URL/URLTest.php @@ -26,6 +26,15 @@ class URLTest extends TestCase $this->assertEquals(null, $url['port']); $this->assertEquals('', $url['path']); $this->assertEquals('', $url['query']); + + $url = URL::parse('appwrite-callback-project://'); + + $this->assertIsArray($url); + $this->assertEquals('appwrite-callback-project', $url['scheme']); + $this->assertEquals('', $url['host']); + $this->assertEquals(null, $url['port']); + $this->assertEquals('', $url['path']); + $this->assertEquals('', $url['query']); } public function testUnparse(): void From 990e1b91bbb680098efb609dced64fafcd6512d3 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Wed, 9 Jul 2025 22:09:18 -0700 Subject: [PATCH 3/4] fix: omit : and @ if user and pass are empty So, unparsing should not end up with https://:@appwrite.io just because user and pass are empty strings. --- src/Appwrite/URL/URL.php | 4 ++-- tests/unit/URL/URLTest.php | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Appwrite/URL/URL.php b/src/Appwrite/URL/URL.php index a511c8dc13..49e95929e7 100644 --- a/src/Appwrite/URL/URL.php +++ b/src/Appwrite/URL/URL.php @@ -68,9 +68,9 @@ class URL $parts['user'] = isset($url['user']) ? $url['user'] : ''; - $parts['pass'] = isset($url['pass']) ? ':' . $url['pass'] : ''; + $parts['pass'] = !empty($url['pass']) ? ':' . $url['pass'] : ''; - $parts['pass'] = ($parts['user'] || $parts['pass']) ? $parts['pass'] . '@' : ''; + $parts['pass'] = ($parts['user'] || !empty($parts['pass'])) ? $parts['pass'] . '@' : ''; $parts['path'] = isset($url['path']) ? $url['path'] : ''; diff --git a/tests/unit/URL/URLTest.php b/tests/unit/URL/URLTest.php index 93415ee20b..ceca1c6304 100644 --- a/tests/unit/URL/URLTest.php +++ b/tests/unit/URL/URLTest.php @@ -95,6 +95,19 @@ class URLTest extends TestCase $this->assertIsString($url); $this->assertEquals('https://eldad:fux@appwrite.io/#bottom', $url); + + $url = URL::unparse([ + 'scheme' => 'https', + 'user' => '', + 'pass' => '', + 'host' => 'appwrite.io', + 'port' => null, + 'path' => '', + 'fragment' => '', + ]); + + $this->assertIsString($url); + $this->assertEquals('https://appwrite.io/#', $url); } public function testParseQuery(): void From 11332ed1f987b53458b8020441b38e58e48dbcd8 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Wed, 9 Jul 2025 22:10:17 -0700 Subject: [PATCH 4/4] chore: fix deprecation warning from passing null into json_decode --- app/controllers/api/account.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 99269f1f18..b87f68abc0 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1424,13 +1424,13 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $name = ''; $nameOAuth = $oauth2->getUserName($accessToken); - $userParam = \json_decode($request->getParam('user'), true); + $userParam = $request->getParam('user'); if (!empty($nameOAuth)) { $name = $nameOAuth; - } elseif (is_array($userParam)) { - $nameParam = $userParam['name']; - if (is_array($nameParam) && isset($nameParam['firstName']) && isset($nameParam['lastName'])) { - $name = $nameParam['firstName'] . ' ' . $nameParam['lastName']; + } elseif ($userParam !== null) { + $user = \json_decode($userParam, true); + if (isset($user['name']['firstName']) && isset($user['name']['lastName'])) { + $name = $user['name']['firstName'] . ' ' . $user['name']['lastName']; } } $email = $oauth2->getUserEmail($accessToken); @@ -1748,8 +1748,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $state['success']['query'] = URLParser::unparseQuery($query); $state['success'] = URLParser::unparse($state['success']); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); - $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache')