diff --git a/plugins/CoreUpdater/Controller.php b/plugins/CoreUpdater/Controller.php
index be3d19408d0..94c8cab9bfe 100644
--- a/plugins/CoreUpdater/Controller.php
+++ b/plugins/CoreUpdater/Controller.php
@@ -171,6 +171,7 @@ public function oneClickUpdate()
try {
$messages = $this->updater->updatePiwik($useHttps);
+ $this->refreshUpdateDetailsToken();
} catch (ArchiveDownloadException $e) {
$view->httpsFail = $useHttps;
$view->error = $e->getMessage();
@@ -244,7 +245,13 @@ public function oneClickResults()
$view = new View('@CoreUpdater/updateHttpError');
$view->error = $error;
} else {
- $updateDetailsToken = $this->createUpdateDetailsTokenIfMissing();
+ $updateDetailsToken = null;
+ if (Piwik::hasUserSuperUserAccess()) {
+ $updateDetailsToken = $this->getUpdateDetailsToken();
+ if ($updateDetailsToken === null) {
+ $updateDetailsToken = $this->refreshUpdateDetailsToken();
+ }
+ }
$runUpdaterUrl = Url::getCurrentUrlWithoutQueryString();
if ($updateDetailsToken !== null) {
@@ -474,14 +481,9 @@ private function checkUpdateDetailsToken(): bool
return $config->General['update_details_token'] === Request::fromRequest()->getStringParameter('updateDetailsToken', '');
}
- private function createUpdateDetailsTokenIfMissing(): ?string
+ private function refreshUpdateDetailsToken(): string
{
$config = Config::getInstance();
-
- if (!empty($config->General['update_details_token'])) {
- return null;
- }
-
$token = Common::generateUniqId();
$config->General['update_details_token'] = $token;
@@ -490,6 +492,17 @@ private function createUpdateDetailsTokenIfMissing(): ?string
return $token;
}
+ private function getUpdateDetailsToken(): ?string
+ {
+ $config = Config::getInstance();
+
+ if (empty($config->General['update_details_token'])) {
+ return null;
+ }
+
+ return $config->General['update_details_token'];
+ }
+
private function removeUpdateDetailsToken(): void
{
$config = Config::getInstance();
diff --git a/plugins/CoreUpdater/templates/runUpdaterAndExit_welcome.twig b/plugins/CoreUpdater/templates/runUpdaterAndExit_welcome.twig
index 6f61acbe854..9da13a413c6 100644
--- a/plugins/CoreUpdater/templates/runUpdaterAndExit_welcome.twig
+++ b/plugins/CoreUpdater/templates/runUpdaterAndExit_welcome.twig
@@ -67,7 +67,7 @@
{{ 'CoreUpdater_UpdateDetailsHidden1'|translate }}
- - {{ 'CoreUpdater_UpdateDetailsHidden2'|translate('
updateTokenDetails', '[General] update_details_token', 'config.ini.php')|raw }}
+ - {{ 'CoreUpdater_UpdateDetailsHidden2'|translate('
updateDetailsToken', '[General] update_details_token', 'config.ini.php')|raw }}
- {{ 'CoreUpdater_UpdateDetailsHidden3'|translate }}
diff --git a/plugins/CoreUpdater/tests/Integration/ControllerTest.php b/plugins/CoreUpdater/tests/Integration/ControllerTest.php
new file mode 100644
index 00000000000..cff7c221408
--- /dev/null
+++ b/plugins/CoreUpdater/tests/Integration/ControllerTest.php
@@ -0,0 +1,170 @@
+originalGet = $_GET;
+ $this->originalPost = $_POST;
+
+ $general = Config::getInstance()->General;
+ $this->hadEnableAutoUpdate = array_key_exists('enable_auto_update', $general);
+ $this->hadEnableInternetFeatures = array_key_exists('enable_internet_features', $general);
+ $this->hadUpdateDetailsToken = array_key_exists('update_details_token', $general);
+ $this->originalEnableAutoUpdate = $general['enable_auto_update'] ?? null;
+ $this->originalEnableInternetFeatures = $general['enable_internet_features'] ?? null;
+ $this->originalUpdateDetailsToken = $general['update_details_token'] ?? null;
+
+ Config::getInstance()->General['enable_auto_update'] = 1;
+ Config::getInstance()->General['enable_internet_features'] = 1;
+ }
+
+ public function tearDown(): void
+ {
+ $_GET = $this->originalGet;
+ $_POST = $this->originalPost;
+
+ FakeAccess::clearAccess();
+
+ $config = Config::getInstance();
+ if ($this->hadEnableAutoUpdate) {
+ $config->General['enable_auto_update'] = $this->originalEnableAutoUpdate;
+ } else {
+ unset($config->General['enable_auto_update']);
+ }
+
+ if ($this->hadEnableInternetFeatures) {
+ $config->General['enable_internet_features'] = $this->originalEnableInternetFeatures;
+ } else {
+ unset($config->General['enable_internet_features']);
+ }
+
+ if ($this->hadUpdateDetailsToken) {
+ $config->General['update_details_token'] = $this->originalUpdateDetailsToken;
+ } else {
+ unset($config->General['update_details_token']);
+ }
+
+ $config->forceSave();
+
+ parent::tearDown();
+ }
+
+ public function testOneClickResultsDoesNotCreateOrExposeTokenForAnonymousRequest()
+ {
+ FakeAccess::clearAccess(false);
+ unset(Config::getInstance()->General['update_details_token']);
+
+ $_GET = [];
+ $_POST = [];
+
+ $result = $this->buildController()->oneClickResults();
+
+ self::assertStringNotContainsString('updateDetailsToken=', $result);
+ self::assertEmpty(Config::getInstance()->General['update_details_token'] ?? null);
+ }
+
+ public function testOneClickResultsOnlyExposesStoredTokenForSuperUserContext()
+ {
+ $token = '1234567890abcdef1234567890abcdef';
+ Config::getInstance()->General['update_details_token'] = $token;
+ Config::getInstance()->forceSave();
+
+ $_GET = [];
+ $_POST = [];
+
+ FakeAccess::clearAccess(true);
+ $superUserResult = $this->buildController()->oneClickResults();
+ self::assertStringContainsString('updateDetailsToken=' . urlencode($token), $superUserResult);
+
+ FakeAccess::clearAccess(false);
+ $anonymousResult = $this->buildController()->oneClickResults();
+ self::assertStringNotContainsString('updateDetailsToken=' . urlencode($token), $anonymousResult);
+ }
+
+ public function testOneClickResultsCreatesAndExposesTokenForSuperUserWhenMissing()
+ {
+ FakeAccess::clearAccess(true);
+ unset(Config::getInstance()->General['update_details_token']);
+
+ $_GET = [];
+ $_POST = [];
+
+ $result = $this->buildController()->oneClickResults();
+ $createdToken = Config::getInstance()->General['update_details_token'] ?? null;
+
+ self::assertNotEmpty($createdToken);
+ self::assertStringContainsString('updateDetailsToken=' . urlencode($createdToken), $result);
+ }
+
+ public function testOneClickUpdateRotatesStoredUpdateDetailsTokenAfterSuccessfulUpdate()
+ {
+ FakeAccess::clearAccess(true);
+ Config::getInstance()->General['update_details_token'] = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
+
+ $_GET = [];
+ $_POST = [];
+ $_GET['nonce'] = Nonce::getNonce('oneClickUpdate');
+
+ ob_start();
+ try {
+ $this->buildController()->oneClickUpdate();
+ $output = ob_get_clean();
+ } catch (\Throwable $e) {
+ ob_end_clean();
+ throw $e;
+ }
+
+ $rotatedToken = Config::getInstance()->General['update_details_token'] ?? '';
+
+ self::assertStringContainsString('action=oneClickResults', $output);
+ self::assertNotSame('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', $rotatedToken);
+ self::assertSame(1, preg_match('/^[a-f0-9]{32}$/', $rotatedToken));
+ }
+
+ public function provideContainerConfig()
+ {
+ return array(
+ 'Piwik\Access' => new FakeAccess(),
+ );
+ }
+
+ private function buildController(): Controller
+ {
+ $updater = $this->createMock(Updater::class);
+ $updater->method('updatePiwik')->willReturn(array());
+
+ return new Controller($updater);
+ }
+}
diff --git a/plugins/CoreUpdater/tests/UI/expected-screenshots/CoreUpdaterDb_main_no_update_token.png b/plugins/CoreUpdater/tests/UI/expected-screenshots/CoreUpdaterDb_main_no_update_token.png
index fb5da11314d..40faaffdec8 100644
--- a/plugins/CoreUpdater/tests/UI/expected-screenshots/CoreUpdaterDb_main_no_update_token.png
+++ b/plugins/CoreUpdater/tests/UI/expected-screenshots/CoreUpdaterDb_main_no_update_token.png
@@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
-oid sha256:32f28a4da0027d6d880e3701108481c831d94337c8ce3f666491140955694a5a
-size 328472
+oid sha256:abafcbeb6fdb9adcb6f6515465425d303a6c3e373579ee718d64b51f19fc2f9c
+size 328301
diff --git a/tests/UI/specs/OneClickUpdate_spec.js b/tests/UI/specs/OneClickUpdate_spec.js
index 448942061d5..13a93c9a153 100644
--- a/tests/UI/specs/OneClickUpdate_spec.js
+++ b/tests/UI/specs/OneClickUpdate_spec.js
@@ -90,8 +90,11 @@ describe("OneClickUpdate", function () {
const updateDetailsToken = readUpdateDetailsTokenFromConfig();
const runUpdaterLink = await page.$eval('.footer a', link => link.getAttribute('href'));
- expect(updateDetailsToken).to.be.ok;
- expect(runUpdaterLink).to.match(new RegExp(`[?&]updateDetailsToken=${updateDetailsToken}(?:&|$)`));
+ if (updateDetailsToken) {
+ expect(runUpdaterLink).to.match(new RegExp(`[?&]updateDetailsToken=${updateDetailsToken}(?:&|$)`));
+ } else {
+ expect(runUpdaterLink).to.not.match(/[?&]updateDetailsToken=/);
+ }
});
it('should login successfully after the update', async function () {