Adding support for connection authentication

mariano · May 18, 2015 · 3a9d7ff · 3a9d7ff
1 parent 8f8cb5a
commit 3a9d7ff
Show file tree

Hide file tree

Showing 13 changed files with 307 additions and 26 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,24 @@
+<?php
+namespace Disque\Command;
+
+use Disque\Command\Response\AuthResponse;
+
+class Auth extends BaseCommand implements CommandInterface
+{
+    /**
+     * Tells the argument types for this command
+     *
+     * @var int
+     */
+    protected $argumentsType = self::ARGUMENTS_TYPE_STRING;
+
+    /**
+     * Get command
+     *
+     * @return string Command
+     */
+    public function getCommand()
+    {
+        return 'AUTH';
+    }
+}
diff --git a/src/Connection/AuthenticationException.php b/src/Connection/AuthenticationException.php
@@ -0,0 +1,6 @@
+<?php
+namespace Disque\Connection;
+
+class AuthenticationException extends ConnectionException
+{
+}
diff --git a/src/Connection/Manager.php b/src/Connection/Manager.php
@@ -2,6 +2,7 @@
 namespace Disque\Connection;
 
 use InvalidArgumentException;
+use Disque\Command\Auth;
 use Disque\Command\CommandInterface;
 use Disque\Command\GetJob;
 use Disque\Command\Hello;
@@ -102,17 +103,18 @@ public function getServers()
      *
      * @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)
     {
         if (!is_string($host) || !is_int($port)) {
             throw new InvalidArgumentException('Invalid server specified');
         }
 
         $port = (int) $port;
-        $this->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,25 +250,50 @@ 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)
     {
         $helloCommand = new Hello();
         $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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,95 @@
+<?php
+namespace Disque\Test\Command;
+
+use PHPUnit_Framework_TestCase;
+use Disque\Command\Argument\InvalidCommandArgumentException;
+use Disque\Command\CommandInterface;
+use Disque\Command\Auth;
+use Disque\Command\Response\InvalidResponseException;
+
+class AuthTest extends PHPUnit_Framework_TestCase
+{
+    public function testInstance()
+    {
+        $c = new Auth();
+        $this->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);
+    }
+}