From 3a9d7ff9de1f58a5f23b16c6c4a662f29665b436 Mon Sep 17 00:00:00 2001 From: Mariano Iglesias Date: Mon, 18 May 2015 10:55:39 -0300 Subject: [PATCH] Adding support for connection authentication --- CHANGELOG.md | 11 +- README.md | 2 +- docs/README.md | 16 ++- src/Client.php | 7 +- src/Command/Auth.php | 24 +++++ src/Connection/AuthenticationException.php | 6 ++ src/Connection/Manager.php | 37 ++++++- src/Connection/ManagerInterface.php | 1 + src/Connection/Socket.php | 2 +- tests/ClientTest.php | 12 +-- tests/Command/AuthTest.php | 95 +++++++++++++++++ tests/Connection/ManagerTest.php | 118 ++++++++++++++++++++- tests/Connection/SocketTest.php | 2 +- 13 files changed, 307 insertions(+), 26 deletions(-) create mode 100644 src/Command/Auth.php create mode 100644 src/Connection/AuthenticationException.php create mode 100644 tests/Command/AuthTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index bcdfc88..d0600d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,17 @@ All Notable changes will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). -## [1.2.2] +## [1.3.0] - 2015-05-18 ### Added - Added support for `WORKING`. - Added `processing()` method to Queue API. +- Added `$password` option to `addServer()` in `Disque\Client`. + +### Changed +- By default when creating a new `Disque\Client` without arguments NO server +is pre-loaded. You will have to manually add servers via `addServer()`, or +specify them to the `Disque\Client` constructor. ## [1.2.1] - 2015-05-14 @@ -94,7 +100,8 @@ parameters were specified, an `InvalidCommandArgumentException` was thrown. - Added support for Predis connections, and allowing adding new connection methods via `ConnectionInterface`. -[1.2.2]: https://github.com/mariano/disque-php/compare/1.2.1...HEAD +[1.3.1]: https://github.com/mariano/disque-php/compare/1.3.1...HEAD +[1.3.0]: https://github.com/mariano/disque-php/compare/tag/1.3.0 [1.2.1]: https://github.com/mariano/disque-php/releases/tag/1.2.1 [1.2.0]: https://github.com/mariano/disque-php/releases/tag/1.2.0 [1.1.0]: https://github.com/mariano/disque-php/releases/tag/1.1.0 diff --git a/README.md b/README.md index 17a67f8..24031d2 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ instead of using the issue tracker. - [x] Higher level API for queueing and retrieving jobs - [x] Method in `Queue` to schedule future jobs based on DateTime - [x] `QSCAN`, `WORKING` -- [ ] Add support for AUTH based connections +- [x] Add support for AUTH based connections - [x] Add support to `WORKING` command on Queue API - [ ] `QSTAT` when they are implemented upstream diff --git a/docs/README.md b/docs/README.md index 8bf761f..2ee77df 100644 --- a/docs/README.md +++ b/docs/README.md @@ -20,11 +20,12 @@ $client = new \Disque\Client([ ]); ``` -If no host is specified, `127.0.0.1:7711` is assumed. Hosts can also be added -via the `addServer($host, $port)` method: +If no host is specified, no server is initialized. Hosts can also be added +via the `addServer($host, $port, $password)` method: ```php $client = new \Disque\Client(); +$client->addServer('127.0.0.1', 7711, 'my_password'); $client->addServer('127.0.0.1', 7712); ``` @@ -341,6 +342,17 @@ $result = $client->connect(); var_dump($result); ``` +You can also specify servers that require a password for connection. To do so, +use the `addServer()` method, like so: + +```php +$client = new \Disque\Client(); +$client->addServer('127.0.0.1', 7711, 'my_password'); +$client->addServer('127.0.0.1, 7712, 'my_password2'); +$result = $client->connect(); +var_dump($result); +``` + The above `connect()` call will return an output similar to the following: ```php diff --git a/src/Client.php b/src/Client.php index ad099ce..876dec8 100644 --- a/src/Client.php +++ b/src/Client.php @@ -59,7 +59,7 @@ class Client * * @param array $servers Servers (`host`:`port`) */ - public function __construct(array $servers = ['127.0.0.1:7711']) + public function __construct(array $servers = []) { foreach ([ 'ACKJOB' => Command\AckJob::class, @@ -115,12 +115,13 @@ public function getConnectionManager() * * @param string $host Host * @param int $port Port + * @param string $password Password to use when connecting to this server * @return void * @throws InvalidArgumentException */ - public function addServer($host, $port = 7711) + public function addServer($host, $port = 7711, $password = null) { - $this->connectionManager->addServer($host, $port); + $this->connectionManager->addServer($host, $port, $password); } /** diff --git a/src/Command/Auth.php b/src/Command/Auth.php new file mode 100644 index 0000000..0c017ad --- /dev/null +++ b/src/Command/Auth.php @@ -0,0 +1,24 @@ +servers[] = compact('host', 'port'); + $this->servers[] = compact('host', 'port', 'password'); } /** @@ -155,6 +157,7 @@ public function isConnected() * Connect to Disque * * @return array Connected node information + * @throws AuthenticationException * @throws ConnectionException */ public function connect() @@ -215,6 +218,7 @@ public function execute(CommandInterface $command) * * @return array Indexed array with `connection` and `hello`. `connection` * could end up being null + * @throws AuthenticationException * @throws ConnectionException */ protected function findAvailableConnection() @@ -246,9 +250,10 @@ protected function findAvailableConnection() /** * Get a node connection and its HELLO result * - * @param array $server Server (with `host`, and `port`) + * @param array $server Server (with `host`, `port`, and `password`) * @return array Indexed array with `connection` and `hello`. `connection` * could end up being null + * @throws AuthenticationException */ protected function getNodeConnection(array $server) { @@ -256,15 +261,39 @@ protected function getNodeConnection(array $server) $connection = $this->buildConnection($server['host'], $server['port']); $hello = []; try { - $connection->connect($this->options); + $this->doConnect($connection, $server, $this->options); $hello = $helloCommand->parse($connection->execute($helloCommand)); } catch (ConnectionException $e) { + $message = $e->getMessage(); + if (stripos($message, 'NOAUTH') === 0) { + throw new AuthenticationException($message); + } $connection = null; $hello = []; } return compact('connection', 'hello'); } + /** + * Actually perform the connection + * + * @param ConnectionInterface $connection Connection + * @param array $server Server (with `host`, `port`, and `password`) + * @param array $options Connection options + */ + private function doConnect(ConnectionInterface $connection, array $server, array $options) + { + $connection->connect($options); + if (!empty($server['password'])) { + $authCommand = new Auth(); + $authCommand->setArguments([$server['password']]); + $response = $authCommand->parse($connection->execute($authCommand)); + if ($response !== 'OK') { + throw new AuthenticationException(); + } + } + } + /** * Build a new connection * diff --git a/src/Connection/ManagerInterface.php b/src/Connection/ManagerInterface.php index 8650399..f192a4e 100644 --- a/src/Connection/ManagerInterface.php +++ b/src/Connection/ManagerInterface.php @@ -65,6 +65,7 @@ public function isConnected(); * Connect to Disque * * @return array Connected node information + * @throws AuthenticationException * @throws ConnectionException */ public function connect(); diff --git a/src/Connection/Socket.php b/src/Connection/Socket.php index f108cf7..3f40716 100644 --- a/src/Connection/Socket.php +++ b/src/Connection/Socket.php @@ -149,7 +149,7 @@ public function receive($keepWaiting = false) return (string) $data; case '-': $data = (string) $data; - throw new ResponseException("Error received from client: {$data}"); + throw new ResponseException($data); case ':': return (int) $data; case '$': diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 2e04360..cb2455d 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -49,9 +49,7 @@ public function testInstance() public function testConstruct() { $c = new MockClient(); - $this->assertSame([ - ['host' => '127.0.0.1', 'port' => 7711] - ], $c->getConnectionManager()->getServers()); + $this->assertSame([], $c->getConnectionManager()->getServers()); } public function testConstructNoServers() @@ -73,8 +71,8 @@ public function testConstructMultipleServers() '127.0.0.1:7712', ]); $this->assertSame([ - ['host' => '127.0.0.1', 'port' => 7711], - ['host' => '127.0.0.1', 'port' => 7712], + ['host' => '127.0.0.1', 'port' => 7711, 'password' => null], + ['host' => '127.0.0.1', 'port' => 7712, 'password' => null], ], $c->getConnectionManager()->getServers()); } @@ -85,8 +83,8 @@ public function testConstructMultipleServersDefaultPort() '127.0.0.1:7712', ]); $this->assertSame([ - ['host' => '127.0.0.1', 'port' => 7711], - ['host' => '127.0.0.1', 'port' => 7712], + ['host' => '127.0.0.1', 'port' => 7711, 'password' => null], + ['host' => '127.0.0.1', 'port' => 7712, 'password' => null], ], $c->getConnectionManager()->getServers()); } diff --git a/tests/Command/AuthTest.php b/tests/Command/AuthTest.php new file mode 100644 index 0000000..bffe81c --- /dev/null +++ b/tests/Command/AuthTest.php @@ -0,0 +1,95 @@ +assertInstanceOf(CommandInterface::class, $c); + } + + public function testGetCommand() + { + $c = new Auth(); + $result = $c->getCommand(); + $this->assertSame('AUTH', $result); + } + + public function testIsBlocking() + { + $c = new Auth(); + $result = $c->isBlocking(); + $this->assertFalse($result); + } + + public function testBuildInvalidArgumentsEmpty() + { + $this->setExpectedException(InvalidCommandArgumentException::class, 'Invalid command arguments. Arguments for command Disque\\Command\\Auth: []'); + $c = new Auth(); + $c->setArguments([]); + } + + public function testBuildInvalidArgumentsEmptyTooMany() + { + $this->setExpectedException(InvalidCommandArgumentException::class, 'Invalid command arguments. Arguments for command Disque\\Command\\Auth: ["test","stuff"]'); + $c = new Auth(); + $c->setArguments(['test', 'stuff']); + } + + public function testBuildInvalidArgumentsEmptyNonNumeric() + { + $this->setExpectedException(InvalidCommandArgumentException::class, 'Invalid command arguments. Arguments for command Disque\\Command\\Auth: {"test":"stuff"}'); + $c = new Auth(); + $c->setArguments(['test' => 'stuff']); + } + + public function testBuildInvalidArgumentsNumericNon0() + { + $this->setExpectedException(InvalidCommandArgumentException::class, 'Invalid command arguments. Arguments for command Disque\\Command\\Auth: {"1":"stuff"}'); + $c = new Auth(); + $c->setArguments([1 => 'stuff']); + } + + public function testBuildInvalidArgumentsNonString() + { + $this->setExpectedException(InvalidCommandArgumentException::class, 'Invalid command arguments. Arguments for command Disque\\Command\\Auth: [false]'); + $c = new Auth(); + $c->setArguments([false]); + } + + public function testBuild() + { + $c = new Auth(); + $c->setArguments(['test']); + $result = $c->getArguments(); + $this->assertSame(['test'], $result); + } + + public function testParseInvalidNonString() + { + $this->setExpectedException(InvalidResponseException::class, 'Invalid command response. Command Disque\\Command\\Auth got: 10'); + $c = new Auth(); + $c->parse(10); + } + + public function testParseInvalidNonStringBoolTrue() + { + $this->setExpectedException(InvalidResponseException::class, 'Invalid command response. Command Disque\\Command\\Auth got: true'); + $c = new Auth(); + $c->parse(true); + } + + public function testParse() + { + $c = new Auth(); + $result = $c->parse('OK'); + $this->assertSame('OK', $result); + } +} \ No newline at end of file diff --git a/tests/Connection/ManagerTest.php b/tests/Connection/ManagerTest.php index bab5828..fb3f6cd 100644 --- a/tests/Connection/ManagerTest.php +++ b/tests/Connection/ManagerTest.php @@ -6,13 +6,16 @@ use InvalidArgumentException; use Mockery as m; use PHPUnit_Framework_TestCase; +use Disque\Command\Auth; use Disque\Command\CommandInterface; use Disque\Command\GetJob; use Disque\Command\Hello; +use Disque\Connection\AuthenticationException; use Disque\Connection\BaseConnection; use Disque\Connection\ConnectionException; use Disque\Connection\ConnectionInterface; use Disque\Connection\Manager; +use Disque\Connection\ResponseException; use Disque\Connection\Socket; class MockManager extends Manager @@ -123,7 +126,7 @@ public function testAddServer() $m = new Manager(); $m->addServer('127.0.0.1', 7712); $this->assertSame([ - ['host' => '127.0.0.1', 'port' => 7712], + ['host' => '127.0.0.1', 'port' => 7712, 'password' => null], ], $m->getServers()); } @@ -132,18 +135,29 @@ public function testAddServerDefaultPort() $m = new Manager(); $m->addServer('other.host'); $this->assertEquals([ - ['host' => 'other.host', 'port' => 7711], + ['host' => 'other.host', 'port' => 7711, 'password' => null], + ], $m->getServers()); + } + + public function testAddServerPassword() + { + $m = new Manager(); + $m->addServer('127.0.0.1', 7712, 'my_password'); + $this->assertSame([ + ['host' => '127.0.0.1', 'port' => 7712, 'password' => 'my_password'], ], $m->getServers()); } public function testAddServers() { $m = new Manager(); - $m->addServer('127.0.0.1', 7711); + $m->addServer('127.0.0.1', 7711, 'my_password1'); $m->addServer('127.0.0.1', 7712); + $m->addServer('127.0.0.1', 7713, 'my_password3'); $this->assertSame([ - ['host' => '127.0.0.1', 'port' => 7711], - ['host' => '127.0.0.1', 'port' => 7712], + ['host' => '127.0.0.1', 'port' => 7711, 'password' => 'my_password1'], + ['host' => '127.0.0.1', 'port' => 7712, 'password' => null], + ['host' => '127.0.0.1', 'port' => 7713, 'password' => 'my_password3'], ], $m->getServers()); } @@ -296,6 +310,100 @@ public function testConnectWithOptions() $m->connect([]); } + public function testConnectWithPasswordMissingPassword() + { + $m = new MockManager(); + $m->addServer('127.0.0.1', 7711); + $m->setAvailableConnection(false); // Passthru + + $connection = m::mock(ConnectionInterface::class) + ->shouldReceive('connect') + ->with([]) + ->once() + ->shouldReceive('execute') + ->with(m::type(Hello::class)) + ->andThrow(new ResponseException('NOAUTH Authentication Required')) + ->once() + ->mock(); + + $m->setBuildConnection($connection); + + $this->setExpectedException(AuthenticationException::class, 'NOAUTH Authentication Required'); + $m->connect([]); + } + + public function testConnectWithPasswordWrongPassword() + { + $m = new MockManager(); + $m->addServer('127.0.0.1', 7711, 'wrong_password'); + $m->setAvailableConnection(false); // Passthru + + $connection = m::mock(ConnectionInterface::class) + ->shouldReceive('connect') + ->with([]) + ->once() + ->shouldReceive('execute') + ->with(m::type(Auth::class)) + ->andThrow(new ResponseException('ERR invalid password')) + ->once() + ->mock(); + + $m->setBuildConnection($connection); + + $this->setExpectedException(ConnectionException::class, 'No servers available'); + $m->connect([]); + } + + public function testConnectWithPasswordWrongResponse() + { + $m = new MockManager(); + $m->addServer('127.0.0.1', 7711, 'right_password'); + $m->setAvailableConnection(false); // Passthru + + $connection = m::mock(ConnectionInterface::class) + ->shouldReceive('connect') + ->with([]) + ->once() + ->shouldReceive('execute') + ->with(m::type(Auth::class)) + ->andReturn('WHATEVER') + ->once() + ->mock(); + + $m->setBuildConnection($connection); + + $this->setExpectedException(ConnectionException::class, 'No servers available'); + $m->connect([]); + } + + public function testConnectWithPasswordRightPassword() + { + $m = new MockManager(); + $m->addServer('127.0.0.1', 7711, 'right_password'); + $m->setAvailableConnection(false); // Passthru + + $connection = m::mock(ConnectionInterface::class) + ->shouldReceive('connect') + ->with([]) + ->once() + ->shouldReceive('execute') + ->with(m::type(Auth::class)) + ->andReturn('OK') + ->once() + ->shouldReceive('execute') + ->with(m::type(Hello::class)) + ->andReturn([ + 'v1', + 'id1', + ['id1', '127.0.0.1', 7711, 'v1'], + ['id2', '127.0.0.1', 7712, 'v1'] + ]) + ->mock(); + + $m->setBuildConnection($connection); + $m->connect([]); + } + public function testFindAvailableConnectionNoneSpecifiedConnectThrowsException() { $m = new MockManager(); diff --git a/tests/Connection/SocketTest.php b/tests/Connection/SocketTest.php index e1f26e2..7daf942 100644 --- a/tests/Connection/SocketTest.php +++ b/tests/Connection/SocketTest.php @@ -163,7 +163,7 @@ public function testSend() public function testReceiveErrorFromClient() { - $this->setExpectedException(ResponseException::class, 'Error received from client: Error from Disque'); + $this->setExpectedException(ResponseException::class, 'Error from Disque'); $socket = fopen('php://memory','rw'); fwrite($socket, "-Error from Disque\r\n");