From 31f8950b45c5daa754aefb9416cc77e3b49e4329 Mon Sep 17 00:00:00 2001 From: Darshan Date: Fri, 3 Jan 2025 15:30:55 +0530 Subject: [PATCH 1/5] add: hostname to audits. --- app/controllers/shared/api.php | 1 + src/Appwrite/Event/Audit.php | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 012dd13c73..182151a6c3 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -506,6 +506,7 @@ App::init() ->setMode($mode) ->setUserAgent($request->getUserAgent('')) ->setIP($request->getIP()) + ->setHostname($request->getHostname()) ->setEvent($route->getLabel('audits.event', '')) ->setProject($project) ->setUser($user); diff --git a/src/Appwrite/Event/Audit.php b/src/Appwrite/Event/Audit.php index 17506bfe6c..406f64b370 100644 --- a/src/Appwrite/Event/Audit.php +++ b/src/Appwrite/Event/Audit.php @@ -11,6 +11,7 @@ class Audit extends Event protected string $mode = ''; protected string $userAgent = ''; protected string $ip = ''; + protected string $hostname = ''; public function __construct(protected Connection $connection) { @@ -113,6 +114,30 @@ class Audit extends Event return $this->ip; } + /** + * Set the hostname. + * + * @param string $hostname + * + * @return self + */ + public function setHostname(string $hostname): self + { + $this->hostname = $hostname; + + return $this; + } + + /** + * Get the hostname. + * + * @return string + */ + public function getHostname(): string + { + return $this->hostname; + } + /** * Executes the event and sends it to the audit worker. * @@ -136,6 +161,7 @@ class Audit extends Event 'ip' => $this->ip, 'userAgent' => $this->userAgent, 'event' => $this->event, + 'hostname' => $this->hostname ]); } } From ff056fa1457f3096ff6fc7ef7456d0bb95979911 Mon Sep 17 00:00:00 2001 From: ChiragAgg5k Date: Wed, 8 Jan 2025 17:01:01 +0530 Subject: [PATCH 2/5] chore: shifted authphone usage tracking to api calls --- app/controllers/api/account.php | 84 ++++++++++++++++++++- app/controllers/api/teams.php | 29 ++++++- src/Appwrite/Event/Messaging.php | 2 +- src/Appwrite/Platform/Workers/Messaging.php | 18 +---- 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 76a3ef8b61..40e248836b 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -17,6 +17,7 @@ use Appwrite\Event\Delete; use Appwrite\Event\Event; use Appwrite\Event\Mail; use Appwrite\Event\Messaging; +use Appwrite\Event\Usage; use Appwrite\Extend\Exception; use Appwrite\Hooks\Hooks; use Appwrite\Network\Validator\Email; @@ -27,7 +28,9 @@ use Appwrite\Utopia\Database\Validator\CustomId; use Appwrite\Utopia\Database\Validator\Queries\Identities; use Appwrite\Utopia\Request; use Appwrite\Utopia\Response; +use libphonenumber\PhoneNumberUtil; use MaxMind\Db\Reader; +use Utopia\Abuse\Abuse; use Utopia\App; use Utopia\Audit\Audit as EventAudit; use Utopia\Config\Config; @@ -2315,7 +2318,10 @@ App::post('/v1/account/tokens/phone') ->inject('queueForEvents') ->inject('queueForMessaging') ->inject('locale') - ->action(function (string $userId, string $phone, Request $request, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Locale $locale) { + ->inject('timelimit') + ->inject('queueForUsage') + ->inject('plan') + ->action(function (string $userId, string $phone, Request $request, Response $response, Document $user, Document $project, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Locale $locale, callable $timelimit, Usage $queueForUsage, array $plan) { if (empty(System::getEnv('_APP_SMS_PROVIDER'))) { throw new Exception(Exception::GENERAL_PHONE_DISABLED, 'Phone provider not configured'); } @@ -2452,6 +2458,27 @@ App::post('/v1/account/tokens/phone') ->setMessage($messageDoc) ->setRecipients([$phone]) ->setProviderType(MESSAGE_TYPE_SMS); + + if (isset($plan['authPhone'])) { + $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days + $timelimit + ->setParam('{organizationId}', $project->getAttribute('organizationId')); + + $abuse = new Abuse($timelimit); + if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { + $helper = PhoneNumberUtil::getInstance(); + $countryCode = $helper->parse($phone)->getCountryCode(); + + if (!empty($countryCode)) { + $queueForUsage + ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); + } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); + } + } } // Set to unhashed secret for events and server responses @@ -3465,7 +3492,10 @@ App::post('/v1/account/verification/phone') ->inject('queueForMessaging') ->inject('project') ->inject('locale') - ->action(function (Request $request, Response $response, Document $user, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Document $project, Locale $locale) { + ->inject('timelimit') + ->inject('queueForUsage') + ->inject('plan') + ->action(function (Request $request, Response $response, Document $user, Database $dbForProject, Event $queueForEvents, Messaging $queueForMessaging, Document $project, Locale $locale, callable $timelimit, Usage $queueForUsage, array $plan) { if (empty(System::getEnv('_APP_SMS_PROVIDER'))) { throw new Exception(Exception::GENERAL_PHONE_DISABLED, 'Phone provider not configured'); } @@ -3548,6 +3578,27 @@ App::post('/v1/account/verification/phone') ->setMessage($messageDoc) ->setRecipients([$user->getAttribute('phone')]) ->setProviderType(MESSAGE_TYPE_SMS); + + if (isset($plan['authPhone'])) { + $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days + $timelimit + ->setParam('{organizationId}', $project->getAttribute('organizationId')); + + $abuse = new Abuse($timelimit); + if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { + $helper = PhoneNumberUtil::getInstance(); + $countryCode = $helper->parse($phone)->getCountryCode(); + + if (!empty($countryCode)) { + $queueForUsage + ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); + } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); + } + } } // Set to unhashed secret for events and server responses @@ -4026,7 +4077,10 @@ App::post('/v1/account/mfa/challenge') ->inject('queueForEvents') ->inject('queueForMessaging') ->inject('queueForMails') - ->action(function (string $factor, Response $response, Database $dbForProject, Document $user, Locale $locale, Document $project, Request $request, Event $queueForEvents, Messaging $queueForMessaging, Mail $queueForMails) { + ->inject('timelimit') + ->inject('queueForUsage') + ->inject('plan') + ->action(function (string $factor, Response $response, Database $dbForProject, Document $user, Locale $locale, Document $project, Request $request, Event $queueForEvents, Messaging $queueForMessaging, Mail $queueForMails, callable $timelimit, Usage $queueForUsage, array $plan) { $expire = DateTime::addSeconds(new \DateTime(), Auth::TOKEN_EXPIRATION_CONFIRM); $code = Auth::codeGenerator(); @@ -4074,6 +4128,7 @@ App::post('/v1/account/mfa/challenge') $message = $message->render(); + $phone = $user->getAttribute('phone'); $queueForMessaging ->setType(MESSAGE_SEND_TYPE_INTERNAL) ->setMessage(new Document([ @@ -4082,8 +4137,29 @@ App::post('/v1/account/mfa/challenge') 'content' => $code, ], ])) - ->setRecipients([$user->getAttribute('phone')]) + ->setRecipients([$phone]) ->setProviderType(MESSAGE_TYPE_SMS); + + if (isset($plan['authPhone'])) { + $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days + $timelimit + ->setParam('{organizationId}', $project->getAttribute('organizationId')); + + $abuse = new Abuse($timelimit); + if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { + $helper = PhoneNumberUtil::getInstance(); + $countryCode = $helper->parse($phone)->getCountryCode(); + + if (!empty($countryCode)) { + $queueForUsage + ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); + } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); + } + } break; case Type::EMAIL: if (empty(System::getEnv('_APP_SMTP_HOST'))) { diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index f829800b98..216807fecf 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -8,6 +8,7 @@ use Appwrite\Event\Delete; use Appwrite\Event\Event; use Appwrite\Event\Mail; use Appwrite\Event\Messaging; +use Appwrite\Event\Usage; use Appwrite\Extend\Exception; use Appwrite\Network\Validator\Email; use Appwrite\Platform\Workers\Deletes; @@ -17,7 +18,9 @@ use Appwrite\Utopia\Database\Validator\Queries\Memberships; use Appwrite\Utopia\Database\Validator\Queries\Teams; use Appwrite\Utopia\Request; use Appwrite\Utopia\Response; +use libphonenumber\PhoneNumberUtil; use MaxMind\Db\Reader; +use Utopia\Abuse\Abuse; use Utopia\App; use Utopia\Audit\Audit; use Utopia\Config\Config; @@ -423,7 +426,10 @@ App::post('/v1/teams/:teamId/memberships') ->inject('queueForMails') ->inject('queueForMessaging') ->inject('queueForEvents') - ->action(function (string $teamId, string $email, string $userId, string $phone, array $roles, string $url, string $name, Response $response, Document $project, Document $user, Database $dbForProject, Locale $locale, Mail $queueForMails, Messaging $queueForMessaging, Event $queueForEvents) { + ->inject('timelimit') + ->inject('queueForUsage') + ->inject('plan') + ->action(function (string $teamId, string $email, string $userId, string $phone, array $roles, string $url, string $name, Response $response, Document $project, Document $user, Database $dbForProject, Locale $locale, Mail $queueForMails, Messaging $queueForMessaging, Event $queueForEvents, callable $timelimit, Usage $queueForUsage, array $plan) { $isAPIKey = Auth::isAppUser(Authorization::getRoles()); $isPrivilegedUser = Auth::isPrivilegedUser(Authorization::getRoles()); @@ -691,6 +697,27 @@ App::post('/v1/teams/:teamId/memberships') ->setMessage($messageDoc) ->setRecipients([$phone]) ->setProviderType('SMS'); + + if (isset($plan['authPhone'])) { + $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days + $timelimit + ->setParam('{organizationId}', $project->getAttribute('organizationId')); + + $abuse = new Abuse($timelimit); + if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { + $helper = PhoneNumberUtil::getInstance(); + $countryCode = $helper->parse($phone)->getCountryCode(); + + if (!empty($countryCode)) { + $queueForUsage + ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); + } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); + } + } } } diff --git a/src/Appwrite/Event/Messaging.php b/src/Appwrite/Event/Messaging.php index ab9b1bee6b..755b8c9158 100644 --- a/src/Appwrite/Event/Messaging.php +++ b/src/Appwrite/Event/Messaging.php @@ -27,7 +27,7 @@ class Messaging extends Event /** * Sets type for the build event. * - * @param string $type Can be `MESSAGE_TYPE_INTERNAL` or `MESSAGE_TYPE_EXTERNAL`. + * @param string $type Can be `MESSAGE_SEND_TYPE_INTERNAL` or `MESSAGE_SEND_TYPE_EXTERNAL`. * @return self */ public function setType(string $type): self diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index 58f6265ff4..1ff032c3e1 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -96,7 +96,7 @@ class Messaging extends Action $message = new Document($payload['message'] ?? []); $recipients = $payload['recipients'] ?? []; - $this->sendInternalSMSMessage($message, $project, $recipients, $queueForUsage, $log); + $this->sendInternalSMSMessage($message, $project, $recipients, $log); break; case MESSAGE_SEND_TYPE_EXTERNAL: $message = $dbForProject->getDocument('messages', $payload['messageId']); @@ -377,7 +377,7 @@ class Messaging extends Action } } - private function sendInternalSMSMessage(Document $message, Document $project, array $recipients, Usage $queueForUsage, Log $log): void + private function sendInternalSMSMessage(Document $message, Document $project, array $recipients, Log $log): void { if (empty(System::getEnv('_APP_SMS_PROVIDER')) || empty(System::getEnv('_APP_SMS_FROM'))) { throw new \Exception('Skipped SMS processing. Missing "_APP_SMS_PROVIDER" or "_APP_SMS_FROM" environment variables.'); @@ -456,24 +456,14 @@ class Messaging extends Action $adapter->getMaxMessagesPerRequest() ); - batch(\array_map(function ($batch) use ($message, $provider, $adapter, $project, $queueForUsage) { - return function () use ($batch, $message, $provider, $adapter, $project, $queueForUsage) { + batch(\array_map(function ($batch) use ($message, $provider, $adapter) { + return function () use ($batch, $message, $provider, $adapter) { $message->setAttribute('to', $batch); $data = $this->buildSmsMessage($message, $provider); try { $adapter->send($data); - - $countryCode = $adapter->getCountryCode($message['to'][0] ?? ''); - if (!empty($countryCode)) { - $queueForUsage - ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); - } - $queueForUsage - ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) - ->setProject($project) - ->trigger(); } catch (\Throwable $th) { throw new \Exception('Failed sending to targets with error: ' . $th->getMessage()); } From 1f240ca27aa765727249beb20cac9e5e7bf5c254 Mon Sep 17 00:00:00 2001 From: ChiragAgg5k Date: Wed, 8 Jan 2025 17:09:55 +0530 Subject: [PATCH 3/5] chore: moved non-countrycode tracking outside abuse --- app/controllers/api/account.php | 24 ++++++++++++------------ app/controllers/api/teams.php | 8 ++++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 40e248836b..b80cf0fd64 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2473,11 +2473,11 @@ App::post('/v1/account/tokens/phone') $queueForUsage ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); } - $queueForUsage - ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) - ->setProject($project) - ->trigger(); } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); } } @@ -3593,11 +3593,11 @@ App::post('/v1/account/verification/phone') $queueForUsage ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); } - $queueForUsage - ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) - ->setProject($project) - ->trigger(); } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); } } @@ -4154,11 +4154,11 @@ App::post('/v1/account/mfa/challenge') $queueForUsage ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); } - $queueForUsage - ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) - ->setProject($project) - ->trigger(); } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); } break; case Type::EMAIL: diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 216807fecf..fc9f959ed2 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -712,11 +712,11 @@ App::post('/v1/teams/:teamId/memberships') $queueForUsage ->addMetric(str_replace('{countryCode}', $countryCode, METRIC_AUTH_METHOD_PHONE_COUNTRY_CODE), 1); } - $queueForUsage - ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) - ->setProject($project) - ->trigger(); } + $queueForUsage + ->addMetric(METRIC_AUTH_METHOD_PHONE, 1) + ->setProject($project) + ->trigger(); } } } From 2178bb44cd01d3c8e0934e6266bf76379a97cd68 Mon Sep 17 00:00:00 2001 From: ChiragAgg5k Date: Wed, 8 Jan 2025 17:14:07 +0530 Subject: [PATCH 4/5] fix: attribute --- app/controllers/api/account.php | 6 +++--- app/controllers/api/teams.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index b80cf0fd64..ff96acfc57 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2462,7 +2462,7 @@ App::post('/v1/account/tokens/phone') if (isset($plan['authPhone'])) { $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days $timelimit - ->setParam('{organizationId}', $project->getAttribute('organizationId')); + ->setParam('{organizationId}', $project->getAttribute('teamId')); $abuse = new Abuse($timelimit); if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { @@ -3582,7 +3582,7 @@ App::post('/v1/account/verification/phone') if (isset($plan['authPhone'])) { $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days $timelimit - ->setParam('{organizationId}', $project->getAttribute('organizationId')); + ->setParam('{organizationId}', $project->getAttribute('teamId')); $abuse = new Abuse($timelimit); if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { @@ -4143,7 +4143,7 @@ App::post('/v1/account/mfa/challenge') if (isset($plan['authPhone'])) { $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days $timelimit - ->setParam('{organizationId}', $project->getAttribute('organizationId')); + ->setParam('{organizationId}', $project->getAttribute('teamId')); $abuse = new Abuse($timelimit); if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index fc9f959ed2..461e90da58 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -701,7 +701,7 @@ App::post('/v1/teams/:teamId/memberships') if (isset($plan['authPhone'])) { $timelimit = $timelimit('organization:{organizationId}', $plan['authPhone'], 30 * 24 * 60 * 60); // 30 days $timelimit - ->setParam('{organizationId}', $project->getAttribute('organizationId')); + ->setParam('{organizationId}', $project->getAttribute('teamId')); $abuse = new Abuse($timelimit); if ($abuse->check() && System::getEnv('_APP_OPTIONS_ABUSE', 'enabled') === 'enabled') { From 0daf4057f36b062bd80df21f50d1ebeaf3271d96 Mon Sep 17 00:00:00 2001 From: Darshan Date: Wed, 8 Jan 2025 19:10:30 +0530 Subject: [PATCH 5/5] fix: benchmark test. --- .github/workflows/tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ab240c1b9..fc03891b35 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -175,6 +175,8 @@ jobs: name: Benchmark runs-on: ubuntu-latest needs: setup + permissions: + pull-requests: write steps: - name: Checkout repository uses: actions/checkout@v4