mirror of
https://github.com/appwrite/appwrite.git
synced 2026-05-26 13:51:13 +00:00
fix(notifications): drop misleading auth metadata, exercise unique-index dedup, fail loudly on missing tracking secret in tests
- Account/Alerts/Track endpoint is scope:public (email clients have no session); SDK Method now declares auth: [] so generators do not require an auth header for an unauthenticated endpoint. - NotificationsTest setUp now creates the production `_key_recipient` UNIQUE composite index on (messageId, channel, userId, teamId), and testPersistAlertReturnsExistingAlertIdOnDuplicate exercises the DuplicateException → return-existing-alertId branch end-to-end (both primary-key collision and unique-index collision via a sibling $id). - Tracking-pixel e2e now asserts _APP_OPENSSL_KEY_V1 is set instead of silently falling back to the .env.example placeholder, so a missing CI secret fails loudly rather than passing against the wrong key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
e125bf6aec
commit
1aa6a05ad8
@@ -3,7 +3,6 @@
|
||||
namespace Appwrite\Platform\Modules\Account\Http\Alerts\Track;
|
||||
|
||||
use Ahc\Jwt\JWT;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\ContentType;
|
||||
use Appwrite\SDK\Method;
|
||||
use Appwrite\SDK\Response as SDKResponse;
|
||||
@@ -39,7 +38,7 @@ class Get extends Action
|
||||
group: 'alerts',
|
||||
name: 'getAlertTrack',
|
||||
description: '/docs/references/account/get-alert-track.md',
|
||||
auth: [AuthType::SESSION, AuthType::JWT],
|
||||
auth: [],
|
||||
responses: [
|
||||
new SDKResponse(
|
||||
code: Response::STATUS_CODE_OK,
|
||||
|
||||
@@ -190,7 +190,8 @@ trait NotificationsBase
|
||||
$alertId = self::$seededAlertId ?? $this->seedWebhookFailureAlert();
|
||||
$this->assertNotEmpty($alertId);
|
||||
|
||||
$secret = System::getEnv('_APP_OPENSSL_KEY_V1') ?: 'your-secret-key';
|
||||
$secret = System::getEnv('_APP_OPENSSL_KEY_V1');
|
||||
$this->assertNotEmpty($secret, '_APP_OPENSSL_KEY_V1 must be set for tracking pixel test');
|
||||
$userId = $this->getRoot()['$id'];
|
||||
|
||||
$jwt = (new JWT($secret, 'HS256', 2592000, 0))->encode([
|
||||
|
||||
@@ -172,6 +172,18 @@ class NotificationsTest extends TestCase
|
||||
$this->database->createAttribute('alerts', 'body', Database::VAR_STRING, 16384, true);
|
||||
$this->database->createAttribute('alerts', 'read', Database::VAR_BOOLEAN, 0, false, false);
|
||||
|
||||
// Mirror the production `_key_recipient` UNIQUE composite index so the
|
||||
// duplicate-handling branch in persistAlert (catch DuplicateException →
|
||||
// return existing alertId) is actually exercised by tests.
|
||||
$this->database->createIndex(
|
||||
'alerts',
|
||||
'_key_recipient',
|
||||
Database::INDEX_UNIQUE,
|
||||
['messageId', 'channel', 'userId', 'teamId'],
|
||||
[Database::LENGTH_KEY, 64, Database::LENGTH_KEY, Database::LENGTH_KEY],
|
||||
[Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC, Database::ORDER_ASC],
|
||||
);
|
||||
|
||||
$this->registry = new Registry();
|
||||
$this->project = new Document(['$id' => 'project-x']);
|
||||
$this->log = new Log();
|
||||
@@ -554,6 +566,97 @@ class NotificationsTest extends TestCase
|
||||
$this->assertLessThan($lastBodyClose, $pixelPosition, 'pixel must be spliced before the final </body>');
|
||||
}
|
||||
|
||||
public function testPersistAlertReturnsExistingAlertIdOnDuplicate(): void
|
||||
{
|
||||
$spy = new SpyEmailAdapter();
|
||||
$this->registry->set('smtp', static fn () => $spy);
|
||||
|
||||
$previousSmtpHost = \getenv('_APP_SMTP_HOST');
|
||||
\putenv('_APP_SMTP_HOST=spy.smtp.test');
|
||||
|
||||
try {
|
||||
$worker = new CountingPersistAlertNotifications();
|
||||
|
||||
// All four unique-index fields populated so the
|
||||
// `_key_recipient` UNIQUE composite (messageId, channel,
|
||||
// userId, teamId) actually fires — SQL UNIQUE semantics treat
|
||||
// NULL as not-equal, so any null in the tuple disables it.
|
||||
$payload = [
|
||||
'project' => ['$id' => 'project-x'],
|
||||
'recipients' => [
|
||||
[
|
||||
'address' => 'user@example.test',
|
||||
'channel' => NOTIFICATION_TYPE_EMAIL,
|
||||
'userId' => 'user-7',
|
||||
'teamId' => 'team-7',
|
||||
],
|
||||
],
|
||||
'subject' => 'Heads up',
|
||||
'body' => 'b',
|
||||
'deduplicationKey' => 'persist-dup',
|
||||
];
|
||||
|
||||
// First dispatch: writes a row through the action loop and
|
||||
// returns the deterministic alertId.
|
||||
$worker->action($this->buildMessage($payload), $this->project, $this->registry, $this->database, $this->log);
|
||||
|
||||
$this->assertSame(1, $worker->persistAlertCalls);
|
||||
$firstAlertId = $worker->persistedIds[0];
|
||||
|
||||
$messageId = \md5('persist-dup');
|
||||
$recipient = [
|
||||
'address' => 'user@example.test',
|
||||
'channel' => NOTIFICATION_TYPE_EMAIL,
|
||||
'userId' => 'user-7',
|
||||
'teamId' => 'team-7',
|
||||
];
|
||||
|
||||
// Second invocation with the SAME messageId/recipient. The
|
||||
// action loop's alreadyDelivered() check short-circuits before
|
||||
// persistAlert, so call persistAlert directly to actually hit
|
||||
// the duplicate branch. The deterministic $id collides on the
|
||||
// primary key → DuplicateException → branch returns the
|
||||
// existing alertId without throwing.
|
||||
$reflection = new \ReflectionMethod($worker, 'persistAlert');
|
||||
$secondAlertId = $reflection->invoke($worker, $this->database, $messageId, $recipient, $payload);
|
||||
|
||||
$this->assertSame($firstAlertId, $secondAlertId, 'duplicate persist must return the existing alertId');
|
||||
|
||||
// Third write: bypass the deterministic $id path and use a
|
||||
// distinct $id with the same recipient tuple. The
|
||||
// `_key_recipient` UNIQUE composite must reject it — proving
|
||||
// the unique-index (not just primary-key) is what backstops the
|
||||
// duplicate-handling branch.
|
||||
$sameTupleDoc = new Document([
|
||||
'$id' => 'sibling-id-' . \uniqid(),
|
||||
'$permissions' => [Permission::read(Role::any())],
|
||||
'messageId' => $messageId,
|
||||
'channel' => NOTIFICATION_TYPE_EMAIL,
|
||||
'userId' => 'user-7',
|
||||
'teamId' => 'team-7',
|
||||
'projectId' => 'project-x',
|
||||
'title' => 'sibling',
|
||||
'body' => 'sibling',
|
||||
'read' => false,
|
||||
]);
|
||||
|
||||
$threw = false;
|
||||
try {
|
||||
$this->database->createDocument('alerts', $sameTupleDoc);
|
||||
} catch (\Utopia\Database\Exception\Duplicate) {
|
||||
$threw = true;
|
||||
}
|
||||
$this->assertTrue($threw, 'unique-index `_key_recipient` must reject a second row sharing the recipient tuple');
|
||||
|
||||
$rows = $this->database->find('alerts', [
|
||||
\Utopia\Database\Query::equal('messageId', [$messageId]),
|
||||
]);
|
||||
$this->assertCount(1, $rows, 'unique-index must prevent a second row from being persisted');
|
||||
} finally {
|
||||
\putenv($previousSmtpHost === false ? '_APP_SMTP_HOST' : '_APP_SMTP_HOST=' . $previousSmtpHost);
|
||||
}
|
||||
}
|
||||
|
||||
public function testPersistAlertReturnsAlertIdAndStoresUserId(): void
|
||||
{
|
||||
$spy = new SpyEmailAdapter();
|
||||
|
||||
Reference in New Issue
Block a user