Improve typing and remove PHP 5.4 hacks

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2023-06-13 11:27:28 +02:00
parent 9dfb0886a1
commit f8ce72926c
No known key found for this signature in database
GPG Key ID: A3E2F658B28C760A
6 changed files with 55 additions and 111 deletions

View File

@ -22,93 +22,26 @@
*/
class Auth {
/** @var Updater */
private $updater;
/** @var string */
private $password;
/**
* @param Updater $updater
* @param string $password
*/
public function __construct(Updater $updater,
$password) {
public function __construct(
private Updater $updater,
private string $password,
) {
$this->updater = $updater;
$this->password = $password;
}
/**
* Compares two strings.
*
* This method implements a constant-time algorithm to compare strings.
* Regardless of the used implementation, it will leak length information.
*
* @param string $knownString The string of known length to compare against
* @param string $userInput The string that the user can control
*
* @return bool true if the two strings are the same, false otherwise
* @license MIT
* @source https://github.com/symfony/security-core/blob/56721d5f5f63da7e08d05aa7668a5a9ef2367e1e/Util/StringUtils.php
*/
private static function equals($knownString, $userInput) {
// Avoid making unnecessary duplications of secret data
if (!is_string($knownString)) {
$knownString = (string) $knownString;
}
if (!is_string($userInput)) {
$userInput = (string) $userInput;
}
if (function_exists('hash_equals')) {
return hash_equals($knownString, $userInput);
}
$knownLen = self::safeStrlen($knownString);
$userLen = self::safeStrlen($userInput);
if ($userLen !== $knownLen) {
return false;
}
$result = 0;
for ($i = 0; $i < $knownLen; ++$i) {
$result |= (ord($knownString[$i]) ^ ord($userInput[$i]));
}
// They are only identical strings if $result is exactly 0...
return 0 === $result;
}
/**
* Returns the number of bytes in a string.
*
* @param string $string The string whose length we wish to obtain
*
* @return int
* @license MIT
* @source https://github.com/symfony/security-core/blob/56721d5f5f63da7e08d05aa7668a5a9ef2367e1e/Util/StringUtils.php
*/
private static function safeStrlen($string) {
// Premature optimization
// Since this cannot be changed at runtime, we can cache it
static $funcExists = null;
if (null === $funcExists) {
$funcExists = function_exists('mb_strlen');
}
if ($funcExists) {
return mb_strlen($string, '8bit');
}
return strlen($string);
}
/**
* Whether the current user is authenticated
*
* @return bool
*/
public function isAuthenticated() {
$storedHash = $this->updater->getConfigOption('updater.secret');
public function isAuthenticated(): bool {
$storedHash = $this->updater->getConfigOptionString('updater.secret');
// As a sanity check the stored hash or the sent password can never be empty
if ($storedHash === '' || $storedHash === null || $this->password === null) {
// As a sanity check the stored hash can never be empty
if ($storedHash === '' || $storedHash === null) {
return false;
}
// As we still support PHP 5.4 we have to use some magic involving "crypt"
return $this->equals($storedHash, crypt($this->password, $storedHash));
return password_verify($this->password, $storedHash);
}
}
@ -143,21 +76,24 @@ try {
}
// Check for authentication
$password = isset($_SERVER['HTTP_X_UPDATER_AUTH']) ? $_SERVER['HTTP_X_UPDATER_AUTH'] : (isset($_POST['updater-secret-input']) ? $_POST['updater-secret-input'] : '');
$password = ($_SERVER['HTTP_X_UPDATER_AUTH'] ?? $_POST['updater-secret-input'] ?? '');
if (!is_string($password)) {
die('Invalid type ' . gettype($password) . ' for password');
}
$auth = new Auth($updater, $password);
// Check if already a step is in process
$currentStep = $updater->currentStep();
$stepNumber = 0;
if ($currentStep !== []) {
$stepState = $currentStep['state'];
$stepNumber = $currentStep['step'];
$stepState = (string)$currentStep['state'];
$stepNumber = (int)$currentStep['step'];
$updater->log('[info] Step ' . $stepNumber . ' is in state "' . $stepState . '".');
if ($stepState === 'start') {
die(
sprintf(
'Step %s is currently in process. Please reload this page later or remove the following file to start from scratch: %s',
'Step %d is currently in process. Please reload this page later or remove the following file to start from scratch: %s',
$stepNumber,
$this->updater->getUpdateStepFileLocation()
)
@ -165,7 +101,7 @@ if ($currentStep !== []) {
}
}
if (isset($_POST['step'])) {
if (isset($_POST['step']) && !is_array($_POST['step'])) {
$updater->log('[info] POST request for step "' . $_POST['step'] . '"');
set_time_limit(0);
try {
@ -220,20 +156,20 @@ if (isset($_POST['step'])) {
$updater->endStep($step);
echo(json_encode(['proceed' => true]));
} catch (UpdateException $e) {
$message = $e->getData();
$data = $e->getData();
try {
$updater->log('[error] POST request failed with UpdateException');
$updater->logException($e);
} catch (LogException $logE) {
$message .= ' (and writing to log failed also with: ' . $logE->getMessage() . ')';
$data[] = ' (and writing to log failed also with: ' . $logE->getMessage() . ')';
}
if (isset($step)) {
$updater->rollbackChanges($step);
}
http_response_code(500);
echo(json_encode(['proceed' => false, 'response' => $message]));
echo(json_encode(['proceed' => false, 'response' => $data]));
} catch (\Exception $e) {
$message = $e->getMessage();

View File

@ -31,6 +31,7 @@ class RecursiveDirectoryIteratorWithoutData extends \RecursiveFilterIterator {
'..',
];
/** @var \SplFileInfo|false */
$current = $this->current();
if (!$current) {
return false;

View File

@ -25,6 +25,7 @@
namespace NC\Updater;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\QuestionHelper;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
@ -36,7 +37,7 @@ class UpdateCommand extends Command {
protected bool $skipBackup = false;
protected bool $skipUpgrade = false;
/** @var array strings of text for stages of updater */
/** @var list<string> strings of text for stages of updater */
protected array $checkTexts = [
0 => '',
1 => 'Check for expected files',
@ -72,8 +73,8 @@ class UpdateCommand extends Command {
}
protected function execute(InputInterface $input, OutputInterface $output) {
$this->skipBackup = $input->getOption('no-backup');
$this->skipUpgrade = $input->getOption('no-upgrade');
$this->skipBackup = (bool)$input->getOption('no-backup');
$this->skipUpgrade = (bool)$input->getOption('no-upgrade');
$version = static::getUpdaterVersion();
$output->writeln('Nextcloud Updater - version: ' . $version);
@ -126,14 +127,14 @@ class UpdateCommand extends Command {
$currentStep = $this->updater->currentStep();
$stepNumber = 0;
if ($currentStep !== []) {
$stepState = $currentStep['state'];
$stepNumber = $currentStep['step'];
$stepState = (string)$currentStep['state'];
$stepNumber = (int)$currentStep['step'];
$this->updater->log('[info] Step ' . $stepNumber . ' is in state "' . $stepState . '".');
if ($stepState === 'start') {
$output->writeln(
sprintf(
'Step %s is currently in process. Please call this command later or remove the following file to start from scratch: %s',
'Step %d is currently in process. Please call this command later or remove the following file to start from scratch: %s',
$stepNumber,
$this->updater->getUpdateStepFileLocation()
)
@ -175,6 +176,7 @@ class UpdateCommand extends Command {
$output->writeln('');
/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion($questionText . '? [y/N] ', false);
@ -288,6 +290,7 @@ class UpdateCommand extends Command {
if ($input->isInteractive()) {
$output->writeln('');
/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion('Should the "occ upgrade" command be executed? [Y/n] ', true);
@ -308,6 +311,7 @@ class UpdateCommand extends Command {
$output->writeln('');
if ($input->isInteractive()) {
/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion($this->checkTexts[11] . ' [y/N] ', false);
@ -345,7 +349,7 @@ class UpdateCommand extends Command {
}
/**
* @return array{proceed:bool,response:string|array} with options 'proceed' which is a boolean and defines if the step succeeded and an optional 'response' string or array
* @return array{proceed:bool,response:string|list<string>} with options 'proceed' which is a boolean and defines if the step succeeded and an optional 'response' string or array
*/
protected function executeStep(int $step): array {
if ($this->updater === null) {

View File

@ -23,11 +23,14 @@
namespace NC\Updater;
class UpdateException extends \Exception {
/** @param list<string> $data */
public function __construct(
protected array $data,
) {
}
/** @return list<string> */
public function getData(): array {
return $this->data;
}

View File

@ -163,11 +163,9 @@ class Updater {
/**
* Returns the specified config option
*
* @return mixed|null Null if the entry is not found
*/
public function getConfigOption(string $key) {
return isset($this->configValues[$key]) ? $this->configValues[$key] : null;
public function getConfigOption(string $key): mixed {
return $this->configValues[$key] ?? null;
}
/**
@ -265,15 +263,14 @@ class Updater {
if (!is_array($appsPaths)) {
throw new \Exception('Configuration key apps_paths should be an array');
}
foreach ($appsPaths as $appsPath) {
if (isset($appsPath['path']) && is_string($appsPath['path'])) {
$appPath = $appsPath['path'];
} else {
if (!is_array($appsPath) || !isset($appsPath['path']) || !is_string($appsPath['path'])) {
throw new \Exception('Invalid configuration in apps_paths configuration key');
}
$parentDir = realpath($this->baseDir . '/../');
$appDir = basename($appPath);
if (strpos($appPath, $parentDir) === 0 && $appDir !== 'apps') {
$appDir = basename($appsPath['path']);
if (strpos($appsPath['path'], $parentDir) === 0 && $appDir !== 'apps') {
$expected[] = $appDir;
}
}
@ -1078,18 +1075,20 @@ EOF;
$this->silentLog('[info] currentStep()');
$updaterDir = $this->getUpdateDirectoryLocation() . '/updater-'.$this->getConfigOptionMandatoryString('instanceid');
$jsonData = [];
if (file_exists($updaterDir. '/.step')) {
$state = file_get_contents($updaterDir . '/.step');
if ($state === false) {
throw new \Exception('Could not read from .step');
}
$jsonData = json_decode($state, true);
if (!is_array($jsonData)) {
throw new \Exception('Can\'t decode .step JSON data');
}
if (!file_exists($updaterDir. '/.step')) {
return [];
}
$state = file_get_contents($updaterDir . '/.step');
if ($state === false) {
throw new \Exception('Could not read from .step');
}
$jsonData = json_decode($state, true);
if (!is_array($jsonData)) {
throw new \Exception('Can\'t decode .step JSON data');
}
return $jsonData;
}

View File

@ -11,6 +11,7 @@
<ignoreFiles>
<directory name="vendor" />
</ignoreFiles>
<file name="index.php" />
</projectFiles>
<extraFiles>
<directory name="vendor" />