Skip to content

Commit

Permalink
Merge pull request #478 from nextcloud/enh/strong-type-classes
Browse files Browse the repository at this point in the history
  • Loading branch information
@skjnldsv
skjnldsv authored Aug 8, 2023
2 parents c47b3d5 + 15136da commit 07f0219
Show file tree
Hide file tree
Showing 9 changed files with 402 additions and 495 deletions.
417 changes: 186 additions & 231 deletions index.php
View file

Large diffs are not rendered by default.

106 changes: 21 additions & 85 deletions index.web.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -143,29 +76,32 @@ public function isAuthenticated() {
}

// 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()
$updater->getUpdateStepFileLocation()
)
);
}
}

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 {
Expand Down Expand Up @@ -220,20 +156,20 @@ public function isAuthenticated() {
$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();

Expand Down
2 changes: 1 addition & 1 deletion lib/RecursiveDirectoryIteratorWithoutData.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@

class RecursiveDirectoryIteratorWithoutData extends \RecursiveFilterIterator {
public function accept(): bool {
/** @var \DirectoryIterator $this */
$excludes = [
'.rnd',
'.well-known',
'data',
'..',
];

/** @var \SplFileInfo|false */
$current = $this->current();
if (!$current) {
return false;
Expand Down
56 changes: 26 additions & 30 deletions lib/UpdateCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,20 @@
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;
use Symfony\Component\Console\Question\ConfirmationQuestion;

class UpdateCommand extends Command {
/** @var Updater */
protected $updater;

/** @var bool */
protected $shouldStop = false;

/** @var bool */
protected $skipBackup = false;

protected ?Updater $updater = null;
protected bool $shouldStop = false;
protected bool $skipBackup = false;
protected bool $skipUpgrade = false;

/** @var array strings of text for stages of updater */
protected $checkTexts = [
/** @var list<string> strings of text for stages of updater */
protected array $checkTexts = [
0 => '',
1 => 'Check for expected files',
2 => 'Check for write permissions',
Expand All @@ -59,7 +54,7 @@ class UpdateCommand extends Command {
12 => 'Done',
];

protected function configure() {
protected function configure(): void {
$this
->setName('update')
->setDescription('Updates the code of an Nextcloud instance')
Expand All @@ -78,8 +73,8 @@ public static function getUpdaterVersion(): string {
}

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);
Expand Down Expand Up @@ -132,14 +127,14 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$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()
)
Expand Down Expand Up @@ -181,6 +176,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {

$output->writeln('');

/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion($questionText . '? [y/N] ', false);

Expand Down Expand Up @@ -294,6 +290,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
if ($input->isInteractive()) {
$output->writeln('');

/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion('Should the "occ upgrade" command be executed? [Y/n] ', true);

Expand All @@ -314,6 +311,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {

$output->writeln('');
if ($input->isInteractive()) {
/** @var QuestionHelper */
$helper = $this->getHelper('question');
$question = new ConfirmationQuestion($this->checkTexts[11] . ' [y/N] ', false);

Expand Down Expand Up @@ -351,10 +349,12 @@ protected function execute(InputInterface $input, OutputInterface $output) {
}

/**
* @param $step integer
* @return array with options 'proceed' which is a boolean and defines if the step succeeded and an optional 'response' string
* @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($step) {
protected function executeStep(int $step): array {
if ($this->updater === null) {
return ['proceed' => false, 'response' => 'Initialization problem'];
}
$this->updater->log('[info] executeStep request for step "' . $step . '"');
try {
if ($step > 12 || $step < 1) {
Expand Down Expand Up @@ -404,19 +404,19 @@ protected function executeStep($step) {
break;
}
$this->updater->endStep($step);
return ['proceed' => true];
return ['proceed' => true, 'response' => ''];
} catch (UpdateException $e) {
$message = $e->getData();
$data = $e->getData();

try {
$this->updater->log('[error] executeStep request failed with UpdateException');
$this->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() . ')';
}

$this->updater->rollbackChanges($step);
return ['proceed' => false, 'response' => $message];
return ['proceed' => false, 'response' => $data];
} catch (\Exception $e) {
$message = $e->getMessage();

Expand All @@ -432,11 +432,7 @@ protected function executeStep($step) {
}
}

/**
* @param OutputInterface $output
* @param integer $stepNumber
*/
protected function showCurrentStatus(OutputInterface $output, $stepNumber) {
protected function showCurrentStatus(OutputInterface $output, int $stepNumber): void {
$output->writeln('Steps that will be executed:');
for ($i = 1; $i < sizeof($this->checkTexts); $i++) {
if ($i === 11) {
Expand All @@ -456,7 +452,7 @@ protected function showCurrentStatus(OutputInterface $output, $stepNumber) {
/**
* gets called by the PCNTL listener once the stop/terminate signal
*/
public function stopCommand() {
public function stopCommand(): void {
$this->shouldStop = true;
}
}
10 changes: 6 additions & 4 deletions lib/UpdateException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
namespace NC\Updater;

class UpdateException extends \Exception {
protected $data;

public function __construct($data) {
$this->data = $data;
/** @param list<string> $data */
public function __construct(
protected array $data,
) {
}

public function getData() {
/** @return list<string> */
public function getData(): array {
return $this->data;
}
}
Loading

0 comments on commit 07f0219

Please sign in to comment.