Skip to content

Navigation Menu

Sign in
Appearance settings
Sign in
Sign up
Appearance settings

Commit 38bc4c0

Browse filesBrowse the repository at this point in the historyBrowse files
nicolas-grekasnicolas-grekas
committed
bug symfony#65636 [Cache] Fix authenticating to the master when using Redis Sentinel (nicolas-grekas)
This PR was merged into the 6.4 branch. Discussion ---------- [Cache] Fix authenticating to the master when using Redis Sentinel | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT A Sentinel DSN opens two connections: one to the Sentinel process, to ask for the current master, and one to that master, to run the commands. They can require different credentials, and many setups leave the Sentinel unauthenticated while the master requires a password. `RedisTrait` resolves credentials into `$params['auth']`, which symfony#63324 leaves empty in Sentinel mode on purpose, so that the DSN userinfo is not sent to the Sentinel. symfony#65421 then made two code paths read `$params['auth']` where they used to read the userinfo: the Predis branch, and the `$redis->auth()` call used with phpredis below 6. In Sentinel mode both now got nothing, so the master connection was made without credentials: ```php RedisAdapter::createConnection('redis://:p4ssw0rd@127.0.0.1:26379?redis_sentinel=mymaster'); // Redis connection failed: NOAUTH Authentication required. ``` This patch takes the resolution that 7.4 already has since symfony#63391: the userinfo authenticates the master, the `auth` query parameter or option authenticates the Sentinel handshake, and each is applied to its own connection. For Predis, the Sentinel credentials are applied to the Sentinel hosts, as they are on 7.4. ```php // master gets "master-pass", the Sentinel handshake gets no credentials RedisAdapter::createConnection('redis://:master-pass@127.0.0.1:26379?redis_sentinel=mymaster'); // master gets "master-pass", the Sentinel handshake gets "sentinel-pass" RedisAdapter::createConnection('redis://:master-pass@127.0.0.1:26379?redis_sentinel=mymaster&auth=sentinel-pass'); // no userinfo: both get "shared-pass", as before RedisAdapter::createConnection('redis://127.0.0.1:26379?redis_sentinel=mymaster&auth=shared-pass'); ``` The tests come from symfony#63391, with the same names, so the merge-up into 7.4 resolves by keeping the 7.4 version. They cover the resolution for the master and for the Sentinel, from the userinfo, from the query string and from the options. While doing that, the `auth` shape check added in symfony#65421 was chained with `elseif` to the check that reports missing Sentinel support. That made `Redis Sentinel support requires one of: ...` reachable for any DSN, not only Sentinel ones, when neither predis, `RedisSentinel` nor `Relay\Sentinel` exists. Both checks now stand on their own, as they do on 7.4. The shape check also covers the Sentinel credentials now. Checks run: the new tests fail on 6.4 for the five Sentinel cases and pass with the patch; `./phpunit src/Symfony/Component/Cache` reports the same failures with and without the patch on my machine; php-cs-fixer is clean. The fix was also verified end to end against a local master with `requirepass` and an unauthenticated Sentinel, with ext-redis 6.3 and with Predis: `NOAUTH Authentication required` before, `PONG` after. The phpredis below 6 path is fixed by the same change but was not run, since that version is not installed here. Commits ------- c365648 [Cache] Fix authenticating to the master when using Redis Sentinel
2 parents 0cb81e0 + c365648 commit 38bc4c0
Copy full SHA for 38bc4c0

2 files changed

+187-8Lines changed: 187 additions & 8 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/Symfony/Component/Cache/Tests/Traits/RedisTraitTest.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Tests/Traits/RedisTraitTest.php
+155Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,4 +210,159 @@ public static function provideInvalidDbIndexDsnParameter(): array
210210
],
211211
];
212212
}
213+
214+
/**
215+
* @dataProvider providePredisMasterAuthResolution
216+
*/
217+
public function testPredisMasterAuthResolution(string $dsn, array $options, string|array|null $expectedMasterAuth)
218+
{
219+
$predisClass = $this->createPredisCaptureClass();
220+
221+
$mock = new class {
222+
use RedisTrait;
223+
};
224+
225+
$mock::createConnection($dsn, ['class' => $predisClass] + $options);
226+
227+
$this->assertAuthMatchesExpected($expectedMasterAuth, $predisClass::$captured['options']['parameters'] ?? []);
228+
}
229+
230+
public static function providePredisMasterAuthResolution(): \Generator
231+
{
232+
yield 'userinfo user+pass' => [
233+
'redis://user:pass@localhost',
234+
[],
235+
['user', 'pass'],
236+
];
237+
238+
yield 'userinfo with @ + query auth array' => [
239+
'redis://user@pass@localhost?auth[]=otheruser&auth[]=otherpass',
240+
[],
241+
['otheruser', 'otherpass'],
242+
];
243+
244+
yield 'query auth array' => [
245+
'redis://localhost?auth[]=user&auth[]=pass',
246+
[],
247+
['user', 'pass'],
248+
];
249+
250+
yield 'options auth array' => [
251+
'redis://localhost',
252+
['auth' => ['user', 'pass']],
253+
['user', 'pass'],
254+
];
255+
256+
yield 'query auth beats options auth' => [
257+
'redis://localhost?auth[]=query-user&auth[]=query-pass',
258+
['auth' => ['opt-user', 'opt-pass']],
259+
['query-user', 'query-pass'],
260+
];
261+
}
262+
263+
/**
264+
* @dataProvider providePredisSentinelAuthResolution
265+
*/
266+
public function testPredisSentinelAuthResolution(string $dsn, array $options, string|array|null $expectedMasterAuth, string|array|null $expectedSentinelAuth)
267+
{
268+
$predisClass = $this->createPredisCaptureClass();
269+
270+
$mock = new class {
271+
use RedisTrait;
272+
};
273+
274+
$mock::createConnection($dsn, ['class' => $predisClass] + $options);
275+
276+
$this->assertAuthMatchesExpected($expectedMasterAuth, $predisClass::$captured['options']['parameters'] ?? []);
277+
$this->assertAuthMatchesExpected($expectedSentinelAuth, $predisClass::$captured['parameters'][0] ?? []);
278+
}
279+
280+
public static function providePredisSentinelAuthResolution(): \Generator
281+
{
282+
yield 'master userinfo, no sentinel auth' => [
283+
'redis://master-user:master-pass@localhost?redis_sentinel=mymaster',
284+
[],
285+
['master-user', 'master-pass'],
286+
null,
287+
];
288+
289+
yield 'sentinel query auth, master userinfo' => [
290+
'redis://master-user:master-pass@localhost?redis_sentinel=mymaster&auth[]=sentinel-user&auth[]=sentinel-pass',
291+
[],
292+
['master-user', 'master-pass'],
293+
['sentinel-user', 'sentinel-pass'],
294+
];
295+
296+
yield 'sentinel options auth when query missing' => [
297+
'redis://master-pass@localhost?redis_sentinel=mymaster',
298+
['auth' => ['sentinel-user', 'sentinel-pass']],
299+
'master-pass',
300+
['sentinel-user', 'sentinel-pass'],
301+
];
302+
303+
yield 'sentinel query auth beats options auth' => [
304+
'redis://master-pass@localhost?redis_sentinel=mymaster&auth[]=query-user&auth[]=query-pass',
305+
['auth' => ['opt-user', 'opt-pass']],
306+
'master-pass',
307+
['query-user', 'query-pass'],
308+
];
309+
310+
yield 'auth shared by master and sentinel when no userinfo' => [
311+
'redis://localhost?redis_sentinel=mymaster&auth=shared-pass',
312+
[],
313+
'shared-pass',
314+
'shared-pass',
315+
];
316+
}
317+
318+
private function assertAuthMatchesExpected(string|array|null $expectedAuth, array $parameters): void
319+
{
320+
if (null === $expectedAuth) {
321+
self::assertArrayNotHasKey('username', $parameters);
322+
self::assertArrayNotHasKey('password', $parameters);
323+
324+
return;
325+
}
326+
327+
if (\is_array($expectedAuth)) {
328+
self::assertSame($expectedAuth[0], $parameters['username'] ?? null);
329+
self::assertSame($expectedAuth[1], $parameters['password'] ?? null);
330+
331+
return;
332+
}
333+
334+
self::assertArrayNotHasKey('username', $parameters);
335+
self::assertSame($expectedAuth, $parameters['password'] ?? null);
336+
}
337+
338+
private function createPredisCaptureClass(): string
339+
{
340+
$predisClass = new class extends \Predis\Client {
341+
public static array $captured = [];
342+
private object $connection;
343+
344+
public function __construct($parameters = null, $options = null)
345+
{
346+
self::$captured = [
347+
'parameters' => $parameters,
348+
'options' => $options,
349+
];
350+
$this->connection = new class {
351+
public function setSentinelTimeout(float $timeout): void
352+
{
353+
}
354+
};
355+
}
356+
357+
/**
358+
* @return object
359+
*/
360+
public function getConnection()
361+
{
362+
return $this->connection;
363+
}
364+
};
365+
366+
return $predisClass::class;
367+
}
213368
}
Collapse file

‎src/Symfony/Component/Cache/Traits/RedisTrait.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Traits/RedisTrait.php
+32-8Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,18 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
194194

195195
if (!isset($params['redis_sentinel'])) {
196196
$params['auth'] ??= $auth;
197-
}
198-
199-
if (\is_array($params['auth']) && (!array_is_list($params['auth']) || 2 !== \count($params['auth']))) {
200-
throw new InvalidArgumentException('Invalid Redis DSN: the "auth" parameter must be a string, or a list of exactly two elements for ACL, "[username, password]".');
197+
$sentinelAuth = null;
201198
} elseif (!class_exists(\Predis\Client::class) && !class_exists(\RedisSentinel::class) && !class_exists(Sentinel::class)) {
202199
throw new CacheException('Redis Sentinel support requires one of: "predis/predis", "ext-redis >= 5.2", "ext-relay".');
200+
} else {
201+
$sentinelAuth = $params['auth'] ?? null;
202+
$params['auth'] = $auth ?? $params['auth'];
203+
}
204+
205+
foreach ([$params['auth'], $sentinelAuth] as $v) {
206+
if (\is_array($v) && (!array_is_list($v) || 2 !== \count($v))) {
207+
throw new InvalidArgumentException('Invalid Redis DSN: the "auth" parameter must be a string, or a list of exactly two elements for ACL, "[username, password]".');
208+
}
203209
}
204210

205211
if (isset($params['lazy'])) {
@@ -234,14 +240,14 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
234240
if ($isRedisExt || $isRelayExt) {
235241
$connect = $params['persistent'] || $params['persistent_id'] ? 'pconnect' : 'connect';
236242

237-
$initializer = static function () use ($class, $isRedisExt, $connect, $params, $hosts, $tls) {
243+
$initializer = static function () use ($class, $isRedisExt, $connect, $params, $sentinelAuth, $hosts, $tls) {
238244
$sentinelClass = $isRedisExt ? \RedisSentinel::class : Sentinel::class;
239245
$redis = new $class();
240246
$hostIndex = 0;
241247
do {
242248
$host = $hosts[$hostIndex]['host'] ?? $hosts[$hostIndex]['path'];
243249
$port = $hosts[$hostIndex]['port'] ?? 0;
244-
$passAuth = null !== $params['auth'] && (!$isRedisExt || \defined('Redis::OPT_NULL_MULTIBULK_AS_NULL'));
250+
$passAuth = null !== $sentinelAuth && (!$isRedisExt || \defined('Redis::OPT_NULL_MULTIBULK_AS_NULL'));
245251
$address = false;
246252

247253
if (isset($hosts[$hostIndex]['host']) && $tls) {
@@ -264,7 +270,7 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
264270
];
265271

266272
if ($passAuth) {
267-
$options['auth'] = $params['auth'];
273+
$options['auth'] = $sentinelAuth;
268274
}
269275

270276
if (null !== $params['ssl'] && version_compare(phpversion('redis'), '6.2.0', '>=')) {
@@ -273,7 +279,7 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
273279

274280
$sentinel = new \RedisSentinel($options);
275281
} else {
276-
$extra = $passAuth ? [$params['auth']] : [];
282+
$extra = $passAuth ? [$sentinelAuth] : [];
277283

278284
$sentinel = @new $sentinelClass($host, $port, $params['timeout'], (string) $params['persistent_id'], $params['retry_interval'], $params['read_timeout'], ...$extra);
279285
}
@@ -405,6 +411,24 @@ public static function createConnection(#[\SensitiveParameter] string $dsn, arra
405411
$params['parameters']['password'] = $params['auth'];
406412
}
407413

414+
if (isset($params['redis_sentinel']) && null !== $sentinelAuth) {
415+
if (\is_array($sentinelAuth)) {
416+
$sentinelUsername = $sentinelAuth[0];
417+
$sentinelPassword = $sentinelAuth[1];
418+
} else {
419+
$sentinelUsername = null;
420+
$sentinelPassword = $sentinelAuth;
421+
}
422+
423+
foreach ($hosts as $i => $host) {
424+
$hosts[$i]['password'] ??= $sentinelPassword;
425+
426+
if (null !== $sentinelUsername) {
427+
$hosts[$i]['username'] ??= $sentinelUsername;
428+
}
429+
}
430+
}
431+
408432
if (isset($params['ssl'])) {
409433
foreach ($hosts as $i => $host) {
410434
$hosts[$i]['ssl'] ??= $params['ssl'];

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.