From d5b9867bed1f93472a6d49777eab34619a01161c Mon Sep 17 00:00:00 2001 From: Matthieu Racine Date: Sun, 3 Mar 2019 14:40:08 +0100 Subject: [PATCH 1/4] Fix #3: Add test on responses count when login, try with legacy=true when multiple responses --- src/Client.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Client.php b/src/Client.php index 85afe6b..c9c67df 100644 --- a/src/Client.php +++ b/src/Client.php @@ -385,8 +385,18 @@ class Client implements Interfaces\ClientInterface // Execute query and get response $response = $this->write($query)->read(false); + // if : + // - we have more than one response + // - response is '!done' + // => problem with legacy version, swap it and retry + // Only tested with ROS pre 6.43, will test with post 6.43 => this could make legacy parameter obsolete ? + if (count($response)>1 && $response[0] === '!done' && !$this->config('legacy')) + { + $this->_config->set('legacy', true); + return $this->login(); + } // Return true if we have only one line from server and this line is !done - return isset($response[0]) && $response[0] === '!done'; + return (1 === count($response)) && isset($response[0]) && ($response[0] === '!done'); } /** From f31da26f58bc3a5c28c7da653c3d00864b648340 Mon Sep 17 00:00:00 2001 From: Matthieu Racine Date: Sun, 3 Mar 2019 15:21:22 +0100 Subject: [PATCH 2/4] Added phpunit tests --- tests/ClientTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/ClientTest.php b/tests/ClientTest.php index cc6a7c2..a899a5d 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -82,6 +82,25 @@ class ClientTest extends TestCase } } + /** + * Test non legacy connection on legacy router (pre 6.43) + * + * login() method recognise legacy router response and swap to legacy mode + */ + public function test__constructLegacy2() + { + try { + $config = new Config(); + $config->set('user', 'admin')->set('pass', 'admin') + ->set('host', '127.0.0.1')->set('port', 18728)->set('legacy', false); + $obj = new Client($config); + $this->assertInternalType('object', $obj); + } catch (\Exception $e) { + $this->assertContains('Must be initialized ', $e->getMessage()); + } + } + + public function test__constructWrongPass() { $this->expectException(ClientException::class); From df4e0a1dd697e7c336ff17879c6c76d02bbd5a3e Mon Sep 17 00:00:00 2001 From: Paul Rock Date: Sun, 3 Mar 2019 18:05:18 +0300 Subject: [PATCH 3/4] endless loop prevention added to login method of Client class --- src/Client.php | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/Client.php b/src/Client.php index c9c67df..21cc980 100644 --- a/src/Client.php +++ b/src/Client.php @@ -358,12 +358,13 @@ class Client implements Interfaces\ClientInterface /** * Authorization logic * + * @param bool $legacyRetry Retry login if we detect legacy version of RouterOS * @return bool * @throws \RouterOS\Exceptions\ClientException * @throws \RouterOS\Exceptions\ConfigException * @throws \RouterOS\Exceptions\QueryException */ - private function login(): bool + private function login(bool $legacyRetry = false): bool { // If legacy login scheme is enabled if ($this->config('legacy')) { @@ -380,23 +381,39 @@ class Client implements Interfaces\ClientInterface $query = (new Query('/login')) ->add('=name=' . $this->config('user')) ->add('=password=' . $this->config('pass')); + + // If we set modern auth scheme but router with legacy firmware then need to retry query, + // but need to prevent endless loop + $legacyRetry = true; } // Execute query and get response $response = $this->write($query)->read(false); - // if : + // if: // - we have more than one response // - response is '!done' // => problem with legacy version, swap it and retry - // Only tested with ROS pre 6.43, will test with post 6.43 => this could make legacy parameter obsolete ? - if (count($response)>1 && $response[0] === '!done' && !$this->config('legacy')) - { - $this->_config->set('legacy', true); - return $this->login(); + // Only tested with ROS pre 6.43, will test with post 6.43 => this could make legacy parameter obsolete? + if ($legacyRetry && $this->isLegacy($response)) { + $this->_config->set('legacy', true); + return $this->login(true); } + // Return true if we have only one line from server and this line is !done - return (1 === count($response)) && isset($response[0]) && ($response[0] === '!done'); + return 1 === \count($response) || (isset($response[0]) && $response[0] === '!done'); + } + + /** + * Detect by login request if firmware is legacy + * + * @param array $response + * @return bool + * @throws ConfigException + */ + private function isLegacy(array &$response): bool + { + return \count($response) > 1 && $response[0] === '!done' && !$this->config('legacy'); } /** From dac8fb6c26a912196bb607a7654d504ada3d3fe4 Mon Sep 17 00:00:00 2001 From: Paul Rock Date: Sun, 3 Mar 2019 18:11:40 +0300 Subject: [PATCH 4/4] small fix of login method --- src/Client.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Client.php b/src/Client.php index 21cc980..6de32c9 100644 --- a/src/Client.php +++ b/src/Client.php @@ -397,11 +397,11 @@ class Client implements Interfaces\ClientInterface // Only tested with ROS pre 6.43, will test with post 6.43 => this could make legacy parameter obsolete? if ($legacyRetry && $this->isLegacy($response)) { $this->_config->set('legacy', true); - return $this->login(true); + return $this->login(); } // Return true if we have only one line from server and this line is !done - return 1 === \count($response) || (isset($response[0]) && $response[0] === '!done'); + return (1 === count($response)) && isset($response[0]) && ($response[0] === '!done'); } /**