Mirrors
/
nextcloud-server
mirror of https://github.com/nextcloud/server
3
0
Fork 0
Code Releases Activity
Browse Source

fix(updater): Stop expiring secret prematurely 

Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
pull/49578/head
Josh Richards 2 years ago
committed by Louis Chmn
parent
7fca1c87ee
commit
f600f26391
4 changed files with 114 additions and 34 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 14
      apps/updatenotification/lib/BackgroundJob/ResetToken.php
  2. 14
      apps/updatenotification/lib/Controller/AdminController.php
  3. 73
      apps/updatenotification/tests/BackgroundJob/ResetTokenTest.php
  4. 47
      apps/updatenotification/tests/Controller/AdminControllerTest.php

14
apps/updatenotification/lib/BackgroundJob/ResetToken.php
View File

@ -12,6 +12,7 @@ use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
use OCP\IAppConfig;
use OCP\IConfig;
use Psr\Log\LoggerInterface;
/**
* Deletes the updater secret after if it is older than 48h
@ -26,10 +27,11 @@ class ResetToken extends TimedJob {
ITimeFactory $time,
private IConfig $config,
private IAppConfig $appConfig,
private LoggerInterface $logger,
) {
parent::__construct($time);
// Run all 10 minutes
parent::setInterval(60 * 10);
// Run once an hour
parent::setInterval(60 * 60);
}
/**
@ -37,13 +39,19 @@ class ResetToken extends TimedJob {
*/
protected function run($argument) {
if ($this->config->getSystemValueBool('config_is_read_only')) {
$this->logger->debug('Skipping `updater.secret` reset since config_is_read_only is set', ['app' => 'updatenotification']);
return;
}
$secretCreated = $this->appConfig->getValueInt('core', 'updater.secret.created', $this->time->getTime());
// Delete old tokens after 2 days
if ($secretCreated >= 172800) {
$secretCreatedDiff = $this->time->getTime() - $secretCreated;
if ($secretCreatedDiff >= 172800) {
$this->config->deleteSystemValue('updater.secret');
$this->appConfig->deleteKey('core', 'updater.secret.created');
$this->logger->warning('Cleared old `updater.secret` that was created ' . $secretCreatedDiff . ' seconds ago', ['app' => 'updatenotification']);
} else {
$this->logger->debug('Keeping existing `updater.secret` that was created ' . $secretCreatedDiff . ' seconds ago', ['app' => 'updatenotification']);
}
}
}

14
apps/updatenotification/lib/Controller/AdminController.php
View File

@ -21,6 +21,7 @@ use OCP\IL10N;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
use OCP\Util;
use Psr\Log\LoggerInterface;
class AdminController extends Controller {
@ -33,14 +34,11 @@ class AdminController extends Controller {
private IAppConfig $appConfig,
private ITimeFactory $timeFactory,
private IL10N $l10n,
private LoggerInterface $logger,
) {
parent::__construct($appName, $request);
}
private function isUpdaterEnabled(): bool {
return !$this->config->getSystemValueBool('upgrade.disable-web');
}
/**
* @param string $channel
* @return DataResponse
@ -55,10 +53,14 @@ class AdminController extends Controller {
* @return DataResponse
*/
public function createCredentials(): DataResponse {
if (!$this->isUpdaterEnabled()) {
if ($this->config->getSystemValueBool('upgrade.disable-web')) {
return new DataResponse(['status' => 'error', 'message' => $this->l10n->t('Web updater is disabled')], Http::STATUS_FORBIDDEN);
}
if ($this->config->getSystemValueBool('config_is_read_only')) {
return new DataResponse(['status' => 'error', 'message' => $this->l10n->t('Configuration is read-only')], Http::STATUS_FORBIDDEN);
}
// Create a new job and store the creation date
$this->jobList->add(ResetToken::class);
$this->appConfig->setValueInt('core', 'updater.secret.created', $this->timeFactory->getTime());
@ -67,6 +69,8 @@ class AdminController extends Controller {
$newToken = $this->secureRandom->generate(64);
$this->config->setSystemValue('updater.secret', password_hash($newToken, PASSWORD_DEFAULT));
$this->logger->warning('Created new `updater.secret`', ['app' => 'updatenotification']);
return new DataResponse($newToken);
}
}

73
apps/updatenotification/tests/BackgroundJob/ResetTokenTest.php
View File

@ -13,35 +13,41 @@ use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IAppConfig;
use OCP\IConfig;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
class ResetTokenTest extends TestCase {
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private ITimeFactory&MockObject $timeFactory;
private BackgroundJobResetToken $resetTokenBackgroundJob;
protected BackgroundJobResetToken $resetTokenBackgroundJob;
protected IConfig&MockObject $config;
protected IAppConfig&MockObject $appConfig;
protected ITimeFactory&MockObject $timeFactory;
protected LoggerInterface&MockObject $logger;
protected function setUp(): void {
parent::setUp();
$this->appConfig = $this->createMock(IAppConfig::class);
$this->config = $this->createMock(IConfig::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->config = $this->createMock(IConfig::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->resetTokenBackgroundJob = new BackgroundJobResetToken(
$this->timeFactory,
$this->config,
$this->appConfig,
$this->logger,
);
}
public function testRunWithNotExpiredToken(): void {
public function testRunWithNotExpiredToken(): void { // Affirm if updater.secret.created <48 hours ago then `updater.secret` is left alone
$this->timeFactory
->expects($this->atLeastOnce())
->method('getTime')
->willReturn(123);
->willReturn(1733069649); // "Sun, 01 Dec 2024 16:14:09 +0000"
$this->appConfig
->expects($this->once())
->method('getValueInt')
->with('core', 'updater.secret.created', 123);
->with('core', 'updater.secret.created', 1733069649)
->willReturn(1733069649 - 1 * 24 * 60 * 60); // 24h prior: "Sat, 30 Nov 2024 16:14:09 +0000"
$this->config
->expects($this->once())
->method('getSystemValueBool')
@ -50,29 +56,53 @@ class ResetTokenTest extends TestCase {
$this->config
->expects($this->never())
->method('deleteSystemValue');
$this->appConfig
->expects($this->never())
->method('deleteKey');
$this->logger
->expects($this->never())
->method('warning');
$this->logger
->expects($this->once())
->method('debug');
static::invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
$this->invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
}
public function testRunWithExpiredToken(): void {
public function testRunWithExpiredToken(): void { // Affirm if updater.secret.created >48 hours ago then `updater.secret` is removed
$this->timeFactory
->expects($this->once())
->expects($this->atLeastOnce())
->method('getTime')
->willReturn(1455045234);
->willReturn(1455045234); // "Tue, 09 Feb 2016 19:13:54 +0000"
$this->appConfig
->expects($this->once())
->method('getValueInt')
->with('core', 'updater.secret.created', 1455045234)
->willReturn(2 * 24 * 60 * 60 + 1); // over 2 days
->willReturn(1455045234 - 3 * 24 * 60 * 60); // 72h prior: "Sat, 06 Feb 2016 19:13:54 +0000"
$this->config
->expects($this->once())
->method('getSystemValueBool')
->with('config_is_read_only')
->willReturn(false);
$this->config
->expects($this->once())
->method('deleteSystemValue')
->with('updater.secret');
$this->appConfig
->expects($this->once())
->method('deleteKey')
->with('core', 'updater.secret.created');
$this->logger
->expects($this->once())
->method('warning');
$this->logger
->expects($this->never())
->method('debug');
static::invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
$this->invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
}
public function testRunWithExpiredTokenAndReadOnlyConfigFile(): void {
public function testRunWithExpiredTokenAndReadOnlyConfigFile(): void { // Affirm if config_is_read_only is set that the secret is never reset
$this->timeFactory
->expects($this->never())
->method('getTime');
@ -87,7 +117,16 @@ class ResetTokenTest extends TestCase {
$this->config
->expects($this->never())
->method('deleteSystemValue');
$this->appConfig
->expects($this->never())
->method('deleteKey');
$this->logger
->expects($this->never())
->method('warning');
$this->logger
->expects($this->once())
->method('debug');
static::invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
$this->invokePrivate($this->resetTokenBackgroundJob, 'run', [null]);
}
}

47
apps/updatenotification/tests/Controller/AdminControllerTest.php
View File

@ -19,18 +19,20 @@ use OCP\IL10N;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
class AdminControllerTest extends TestCase {
private IRequest&MockObject $request;
private IJobList&MockObject $jobList;
private ISecureRandom&MockObject $secureRandom;
private IConfig&MockObject $config;
private ITimeFactory&MockObject $timeFactory;
private IL10N&MockObject $l10n;
private IAppConfig&MockObject $appConfig;
protected IRequest&MockObject $request;
protected IJobList&MockObject $jobList;
protected ISecureRandom&MockObject $secureRandom;
protected IConfig&MockObject $config;
protected ITimeFactory&MockObject $timeFactory;
protected IL10N&MockObject $l10n;
protected IAppConfig&MockObject $appConfig;
protected LoggerInterface&MockObject $logger;
private AdminController $adminController;
protected AdminController $adminController;
protected function setUp(): void {
parent::setUp();
@ -42,6 +44,7 @@ class AdminControllerTest extends TestCase {
$this->appConfig = $this->createMock(IAppConfig::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->l10n = $this->createMock(IL10N::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->adminController = new AdminController(
'updatenotification',
@ -51,7 +54,8 @@ class AdminControllerTest extends TestCase {
$this->config,
$this->appConfig,
$this->timeFactory,
$this->l10n
$this->l10n,
$this->logger,
);
}
@ -81,4 +85,29 @@ class AdminControllerTest extends TestCase {
$expected = new DataResponse('MyGeneratedToken');
$this->assertEquals($expected, $this->adminController->createCredentials());
}
public function testCreateCredentialsAndWebUpdaterDisabled(): void {
$this->config
->expects($this->once())
->method('getSystemValueBool')
->with('upgrade.disable-web')
->willReturn(true);
$this->jobList
->expects($this->never())
->method('add');
$this->secureRandom
->expects($this->never())
->method('generate');
$this->config
->expects($this->never())
->method('setSystemValue');
$this->timeFactory
->expects($this->never())
->method('getTime');
$this->appConfig
->expects($this->never())
->method('setValueInt');
$this->adminController->createCredentials();
}
}
Write Preview
Loading…
Cancel
Save