From 1a7587a17592dbde0058edeecd2ceecf555303c2 Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Wed, 10 Dec 2025 17:51:35 +0100 Subject: [PATCH] chore: initial refactor to inject based approach --- app/controllers/api/account.php | 63 +++--- app/controllers/api/locale.php | 5 +- app/controllers/api/teams.php | 7 +- app/controllers/api/users.php | 12 +- app/controllers/general.php | 7 +- app/controllers/shared/api.php | 7 +- app/controllers/shared/api/auth.php | 5 +- app/init/resources.php | 33 +++ .../Functions/Http/Executions/Create.php | 4 +- src/Appwrite/Utopia/Request.php | 41 ---- tests/e2e/Services/Account/AccountBase.php | 2 +- tests/unit/Utopia/RequestTest.php | 198 +++++++++--------- 12 files changed, 198 insertions(+), 186 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 555b9e578e..4901b6916b 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -194,7 +194,7 @@ function sendSessionAlert(Locale $locale, Document $user, Document $project, Doc } ; -$createSession = function (string $userId, string $secret, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Mail $queueForMails, Store $store, ProofsToken $proofForToken, ProofsCode $proofForCode) { +$createSession = function (string $userId, string $secret, string $trustedIp, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Mail $queueForMails, Store $store, ProofsToken $proofForToken, ProofsCode $proofForCode) { /** @var Appwrite\Utopia\Database\Documents\User $userFromRequest */ $userFromRequest = Authorization::skip(fn () => $dbForProject->getDocument('users', $userId)); @@ -214,7 +214,7 @@ $createSession = function (string $userId, string $secret, Request $request, Res $duration = $project->getAttribute('auths', [])['duration'] ?? TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $sessionSecret = $proofForToken->generate(); $factor = (match ($verifiedToken->getAttribute('type')) { @@ -243,7 +243,7 @@ $createSession = function (string $userId, string $secret, Request $request, Res 'provider' => $provider, 'secret' => $proofForToken->hash($sessionSecret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'factors' => [$factor], 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => DateTime::addSeconds(new \DateTime(), $duration) @@ -364,7 +364,8 @@ App::post('/v1/account') ->inject('project') ->inject('dbForProject') ->inject('hooks') - ->action(function (string $userId, string $email, string $password, string $name, Request $request, Response $response, Document $user, Document $project, Database $dbForProject, Hooks $hooks) { + ->inject('trustedIp') + ->action(function (string $userId, string $email, string $password, string $name, string $trustedIp, Response $response, Document $user, Document $project, Database $dbForProject, Hooks $hooks) { $email = \strtolower($email); if ('console' === $project->getId()) { @@ -375,7 +376,7 @@ App::post('/v1/account') throw new Exception(Exception::USER_EMAIL_NOT_WHITELISTED); } - if (!empty($whitelistIPs) && !\in_array($request->getIP(), $whitelistIPs)) { + if (!empty($whitelistIPs) && !\in_array($trustedIp, $whitelistIPs)) { throw new Exception(Exception::USER_IP_NOT_WHITELISTED); } } @@ -957,7 +958,8 @@ App::post('/v1/account/sessions/email') ->inject('store') ->inject('proofForPassword') ->inject('proofForToken') - ->action(function (string $email, string $password, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Mail $queueForMails, Hooks $hooks, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken) { + ->inject('trustedIp') + ->action(function (string $email, string $password, string $trustedIp, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Mail $queueForMails, Hooks $hooks, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken) { $email = \strtolower($email); $protocol = $request->getProtocol(); @@ -981,7 +983,7 @@ App::post('/v1/account/sessions/email') $duration = $project->getAttribute('auths', [])['duration'] ?? TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $secret = $proofForToken->generate(); $session = new Document(array_merge( [ @@ -992,7 +994,7 @@ App::post('/v1/account/sessions/email') 'providerUid' => $email, 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'factors' => ['password'], 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => DateTime::addSeconds(new \DateTime(), $duration) @@ -1101,7 +1103,8 @@ App::post('/v1/account/sessions/anonymous') ->inject('store') ->inject('proofForPassword') ->inject('proofForToken') - ->action(function (Request $request, Response $response, Locale $locale, User $user, Document $project, Database $dbForProject, Reader $geodb, Event $queueForEvents, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken) { + ->inject('trustedIp') + ->action(function (Request $request, Response $response, Locale $locale, User $user, Document $project, Database $dbForProject, Reader $geodb, Event $queueForEvents, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken, string $trustedIp) { $protocol = $request->getProtocol(); if ('console' === $project->getId()) { @@ -1151,7 +1154,7 @@ App::post('/v1/account/sessions/anonymous') // Create session token $duration = $project->getAttribute('auths', [])['duration'] ?? TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $secret = $proofForToken->generate(); $session = new Document(array_merge( @@ -1162,7 +1165,7 @@ App::post('/v1/account/sessions/anonymous') 'provider' => SESSION_PROVIDER_ANONYMOUS, 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'factors' => ['anonymous'], 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => DateTime::addSeconds(new \DateTime(), $duration) @@ -1253,6 +1256,7 @@ App::post('/v1/account/sessions/token') ->inject('store') ->inject('proofForToken') ->inject('proofForCode') + ->inject('trustedIp') ->action($createSession); App::get('/v1/account/sessions/oauth2/:provider') @@ -1448,7 +1452,8 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ->inject('store') ->inject('proofForPassword') ->inject('proofForToken') - ->action(function (string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response, Document $project, array $platforms, Document $devKey, User $user, Database $dbForProject, Reader $geodb, Event $queueForEvents, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken) use ($oauthDefaultSuccess) { + ->inject('trustedIp') + ->action(function (string $provider, string $code, string $state, string $error, string $error_description, string $trustedIp, Request $request, Response $response, Document $project, array $platforms, Document $devKey, User $user, Database $dbForProject, Reader $geodb, Event $queueForEvents, Store $store, ProofsPassword $proofForPassword, ProofsToken $proofForToken) use ($oauthDefaultSuccess) { $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https'; $port = $request->getPort(); $callbackBase = $protocol . '://' . $request->getHostname(); @@ -1812,7 +1817,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'secret' => $proofForTokenOAuth2->hash($secret), // One way hash encryption to protect DB leak 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); @@ -1836,7 +1841,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') // If the `token` param is not set, we persist the session in a cookie } else { $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $secret = $proofForToken->generate(); $session = new Document(array_merge([ @@ -1850,7 +1855,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'providerAccessTokenExpiry' => DateTime::addSeconds(new \DateTime(), (int)$accessTokenExpiry), 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'factors' => [TYPE::EMAIL, 'oauth2'], // include a special oauth2 factor to bypass MFA checks 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => DateTime::addSeconds(new \DateTime(), $duration) @@ -2048,7 +2053,8 @@ App::post('/v1/account/tokens/magic-url') ->inject('queueForEvents') ->inject('queueForMails') ->inject('proofForPassword') - ->action(function (string $userId, string $email, string $url, bool $phrase, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsPassword $proofForPassword) { + ->inject('trustedIp') + ->action(function (string $userId, string $email, string $url, bool $phrase, string $trustedIp, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsPassword $proofForPassword, string $trustedIp) { if (empty(System::getEnv('_APP_SMTP_HOST'))) { throw new Exception(Exception::GENERAL_SMTP_DISABLED, 'SMTP disabled'); } @@ -2138,7 +2144,7 @@ App::post('/v1/account/tokens/magic-url') 'secret' => $proofForToken->hash($tokenSecret), // One way hash encryption to protect DB leak 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); @@ -2316,7 +2322,8 @@ App::post('/v1/account/tokens/email') ->inject('queueForMails') ->inject('proofForPassword') ->inject('proofForCode') - ->action(function (string $userId, string $email, bool $phrase, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsPassword $proofForPassword, ProofsCode $proofForCode) { + ->inject('trustedIp') + ->action(function (string $userId, string $email, string $trustedIp, bool $phrase, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsPassword $proofForPassword, ProofsCode $proofForCode) { if (empty(System::getEnv('_APP_SMTP_HOST'))) { throw new Exception(Exception::GENERAL_SMTP_DISABLED, 'SMTP disabled'); } @@ -2422,7 +2429,7 @@ App::post('/v1/account/tokens/email') 'secret' => $proofForCode->hash($tokenSecret), // One way hash encryption to protect DB leak 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); @@ -2698,7 +2705,8 @@ App::post('/v1/account/tokens/phone') ->inject('plan') ->inject('store') ->inject('proofForCode') - ->action(function (string $userId, string $phone, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Locale $locale, callable $timelimit, StatsUsage $queueForStatsUsage, array $plan, Store $store, ProofsCode $proofForCode) { + ->inject('trustedIp') + ->action(function (string $userId, string $phone, Request $request, Response $response, User $user, Document $project, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Locale $locale, callable $timelimit, StatsUsage $queueForStatsUsage, array $plan, Store $store, ProofsCode $proofForCode, string $trustedIp) { if (empty(System::getEnv('_APP_SMS_PROVIDER'))) { throw new Exception(Exception::GENERAL_PHONE_DISABLED, 'Phone provider not configured'); } @@ -2793,7 +2801,7 @@ App::post('/v1/account/tokens/phone') 'secret' => $proofForCode->hash($secret), 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); @@ -3475,7 +3483,8 @@ App::post('/v1/account/recovery') ->inject('queueForMails') ->inject('queueForEvents') ->inject('proofForToken') - ->action(function (string $email, string $url, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Mail $queueForMails, Event $queueForEvents, ProofsToken $proofForToken) { + ->inject('trustedIp') + ->action(function (string $email, string $url, Request $request, Response $response, User $user, Database $dbForProject, Document $project, Locale $locale, Mail $queueForMails, Event $queueForEvents, ProofsToken $proofForToken, string $trustedIp) { if (empty(System::getEnv('_APP_SMTP_HOST'))) { throw new Exception(Exception::GENERAL_SMTP_DISABLED, 'SMTP Disabled'); @@ -3508,7 +3517,7 @@ App::post('/v1/account/recovery') 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($profile->getId())->toString()); @@ -3770,7 +3779,8 @@ App::post('/v1/account/verifications/email') ->inject('queueForEvents') ->inject('queueForMails') ->inject('proofForToken') - ->action(function (string $url, Request $request, Response $response, Document $project, User $user, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsToken $proofForToken) { + ->inject('trustedIp') + ->action(function (string $url, Request $request, Response $response, Document $project, User $user, Database $dbForProject, Locale $locale, Event $queueForEvents, Mail $queueForMails, ProofsToken $proofForToken, string $trustedIp) { if (empty(System::getEnv('_APP_SMTP_HOST'))) { throw new Exception(Exception::GENERAL_SMTP_DISABLED, 'SMTP Disabled'); @@ -3796,7 +3806,7 @@ App::post('/v1/account/verifications/email') 'secret' => $proofForToken->hash($verificationSecret), // One way hash encryption to protect DB leak 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); @@ -4057,6 +4067,7 @@ App::post('/v1/account/verifications/phone') ->inject('queueForStatsUsage') ->inject('plan') ->inject('proofForCode') + ->inject('trustedIp') ->action(function (Request $request, Response $response, User $user, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Document $project, Locale $locale, callable $timelimit, StatsUsage $queueForStatsUsage, array $plan, ProofsCode $proofForCode) { if (empty(System::getEnv('_APP_SMS_PROVIDER'))) { throw new Exception(Exception::GENERAL_PHONE_DISABLED, 'Phone provider not configured'); @@ -4093,7 +4104,7 @@ App::post('/v1/account/verifications/phone') 'secret' => $proofForCode->hash($secret), 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, ]); Authorization::setRole(Role::user($user->getId())->toString()); diff --git a/app/controllers/api/locale.php b/app/controllers/api/locale.php index 69bf766323..295987784c 100644 --- a/app/controllers/api/locale.php +++ b/app/controllers/api/locale.php @@ -32,11 +32,12 @@ App::get('/v1/locale') ->inject('response') ->inject('locale') ->inject('geodb') - ->action(function (Request $request, Response $response, Locale $locale, Reader $geodb) { + ->inject('trustedIp') + ->action(function (Request $request, Response $response, Locale $locale, Reader $geodb, string $trustedIp) { $eu = Config::getParam('locale-eu'); $currencies = Config::getParam('locale-currencies'); $output = []; - $ip = $request->getIP(); + $ip = $trustedIp; $output['ip'] = $ip; diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 6628cec50a..f51de587cb 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -1210,7 +1210,8 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') ->inject('queueForEvents') ->inject('store') ->inject('proofForToken') - ->action(function (string $teamId, string $membershipId, string $userId, string $secret, Request $request, Response $response, Document $user, Database $dbForProject, Document $project, Reader $geodb, Event $queueForEvents, Store $store, Token $proofForToken) { + ->inject('trustedIp') + ->action(function (string $teamId, string $membershipId, string $userId, string $secret, Request $request, Response $response, Document $user, Database $dbForProject, Document $project, Reader $geodb, Event $queueForEvents, Store $store, Token $proofForToken, string $trustedIp) { $protocol = $request->getProtocol(); $membership = $dbForProject->getDocument('memberships', $membershipId); @@ -1262,7 +1263,7 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') Authorization::setRole(Role::user($user->getId())->toString()); $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $authDuration = $project->getAttribute('auths', [])['duration'] ?? TOKEN_EXPIRATION_LOGIN_LONG; $expire = DateTime::addSeconds(new \DateTime(), $authDuration); $secret = $proofForToken->generate(); @@ -1279,7 +1280,7 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') 'providerUid' => $user->getAttribute('email'), 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'factors' => ['email'], 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => DateTime::addSeconds(new \DateTime(), $authDuration) diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 8ff7c12cae..99c375ccf1 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -2208,7 +2208,8 @@ App::post('/v1/users/:userId/sessions') ->inject('queueForEvents') ->inject('store') ->inject('proofForToken') - ->action(function (string $userId, Request $request, Response $response, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Store $store, Token $proofForToken) { + ->inject('trustedIp') + ->action(function (string $userId, Request $request, Response $response, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents, Store $store, Token $proofForToken, string $trustedIp) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { throw new Exception(Exception::USER_NOT_FOUND); @@ -2216,7 +2217,7 @@ App::post('/v1/users/:userId/sessions') $secret = $proofForToken->generate(); $detector = new Detector($request->getUserAgent('UNKNOWN')); - $record = $geodb->get($request->getIP()); + $record = $geodb->get($trustedIp); $duration = $project->getAttribute('auths', [])['duration'] ?? TOKEN_EXPIRATION_LOGIN_LONG; $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); @@ -2230,7 +2231,7 @@ App::post('/v1/users/:userId/sessions') 'secret' => $proofForToken->hash($secret), // One way hash encryption to protect DB leak 'userAgent' => $request->getUserAgent('UNKNOWN'), 'factors' => ['server'], - 'ip' => $request->getIP(), + 'ip' => $trustedIp, 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', 'expire' => $expire, ], @@ -2297,7 +2298,8 @@ App::post('/v1/users/:userId/tokens') ->inject('response') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $userId, int $length, int $expire, Request $request, Response $response, Database $dbForProject, Event $queueForEvents) { + ->inject('trustedIp') + ->action(function (string $userId, int $length, int $expire, Request $request, Response $response, Database $dbForProject, Event $queueForEvents, string $trustedIp) { $user = $dbForProject->getDocument('users', $userId); if ($user->isEmpty()) { @@ -2317,7 +2319,7 @@ App::post('/v1/users/:userId/tokens') 'secret' => $proofForToken->hash($secret), 'expire' => $expire, 'userAgent' => $request->getUserAgent('UNKNOWN'), - 'ip' => $request->getIP() + 'ip' => $trustedIp ]); $token = $dbForProject->createDocument('tokens', $token); diff --git a/app/controllers/general.php b/app/controllers/general.php index f034da6b24..9f83eb2538 100644 --- a/app/controllers/general.php +++ b/app/controllers/general.php @@ -365,7 +365,7 @@ function router(App $utopia, Database $dbForPlatform, callable $getProjectDB, Sw $headers['x-appwrite-country-code'] = ''; $headers['x-appwrite-continent-code'] = ''; $headers['x-appwrite-continent-eu'] = 'false'; - $ip = $request->getIP(); + $ip = $trustedIp; $headers['x-appwrite-client-ip'] = $ip; $jwtExpiry = $resource->getAttribute('timeout', 900) + 60; // 1min extra to account for possible cold-starts @@ -1178,7 +1178,8 @@ App::error() ->inject('log') ->inject('queueForStatsUsage') ->inject('devKey') - ->action(function (Throwable $error, App $utopia, Request $request, Response $response, Document $project, ?Logger $logger, Log $log, StatsUsage $queueForStatsUsage) { + ->inject('trustedIp') + ->action(function (Throwable $error, App $utopia, Request $request, Response $response, Document $project, ?Logger $logger, Log $log, StatsUsage $queueForStatsUsage, string $trustedIp) { $version = System::getEnv('_APP_VERSION', 'UNKNOWN'); $route = $utopia->getRoute(); $class = \get_class($error); @@ -1289,7 +1290,7 @@ App::error() if (isset($user) && !$user->isEmpty()) { $log->setUser(new User($user->getId())); } else { - $log->setUser(new User('guest-' . hash('sha256', $request->getIP()))); + $log->setUser(new User('guest-' . hash('sha256', $trustedIp))); } try { diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 16d44481b6..9d521eb249 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -503,7 +503,8 @@ App::init() ->inject('plan') ->inject('devKey') ->inject('telemetry') - ->action(function (App $utopia, Request $request, Response $response, Document $project, Document $user, Publisher $publisher, Publisher $publisherFunctions, Publisher $publisherWebhooks, Event $queueForEvents, Messaging $queueForMessaging, Audit $queueForAudits, Delete $queueForDeletes, EventDatabase $queueForDatabase, Build $queueForBuilds, StatsUsage $queueForStatsUsage, Database $dbForProject, callable $timelimit, Document $resourceToken, string $mode, ?Key $apiKey, array $plan, Document $devKey, Telemetry $telemetry) use ($usageDatabaseListener, $eventDatabaseListener) { + ->inject('trustedIp') + ->action(function (App $utopia, Request $request, Response $response, Document $project, Document $user, Publisher $publisher, Publisher $publisherFunctions, Publisher $publisherWebhooks, Event $queueForEvents, Messaging $queueForMessaging, Audit $queueForAudits, Delete $queueForDeletes, EventDatabase $queueForDatabase, Build $queueForBuilds, StatsUsage $queueForStatsUsage, Database $dbForProject, callable $timelimit, Document $resourceToken, string $mode, ?Key $apiKey, array $plan, Document $devKey, Telemetry $telemetry, string $trustedIp) use ($usageDatabaseListener, $eventDatabaseListener) { $route = $utopia->getRoute(); @@ -531,7 +532,7 @@ App::init() ->setParam('{projectId}', $project->getId()) ->setParam('{userId}', $user->getId()) ->setParam('{userAgent}', $request->getUserAgent('')) - ->setParam('{ip}', $request->getIP()) + ->setParam('{ip}', $trustedIp) ->setParam('{url}', $request->getHostname() . $route->getPath()) ->setParam('{method}', $request->getMethod()) ->setParam('{chunkId}', (int)($start / ($end + 1 - $start))); @@ -588,7 +589,7 @@ App::init() $queueForAudits ->setMode($mode) ->setUserAgent($request->getUserAgent('')) - ->setIP($request->getIP()) + ->setIP($trustedIp) ->setHostname($request->getHostname()) ->setEvent($route->getLabel('audits.event', '')) ->setProject($project); diff --git a/app/controllers/shared/api/auth.php b/app/controllers/shared/api/auth.php index efa733fc34..7c3b3ba645 100644 --- a/app/controllers/shared/api/auth.php +++ b/app/controllers/shared/api/auth.php @@ -36,11 +36,12 @@ App::init() ->inject('request') ->inject('project') ->inject('geodb') - ->action(function (App $utopia, Request $request, Document $project, Reader $geodb) { + ->inject('trustedIp') + ->action(function (App $utopia, Request $request, Document $project, Reader $geodb, string $trustedIp) { $denylist = System::getEnv('_APP_CONSOLE_COUNTRIES_DENYLIST', ''); if (!empty($denylist && $project->getId() === 'console')) { $countries = explode(',', $denylist); - $record = $geodb->get($request->getIP()) ?? []; + $record = $geodb->get($trustedIp) ?? []; $country = $record['country']['iso_code'] ?? ''; if (in_array($country, $countries)) { throw new Exception(Exception::GENERAL_REGION_ACCESS_DENIED); diff --git a/app/init/resources.php b/app/init/resources.php index 98162d3a2b..4d480f407f 100644 --- a/app/init/resources.php +++ b/app/init/resources.php @@ -748,6 +748,39 @@ App::setResource('servers', function () { return $languages; }); +App::setResource('trustedIp', function(Request $request) { + + // Setup the fallback + $remoteAddr = $this->getServer('remote_addr') ?? '0.0.0.0'; + + // Fetch and parse the list of trusted headers from configuration + $trustedHeadersConfig = System::getEnv('_APP_TRUSTED_HEADERS', 'x-forwarded-for'); + + $trustedHeaders = explode(',', $trustedHeadersConfig); + $trustedHeaders = array_map('trim', $trustedHeaders); + $trustedHeaders = array_map('strtolower', $trustedHeaders); + $trustedHeaders = array_filter($trustedHeaders); + + foreach ($trustedHeaders as $header) { + $headerValue = $this->getHeader($header); + + if (empty($headerValue)) { + continue; + } + + // Leftmost IP address is the address of the originating client + $ips = explode(',', $headerValue); + $ip = trim($ips[0]); + + // Validate IP format (supports both IPv4 and IPv6) + if (filter_var($ip, FILTER_VALIDATE_IP)) { + return $ip; + } + } + + return $remoteAddr; +}, ['request']); + App::setResource('promiseAdapter', function ($register) { return $register->get('promiseAdapter'); }, ['register']); diff --git a/src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php b/src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php index 1367cf337f..1cfde9be2c 100644 --- a/src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php +++ b/src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php @@ -98,6 +98,7 @@ class Create extends Base ->inject('store') ->inject('proofForToken') ->inject('executor') + ->inject('trustedIp') ->callback($this->action(...)); } @@ -109,6 +110,7 @@ class Create extends Base string $method, mixed $headers, ?string $scheduledAt, + string $trustedIp, Response $response, Request $request, Document $project, @@ -236,7 +238,7 @@ class Create extends Base $headers['x-appwrite-country-code'] = ''; $headers['x-appwrite-continent-code'] = ''; $headers['x-appwrite-continent-eu'] = 'false'; - $ip = $request->getIP(); + $ip = $trustedIp; $headers['x-appwrite-client-ip'] = $ip; if (!empty($ip)) { diff --git a/src/Appwrite/Utopia/Request.php b/src/Appwrite/Utopia/Request.php index 6b73d4e8b6..2c51345c6e 100644 --- a/src/Appwrite/Utopia/Request.php +++ b/src/Appwrite/Utopia/Request.php @@ -234,45 +234,4 @@ class Request extends UtopiaRequest ksort($params); return md5($this->getURI() . '*' . serialize($params) . '*' . APP_CACHE_BUSTER); } - - /** - * Get IP - * - * Returns users IP address based on a list of trusted headers. - * Assumes application is only accessible through a trusted proxy. - * - * @return string - */ - public function getIP(): string - { - // Setup the fallback - $remoteAddr = $this->getServer('remote_addr') ?? '0.0.0.0'; - - // Fetch and parse the list of trusted headers from configuration - $trustedHeadersConfig = System::getEnv('_APP_TRUSTED_HEADERS', 'x-forwarded-for'); - - $trustedHeaders = explode(',', $trustedHeadersConfig); - $trustedHeaders = array_map('trim', $trustedHeaders); - $trustedHeaders = array_map('strtolower', $trustedHeaders); - $trustedHeaders = array_filter($trustedHeaders); - - foreach ($trustedHeaders as $header) { - $headerValue = $this->getHeader($header); - - if (empty($headerValue)) { - continue; - } - - // Leftmost IP address is the address of the originating client - $ips = explode(',', $headerValue); - $ip = trim($ips[0]); - - // Validate IP format (supports both IPv4 and IPv6) - if (filter_var($ip, FILTER_VALIDATE_IP)) { - return $ip; - } - } - - return $remoteAddr; - } } diff --git a/tests/e2e/Services/Account/AccountBase.php b/tests/e2e/Services/Account/AccountBase.php index 2564fdee1d..67664d2c9c 100644 --- a/tests/e2e/Services/Account/AccountBase.php +++ b/tests/e2e/Services/Account/AccountBase.php @@ -342,7 +342,7 @@ trait AccountBase $this->assertEquals($response['headers']['status-code'], 204); } - public function testGetIp(): void + public function testTrustedIp(): void { $email = uniqid() . 'user@localhost.test'; $password = 'password'; diff --git a/tests/unit/Utopia/RequestTest.php b/tests/unit/Utopia/RequestTest.php index e96726ba54..be9e3720e7 100644 --- a/tests/unit/Utopia/RequestTest.php +++ b/tests/unit/Utopia/RequestTest.php @@ -185,141 +185,141 @@ class RequestTest extends TestCase $this->request->setRoute($route); } - public function testGetIPWithDefaultFallback(): void - { - // No headers set, should return remote_addr - $ip = $this->request->getIP(); + // public function testGetIPWithDefaultFallback(): void + // { + // // No headers set, should return remote_addr + // $ip = $this->request->getIP(); - $this->assertIsString($ip); - // Default fallback when nothing is set - $this->assertSame('0.0.0.0', $ip); - } + // $this->assertIsString($ip); + // // Default fallback when nothing is set + // $this->assertSame('0.0.0.0', $ip); + // } - public function testGetIPWithRemoteAddr(): void - { - // Set remote_addr in server variables - $this->request->setServer('remote_addr', '192.168.1.100'); + // public function testGetIPWithRemoteAddr(): void + // { + // // Set remote_addr in server variables + // $this->request->setServer('remote_addr', '192.168.1.100'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - $this->assertSame('192.168.1.100', $ip); - } + // $this->assertSame('192.168.1.100', $ip); + // } - public function testGetIPWithXForwardedFor(): void - { - // Set X-Forwarded-For header with single IP - $this->request->addHeader('x-forwarded-for', '203.0.113.195'); + // public function testGetIPWithXForwardedFor(): void + // { + // // Set X-Forwarded-For header with single IP + // $this->request->addHeader('x-forwarded-for', '203.0.113.195'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - $this->assertSame('203.0.113.195', $ip); - } + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPWithMultipleProxies(): void - { - // Set X-Forwarded-For with multiple IPs (leftmost is client) - $this->request->addHeader('x-forwarded-for', '203.0.113.195, 70.41.3.18, 150.172.238.178'); + // public function testGetIPWithMultipleProxies(): void + // { + // // Set X-Forwarded-For with multiple IPs (leftmost is client) + // $this->request->addHeader('x-forwarded-for', '203.0.113.195, 70.41.3.18, 150.172.238.178'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should return the leftmost (original client) IP - $this->assertSame('203.0.113.195', $ip); - } + // // Should return the leftmost (original client) IP + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPWithWhitespaceInHeader(): void - { - // Test that whitespace is properly trimmed - $this->request->addHeader('x-forwarded-for', ' 203.0.113.195 , 70.41.3.18 '); + // public function testGetIPWithWhitespaceInHeader(): void + // { + // // Test that whitespace is properly trimmed + // $this->request->addHeader('x-forwarded-for', ' 203.0.113.195 , 70.41.3.18 '); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - $this->assertSame('203.0.113.195', $ip); - } + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPWithInvalidIP(): void - { - // Set invalid IP in X-Forwarded-For - // When the leftmost IP is invalid, the entire header is skipped - // and we fallback to remote_addr (intentional security behavior) - $this->request->addHeader('x-forwarded-for', 'not-an-ip, 203.0.113.195'); - $this->request->setServer('remote_addr', '192.168.1.100'); + // public function testGetIPWithInvalidIP(): void + // { + // // Set invalid IP in X-Forwarded-For + // // When the leftmost IP is invalid, the entire header is skipped + // // and we fallback to remote_addr (intentional security behavior) + // $this->request->addHeader('x-forwarded-for', 'not-an-ip, 203.0.113.195'); + // $this->request->setServer('remote_addr', '192.168.1.100'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should fallback to remote_addr since leftmost IP is invalid - $this->assertSame('192.168.1.100', $ip); - } + // // Should fallback to remote_addr since leftmost IP is invalid + // $this->assertSame('192.168.1.100', $ip); + // } - public function testGetIPWithCustomTrustedHeader(): void - { - // Assuming you can set environment variable in test - putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip'); + // public function testGetIPWithCustomTrustedHeader(): void + // { + // // Assuming you can set environment variable in test + // putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip'); - $this->request->addHeader('cf-connecting-ip', '203.0.113.195'); - $this->request->addHeader('x-forwarded-for', '198.51.100.178'); + // $this->request->addHeader('cf-connecting-ip', '203.0.113.195'); + // $this->request->addHeader('x-forwarded-for', '198.51.100.178'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should use cf-connecting-ip since it's the trusted header - $this->assertSame('203.0.113.195', $ip); - } + // // Should use cf-connecting-ip since it's the trusted header + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPWithMultipleTrustedHeaders(): void - { - putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip, x-real-ip, x-forwarded-for'); + // public function testGetIPWithMultipleTrustedHeaders(): void + // { + // putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip, x-real-ip, x-forwarded-for'); - // Only set the second header - $this->request->addHeader('x-real-ip', '203.0.113.195'); - $this->request->addHeader('x-forwarded-for', '203.0.113.192'); + // // Only set the second header + // $this->request->addHeader('x-real-ip', '203.0.113.195'); + // $this->request->addHeader('x-forwarded-for', '203.0.113.192'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - $this->assertSame('203.0.113.195', $ip); - } + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPHeaderPriority(): void - { - putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip, x-forwarded-for'); + // public function testGetIPHeaderPriority(): void + // { + // putenv('_APP_TRUSTED_HEADERS=cf-connecting-ip, x-forwarded-for'); - // Set both headers, cf-connecting-ip should take priority - $this->request->addHeader('cf-connecting-ip', '203.0.113.195'); - $this->request->addHeader('x-forwarded-for', '198.51.100.178'); + // // Set both headers, cf-connecting-ip should take priority + // $this->request->addHeader('cf-connecting-ip', '203.0.113.195'); + // $this->request->addHeader('x-forwarded-for', '198.51.100.178'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should return the first trusted header's value - $this->assertSame('203.0.113.195', $ip); - } + // // Should return the first trusted header's value + // $this->assertSame('203.0.113.195', $ip); + // } - public function testGetIPWithIPv6(): void - { - $this->request->addHeader('x-forwarded-for', '2001:db8:85a3:8d3:1319:8a2e:370:7348'); + // public function testGetIPWithIPv6(): void + // { + // $this->request->addHeader('x-forwarded-for', '2001:db8:85a3:8d3:1319:8a2e:370:7348'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - $this->assertSame('2001:db8:85a3:8d3:1319:8a2e:370:7348', $ip); - } + // $this->assertSame('2001:db8:85a3:8d3:1319:8a2e:370:7348', $ip); + // } - public function testGetIPWithEmptyHeader(): void - { - $this->request->addHeader('x-forwarded-for', ''); - $this->request->setServer('remote_addr', '192.168.1.100'); + // public function testGetIPWithEmptyHeader(): void + // { + // $this->request->addHeader('x-forwarded-for', ''); + // $this->request->setServer('remote_addr', '192.168.1.100'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should fallback to remote_addr when header is empty - $this->assertSame('192.168.1.100', $ip); - } + // // Should fallback to remote_addr when header is empty + // $this->assertSame('192.168.1.100', $ip); + // } - public function testGetIPWithEmptyTrustedHeadersConfig(): void - { - putenv('_APP_TRUSTED_HEADERS= , , '); + // public function testGetIPWithEmptyTrustedHeadersConfig(): void + // { + // putenv('_APP_TRUSTED_HEADERS= , , '); - $this->request->setServer('remote_addr', '192.168.1.100'); + // $this->request->setServer('remote_addr', '192.168.1.100'); - $ip = $this->request->getIP(); + // $ip = $this->request->getIP(); - // Should fallback to remote_addr when config is effectively empty - $this->assertSame('192.168.1.100', $ip); - } + // // Should fallback to remote_addr when config is effectively empty + // $this->assertSame('192.168.1.100', $ip); + // } }