fix(auth): Adds default user if only password used for authentication (#1535)

This commit is contained in:
Vladyslav Vildanov
2025-05-13 19:01:21 +03:00
committed by GitHub
parent 9b1c6d2c8f
commit 06c904af37
17 changed files with 187 additions and 36 deletions
+26 -3
View File
@@ -12,6 +12,8 @@ services:
retries: 3
ports:
- "6379:6379"
command:
- --requirepass "foobar"
profiles:
- all
- official
@@ -23,9 +25,10 @@ services:
environment:
- TLS_ENABLED=yes
- REDIS_CLUSTER=no
- REDIS_PASSWORD=foobar
- PORT=6379
- TLS_PORT=6666
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --tls-auth-clients optional --save ""}
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --tls-auth-clients optional --save "" --requirepass "foobar"}
ports:
- 6379:6379
- 6666:6666 # TLS port
@@ -36,17 +39,36 @@ services:
- clients
- standalone
redis-unprotected:
image: ${REDIS_IMAGE_NAME:-redislabs/client-libs-test:7.4.2}
container_name: redis-unprotected
environment:
- TLS_ENABLED=no
- REDIS_CLUSTER=no
- PORT=6379
- TLS_PORT=6666
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --tls-auth-clients optional --save ""}
ports:
- 6380:6379
volumes:
- "./dockers/unprotected:/redis/work"
profiles:
- all
- clients
- unprotected
redis-cluster:
image: ${REDIS_IMAGE_NAME:-redislabs/client-libs-test:7.4.2}
container_name: redis-cluster
environment:
- REDIS_CLUSTER=yes
- REDIS_PASSWORD=foobar
- NODES=6
- REPLICAS=1
- TLS_ENABLED=yes
- PORT=6372
- TLS_PORT=27379
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --tls-auth-clients optional --save ""}
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --tls-auth-clients optional --save "" --requirepass "foobar"}
ports:
- "6372-6377:6372-6377"
volumes:
@@ -63,8 +85,9 @@ services:
- "6479:6379"
environment:
- REDIS_CLUSTER=no
- REDIS_PASSWORD=foobar
- PORT=6379
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --save ""}
command: ${REDIS_EXTRA_ARGS:---enable-debug-command yes --enable-module-command yes --save "" --requirepass "foobar"}
volumes:
- "./dockers/redis-stack:/redis/work"
profiles:
+17
View File
@@ -78,6 +78,13 @@ jobs:
compose-file: .github/docker-compose.yml
services: ${{ env.DOCKER_SERVICE }}
- name: Start Redis unprotected image
uses: hoverkraft-tech/compose-action@v2.0.1
if: ${{ matrix.redis > '4.0' }}
with:
compose-file: .github/docker-compose.yml
services: redis-unprotected
- name: Start Redis stack image
id: stack_infra
uses: hoverkraft-tech/compose-action@v2.0.1
@@ -123,6 +130,14 @@ jobs:
if: ${{ matrix.php == '8.4' && matrix.redis == '7.4' }}
run: vendor/bin/phpunit -c phpunit.relay.xml --coverage-php build/cov/coverage-relay.cov --coverage-filter ./src
- name: Run tests against unprotected Redis
if: ${{ (matrix.php != '8.4' || matrix.redis != '7.4') && matrix.redis > '4.0'}}
run: vendor/bin/phpunit --group unprotected
- name: Run tests against unprotected Redis with coverage
if: ${{ matrix.php == '8.4' && matrix.redis == '7.4' }}
run: vendor/bin/phpunit --group unprotected --coverage-php build/cov/coverage-unprotected.cov --coverage-filter ./src
- name: Run stack tests
if: ${{ (matrix.php != '8.4' || matrix.redis != '7.4') && matrix.redis >= '7.2' }}
run: vendor/bin/phpunit --group realm-stack
@@ -156,11 +171,13 @@ jobs:
- name: Run tests against cluster using Relay
if: ${{ matrix.php != '8.4' && matrix.redis == '7.4' }}
run: |
sleep 5
vendor/bin/phpunit -c phpunit.relay.xml --group cluster
- name: Run tests against cluster using Relay with coverage
if: ${{ matrix.php == '8.4' && matrix.redis == '7.4' }}
run: |
sleep 5
vendor/bin/phpunit -c phpunit.relay.xml --group cluster --coverage-php build/cov/coverage-cluster-relay.cov --coverage-filter ./src
- name: Merge coverage reports
+4
View File
@@ -1,4 +1,8 @@
## Changelog
## Unreleased
### Fixed
- fix(auth): Adds default user if only password used for authentication (#1535)
## Unreleased
### Maintenance
+7 -1
View File
@@ -29,6 +29,7 @@
<group>ext-curl</group>
<group>cluster</group>
<group>gears</group>
<group>unprotected</group>
</exclude>
</groups>
@@ -41,12 +42,17 @@
<php>
<const name="REDIS_SERVER_HOST" value="127.0.0.1" />
<const name="REDIS_SERVER_PORT" value="6379" />
<const name="REDIS_UNPROTECTED_SERVER_PORT" value="6380" />
<const name="REDIS_SERVER_DBNUM" value="0" />
<const name="REDIS_PASSWORD" value="foobar" />
<env name="USE_RELAY" value="true" />
<env name="REDIS_STACK_SERVER_PORT" value="6479" />
<!-- Redis Cluster -->
<!-- Only master nodes endpoints included -->
<const name="REDIS_CLUSTER_ENDPOINTS" value="127.0.0.1:6372,127.0.0.1:6373,127.0.0.1:6374" />
<const
name="REDIS_CLUSTER_ENDPOINTS"
value="127.0.0.1:6372?password=foobar,127.0.0.1:6373?password=foobar,127.0.0.1:6374?password=foobar"
/>
</php>
</phpunit>
+7 -1
View File
@@ -29,6 +29,7 @@
<group>cluster</group>
<group>gears</group>
<group>gears-cluster</group>
<group>unprotected</group>
<!-- <group>connected</group> -->
<!-- <group>disconnected</group> -->
<!-- <group>commands</group> -->
@@ -45,12 +46,17 @@
<php>
<const name="REDIS_SERVER_HOST" value="127.0.0.1" />
<const name="REDIS_SERVER_PORT" value="6379" />
<const name="REDIS_UNPROTECTED_SERVER_PORT" value="6380" />
<const name="REDIS_SERVER_DBNUM" value="0" />
<const name="REDIS_PASSWORD" value="foobar" />
<env name="USE_RELAY" value="false" />
<env name="REDIS_STACK_SERVER_PORT" value="6479" />
<!-- Redis Cluster -->
<!-- Only master nodes endpoints included -->
<const name="REDIS_CLUSTER_ENDPOINTS" value="127.0.0.1:6372,127.0.0.1:6373,127.0.0.1:6374" />
<const
name="REDIS_CLUSTER_ENDPOINTS"
value="127.0.0.1:6372?password=foobar,127.0.0.1:6373?password=foobar,127.0.0.1:6374?password=foobar"
/>
</php>
</phpunit>
+2
View File
@@ -17,8 +17,10 @@ use Predis\Response\Status;
/**
* @method array cat(string $category = null)
* @method Status dryRun(string $username, string $command, ...$arguments)
* @method int delUser(string ...$username)
* @method array getUser(string $username)
* @method Status setUser(string $username, string ...$rules)
* @method string whoami()
*/
class ACL extends AbstractContainer
{
+5 -4
View File
@@ -164,13 +164,14 @@ class Factory implements FactoryInterface
{
$parameters = $connection->getParameters();
if (isset($parameters->password) && strlen($parameters->password)) {
if (!empty($parameters->password)) {
$cmdAuthArgs = [$parameters->protocol, 'AUTH'];
isset($parameters->username) && strlen($parameters->username)
? array_push($cmdAuthArgs, $parameters->username, $parameters->password)
: $cmdAuthArgs[] = $parameters->password;
if (empty($parameters->username)) {
$parameters->username = 'default';
}
array_push($cmdAuthArgs, $parameters->username, $parameters->password);
array_push($cmdAuthArgs, 'SETNAME', 'predis');
$connection->addConnectCommand(
+6 -2
View File
@@ -316,9 +316,13 @@ class StreamConnection extends AbstractConnection
if (in_array('AUTH', $failedCommand->getArguments(), true)) {
$parameters = $this->getParameters();
$auth = new RawCommand('AUTH', [$parameters->username, $parameters->password]);
// If Redis <= 6.0
$auth = new RawCommand('AUTH', [$parameters->password]);
$response = $this->executeCommand($auth);
$this->handleOnConnectResponse($response, $auth);
if ($response instanceof ErrorResponseInterface) {
$this->onConnectionError("Failed: {$response->getMessage()}");
}
}
$setName = new RawCommand('CLIENT', ['SETNAME', 'predis']);
+24 -4
View File
@@ -14,6 +14,7 @@ namespace Predis\Connection;
use PHPUnit\Framework\MockObject\MockObject;
use Predis\Command\CommandInterface;
use Predis\Command\RawCommand;
use PredisTestCase;
/**
@@ -492,6 +493,7 @@ abstract class PredisConnectionTestCase extends PredisTestCase
'host' => constant('REDIS_SERVER_HOST'),
'port' => constant('REDIS_SERVER_PORT'),
'database' => constant('REDIS_SERVER_DBNUM'),
'password' => getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD'),
'read_write_timeout' => 2,
];
}
@@ -529,9 +531,9 @@ abstract class PredisConnectionTestCase extends PredisTestCase
*
* @return NodeConnectionInterface
*/
protected function createConnection(bool $initialize = false): NodeConnectionInterface
protected function createConnection(bool $initialize = false, bool $noAuth = false): NodeConnectionInterface
{
return $this->createConnectionWithParams([], $initialize);
return $this->createConnectionWithParams([], $initialize, $noAuth);
}
/**
@@ -542,8 +544,11 @@ abstract class PredisConnectionTestCase extends PredisTestCase
*
* @return NodeConnectionInterface
*/
protected function createConnectionWithParams($parameters, $initialize = false): NodeConnectionInterface
{
protected function createConnectionWithParams(
$parameters,
$initialize = false,
bool $noAuth = false
): NodeConnectionInterface {
$class = $this->getConnectionClass();
$commands = $this->getCommandFactory();
@@ -553,6 +558,21 @@ abstract class PredisConnectionTestCase extends PredisTestCase
$connection = new $class($parameters);
if (!$noAuth) {
if (isset($parameters->password) && strlen($parameters->password)) {
if (!isset($parameters->username) || !strlen($parameters->username)) {
$parameters->username = 'default';
}
$connection->addConnectCommand(
new RawCommand(
'HELLO',
[$parameters->protocol, 'AUTH', $parameters->username, $parameters->password]
)
);
}
}
if ($initialize) {
$connection->addConnectCommand(
$commands->create('select', [$parameters->database])
+29 -1
View File
@@ -172,11 +172,23 @@ abstract class PredisTestCase extends PHPUnit\Framework\TestCase
return $this->prepareClusterEndpoints();
}
$password = getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD');
if ($this->isStackTest()) {
$port = getenv('REDIS_STACK_SERVER_PORT');
} elseif ($this->isUnprotectedTest()) {
$port = constant('REDIS_UNPROTECTED_SERVER_PORT');
$password = '';
} else {
$port = constant('REDIS_SERVER_PORT');
}
return [
'scheme' => 'tcp',
'host' => constant('REDIS_SERVER_HOST'),
'port' => ($this->isStackTest()) ? getenv('REDIS_STACK_SERVER_PORT') : constant('REDIS_SERVER_PORT'),
'port' => $port,
'database' => constant('REDIS_SERVER_DBNUM'),
'password' => $password,
];
}
@@ -604,6 +616,22 @@ abstract class PredisTestCase extends PHPUnit\Framework\TestCase
&& in_array('realm-stack', $annotations['class']['group'], true);
}
/**
* Check annotations if it's matches to unprotected test scenario.
*
* @return bool
*/
protected function isUnprotectedTest(): bool
{
$annotations = TestUtil::parseTestMethodAnnotations(
get_class($this),
$this->getName(false)
);
return isset($annotations['method']['group'])
&& in_array('unprotected', $annotations['method']['group'], true);
}
/**
* Parse comma-separated cluster endpoints and convert them into tcp strings.
*
+40
View File
@@ -1380,6 +1380,46 @@ class ClientTest extends PredisTestCase
$this->assertSame('value', $client->get('key'));
}
/**
* @group connected
* @group unprotected
* @requiresRedisVersion >= 2.0.0
* @return void
*/
public function testClientAuthenticationAgainstUnprotectedServer(): void
{
$client = new Client($this->getParameters());
$this->assertEquals('OK', $client->set('key', 'value'));
$this->assertSame('value', $client->get('key'));
// AUTH doesn't throw exception if no authentication requires.
$clientWithPassword = new Client($this->getParameters(
['password' => getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD')])
);
$this->assertEquals('OK', $clientWithPassword->set('key', 'value'));
$this->assertSame('value', $clientWithPassword->get('key'));
$this->assertEquals(
'OK',
$client->acl->setUser(
'test_user',
'on',
'>foobar',
'allcommands',
'allkeys'
)
);
$clientTestUser = new Client($this->getParameters(
['username' => 'test_user', 'password' => 'foobar'])
);
$this->assertEquals('test_user', $clientTestUser->acl->whoami());
$this->assertEquals('OK', $clientTestUser->set('key', 'value'));
$this->assertSame('value', $clientTestUser->get('key'));
$this->assertEquals(1, $clientTestUser->acl->delUser('test_user'));
}
// ******************************************************************** //
// ---- HELPER METHODS ------------------------------------------------ //
// ******************************************************************** //
@@ -69,6 +69,7 @@ class CompositeStreamConnectionTest extends PredisConnectionTestCase
/** @var CompositeConnectionInterface */
$connection = $this->createConnection(true);
$commands = $this->getCommandFactory();
$connection->connect();
$connection->getProtocol()->useIterableMultibulk(true);
+2 -2
View File
@@ -307,7 +307,7 @@ class FactoryTest extends PredisTestCase
->expects($this->exactly(4))
->method('addConnectCommand')
->withConsecutive(
[$this->isRedisCommand('HELLO', [2, 'AUTH', 'foobar', 'SETNAME', 'predis'])],
[$this->isRedisCommand('HELLO', [2, 'AUTH', 'default', 'foobar', 'SETNAME', 'predis'])],
[$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])],
[$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])],
[$this->isRedisCommand('SELECT', ['0'])]
@@ -338,7 +338,7 @@ class FactoryTest extends PredisTestCase
$connection->expects($this->exactly(3))
->method('addConnectCommand')
->withConsecutive(
[$this->isRedisCommand('HELLO', [2, 'AUTH', 'foobar', 'SETNAME', 'predis'])],
[$this->isRedisCommand('HELLO', [2, 'AUTH', 'default', 'foobar', 'SETNAME', 'predis'])],
[$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])],
[$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])]
);
@@ -25,7 +25,7 @@ use RuntimeException;
/**
* @method StreamConnection createConnection(bool $initialize = false)
* @method StreamConnection createConnectionWithParams($parameters, $initialize = false)
* @method StreamConnection createConnectionWithParams($parameters, $initialize = false, bool $noAuth = false)
*/
class StreamConnectionTest extends PredisConnectionTestCase
{
@@ -844,22 +844,6 @@ class StreamConnectionTest extends PredisConnectionTestCase
$this->assertNotNull($connection->getClientId());
}
/**
* @group connected
* @return void
* @requiresRedisVersion >= 6.2.0
*/
public function testDoNotSetClientIdOnResp2ConnectionIfNotHelloCommand(): void
{
$connection = $this->createConnectionWithParams([]);
$connection->addConnectCommand(
new RawCommand('INFO')
);
$connection->connect();
$this->assertNull($connection->getClientId());
}
/**
* @group connected
* @return void
@@ -920,7 +904,7 @@ class StreamConnectionTest extends PredisConnectionTestCase
{
$failedCommand = new RawCommand('HELLO', ['FOOBAR', 'AUTH', 'foobar']);
$connection = $this->createConnection();
$connection = $this->createConnectionWithParams(['password' => ''], false, true);
$connection->addConnectCommand($failedCommand);
$this->expectException(ConnectionException::class);
+1
View File
@@ -211,6 +211,7 @@ class ConsumerTest extends PredisTestCase
'host' => constant('REDIS_SERVER_HOST'),
'port' => constant('REDIS_SERVER_PORT'),
'database' => constant('REDIS_SERVER_DBNUM'),
'password' => getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD'),
// Prevents suite from handing on broken test
'read_write_timeout' => 2,
];
@@ -14,6 +14,7 @@ namespace Predis\Response\Iterator;
use Predis\Client;
use Predis\ClientInterface;
use Predis\Command\RawCommand;
use Predis\Connection\CompositeStreamConnection;
use Predis\Protocol\Text\ProtocolProcessor as TextProtocolProcessor;
use PredisTestCase;
@@ -119,6 +120,12 @@ class MultiBulkTest extends PredisTestCase
$protocol->useIterableMultibulk(true);
$connection = new CompositeStreamConnection($parameters, $protocol);
$connection->addConnectCommand(
new RawCommand(
'AUTH',
[getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD')]
)
);
$client = new Client($connection);
$client->connect();
@@ -14,6 +14,7 @@ namespace Predis\Response\Iterator;
use Predis\Client;
use Predis\ClientInterface;
use Predis\Command\RawCommand;
use Predis\Connection\CompositeStreamConnection;
use Predis\Connection\NodeConnectionInterface;
use Predis\Protocol\Text\ProtocolProcessor as TextProtocolProcessor;
@@ -125,6 +126,12 @@ class MultiBulkTupleTest extends PredisTestCase
$protocol->useIterableMultibulk(true);
$connection = new CompositeStreamConnection($parameters, $protocol);
$connection->addConnectCommand(
new RawCommand(
'AUTH',
[getenv('REDIS_PASSWORD') ?: constant('REDIS_PASSWORD')]
)
);
$client = new Client($connection);
$client->connect();