Merge pull request #10130 from appwrite/fix-oauth2-redirect-validation

fix: success validation in oauth2 redirect
This commit is contained in:
Steven Nguyen
2025-07-10 09:06:43 -07:00
committed by GitHub
3 changed files with 49 additions and 14 deletions
+11 -11
View File
@@ -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 = [];
@@ -1422,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);
@@ -1746,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')
+16 -3
View File
@@ -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);
}
/**
@@ -55,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'] : '';
+22
View File
@@ -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
@@ -86,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