diff options
author | Matthieu Aubry <matt@piwik.org> | 2014-10-09 15:24:42 +0400 |
---|---|---|
committer | Matthieu Aubry <matt@piwik.org> | 2014-10-09 15:24:42 +0400 |
commit | 9c32a948e1fd70c1db09147f634cde4a6bd4e5c4 (patch) | |
tree | 9f49bb9b66bc3c0f3166508838473302052f432b | |
parent | 3f6141c12df003838d45cb2fa60b9ea49f4d9dac (diff) | |
parent | cba98d0124a52a13be92e0bd19230592832e63a5 (diff) |
Merge pull request #6407 from piwik/6110_cdt
setting 'cdt' tracker parameter in recent past (in the last 4 hours) should not require token_auth fixes #6110
-rw-r--r-- | core/Tracker.php | 57 | ||||
-rw-r--r-- | core/Tracker/Request.php | 93 | ||||
-rw-r--r-- | core/Tracker/Visit.php | 1 | ||||
-rw-r--r-- | plugins/CoreConsole/Commands/TestsRun.php | 15 | ||||
-rw-r--r-- | plugins/UserSettings/tests/Fixtures/LanguageFixture.php | 1 | ||||
-rwxr-xr-x | tests/LocalTracker.php | 2 | ||||
-rw-r--r-- | tests/PHPUnit/Fixtures/OneVisitWithAbnormalPageviewUrls.php | 2 | ||||
-rw-r--r-- | tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php | 1 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/BackwardsCompatibility1XTest.php | 24 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/Core/Tracker/ActionTest.php | 1 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/Core/Tracker/Visit2Test.php | 5 |
11 files changed, 105 insertions, 97 deletions
diff --git a/core/Tracker.php b/core/Tracker.php index 72bd3cd289..bd72e04920 100644 --- a/core/Tracker.php +++ b/core/Tracker.php @@ -44,9 +44,6 @@ class Tracker const LENGTH_HEX_ID_STRING = 16; const LENGTH_BINARY_ID = 8; - protected static $forcedDateTime = null; - protected static $forcedIpString = null; - protected static $pluginsNotToLoad = array(); protected static $pluginsToLoad = array(); @@ -90,21 +87,9 @@ class Tracker public function clear() { - self::$forcedIpString = null; - self::$forcedDateTime = null; $this->stateValid = self::STATE_NOTHING_TO_NOTICE; } - public static function setForceIp($ipString) - { - self::$forcedIpString = $ipString; - } - - public static function setForceDateTime($dateTime) - { - self::$forcedDateTime = $dateTime; - } - /** * Do not load the specified plugins (used during testing, to disable Provider plugin) * @param array $plugins @@ -488,15 +473,13 @@ class Tracker /** * Initialization + * @param Request $request */ protected function init(Request $request) { $this->loadTrackerPlugins($request); - $this->handleTrackingApi($request); $this->handleDisabledTracker(); $this->handleEmptyRequest($request); - - Common::printDebug("Current datetime: " . date("Y-m-d H:i:s", $request->getCurrentTimestamp())); } /** @@ -748,29 +731,6 @@ class Tracker return Common::getRequestVar('token_auth', false); } - /** - * This method allows to set custom IP + server time + visitor ID, when using Tracking API. - * These two attributes can be only set by the Super User (passing token_auth). - */ - protected function handleTrackingApi(Request $request) - { - if (!$request->isAuthenticated()) { - return; - } - - // Custom IP to use for this visitor - $customIp = $request->getParam('cip'); - if (!empty($customIp)) { - $this->setForceIp($customIp); - } - - // Custom server date time to use - $customDatetime = $request->getParam('cdt'); - if (!empty($customDatetime)) { - $this->setForceDateTime($customDatetime); - } - } - public static function setTestEnvironment($args = null, $requestMethod = null) { if (is_null($args)) { @@ -816,18 +776,6 @@ class Tracker \Piwik\Plugins\PrivacyManager\IPAnonymizer::activate(); } - // Custom IP to use for this visitor - $customIp = Common::getRequestVar('cip', false, null, $args); - if (!empty($customIp)) { - self::setForceIp($customIp); - } - - // Custom server date time to use - $customDatetime = Common::getRequestVar('cdt', false, null, $args); - if (!empty($customDatetime)) { - self::setForceDateTime($customDatetime); - } - $pluginsDisabled = array('Provider'); // Disable provider plugin, because it is so slow to do many reverse ip lookups @@ -870,8 +818,7 @@ class Tracker try { if ($this->isVisitValid()) { - $request->setForceDateTime(self::$forcedDateTime); - $request->setForceIp(self::$forcedIpString); + Common::printDebug("Current datetime: " . date("Y-m-d H:i:s", $request->getCurrentTimestamp())); $visit = $this->getNewVisitObject(); $visit->setRequest($request); diff --git a/core/Tracker/Request.php b/core/Tracker/Request.php index 212abbd0d2..aad200660f 100644 --- a/core/Tracker/Request.php +++ b/core/Tracker/Request.php @@ -35,6 +35,8 @@ class Request const UNKNOWN_RESOLUTION = 'unknown'; + const CUSTOM_TIMESTAMP_DOES_NOT_REQUIRE_TOKENAUTH_WHEN_NEWER_THAN = 14400; // 4 hours + /** * @param $params * @param bool|string $tokenAuth @@ -47,7 +49,6 @@ class Request $this->params = $params; $this->tokenAuth = $tokenAuth; $this->timestamp = time(); - $this->enforcedIp = false; // When the 'url' and referrer url parameter are not given, we might be in the 'Simple Image Tracker' mode. // The URL can default to the Referrer, which will be in this case @@ -319,13 +320,54 @@ class Request public function getCurrentTimestamp() { + $cdt = $this->getCustomTimestamp(); + if(!empty($cdt)) { + return $cdt; + } return $this->timestamp; } - protected function isTimestampValid($time) + protected function getCustomTimestamp() + { + $cdt = $this->getParam('cdt'); + if (empty($cdt)) { + return false; + } + if (!is_numeric($cdt)) { + $cdt = strtotime($cdt); + } + if (!$this->isTimestampValid($cdt, $this->timestamp)) { + Common::printDebug(sprintf("Datetime %s is not valid", date("Y-m-d H:i:m", $cdt))); + return false; + } + + // If timestamp in the past, token_auth is required + $timeFromNow = $this->timestamp - $cdt; + $isTimestampRecent = $timeFromNow < self::CUSTOM_TIMESTAMP_DOES_NOT_REQUIRE_TOKENAUTH_WHEN_NEWER_THAN; + if (!$isTimestampRecent) { + if(!$this->isAuthenticated()) { + Common::printDebug(sprintf("Custom timestamp is %s seconds old, requires &token_auth...", $timeFromNow)); + Common::printDebug("WARN: Tracker API 'cdt' was used with invalid token_auth"); + return false; + } + } + return $cdt; + } + + /** + * Returns true if the timestamp is valid ie. timestamp is sometime in the last 10 years and is not in the future. + * + * @param $time int Timestamp to test + * @param $now int Current timestamp + * @return bool + */ + protected function isTimestampValid($time, $now = null) { - return $time <= $this->getCurrentTimestamp() - && $time > $this->getCurrentTimestamp() - 10 * 365 * 86400; + if(empty($now)) { + $now = $this->getCurrentTimestamp(); + } + return $time <= $now + && $time > $now - 10 * 365 * 86400; } public function getIdSite() @@ -521,33 +563,11 @@ class Request public function getIp() { - if (!empty($this->enforcedIp)) { - $ipString = $this->enforcedIp; - } else { - $ipString = IP::getIpFromHeader(); - } - + $ipString = $this->getIpString(); $ip = IP::P2N($ipString); return $ip; } - public function setForceIp($ip) - { - if (!empty($ip)) { - $this->enforcedIp = $ip; - } - } - - public function setForceDateTime($dateTime) - { - if (!is_numeric($dateTime)) { - $dateTime = strtotime($dateTime); - } - if (!empty($dateTime)) { - $this->timestamp = $dateTime; - } - } - public function getForcedUserId() { $userId = $this->getParam('uid'); @@ -611,4 +631,23 @@ class Request { return substr( sha1( $userId ), 0, 16); } + + /** + * @return mixed|string + * @throws Exception + */ + private function getIpString() + { + $cip = $this->getParam('cip'); + + if(empty($cip)) { + return IP::getIpFromHeader(); + } + + if(!$this->isAuthenticated()) { + Common::printDebug("WARN: Tracker API 'cip' was used with invalid token_auth"); + return IP::getIpFromHeader(); + } + return $cip; + } } diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 0ce45c3659..c8fb12d389 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -467,6 +467,7 @@ class Visit implements VisitInterface $debugVisitInfo = $this->visitorInfo; $debugVisitInfo['idvisitor'] = bin2hex($debugVisitInfo['idvisitor']); $debugVisitInfo['config_id'] = bin2hex($debugVisitInfo['config_id']); + $debugVisitInfo['location_ip'] = IP::N2P($debugVisitInfo['location_ip']); Common::printDebug($debugVisitInfo); } diff --git a/plugins/CoreConsole/Commands/TestsRun.php b/plugins/CoreConsole/Commands/TestsRun.php index 5f940acb83..5e00bd6230 100644 --- a/plugins/CoreConsole/Commands/TestsRun.php +++ b/plugins/CoreConsole/Commands/TestsRun.php @@ -88,7 +88,7 @@ class TestsRun extends ConsoleCommand private function executeTestFile($testFile, $options, $command, OutputInterface $output) { $params = $options . " " . $testFile; - $cmd = sprintf("cd %s/tests/PHPUnit && %s %s", PIWIK_DOCUMENT_ROOT, $command, $params); + $cmd = $this->getCommand($command, $params); $output->writeln('Executing command: <info>' . $cmd . '</info>'); passthru($cmd); $output->writeln(""); @@ -102,7 +102,7 @@ class TestsRun extends ConsoleCommand foreach ($groups as $group) { $params = '--group ' . $group . ' ' . str_replace('%group%', $group, $options); - $cmd = sprintf('cd %s/tests/PHPUnit && %s %s', PIWIK_DOCUMENT_ROOT, $command, $params); + $cmd = $this->getCommand($command, $params); $output->writeln('Executing command: <info>' . $cmd . '</info>'); passthru($cmd); $output->writeln(""); @@ -113,4 +113,15 @@ class TestsRun extends ConsoleCommand { return array('Core', 'Plugins', 'Integration', 'UI'); } + + /** + * @param $command + * @param $params + * @return string + */ + private function getCommand($command, $params) + { + $cmd = sprintf('cd %s/tests/PHPUnit && %s %s', PIWIK_DOCUMENT_ROOT, $command, $params); + return $cmd; + } }
\ No newline at end of file diff --git a/plugins/UserSettings/tests/Fixtures/LanguageFixture.php b/plugins/UserSettings/tests/Fixtures/LanguageFixture.php index 01c5d626a9..227ea016ea 100644 --- a/plugins/UserSettings/tests/Fixtures/LanguageFixture.php +++ b/plugins/UserSettings/tests/Fixtures/LanguageFixture.php @@ -54,6 +54,7 @@ class LanguageFixture extends Fixture $this->dateTime, $defaultInit = false ); + $tracker->setTokenAuth(self::getTokenAuth()); $hour = 1; foreach ($this->getBrowserLangs() as $browserLang) { diff --git a/tests/LocalTracker.php b/tests/LocalTracker.php index afa0a6a4bc..ecddbe5c9f 100755 --- a/tests/LocalTracker.php +++ b/tests/LocalTracker.php @@ -45,8 +45,6 @@ class Piwik_LocalTracker extends PiwikTracker // unset cached values Cache::$trackerCache = null; - Tracker::setForceIp(null); - Tracker::setForceDateTime(null); // save some values $plugins = Config::getInstance()->Plugins['Plugins']; diff --git a/tests/PHPUnit/Fixtures/OneVisitWithAbnormalPageviewUrls.php b/tests/PHPUnit/Fixtures/OneVisitWithAbnormalPageviewUrls.php index 7a316bcc78..ba1f56d207 100644 --- a/tests/PHPUnit/Fixtures/OneVisitWithAbnormalPageviewUrls.php +++ b/tests/PHPUnit/Fixtures/OneVisitWithAbnormalPageviewUrls.php @@ -41,7 +41,7 @@ class OneVisitWithAbnormalPageviewUrls extends Fixture { $dateTime = $this->dateTime; $idSite = $this->idSite; - $t = self::getTracker($idSite, $dateTime, $defaultInit = true, $useThirdPartyCookie = 1); + $t = self::getTracker($idSite, $dateTime, $defaultInit = true); $t->setUrlReferrer('http://www.google.com/search?q=piwik'); $t->setUrl('http://example.org/foo/bar.html'); diff --git a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php index 4896e6fe2a..adc6b2e3e0 100644 --- a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php +++ b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php @@ -63,6 +63,7 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture // Create a new Tracker object, with different attributes $t2 = self::getTracker($idSite, $dateTime, $defaultInit = false); + $t2->setTokenAuth(self::getTokenAuth()); // Make sure the ID is different at first $visitorId2 = $t2->getVisitorId(); diff --git a/tests/PHPUnit/Integration/BackwardsCompatibility1XTest.php b/tests/PHPUnit/Integration/BackwardsCompatibility1XTest.php index 8e68ab6610..e8756f67a9 100644 --- a/tests/PHPUnit/Integration/BackwardsCompatibility1XTest.php +++ b/tests/PHPUnit/Integration/BackwardsCompatibility1XTest.php @@ -43,19 +43,31 @@ class BackwardsCompatibility1XTest extends IntegrationTestCase Db::query("TRUNCATE TABLE " . Common::prefixTable($table)); } - // add two visits from same visitor on dec. 29 - $t = Fixture::getTracker(1, '2012-12-29 01:01:30', $defaultInit = true); + self::trackTwoVisitsOnSameDay(); + + // launch archiving + VisitFrequencyApi::getInstance()->get(1, 'year', '2012-12-29'); + } + + + /** + * add two visits from same visitor on dec. 29 + */ + private static function trackTwoVisitsOnSameDay() + { + $t = Fixture::getTracker(1, '2012-12-29 01:01:30', $defaultInit = true, $useLocal = true); + $t->enableBulkTracking(); + $t->setUrl('http://site.com/index.htm'); $t->setIp('136.5.3.2'); - Fixture::checkResponse($t->doTrackPageView('incredible title!')); + $t->doTrackPageView('incredible title!'); $t->setForceVisitDateTime('2012-12-29 03:01:30'); $t->setUrl('http://site.com/other/index.htm'); $t->DEBUG_APPEND_URL = '&_idvc=2'; // make sure visit is marked as returning - Fixture::checkResponse($t->doTrackPageView('other incredible title!')); + $t->doTrackPageView('other incredible title!'); - // launch archiving - VisitFrequencyApi::getInstance()->get(1, 'year', '2012-12-29'); + $t->doBulkTrack(); } /** diff --git a/tests/PHPUnit/Integration/Core/Tracker/ActionTest.php b/tests/PHPUnit/Integration/Core/Tracker/ActionTest.php index 02b3f1e8c1..792ccbbc3f 100644 --- a/tests/PHPUnit/Integration/Core/Tracker/ActionTest.php +++ b/tests/PHPUnit/Integration/Core/Tracker/ActionTest.php @@ -20,6 +20,7 @@ use Piwik\Plugin\Manager as PluginManager; * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later * * @group Core + * @group ActionTest */ class Core_Tracker_ActionTest extends DatabaseTestCase { diff --git a/tests/PHPUnit/Integration/Core/Tracker/Visit2Test.php b/tests/PHPUnit/Integration/Core/Tracker/Visit2Test.php index 7c3b2e9ed5..de9d0810ec 100644 --- a/tests/PHPUnit/Integration/Core/Tracker/Visit2Test.php +++ b/tests/PHPUnit/Integration/Core/Tracker/Visit2Test.php @@ -134,6 +134,7 @@ class FakeTrackerVisit extends \Piwik\Tracker\Visit /** * @group Core + * @group VisitTest */ class VisitTest extends DatabaseTestCase { @@ -156,7 +157,6 @@ class VisitTest extends DatabaseTestCase public function test_handleNewVisitWithoutConversion_shouldTriggerDimensions() { $request = new \Piwik\Tracker\Request(array()); - $request->setForceIp('127.0.0.1'); $visitor = new \Piwik\Tracker\Visitor($request, ''); $visit = new FakeTrackerVisit($request); @@ -179,7 +179,6 @@ class VisitTest extends DatabaseTestCase public function test_handleNewVisitWithConversion_shouldTriggerDimensions() { $request = new \Piwik\Tracker\Request(array()); - $request->setForceIp('127.0.0.1'); $visitor = new \Piwik\Tracker\Visitor($request, ''); $visit = new FakeTrackerVisit($request); @@ -198,7 +197,6 @@ class VisitTest extends DatabaseTestCase public function test_handleExistingVisitWithoutConversion_shouldTriggerDimensions() { $request = new \Piwik\Tracker\Request(array()); - $request->setForceIp('127.0.0.1'); $visitor = new \Piwik\Tracker\Visitor($request, ''); $visit = new FakeTrackerVisit($request); @@ -222,7 +220,6 @@ class VisitTest extends DatabaseTestCase public function test_handleExistingVisitWithConversion_shouldTriggerDimensions() { $request = new \Piwik\Tracker\Request(array()); - $request->setForceIp('127.0.0.1'); $visitor = new \Piwik\Tracker\Visitor($request, ''); $visit = new FakeTrackerVisit($request); |