In two UsersManager API methods, do not apply offset if limit is not also applied. (#13764)

2 files changed, 35 insertions, 8 deletions

diff --git a/plugins/UsersManager/Model.php b/plugins/UsersManager/Model.php
index a4874c70b5..afba31a4da 100644
--- a/plugins/UsersManager/Model.php
+++ b/plugins/UsersManager/Model.php
@@ -178,13 +178,13 @@ class Model
         $bind = array_merge($bind, $whereBind);
 
         $limitSql = '';
+        $offsetSql = '';
         if ($limit) {
             $limitSql = "LIMIT " . (int)$limit;
-        }
 
-        $offsetSql = '';
-        if ($offset) {
-            $offsetSql = "OFFSET " . (int)$offset;
+            if ($offset) {
+                $offsetSql = "OFFSET " . (int)$offset;
+            }
         }
 
         $sql = 'SELECT SQL_CALC_FOUND_ROWS s.idsite as idsite, s.name as site_name, GROUP_CONCAT(a.access SEPARATOR "|") as access
@@ -452,13 +452,13 @@ class Model
         $bind = array_merge($bind, $whereBind);
 
         $limitSql = '';
+        $offsetSql = '';
         if ($limit) {
             $limitSql = "LIMIT " . (int)$limit;
-        }
 
-        $offsetSql = '';
-        if ($offset) {
-            $offsetSql = "OFFSET " . (int)$offset;
+            if ($offset) {
+                $offsetSql = "OFFSET " . (int)$offset;
+            }
         }
 
         $sql = 'SELECT SQL_CALC_FOUND_ROWS u.*, GROUP_CONCAT(a.access SEPARATOR "|") as access
diff --git a/plugins/UsersManager/tests/Integration/APITest.php b/plugins/UsersManager/tests/Integration/APITest.php
index 83f4b2be36..59ba9bad33 100644
--- a/plugins/UsersManager/tests/Integration/APITest.php
+++ b/plugins/UsersManager/tests/Integration/APITest.php
@@ -369,6 +369,19 @@ class APITest extends IntegrationTestCase
         $this->assertEquals($expected, $users);
     }
 
+    public function test_getUsersPlusRole_shouldIgnoreOffsetIfLimitIsNotSupplied()
+    {
+        $this->addUserWithAccess('userLogin2', 'view', 1);
+        $this->setCurrentUser('userLogin2', 'view', 1);
+
+        $users = $this->api->getUsersPlusRole(1, $limit = null, $offset = 1);
+        $this->cleanUsers($users);
+        $expected = [
+            ['login' => 'userLogin2', 'alias' => 'userLogin2', 'role' => 'view', 'capabilities' => []],
+        ];
+        $this->assertEquals($expected, $users);
+    }
+
     public function test_getUsersPlusRole_shouldNotAllowSuperuserFilter_ifUserIsNotSuperUser()
     {
         $this->addUserWithAccess('userLogin2', 'view', 1);
@@ -551,6 +564,20 @@ class APITest extends IntegrationTestCase
         $this->assertEquals($expected, $access);
     }
 
+    public function getSitesAccessForUser_shouldIgnoreOffsetIfLimitNotSupplied()
+    {
+        $this->api->setUserAccess('userLogin', 'admin', [1]);
+        $this->api->setUserAccess('userLogin', 'view', [2]);
+        $this->api->setUserAccess('userLogin', 'view', [3]);
+
+        $access = $this->api->getSitesAccessForUser('userLogin', $limit = null, $offset = 1);
+        $expected = [
+            ['idsite' => '2', 'site_name' => 'Piwik test', 'role' => 'view', 'capabilities' => []],
+            ['idsite' => '3', 'site_name' => 'Piwik test', 'role' => 'view', 'capabilities' => []],
+        ];
+        $this->assertEquals($expected, $access);
+    }
+
     public function test_getSitesAccessForUser_shouldApplyLimitAndOffsetCorrectly()
     {
         $this->api->setUserAccess('userLogin', 'admin', [1]);