Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions src/Utopia/Messaging/Adapter/SMS/AlibabaCloud.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php

namespace Utopia\Messaging\Adapter\SMS;

use Utopia\Messaging\Adapter\SMS as SMSAdapter;
use Utopia\Messaging\Messages\SMS as SMSMessage;
use Utopia\Messaging\Response;

class AlibabaCloud extends SMSAdapter
{
protected const NAME = 'AlibabaCloud';

/**
* @param string $accessKeyId Alibaba Cloud Access Key ID
* @param string $accessKeySecret Alibaba Cloud Access Key Secret
* @param string $signName Alibaba Cloud SMS Sign Name
* @param string $templateCode Alibaba Cloud SMS Template Code
*/
public function __construct(
private string $accessKeyId,
private string $accessKeySecret,
private string $signName,
private string $templateCode,
) {
}

/**
* Get adapter name.
*/
public function getName(): string
{
return static::NAME;
}

/**
* Get max messages per request.
*/
public function getMaxMessagesPerRequest(): int
{
return 1;
}

/**
* {@inheritdoc}
*/
protected function process(SMSMessage $message): array
{
$response = new Response($this->getType());

foreach ($message->getTo() as $to) {
$params = [
'AccessKeyId' => $this->accessKeyId,
'Action' => 'SendSms',
'Format' => 'JSON',
'PhoneNumbers' => $to,
'RegionId' => 'cn-hangzhou',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 RegionId is hardcoded to 'cn-hangzhou'

The Alibaba Cloud SMS API supports multiple regions (e.g., ap-southeast-1 for Southeast Asia, eu-central-1 for Europe). Hardcoding cn-hangzhou means all traffic is routed through mainland China, which may incur higher latency and potential regulatory concerns for international deployments. Consider accepting regionId as an optional constructor parameter so callers can choose the appropriate datacenter.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

'SignName' => $this->signName,
'SignatureMethod' => 'HMAC-SHA1',
'SignatureNonce' => \uniqid(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 uniqid() may produce collisions under concurrent load — it is based on the current microsecond timestamp. Under high request volumes, two simultaneous calls could generate identical nonces, which the Alibaba Cloud API may reject as replay attempts. Prefer bin2hex(random_bytes(16)) to generate a cryptographically random nonce.

Suggested change
'SignatureNonce' => \uniqid(),
'SignatureNonce' => \bin2hex(\random_bytes(16)),

'SignatureVersion' => '1.0',
'TemplateCode' => $this->templateCode,
'TemplateParam' => \json_encode(['code' => $message->getContent()]),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 TemplateParam key hardcoded to 'code'

The adapter always wraps the message content under the key 'code', meaning it only works with Alibaba Cloud SMS templates that define a ${code} placeholder variable. Any template using a different variable name (e.g., ${name}, ${product}, ${otp}) will receive an incorrect or empty substitution and the API will return an error. The TemplateParam JSON should either be passed through as the raw content (letting callers supply the full JSON structure) or the parameter key should be user-configurable via the constructor.

'Timestamp' => \gmdate('Y-m-d\TH:i:s\Z'),
'Version' => '2017-05-25',
];

$params['Signature'] = $this->generateSignature($params);

$result = $this->request(
method: 'GET',
url: 'https://dysmsapi.aliyuncs.com',
headers: [],
body: $params
);

if ($result['statusCode'] >= 200 && $result['statusCode'] < 300 && ($result['response']['Code'] ?? '') === 'OK') {
$response->incrementDeliveredTo();
$response->addResult($to);
} else {
$response->addResult($to, $result['response']['Message'] ?? 'Unknown error');
}
}

return $response->toArray();
}

/**
* Generate Alibaba Cloud API Signature.
*/
private function generateSignature(array $params): string
{
\ksort($params);

$canonicalizedQueryString = '';
foreach ($params as $key => $value) {
$canonicalizedQueryString .= '&' . $this->percentEncode($key) . '=' . $this->percentEncode($value);
}

$stringToSign = 'GET&' . $this->percentEncode('/') . '&' . $this->percentEncode(\substr($canonicalizedQueryString, 1));

$signature = \base64_encode(\hash_hmac('sha1', $stringToSign, $this->accessKeySecret . '&', true));

return $signature;
}

private function percentEncode(string $str): string
{
$res = \urlencode($str);
$res = \str_replace(['+', '*'], ['%20', '%2A'], $res);
$res = \preg_replace('/%7E/i', '~', $res);

return $res;
}
Comment on lines +106 to +113
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

preg_replace can return null on error.

If preg_replace encounters an error, it returns null, but the function is typed to return string. This could cause a type error.

🛡️ Proposed fix
 private function percentEncode(string $str): string
 {
     $res = \urlencode($str);
     $res = \str_replace(['+', '*'], ['%20', '%2A'], $res);
-    $res = \preg_replace('/%7E/i', '~', $res);
+    $res = \preg_replace('/%7E/i', '~', $res) ?? $res;

     return $res;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function percentEncode(string $str): string
{
$res = \urlencode($str);
$res = \str_replace(['+', '*'], ['%20', '%2A'], $res);
$res = \preg_replace('/%7E/i', '~', $res);
return $res;
}
private function percentEncode(string $str): string
{
$res = \urlencode($str);
$res = \str_replace(['+', '*'], ['%20', '%2A'], $res);
$res = \preg_replace('/%7E/i', '~', $res) ?? $res;
return $res;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Utopia/Messaging/Adapter/SMS/AlibabaCloud.php` around lines 106 - 113,
The percentEncode function uses preg_replace('/%7E/i', '~', $res) which can
return null on error but percentEncode is typed to return string; update
percentEncode to handle a null result from preg_replace (in function
percentEncode) by checking the return value and falling back to the original
$res or casting to string before returning so the method always returns a
string, ensuring any preg_replace failure is safely handled.

}
32 changes: 32 additions & 0 deletions tests/Messaging/Adapter/SMS/AlibabaCloudTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace Utopia\Tests\Adapter\SMS;

use Utopia\Messaging\Adapter\SMS\AlibabaCloud;
use Utopia\Messaging\Messages\SMS;
use Utopia\Tests\Adapter\Base;

class AlibabaCloudTest extends Base
{
/**
* @throws \Exception
*/
public function testSendSMS(): void
{
$sender = new AlibabaCloud(
\getenv('ALIBABA_CLOUD_ACCESS_KEY_ID'),
\getenv('ALIBABA_CLOUD_ACCESS_KEY_SECRET'),
\getenv('ALIBABA_CLOUD_SIGN_NAME'),
\getenv('ALIBABA_CLOUD_TEMPLATE_CODE')
);

$message = new SMS(
to: [\getenv('ALIBABA_CLOUD_TO')],
content: \json_encode(['code' => '123456']),
);
Comment on lines +23 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Message content format may cause double JSON encoding.

The test passes \json_encode(['code' => '123456']) as content, but the adapter wraps this in another JSON object with the 'code' key (line 62 of the adapter). This results in double encoding: {"code":"{\"code\":\"123456\"}"}.

If the adapter fix to pass content directly as TemplateParam is applied, update this test accordingly:

🐛 Proposed fix (if adapter is updated to use content directly)
         $message = new SMS(
             to: [\getenv('ALIBABA_CLOUD_TO')],
-            content: \json_encode(['code' => '123456']),
+            content: '{"code":"123456"}',
         );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$message = new SMS(
to: [\getenv('ALIBABA_CLOUD_TO')],
content: \json_encode(['code' => '123456']),
);
$message = new SMS(
to: [\getenv('ALIBABA_CLOUD_TO')],
content: '{"code":"123456"}',
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Messaging/Adapter/SMS/AlibabaCloudTest.php` around lines 23 - 26, The
test is passing a JSON string as SMS content which causes double-encoding when
the adapter wraps it again; update the test in AlibabaCloudTest so SMS is
constructed with content as an associative array (e.g., ['code' => '123456']) or
otherwise match the adapter’s expected TemplateParam shape instead of passing
json_encode(...), and adjust any assertions that inspect the resulting
TemplateParam payload to expect the single-layer JSON (or raw array) produced by
the adapter; update references to the SMS constructor call (symbol: SMS) in the
test to pass the direct array content.

Comment on lines +23 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Content is double-encoded into TemplateParam

The test sets content to json_encode(['code' => '123456']) = '{"code":"123456"}'. Inside process(), that string is then wrapped again with json_encode(['code' => $message->getContent()]), producing TemplateParam = {"code":"{\"code\":\"123456\"}"}. The Alibaba Cloud API receives code = {"code":"123456"} (a JSON string) rather than 123456, which will either fail validation or inject the wrong value into the SMS template. The test content should be the plain OTP value — '123456' — not a pre-encoded JSON object.


$result = $sender->send($message);

$this->assertResponse($result);
}
}