Add possibility to restrict piwik access by ip (#12242)

* add possibility to restrict piwik login by ip

* better whitelist implementation

* move classes to corehome

* better error message

* better config

* make sure ips can be overwritten via DI

* fix ui tests

6 files changed, 322 insertions, 92 deletions

diff --git a/plugins/CoreHome/CoreHome.php b/plugins/CoreHome/CoreHome.php
index 33949a5032..33a8b3e847 100644
--- a/plugins/CoreHome/CoreHome.php
+++ b/plugins/CoreHome/CoreHome.php
@@ -9,6 +9,8 @@
 namespace Piwik\Plugins\CoreHome;
 use Piwik\Columns\ComputedMetricFactory;
 use Piwik\Columns\MetricsList;
+use Piwik\IP;
+use Piwik\Piwik;
 use Piwik\Plugin\ArchivedMetric;
 use Piwik\Plugin\ComputedMetric;
 
@@ -34,10 +36,27 @@ class CoreHome extends \Piwik\Plugin
             'AssetManager.getJavaScriptFiles'        => 'getJsFiles',
             'AssetManager.filterMergedJavaScripts'   => 'filterMergedJavaScripts',
             'Translate.getClientSideTranslationKeys' => 'getClientSideTranslationKeys',
-            'Metric.addComputedMetrics'              => 'addComputedMetrics'
+            'Metric.addComputedMetrics'              => 'addComputedMetrics',
+            'Request.initAuthenticationObject' => 'initAuthenticationObject',
         );
     }
 
+    public function initAuthenticationObject()
+    {
+        $isApi = Piwik::getModule() === 'API' && (Piwik::getAction() == '' || Piwik::getAction() == 'index');
+
+        if ($isApi) {
+            // will be checked in API itself to make sure we return an API response in the proper format.
+            return;
+        }
+
+        $whitelist = new LoginWhitelist();
+        if ($whitelist->shouldCheckWhitelist()) {
+            $ip = IP::getIpFromHeader();
+            $whitelist->checkIsWhitelisted($ip);
+        }
+    }
+
     public function addComputedMetrics(MetricsList $list, ComputedMetricFactory $computedMetricFactory)
     {
         $metrics = $list->getMetrics();
diff --git a/plugins/CoreHome/LoginWhitelist.php b/plugins/CoreHome/LoginWhitelist.php
new file mode 100644
index 0000000000..4e9a8c28ff
--- /dev/null
+++ b/plugins/CoreHome/LoginWhitelist.php
@@ -0,0 +1,85 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Plugins\CoreHome;
+
+use Piwik\Common;
+use Piwik\Config;
+use Piwik\Container\StaticContainer;
+use Piwik\Network\IP as NetworkIp;
+use Piwik\NoAccessException;
+use Piwik\Piwik;
+
+/**
+ * This class is in CoreHome since some alternative Login plugins disable the Login plugin and we want to ensure the
+ * feature works for all login plugins.
+ */
+class LoginWhitelist
+{
+    public function shouldWhitelistApplyToAPI()
+    {
+        $general = $this->getGeneralConfig();
+        return !empty($general['login_whitelist_apply_to_reporting_api_requests']);
+    }
+
+    public function shouldCheckWhitelist()
+    {
+        if (Common::isPhpCliMode()) {
+            return false;
+        }
+
+        $ips = $this->getWhitelistedLoginIps();
+        return !empty($ips);
+    }
+
+    public function checkIsWhitelisted($ipString)
+    {
+        if (!$this->isIpWhitelisted($ipString)) {
+            throw new NoAccessException(Piwik::translate('CoreHome_ExceptionNotWhitelistedIP', $ipString));
+        }
+    }
+
+    public function isIpWhitelisted($userIpString)
+    {
+        $userIp = NetworkIp::fromStringIP($userIpString);
+        $ipsWhitelisted = $this->getWhitelistedLoginIps();
+
+        if (empty($ipsWhitelisted)) {
+            return false;
+        }
+
+        return $userIp->isInRanges($ipsWhitelisted);
+    }
+
+    /**
+     * @return array
+     */
+    protected function getWhitelistedLoginIps()
+    {
+        $ips = StaticContainer::get('login.whitelist.ips');
+
+        if (!empty($ips) && is_array($ips)) {
+            $ips = array_map(function ($ip) {
+                return trim($ip);
+            }, $ips);
+            $ips = array_filter($ips, function ($ip) {
+                return !empty($ip);
+            });
+            return array_unique(array_values($ips));
+        }
+
+        return array();
+    }
+
+    private function getGeneralConfig()
+    {
+        $config = Config::getInstance();
+        $general = $config->General;
+        return $general;
+    }
+}
diff --git a/plugins/CoreHome/lang/en.json b/plugins/CoreHome/lang/en.json
index 1880e6eadf..36f1234e32 100644
--- a/plugins/CoreHome/lang/en.json
+++ b/plugins/CoreHome/lang/en.json
@@ -18,6 +18,7 @@
         "DonateCall2": "Piwik needs your continued support to grow and thrive.",
         "DonateCall3": "If you feel that Piwik has added significant value to your business or endeavour, %1$splease consider donating%2$s or %3$spurchasing a premium feature%4$s. Every penny will help.",
         "DonateFormInstructions": "Click on the slider to select an amount, then click subscribe to donate.",
+        "ExceptionNotWhitelistedIP": "You cannot use this Piwik as your IP %s is not whitelisted",
         "ExcludeRowsWithLowPopulation": "All rows are shown %s Exclude low population",
         "ExternalHelp": "Help (opens in new tab)",
         "FlattenDataTable": "The report is hierarchical %s Make it flat",
diff --git a/plugins/CoreHome/tests/Integration/LoginWhitelistTest.php b/plugins/CoreHome/tests/Integration/LoginWhitelistTest.php
new file mode 100644
index 0000000000..fd74d52607
--- /dev/null
+++ b/plugins/CoreHome/tests/Integration/LoginWhitelistTest.php
@@ -0,0 +1,213 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\tests\Integration;
+
+use Piwik\Common;
+use Piwik\Config;
+use Piwik\NoAccessException;
+use Piwik\Plugins\CoreHome\LoginWhitelist;
+use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+class CustomLoginWhitelist extends LoginWhitelist {
+
+    public function getWhitelistedLoginIps()
+    {
+        return parent::getWhitelistedLoginIps();
+    }
+
+    public function isIpWhitelisted($ip)
+    {
+        return parent::isIpWhitelisted($ip);
+    }
+}
+
+/**
+ * @group Plugins
+ * @group LoginWhitelist
+ * @group LoginWhitelistTest
+ */
+class LoginWhitelistTest extends IntegrationTestCase
+{
+    /**
+     * @var CustomLoginWhitelist
+     */
+    private $whitelist;
+
+    private $cliMode;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->cliMode = Common::$isCliMode;
+        Common::$isCliMode = false;
+
+        $this->whitelist = new CustomLoginWhitelist();
+    }
+
+    public function tearDown()
+    {
+        Common::$isCliMode = $this->cliMode;
+        parent::tearDown();
+    }
+
+    public function test_shouldWhitelistApplyToAPI_shouldBeEnabledByDefault()
+    {
+        $this->assertTrue($this->whitelist->shouldWhitelistApplyToAPI());
+    }
+
+    public function test_shouldWhitelistApplyToAPI_canBeDisabled()
+    {
+        $this->setGeneralConfig('login_whitelist_apply_to_reporting_api_requests', '0');
+        $this->assertFalse($this->whitelist->shouldWhitelistApplyToAPI());
+    }
+
+    public function test_shouldWhitelistApplyToAPI_enabled()
+    {
+        $this->setGeneralConfig('login_whitelist_apply_to_reporting_api_requests', '1');
+        $this->assertTrue($this->whitelist->shouldWhitelistApplyToAPI());
+    }
+
+    public function test_shouldCheckWhitelist_shouldNotBeCheckedByDefaultAndNotHaveAnyIps()
+    {
+        $this->assertFalse($this->whitelist->shouldCheckWhitelist());
+    }
+
+    public function test_shouldCheckWhitelist_shouldBeCheckedIfHasAtLeastOneIp()
+    {
+        $this->setGeneralConfig('login_whitelist_ip', ['192.168.33.1']);
+        $this->assertTrue($this->whitelist->shouldCheckWhitelist());
+    }
+
+    public function test_shouldCheckWhitelist_shouldNotBeCheckedIfExecutedFromCLI()
+    {
+        Common::$isCliMode = true;
+        $this->setGeneralConfig('login_whitelist_ip', ['192.168.33.1']);
+        $this->assertFalse($this->whitelist->shouldCheckWhitelist());
+    }
+
+    public function test_shouldCheckWhitelist_shouldNotBeCheckedIfOnlyEmptyEntries()
+    {
+        $this->setGeneralConfig('login_whitelist_ip', ['', ' ']);
+        $this->assertFalse($this->whitelist->shouldCheckWhitelist());
+    }
+
+    public function test_getWhitelistedLoginIps_shouldReturnEmptyArrayByDefault()
+    {
+        $this->assertSame($this->whitelist->getWhitelistedLoginIps(), []);
+    }
+
+    public function test_getWhitelistedLoginIps_shouldReturnIpsAndTrimIfNeeded()
+    {
+        $this->setGeneralConfig('login_whitelist_ip', ['192.168.33.1', ' 127.0.0.1 ', '2001:0db8:85a3:0000:0000:8a2e:0370:7334']);
+        $this->assertSame(['192.168.33.1', '127.0.0.1', '2001:0db8:85a3:0000:0000:8a2e:0370:7334'], $this->whitelist->getWhitelistedLoginIps());
+    }
+
+    public function test_getWhitelistedLoginIps_shouldNotBeCheckedIfOnlyEmptyEntries()
+    {
+        $this->setGeneralConfig('login_whitelist_ip', ['', '192.168.33.1 ', ' ']);
+        $this->assertSame(['192.168.33.1'], $this->whitelist->getWhitelistedLoginIps());
+    }
+
+    public function test_getWhitelistedLoginIps_shouldNotReturnDuplicates()
+    {
+        $this->setGeneralConfig('login_whitelist_ip', [' 192.168.33.1', '192.168.33.1 ', ' 192.168.33.1 ', '192.168.33.1']);
+        $this->assertSame(['192.168.33.1'], $this->whitelist->getWhitelistedLoginIps());
+    }
+
+    /**
+     * @dataProvider getIpWhitelistedTests
+     */
+    public function test_isIpWhitelisted($expectedIsWhitelisted, $ipString)
+    {
+        $ipsWhitelisted = [
+            '127.0.0.1',
+            '192.168.33.1',
+            '2001:0db8:85a3:0000:0000:8a2e:0370:7334',
+            '204.93.240.*',
+            '204.93.177.0/25',
+            '2001:db9::/48'
+        ];
+        $this->setGeneralConfig('login_whitelist_ip', $ipsWhitelisted);
+        $this->assertSame($expectedIsWhitelisted, $this->whitelist->isIpWhitelisted($ipString));
+    }
+
+    /**
+     * @dataProvider getIpWhitelistedTests
+     */
+    public function test_isIpWhitelisted_WhenNoIpsConfigured_AllIpsAreWhitelisted($expectedIsWhitelisted, $ipString)
+    {
+        $this->assertFalse($this->whitelist->isIpWhitelisted($ipString));
+    }
+
+    /**
+     * @dataProvider getIpWhitelistedTests
+     */
+    public function test_checkIsWhitelisted($expectedIsWhitelisted, $ipString)
+    {
+        $ipsWhitelisted = [
+            '127.0.0.1',
+            '192.168.33.1',
+            '2001:0db8:85a3:0000:0000:8a2e:0370:7334',
+            '204.93.240.*',
+            '204.93.177.0/25',
+            '2001:db9::/48'
+        ];
+        $this->setGeneralConfig('login_whitelist_ip', $ipsWhitelisted);
+
+        if ($expectedIsWhitelisted) {
+            $this->whitelist->checkIsWhitelisted($ipString);
+            $this->assertTrue(true);
+        } else {
+            try {
+                $this->whitelist->checkIsWhitelisted($ipString);
+                $this->fail('An expected exception has not been thrown');
+            } catch (NoAccessException $e) {
+                $this->assertTrue(true);
+            }
+        }
+    }
+
+    public function getIpWhitelistedTests()
+    {
+        return array(
+            array(true, '127.0.0.1'),
+            array(true, '192.168.33.1'),
+            array(true, '2001:0db8:85a3:0000:0000:8a2e:0370:7334'),
+            array(true, '204.93.240.5'),
+            array(true, '204.93.177.5'),
+            array(true, '2001:db9:0000:ffff:ffff:ffff:ffff:ffff'),
+
+
+            array(false, '127.0.0.2'),
+            array(false, '192.168.33.2'),
+            array(false, '2001:0db8:85a3:0000:0000:8a2e:0370:7333'),
+            array(false, '204.93.239.5'),
+            array(false, '204.93.177.255'),
+            array(false, '2001:db8:0000:ffff:ffff:ffff:ffff:ffff'),
+        );
+    }
+
+    private function setGeneralConfig($name, $value)
+    {
+        $config = Config::getInstance();
+        $general = $config->General;
+        $general[$name] = $value;
+        $config->General = $general;
+        $config->forceSave();
+    }
+
+    public function provideContainerConfig()
+    {
+        return array(
+            'Piwik\Access' => new FakeAccess()
+        );
+    }
+}
diff --git a/plugins/Login/Login.php b/plugins/Login/Login.php
index fbb20e401f..c88756a160 100644
--- a/plugins/Login/Login.php
+++ b/plugins/Login/Login.php
@@ -15,7 +15,6 @@ use Piwik\Container\StaticContainer;
 use Piwik\Cookie;
 use Piwik\FrontController;
 use Piwik\Piwik;
-use Piwik\Session;
 
 /**
  *
diff --git a/plugins/Login/tests/Integration/LoginTest.php b/plugins/Login/tests/Integration/LoginTest.php
index 4aac46dbc1..9a80b2aaf8 100644
--- a/plugins/Login/tests/Integration/LoginTest.php
+++ b/plugins/Login/tests/Integration/LoginTest.php
@@ -9,7 +9,10 @@
 namespace Piwik\Plugins\Login\tests\Integration;
 
 use Piwik\AuthResult;
+use Piwik\Common;
+use Piwik\Config;
 use Piwik\DbHelper;
+use Piwik\NoAccessException;
 use Piwik\Plugins\Login\Auth;
 use Piwik\Plugins\UsersManager\API;
 use Piwik\Tests\Framework\Mock\FakeAccess;
@@ -40,9 +43,6 @@ class LoginTest extends IntegrationTestCase
         $this->auth = new Auth();
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureNoLoginNoTokenAuth()
     {
         // no login; no token auth
@@ -50,9 +50,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureEmptyLoginNoTokenAuth()
     {
         // empty login; no token auth
@@ -61,9 +58,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureNonExistentUser()
     {
         // non-existent user
@@ -72,9 +66,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousNotExisting()
     {
         // anonymous user doesn't exist yet
@@ -82,9 +73,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousNotExistentEmptyLogin()
     {
         // empty login; anonymous user doesn't exist yet
@@ -93,9 +81,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousNotExistentEmptyLoginWithTokenAuth()
     {
         // API authentication; anonymous user doesn't exist yet
@@ -103,9 +88,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousNotExistentWithLoginAndTokenAuth()
     {
         // anonymous user doesn't exist yet
@@ -113,9 +95,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousWithLogin()
     {
         DbHelper::createAnonymousUser();
@@ -125,9 +104,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousEmptyLoginWithTokenAuth()
     {
         DbHelper::createAnonymousUser();
@@ -137,9 +113,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureAnonymousLoginTokenAuthMissmatch()
     {
         DbHelper::createAnonymousUser();
@@ -149,9 +122,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successAnonymousWithTokenAuth()
     {
         DbHelper::createAnonymousUser();
@@ -161,9 +131,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertUserLogin($rc, $login = 'anonymous', $tokenLength = 9);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successAnonymous()
     {
         DbHelper::createAnonymousUser();
@@ -173,9 +140,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertUserLogin($rc, $login = 'anonymous', $tokenLength = 9);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserEmptyTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -185,9 +149,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserInvalidTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -197,9 +158,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserInvalidTokenAuth2()
     {
         $user = $this->_setUpUser();
@@ -209,9 +167,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserEmptyLogin()
     {
         $user = $this->_setUpUser();
@@ -221,9 +176,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserWithSuperUserAccessEmptyLogin()
     {
         $user = $this->_setUpUser();
@@ -234,9 +186,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserLoginTokenAuthMissmatch()
     {
         $this->_setUpUser();
@@ -246,9 +195,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserLoginTokenAuthMissmatch2()
     {
         $user = $this->_setUpUser();
@@ -258,9 +204,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserLoginTokenAuthMissmatch3()
     {
         $user = $this->_setUpUser();
@@ -270,9 +213,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failureUserWithSuperUserAccessLoginTokenAuthMissmatch()
     {
         $user = $this->_setUpUser();
@@ -283,9 +223,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successUserTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -295,9 +232,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertUserLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successUserWithSuperUserAccessByTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -308,9 +242,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertSuperUserLogin($rc, 'user');
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successUserLoginAndTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -320,9 +251,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertUserLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successUserWithSuperUserAccessLoginAndTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -333,9 +261,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertSuperUserLogin($rc, 'user');
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successLoginAndHashedTokenAuth()
     {
         $user = $this->_setUpUser();
@@ -346,9 +271,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertUserLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successWithValidPassword()
     {
         $user = $this->_setUpUser();
@@ -361,9 +283,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertEquals($user['tokenAuth'], $rc->getTokenAuth());
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_successWithSuperUserPassword()
     {
         $user = $this->_setUpUser();
@@ -376,9 +295,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertSuperUserLogin($rc, 'user');
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_failsWithInvalidPassword()
     {
         $user = $this->_setUpUser();
@@ -389,9 +305,6 @@ class LoginTest extends IntegrationTestCase
         $this->assertFailedLogin($rc);
     }
 
-    /**
-     * @group Plugins
-     */
     public function test_authenticate_prioritizesPasswordAuthentication()
     {
         $user = $this->_setUpUser();