Merge pull request #4 from matracine/login-legacy

Fix #3: Add test on responses count when login, try with legacy=true …
EvilFreelancer · Mar 3, 2019 · 952e4ef · 952e4ef
2 parents 40e82b7 + dac8fb6
commit 952e4ef
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 2 deletions.
diff --git a/src/Client.php b/src/Client.php
@@ -358,12 +358,13 @@ private function pregResponse(string $value, &$matches)
     /**
      * 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,13 +381,39 @@ private function login(): bool
             $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:
+        //  - 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 ($legacyRetry && $this->isLegacy($response)) {
+            $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');
+    }
+
+    /**
+     * 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');
     }
 
     /**

diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -82,6 +82,25 @@ public function test__constructLegacy()
         }
     }
 
+    /**
+     * 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);