From c4fb7ecbc1b76844b41525618aec76bd31129a78 Mon Sep 17 00:00:00 2001 From: shimon Date: Tue, 12 Aug 2025 15:20:30 +0300 Subject: [PATCH] Refactor and clean up debugging output across multiple files, including mock.php, account.php, and OAuth2 classes. Removed unnecessary var_dump statements for improved code clarity and consistency. Updated MongoDB condition checks in test cases to ensure case-insensitive comparisons. --- app/controllers/api/account.php | 18 +-- app/controllers/mock.php | 2 - app/controllers/shared/api.php | 10 +- src/Appwrite/Auth/OAuth2.php | 138 +----------------- src/Appwrite/Auth/OAuth2/Mock.php | 10 +- .../Databases/Grids/DatabasesBase.php | 10 +- .../Projects/ProjectsConsoleClientTest.php | 8 +- tests/e2e/Services/Users/UsersBase.php | 2 +- 8 files changed, 30 insertions(+), 168 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 68d0ca6a54..00efccd187 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1258,8 +1258,6 @@ App::get('/v1/account/sessions/oauth2/:provider') 'token' => false, ], $scopes); - //var_dump('Url: /v1/account/sessions/oauth2/:provider redirecting to -> '. $oauth2->getLoginURL()); - $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache') @@ -1289,14 +1287,11 @@ App::get('/v1/account/sessions/oauth2/callback/:provider/:projectId') } elseif ($protocol === 'http' && $port !== '80') { $callbackBase .= ':' . $port; } - + $params = $request->getParams(); $params['project'] = $projectId; unset($params['projectId']); - - var_dump('Url : /v1/account/sessions/oauth2/callback/'.$provider. '/ '.$projectId. 'redirect to '. $callbackBase . '/v1/account/sessions/oauth2/' . $provider . '/redirect?' - . \http_build_query($params)); - + $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache') @@ -1375,6 +1370,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } elseif ($protocol === 'http' && $port !== '80') { $callbackBase .= ':' . $port; } + $callback = $callbackBase . '/v1/account/sessions/oauth2/callback/' . $provider . '/' . $project->getId(); $defaultState = ['success' => $project->getAttribute('url', ''), 'failure' => '']; $redirect = new Redirect($platforms); @@ -1456,9 +1452,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $accessToken = ''; $refreshToken = ''; $accessTokenExpiry = 0; - - var_dump('Url: /v1/account/sessions/oauth2/' .$provider. '/redirect: Before attempting to get the tokens'); - + try { $accessToken = $oauth2->getAccessToken($code); $refreshToken = $oauth2->getRefreshToken($code); @@ -1473,8 +1467,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ); } - var_dump('Url: /v1/account/sessions/oauth2/' .$provider. '/redirect: After getting the tokens'); - $oauth2ID = $oauth2->getUserID($accessToken); if (empty($oauth2ID)) { $failureRedirect(Exception::USER_MISSING_ID); @@ -1851,7 +1843,7 @@ App::get('/v1/account/tokens/oauth2/:provider') } elseif ($protocol === 'http' && $port !== '80') { $callbackBase .= ':' . $port; } - + var_dump( $callbackBase . '/v1/account/sessions/oauth2/callback/' . $provider . '/' . $project->getId()); $callback = $callbackBase . '/v1/account/sessions/oauth2/callback/' . $provider . '/' . $project->getId(); $providerEnabled = $project->getAttribute('oAuthProviders', [])[$provider . 'Enabled'] ?? false; diff --git a/app/controllers/mock.php b/app/controllers/mock.php index 1b62a82898..59a3ccd92c 100644 --- a/app/controllers/mock.php +++ b/app/controllers/mock.php @@ -31,7 +31,6 @@ App::get('/v1/mock/tests/general/oauth2') ->param('state', '', new Text(1024), 'OAuth2 state.') ->inject('response') ->action(function (string $client_id, string $redirectURI, string $scope, string $state, Response $response) { - //var_dump('Url: /v1/mock/tests/general/oauth2 redirecting to -> ' . $redirectURI . '?' . \http_build_query(['code' => 'abcdef', 'state' => $state])); $response->redirect($redirectURI . '?' . \http_build_query(['code' => 'abcdef', 'state' => $state])); }); @@ -49,7 +48,6 @@ App::get('/v1/mock/tests/general/oauth2/token') ->param('refresh_token', '', new Text(100), 'OAuth2 refresh token.', true) ->inject('response') ->action(function (string $client_id, string $client_secret, string $grantType, string $redirectURI, string $code, string $refreshToken, Response $response) { - // var_dump('Url: /v1/mock/tests/general/oauth2/token'); if ($client_id != '1') { throw new Exception(Exception::GENERAL_MOCK, 'Invalid client ID'); diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index ac88a16d2c..65da62eb1d 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -438,9 +438,10 @@ App::init() /* * Abuse Check */ + $abuseKeyLabel = $route->getLabel('abuse-key', 'url:{url},ip:{ip}'); $timeLimitArray = []; - + $abuseKeyLabel = (!is_array($abuseKeyLabel)) ? [$abuseKeyLabel] : $abuseKeyLabel; foreach ($abuseKeyLabel as $abuseKey) { @@ -476,7 +477,7 @@ App::init() $limit = $timeLimit->limit(); $time = $timeLimit->time() + $route->getLabel('abuse-time', 3600); - + if ($limit && ($remaining < $closestLimit || is_null($closestLimit))) { $closestLimit = $remaining; $response @@ -486,7 +487,10 @@ App::init() } $enabled = System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') !== 'disabled'; - + var_dump( + [ + 'enabled' => $enabled, + ]); if ( $enabled // Abuse is enabled && !$isAppUser // User is not API key diff --git a/src/Appwrite/Auth/OAuth2.php b/src/Appwrite/Auth/OAuth2.php index 61be106182..0d6faede5c 100644 --- a/src/Appwrite/Auth/OAuth2.php +++ b/src/Appwrite/Auth/OAuth2.php @@ -182,40 +182,17 @@ abstract class OAuth2 * @param string $url * @param array $headers * @param string $payload - * @param bool $debug * * @return string */ - protected function request(string $method, string $url = '', array $headers = [], string $payload = '', bool $debug = true): string + protected function request(string $method, string $url = '', array $headers = [], string $payload = ''): string { - if ($debug) { - error_log("OAuth2 Debug: Starting request to $url with method $method"); - } - $ch = \curl_init($url); \curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $method); \curl_setopt($ch, CURLOPT_HEADER, 0); \curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); \curl_setopt($ch, CURLOPT_USERAGENT, 'Appwrite OAuth2'); - - // Set timeout options - \curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 10); // 10 seconds to establish connection - \curl_setopt($ch, CURLOPT_TIMEOUT,5); // 30 seconds total timeout - - // Additional options to prevent silent failures - \curl_setopt($ch, CURLOPT_FAILONERROR, false); // Don't fail on HTTP errors, we'll handle them - \curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); // Follow redirects - \curl_setopt($ch, CURLOPT_MAXREDIRS, 5); // Max 5 redirects - \curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false); // Verify SSL - \curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, false); // Verify SSL host - - // Enable verbose debugging if requested - if ($debug) { - \curl_setopt($ch, CURLOPT_VERBOSE, true); - $verbose = fopen('php://temp', 'w+'); - \curl_setopt($ch, CURLOPT_STDERR, $verbose); - } if (!empty($payload)) { \curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); @@ -223,119 +200,18 @@ abstract class OAuth2 } \curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); - - if ($debug) { - error_log("OAuth2 Debug: Executing cURL request..."); - } - + // Send the request & save response to $response $response = \curl_exec($ch); - - if ($debug) { - error_log("OAuth2 Debug: cURL execution completed. Response type: " . gettype($response)); - if ($response === false) { - error_log("OAuth2 Debug: cURL returned false"); - } elseif ($response === null) { - error_log("OAuth2 Debug: cURL returned null"); - } else { - error_log("OAuth2 Debug: Response length: " . strlen($response)); - } - } - - // Check for cURL errors - $error = \curl_error($ch); - $errno = \curl_errno($ch); - $info = \curl_getinfo($ch); - - if ($debug) { - error_log("OAuth2 Debug: cURL errno: $errno, error: $error"); - error_log("OAuth2 Debug: HTTP code: " . ($info['http_code'] ?? 'unknown')); - error_log("OAuth2 Debug: Total time: " . ($info['total_time'] ?? 'unknown') . "s"); - } - - // Get verbose debug info if enabled - $verboseLog = ''; - if ($debug && isset($verbose)) { - rewind($verbose); - $verboseLog = stream_get_contents($verbose); - fclose($verbose); - if ($verboseLog) { - error_log("OAuth2 Debug: Verbose log: " . $verboseLog); - } - } - + + $code = curl_getinfo($ch, CURLINFO_HTTP_CODE); + \curl_close($ch); - // Handle cURL errors - check for both explicit errors and silent failures - if ($errno !== CURLE_OK) { - $errorMessage = "cURL error: $error (errno: $errno)"; - - // Add specific timeout error messages - if ($errno === CURLE_OPERATION_TIMEDOUT) { - $errorMessage .= " - Request timed out after {$info['total_time']} seconds"; - } elseif ($errno === CURLE_COULDNT_CONNECT) { - $errorMessage .= " - Could not connect to server"; - } elseif ($errno === CURLE_COULDNT_RESOLVE_HOST) { - $errorMessage .= " - Could not resolve hostname"; - } elseif ($errno === CURLE_SSL_CONNECT_ERROR) { - $errorMessage .= " - SSL connection failed"; - } elseif ($errno === CURLE_SSL_CERTPROBLEM) { - $errorMessage .= " - SSL certificate problem"; - } - - // Include verbose log in debug mode - if ($debug && $verboseLog) { - $errorMessage .= "\nVerbose log:\n" . $verboseLog; - } - - throw new Exception($errorMessage, $errno); - } - - // Check for silent failures - when cURL returns false but no error - if ($response === false && $errno === CURLE_OK) { - $errorMessage = "cURL returned false but no error was reported"; - if ($debug) { - $errorMessage .= "\nDebug info: " . json_encode($info); - } - throw new Exception($errorMessage, 0); - } - - // Check for null response - if ($response === null) { - $errorMessage = "cURL returned null response"; - if ($debug) { - $errorMessage .= "\nDebug info: " . json_encode($info); - } - throw new Exception($errorMessage, 0); - } - - // Check for empty response with successful HTTP code - if (empty($response) && ($info['http_code'] >= 200 && $info['http_code'] < 300)) { - if ($debug) { - error_log("OAuth2 Debug: Warning - Empty response with successful HTTP code: " . $info['http_code']); - } - } - - $code = $info['http_code']; - if ($code >= 400) { - $errorMessage = "HTTP error $code: $response"; - if ($debug) { - $errorMessage .= "\nRequest info: " . json_encode([ - 'url' => $url, - 'method' => $method, - 'headers' => $headers, - 'payload_length' => strlen($payload), - 'curl_info' => $info - ]); - } - throw new Exception($errorMessage, $code); - } - - if ($debug) { - error_log("OAuth2 Debug: Request successful, returning response"); + throw new Exception($response, $code); } return (string)$response; } -} +} \ No newline at end of file diff --git a/src/Appwrite/Auth/OAuth2/Mock.php b/src/Appwrite/Auth/OAuth2/Mock.php index 77ce2a9e34..0325396692 100644 --- a/src/Appwrite/Auth/OAuth2/Mock.php +++ b/src/Appwrite/Auth/OAuth2/Mock.php @@ -56,16 +56,8 @@ class Mock extends OAuth2 */ protected function getTokens(string $code): array { - if (empty($this->tokens)) { - var_dump('Tring to get tokes from url: http://localhost/' . $this->version . '/mock/tests/general/oauth2/token?' . - \http_build_query([ - 'client_id' => $this->appID, - 'redirect_uri' => $this->callback, - 'client_secret' => $this->appSecret, - 'code' => $code - ])); - var_dump('Sending.......'); + if (empty($this->tokens)) { $this->tokens = \json_decode($this->request( 'GET', 'http://localhost/' . $this->version . '/mock/tests/general/oauth2/token?' . diff --git a/tests/e2e/Services/Databases/Grids/DatabasesBase.php b/tests/e2e/Services/Databases/Grids/DatabasesBase.php index c3876820e5..8625c582d4 100644 --- a/tests/e2e/Services/Databases/Grids/DatabasesBase.php +++ b/tests/e2e/Services/Databases/Grids/DatabasesBase.php @@ -4442,7 +4442,7 @@ trait DatabasesBase public function testOneToOneRelationship(array $data): array { - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $this->markTestSkipped('MongoDB is not supported for this test'); } @@ -4689,7 +4689,7 @@ trait DatabasesBase public function testOneToManyRelationship(array $data): array { - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $this->markTestSkipped('MongoDB is not supported for this test'); } @@ -4849,7 +4849,7 @@ trait DatabasesBase public function testManyToOneRelationship(array $data): array { - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $this->markTestSkipped('MongoDB is not supported for this test'); } @@ -5006,7 +5006,7 @@ trait DatabasesBase public function testManyToManyRelationship(array $data): array { - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $this->markTestSkipped('MongoDB is not supported for this test'); } @@ -5404,7 +5404,7 @@ trait DatabasesBase public function testUpdateWithExistingRelationships(array $data): void { - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $this->markTestSkipped('MongoDB is not supported for this test'); } diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index ca83b3525e..dc8318c91a 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -448,7 +448,7 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(404, $response['headers']['status-code']); $projectId = str_repeat('very_long_id', 10); - if('mongodb' === System::getEnv('_APP_DB_ADAPTER', 'mongodb')){ // to support mongodb UID length + if('mongodb' === strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ // to support mongodb UID length $projectId = str_repeat('long_id', 20); } @@ -4809,7 +4809,7 @@ class ProjectsConsoleClientTest extends Scope /** * Test for SUCCESS */ - for ($i = 0; $i < 15; $i++) { + for ($i = 0; $i < 10; $i++) { $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', [ 'content-type' => 'application/json', 'x-appwrite-project' => $projectId, @@ -4817,7 +4817,7 @@ class ProjectsConsoleClientTest extends Scope 'email' => 'user@appwrite.io', 'password' => 'password' ]); - //var_dump($response['headers']['status-code']); + $this->assertEquals(401, $response['headers']['status-code']); } $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', [ @@ -4827,7 +4827,7 @@ class ProjectsConsoleClientTest extends Scope 'email' => 'user@appwrite.io', 'password' => 'password' ]); - //var_dump($response['headers']['status-code']); + $this->assertEquals(429, $response['headers']['status-code']); $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', [ diff --git a/tests/e2e/Services/Users/UsersBase.php b/tests/e2e/Services/Users/UsersBase.php index 617593483e..7db5c335d1 100644 --- a/tests/e2e/Services/Users/UsersBase.php +++ b/tests/e2e/Services/Users/UsersBase.php @@ -695,7 +695,7 @@ trait UsersBase $this->assertCount(1, $response['body']['users']); //mongodb fulltext search support only in complete words. - if('mongodb' !== System::getEnv('_APP_DB_ADAPTER', 'mongodb')) { + if('mongodb' !== strtolower(System::getEnv('_APP_DB_ADAPTER', 'mongodb'))){ $response = $this->client->call(Client::METHOD_GET, '/users', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'],