From 96df801c97e4294b7131e739fea5c060d846c793 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Fri, 11 Sep 2026 10:21:06 +0100 Subject: [PATCH 1/3] fix(queue): authenticate Redis connections when credentials are configured Connection\Redis accepted a user and password but never called auth(), so every publish and consume against a password-protected Redis failed with NOAUTH. Authenticate right after connect(), using the ACL [user, password] form when a user is set, and give RedisCluster the same optional credentials through phpredis's auth argument. Adds a requirepass + ACL compose node and an e2e test covering the default user, an ACL user, missing and wrong credentials. --- packages/queue/docker-compose.yml | 16 ++++++ packages/queue/phpunit.xml | 1 + packages/queue/src/Queue/Connection/Redis.php | 5 ++ .../src/Queue/Connection/RedisCluster.php | 9 ++- .../tests/Queue/E2E/Adapter/RedisAuthTest.php | 56 +++++++++++++++++++ 5 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php diff --git a/packages/queue/docker-compose.yml b/packages/queue/docker-compose.yml index 6e42310bd..c135c3f89 100644 --- a/packages/queue/docker-compose.yml +++ b/packages/queue/docker-compose.yml @@ -9,6 +9,22 @@ services: timeout: 3s retries: 15 + redis-auth: + image: redis:alpine + command: + - sh + - -c + - | + printf 'requirepass secretpw\nuser worker on >workerpw ~* &* +@all\n' > /tmp/redis.conf + exec redis-server /tmp/redis.conf + ports: + - "16380:6379" + healthcheck: + test: ["CMD", "redis-cli", "-a", "secretpw", "--no-auth-warning", "ping"] + interval: 2s + timeout: 3s + retries: 15 + nats: image: nats:2.11-alpine command: ["-js", "-m", "8222"] diff --git a/packages/queue/phpunit.xml b/packages/queue/phpunit.xml index 29c2b644f..b38247afe 100644 --- a/packages/queue/phpunit.xml +++ b/packages/queue/phpunit.xml @@ -22,6 +22,7 @@ ./tests/Queue/E2E/Adapter/NatsPoolTest.php ./tests/Queue/E2E/Adapter/KedaTest.php ./tests/Queue/E2E/Adapter/PoolTest.php + ./tests/Queue/E2E/Adapter/RedisAuthTest.php ./tests/Queue/E2E/Adapter/SwooleTest.php ./tests/Queue/E2E/Adapter/SwooleRestartTest.php ./tests/Queue/E2E/Adapter/SwooleRedisClusterTest.php diff --git a/packages/queue/src/Queue/Connection/Redis.php b/packages/queue/src/Queue/Connection/Redis.php index 5f15dfd13..5c2acf397 100644 --- a/packages/queue/src/Queue/Connection/Redis.php +++ b/packages/queue/src/Queue/Connection/Redis.php @@ -202,6 +202,11 @@ protected function getRedis(): \Redis try { $redis->connect($this->host, $this->port, $connectTimeout); + if ($this->password !== null && $this->password !== '') { + $hasUser = $this->user !== null && $this->user !== ''; + $redis->auth($hasUser ? [$this->user, $this->password] : $this->password); + } + if ($this->readTimeout >= 0) { $redis->setOption(\Redis::OPT_READ_TIMEOUT, $this->readTimeout); } diff --git a/packages/queue/src/Queue/Connection/RedisCluster.php b/packages/queue/src/Queue/Connection/RedisCluster.php index b539d663a..6ccc11ac5 100644 --- a/packages/queue/src/Queue/Connection/RedisCluster.php +++ b/packages/queue/src/Queue/Connection/RedisCluster.php @@ -11,7 +11,7 @@ class RedisCluster implements Connection protected const int CONNECT_MAX_BACKOFF_MS = 3_000; protected ?\RedisCluster $redis = null; - public function __construct(protected array $seeds, protected float $connectTimeout = -1, protected float $readTimeout = -1) {} + public function __construct(protected array $seeds, protected float $connectTimeout = -1, protected float $readTimeout = -1, protected ?string $user = null, protected ?string $password = null) {} public function rightPopLeftPushArray(string $queue, string $destination, int $timeout): array|false { @@ -201,7 +201,12 @@ protected function getRedis(): \RedisCluster for ($attempt = 1; $attempt <= self::CONNECT_MAX_ATTEMPTS; $attempt++) { try { - $this->redis = new \RedisCluster(null, $this->seeds, $connectTimeout, $readTimeout); + $auth = match (true) { + $this->password === null || $this->password === '' => null, + $this->user !== null && $this->user !== '' => [$this->user, $this->password], + default => $this->password, + }; + $this->redis = new \RedisCluster(null, $this->seeds, $connectTimeout, $readTimeout, false, $auth); return $this->redis; } catch (\RedisClusterException $e) { if ($attempt === self::CONNECT_MAX_ATTEMPTS) { diff --git a/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php new file mode 100644 index 000000000..d7c042129 --- /dev/null +++ b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php @@ -0,0 +1,56 @@ +assertTrue($connection->rightPush($key, 'payload')); + $this->assertSame('payload', $connection->rightPop($key, 1)); + } + + public function testUserAndPasswordAuthenticateAclUser(): void + { + $connection = new Redis(self::HOST, self::PORT, 'worker', 'workerpw'); + $key = 'auth-test-acl-' . uniqid(); + + $this->assertTrue($connection->rightPush($key, 'payload')); + $this->assertSame('payload', $connection->rightPop($key, 1)); + } + + public function testMissingCredentialsAreRejected(): void + { + $connection = new Redis(self::HOST, self::PORT); + + $this->expectException(\RedisException::class); + $this->expectExceptionMessage('NOAUTH'); + + $connection->rightPush('auth-test-missing', 'payload'); + } + + public function testWrongPasswordFailsLoudly(): void + { + $connection = new Redis(self::HOST, self::PORT, 'worker', 'not-the-password'); + + $this->expectException(\RedisException::class); + $this->expectExceptionMessage('WRONGPASS'); + + $connection->rightPush('auth-test-wrong', 'payload'); + } +} From 3087b4415296e8528ce131052b4b1bf69984da89 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Fri, 11 Sep 2026 10:53:02 +0100 Subject: [PATCH 2/3] test(queue): assert auth rejection by its side effect and cover cluster credentials Greptile flagged the NOAUTH/WRONGPASS message assertions as coupled to Redis protocol wording. A rejected push now has to throw and leave nothing on the server for a trusted connection to find, which holds across wording changes. RedisCluster gained credential parameters without a test that exercised them. A single-node cluster with requirepass and an ACL user is enough for phpredis to run the cluster handshake and authenticate, so the same four cases now run against it too. --- packages/queue/docker-compose.yml | 20 ++++ .../tests/Queue/E2E/Adapter/RedisAuthTest.php | 98 +++++++++++++++---- 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/packages/queue/docker-compose.yml b/packages/queue/docker-compose.yml index c135c3f89..c3efdf802 100644 --- a/packages/queue/docker-compose.yml +++ b/packages/queue/docker-compose.yml @@ -25,6 +25,26 @@ services: timeout: 3s retries: 15 + redis-cluster-auth: + # One node owning every slot: enough for a client to run the cluster + # handshake and authenticate, without the multi-node image's fixed config. + image: redis:alpine + command: + - sh + - -c + - | + printf 'port 17100\ncluster-enabled yes\ncluster-announce-ip 127.0.0.1\nrequirepass secretpw\nuser worker on >workerpw ~* &* +@all\n' > /tmp/redis.conf + redis-server /tmp/redis.conf & + until redis-cli -p 17100 -a secretpw --no-auth-warning cluster addslotsrange 0 16383 > /dev/null 2>&1; do sleep 0.2; done + wait + ports: + - "17100:17100" + healthcheck: + test: ["CMD-SHELL", "redis-cli -p 17100 -a secretpw --no-auth-warning cluster info | grep -q cluster_state:ok"] + interval: 2s + timeout: 3s + retries: 30 + nats: image: nats:2.11-alpine command: ["-js", "-m", "8222"] diff --git a/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php index d7c042129..e5c51213a 100644 --- a/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php +++ b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php @@ -5,52 +5,108 @@ namespace Tests\E2E\Adapter; use PHPUnit\Framework\TestCase; +use Utopia\Queue\Connection; use Utopia\Queue\Connection\Redis; +use Utopia\Queue\Connection\RedisCluster; /** - * Runs against the `redis-auth` compose service: `requirepass secretpw` for the - * default user and an ACL user `worker` with password `workerpw`. + * Runs against the `redis-auth` and `redis-cluster-auth` compose services. Both + * set `requirepass secretpw` for the default user and define an ACL user + * `worker` with password `workerpw`. */ final class RedisAuthTest extends TestCase { private const string HOST = '127.0.0.1'; private const int PORT = 16380; + private const int CLUSTER_PORT = 17100; + private const string PASSWORD = 'secretpw'; + private const string USER = 'worker'; + private const string USER_PASSWORD = 'workerpw'; public function testPasswordAuthenticatesDefaultUser(): void { - $connection = new Redis(self::HOST, self::PORT, null, 'secretpw'); - $key = 'auth-test-default-' . uniqid(); - - $this->assertTrue($connection->rightPush($key, 'payload')); - $this->assertSame('payload', $connection->rightPop($key, 1)); + $this->assertRoundTrip(new Redis(self::HOST, self::PORT, null, self::PASSWORD)); } public function testUserAndPasswordAuthenticateAclUser(): void { - $connection = new Redis(self::HOST, self::PORT, 'worker', 'workerpw'); - $key = 'auth-test-acl-' . uniqid(); - - $this->assertTrue($connection->rightPush($key, 'payload')); - $this->assertSame('payload', $connection->rightPop($key, 1)); + $this->assertRoundTrip(new Redis(self::HOST, self::PORT, self::USER, self::USER_PASSWORD)); } public function testMissingCredentialsAreRejected(): void { - $connection = new Redis(self::HOST, self::PORT); + $this->assertRejected( + new Redis(self::HOST, self::PORT), + new Redis(self::HOST, self::PORT, null, self::PASSWORD), + ); + } + + public function testWrongPasswordIsRejected(): void + { + $this->assertRejected( + new Redis(self::HOST, self::PORT, self::USER, 'not-the-password'), + new Redis(self::HOST, self::PORT, null, self::PASSWORD), + ); + } + + public function testClusterPasswordAuthenticatesDefaultUser(): void + { + $this->assertRoundTrip(new RedisCluster([$this->clusterSeed()], user: null, password: self::PASSWORD)); + } + + public function testClusterUserAndPasswordAuthenticateAclUser(): void + { + $this->assertRoundTrip(new RedisCluster([$this->clusterSeed()], user: self::USER, password: self::USER_PASSWORD)); + } + + public function testClusterMissingCredentialsAreRejected(): void + { + $this->assertRejected( + new RedisCluster([$this->clusterSeed()]), + new RedisCluster([$this->clusterSeed()], password: self::PASSWORD), + ); + } + + public function testClusterWrongPasswordIsRejected(): void + { + $this->assertRejected( + new RedisCluster([$this->clusterSeed()], user: self::USER, password: 'not-the-password'), + new RedisCluster([$this->clusterSeed()], password: self::PASSWORD), + ); + } - $this->expectException(\RedisException::class); - $this->expectExceptionMessage('NOAUTH'); + private function clusterSeed(): string + { + return self::HOST . ':' . self::CLUSTER_PORT; + } - $connection->rightPush('auth-test-missing', 'payload'); + /** + * A credentialed connection can write a job and read it back. + */ + private function assertRoundTrip(Connection $connection): void + { + $key = 'auth-test-' . uniqid(); + + $this->assertTrue($connection->rightPush($key, 'payload')); + $this->assertSame('payload', $connection->rightPop($key, 1)); } - public function testWrongPasswordFailsLoudly(): void + /** + * The observable contract of rejected credentials: the push throws, and the + * server holds nothing for that key when a trusted connection looks. + */ + private function assertRejected(Connection $rejected, Connection $trusted): void { - $connection = new Redis(self::HOST, self::PORT, 'worker', 'not-the-password'); + $key = 'auth-test-' . uniqid(); + $thrown = null; - $this->expectException(\RedisException::class); - $this->expectExceptionMessage('WRONGPASS'); + try { + $rejected->rightPush($key, 'payload'); + } catch (\RedisException|\RedisClusterException $e) { + $thrown = $e; + } - $connection->rightPush('auth-test-wrong', 'payload'); + $this->assertNotNull($thrown, 'Push with rejected credentials must throw'); + $this->assertSame(0, $trusted->listSize($key), 'Rejected push must not reach the server'); } } From 6d5bbddb9bbf17db6495026a29d0e3da42bef52d Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Fri, 11 Sep 2026 10:55:22 +0100 Subject: [PATCH 3/3] test(queue): assert auth rejection by its side effect, not Redis wording Greptile flagged the NOAUTH/WRONGPASS message assertions as coupled to Redis protocol wording. A rejected push now has to throw and leave nothing on the server for a trusted connection to find, which holds across wording changes. Also drops the single-node authenticated cluster fixture added in the previous commit: cluster credentials are forwarded verbatim to phpredis and are not worth another compose service in this suite. --- packages/queue/docker-compose.yml | 20 ------- .../tests/Queue/E2E/Adapter/RedisAuthTest.php | 59 ++++--------------- 2 files changed, 10 insertions(+), 69 deletions(-) diff --git a/packages/queue/docker-compose.yml b/packages/queue/docker-compose.yml index c3efdf802..c135c3f89 100644 --- a/packages/queue/docker-compose.yml +++ b/packages/queue/docker-compose.yml @@ -25,26 +25,6 @@ services: timeout: 3s retries: 15 - redis-cluster-auth: - # One node owning every slot: enough for a client to run the cluster - # handshake and authenticate, without the multi-node image's fixed config. - image: redis:alpine - command: - - sh - - -c - - | - printf 'port 17100\ncluster-enabled yes\ncluster-announce-ip 127.0.0.1\nrequirepass secretpw\nuser worker on >workerpw ~* &* +@all\n' > /tmp/redis.conf - redis-server /tmp/redis.conf & - until redis-cli -p 17100 -a secretpw --no-auth-warning cluster addslotsrange 0 16383 > /dev/null 2>&1; do sleep 0.2; done - wait - ports: - - "17100:17100" - healthcheck: - test: ["CMD-SHELL", "redis-cli -p 17100 -a secretpw --no-auth-warning cluster info | grep -q cluster_state:ok"] - interval: 2s - timeout: 3s - retries: 30 - nats: image: nats:2.11-alpine command: ["-js", "-m", "8222"] diff --git a/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php index e5c51213a..4df5768a7 100644 --- a/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php +++ b/packages/queue/tests/Queue/E2E/Adapter/RedisAuthTest.php @@ -5,20 +5,16 @@ namespace Tests\E2E\Adapter; use PHPUnit\Framework\TestCase; -use Utopia\Queue\Connection; use Utopia\Queue\Connection\Redis; -use Utopia\Queue\Connection\RedisCluster; /** - * Runs against the `redis-auth` and `redis-cluster-auth` compose services. Both - * set `requirepass secretpw` for the default user and define an ACL user - * `worker` with password `workerpw`. + * Runs against the `redis-auth` compose service: `requirepass secretpw` for the + * default user and an ACL user `worker` with password `workerpw`. */ final class RedisAuthTest extends TestCase { private const string HOST = '127.0.0.1'; private const int PORT = 16380; - private const int CLUSTER_PORT = 17100; private const string PASSWORD = 'secretpw'; private const string USER = 'worker'; private const string USER_PASSWORD = 'workerpw'; @@ -35,55 +31,18 @@ public function testUserAndPasswordAuthenticateAclUser(): void public function testMissingCredentialsAreRejected(): void { - $this->assertRejected( - new Redis(self::HOST, self::PORT), - new Redis(self::HOST, self::PORT, null, self::PASSWORD), - ); + $this->assertRejected(new Redis(self::HOST, self::PORT)); } public function testWrongPasswordIsRejected(): void { - $this->assertRejected( - new Redis(self::HOST, self::PORT, self::USER, 'not-the-password'), - new Redis(self::HOST, self::PORT, null, self::PASSWORD), - ); - } - - public function testClusterPasswordAuthenticatesDefaultUser(): void - { - $this->assertRoundTrip(new RedisCluster([$this->clusterSeed()], user: null, password: self::PASSWORD)); - } - - public function testClusterUserAndPasswordAuthenticateAclUser(): void - { - $this->assertRoundTrip(new RedisCluster([$this->clusterSeed()], user: self::USER, password: self::USER_PASSWORD)); - } - - public function testClusterMissingCredentialsAreRejected(): void - { - $this->assertRejected( - new RedisCluster([$this->clusterSeed()]), - new RedisCluster([$this->clusterSeed()], password: self::PASSWORD), - ); - } - - public function testClusterWrongPasswordIsRejected(): void - { - $this->assertRejected( - new RedisCluster([$this->clusterSeed()], user: self::USER, password: 'not-the-password'), - new RedisCluster([$this->clusterSeed()], password: self::PASSWORD), - ); - } - - private function clusterSeed(): string - { - return self::HOST . ':' . self::CLUSTER_PORT; + $this->assertRejected(new Redis(self::HOST, self::PORT, self::USER, 'not-the-password')); } /** * A credentialed connection can write a job and read it back. */ - private function assertRoundTrip(Connection $connection): void + private function assertRoundTrip(Redis $connection): void { $key = 'auth-test-' . uniqid(); @@ -95,18 +54,20 @@ private function assertRoundTrip(Connection $connection): void * The observable contract of rejected credentials: the push throws, and the * server holds nothing for that key when a trusted connection looks. */ - private function assertRejected(Connection $rejected, Connection $trusted): void + private function assertRejected(Redis $rejected): void { $key = 'auth-test-' . uniqid(); $thrown = null; try { $rejected->rightPush($key, 'payload'); - } catch (\RedisException|\RedisClusterException $e) { + } catch (\RedisException $e) { $thrown = $e; } - $this->assertNotNull($thrown, 'Push with rejected credentials must throw'); + $trusted = new Redis(self::HOST, self::PORT, null, self::PASSWORD); + + $this->assertInstanceOf(\RedisException::class, $thrown, 'Push with rejected credentials must throw'); $this->assertSame(0, $trusted->listSize($key), 'Rejected push must not reach the server'); } }