From cf56157ec720eec588b2b8e511855d08a3ec4acd Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Sat, 26 Jan 2019 00:51:28 +0100 Subject: [PATCH] Move password confirmation validation to frontends Since this is not strictly speaking a domain invariant, but rather specific to the user interface where passwords are not displayed, and should therefore be entered twice to prevent mistakes going unnoticed, this stuff should be checked in the frontend, not in the install steps. Next step: Ensure that all domain-specific validation is done in the installer's domain layer. This will ensure these validations cannot be forgotten, and keep the frontends DRY. --- .../src/Install/Console/FileDataProvider.php | 1 - .../src/Install/Console/InstallCommand.php | 8 --- .../src/Install/Console/UserDataProvider.php | 34 ++++++++-- .../Install/Controller/InstallController.php | 68 ++++++++++++------- .../src/Install/Steps/CreateAdminUser.php | 5 -- .../core/src/Install/ValidationFailed.php | 18 +++++ 6 files changed, 90 insertions(+), 44 deletions(-) create mode 100644 framework/core/src/Install/ValidationFailed.php diff --git a/framework/core/src/Install/Console/FileDataProvider.php b/framework/core/src/Install/Console/FileDataProvider.php index 09975b861..bd39e7deb 100644 --- a/framework/core/src/Install/Console/FileDataProvider.php +++ b/framework/core/src/Install/Console/FileDataProvider.php @@ -77,7 +77,6 @@ class FileDataProvider implements DataProviderInterface return $this->adminUser + [ 'username' => 'admin', 'password' => 'password', - 'password_confirmation' => 'password', 'email' => 'admin@example.com', ]; } diff --git a/framework/core/src/Install/Console/InstallCommand.php b/framework/core/src/Install/Console/InstallCommand.php index 7213e5ef2..1138ba657 100644 --- a/framework/core/src/Install/Console/InstallCommand.php +++ b/framework/core/src/Install/Console/InstallCommand.php @@ -120,14 +120,6 @@ class InstallCommand extends AbstractCommand $admin = $this->dataSource->getAdminUser(); - if (strlen($admin['password']) < 8) { - throw new Exception('Password must be at least 8 characters.'); - } - - if ($admin['password'] !== $admin['password_confirmation']) { - throw new Exception('The password did not match its confirmation.'); - } - if (! filter_var($admin['email'], FILTER_VALIDATE_EMAIL)) { throw new Exception('You must enter a valid email.'); } diff --git a/framework/core/src/Install/Console/UserDataProvider.php b/framework/core/src/Install/Console/UserDataProvider.php index 2de2a2677..6641f4265 100644 --- a/framework/core/src/Install/Console/UserDataProvider.php +++ b/framework/core/src/Install/Console/UserDataProvider.php @@ -64,13 +64,33 @@ class UserDataProvider implements DataProviderInterface public function getAdminUser() { return [ - 'username' => $this->ask('Admin username:'), - 'password' => $this->secret('Admin password:'), - 'password_confirmation' => $this->secret('Admin password (confirmation):'), - 'email' => $this->ask('Admin email address:'), + 'username' => $this->ask('Admin username:'), + 'password' => $this->askForAdminPassword(), + 'email' => $this->ask('Admin email address:'), ]; } + private function askForAdminPassword() + { + while (true) { + $password = $this->secret('Admin password:'); + + if (strlen($password) < 8) { + $this->validationError('Password must be at least 8 characters.'); + continue; + } + + $confirmation = $this->secret('Admin password (confirmation):'); + + if ($password !== $confirmation) { + $this->validationError('The password did not match its confirmation.'); + continue; + } + + return $password; + } + } + public function getSettings() { $title = $this->ask('Forum title:'); @@ -99,6 +119,12 @@ class UserDataProvider implements DataProviderInterface return $this->questionHelper->ask($this->input, $this->output, $question); } + protected function validationError($message) + { + $this->output->writeln("$message"); + $this->output->writeln('Please try again.'); + } + public function isDebugMode(): bool { return false; diff --git a/framework/core/src/Install/Controller/InstallController.php b/framework/core/src/Install/Controller/InstallController.php index 8dd46e9a3..e3c0e6ba4 100644 --- a/framework/core/src/Install/Controller/InstallController.php +++ b/framework/core/src/Install/Controller/InstallController.php @@ -14,6 +14,7 @@ namespace Flarum\Install\Controller; use Flarum\Http\SessionAuthenticator; use Flarum\Install\Installation; use Flarum\Install\StepFailed; +use Flarum\Install\ValidationFailed; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\RequestHandlerInterface; @@ -59,32 +60,35 @@ class InstallController implements RequestHandlerInterface $baseUrl = rtrim((string) $request->getUri(), '/'); - $pipeline = $this->installation - ->baseUrl($baseUrl) - ->databaseConfig([ - 'driver' => 'mysql', - 'host' => $host, - 'port' => $port, - 'database' => array_get($input, 'mysqlDatabase'), - 'username' => array_get($input, 'mysqlUsername'), - 'password' => array_get($input, 'mysqlPassword'), - 'charset' => 'utf8mb4', - 'collation' => 'utf8mb4_unicode_ci', - 'prefix' => array_get($input, 'tablePrefix'), - 'strict' => false, - ]) - ->adminUser([ - 'username' => array_get($input, 'adminUsername'), - 'password' => array_get($input, 'adminPassword'), - 'password_confirmation' => array_get($input, 'adminPasswordConfirmation'), - 'email' => array_get($input, 'adminEmail'), - ]) - ->settings([ - 'forum_title' => array_get($input, 'forumTitle'), - 'mail_from' => 'noreply@'.preg_replace('/^www\./i', '', parse_url($baseUrl, PHP_URL_HOST)), - 'welcome_title' => 'Welcome to '.array_get($input, 'forumTitle'), - ]) - ->build(); + try { + $pipeline = $this->installation + ->baseUrl($baseUrl) + ->databaseConfig([ + 'driver' => 'mysql', + 'host' => $host, + 'port' => $port, + 'database' => array_get($input, 'mysqlDatabase'), + 'username' => array_get($input, 'mysqlUsername'), + 'password' => array_get($input, 'mysqlPassword'), + 'charset' => 'utf8mb4', + 'collation' => 'utf8mb4_unicode_ci', + 'prefix' => array_get($input, 'tablePrefix'), + 'strict' => false, + ]) + ->adminUser([ + 'username' => array_get($input, 'adminUsername'), + 'password' => $this->getConfirmedAdminPassword($input), + 'email' => array_get($input, 'adminEmail'), + ]) + ->settings([ + 'forum_title' => array_get($input, 'forumTitle'), + 'mail_from' => 'noreply@'.preg_replace('/^www\./i', '', parse_url($baseUrl, PHP_URL_HOST)), + 'welcome_title' => 'Welcome to '.array_get($input, 'forumTitle'), + ]) + ->build(); + } catch (ValidationFailed $e) { + return new Response\HtmlResponse($e->getMessage(), 500); + } try { $pipeline->run(); @@ -97,4 +101,16 @@ class InstallController implements RequestHandlerInterface return new Response\EmptyResponse; } + + private function getConfirmedAdminPassword(array $input) + { + $password = array_get($input, 'adminPassword'); + $confirmation = array_get($input, 'adminPasswordConfirmation'); + + if ($password !== $confirmation) { + throw new ValidationFailed('The admin password did not match its confirmation.'); + } + + return $password; + } } diff --git a/framework/core/src/Install/Steps/CreateAdminUser.php b/framework/core/src/Install/Steps/CreateAdminUser.php index af51fa04d..faed9c6c3 100644 --- a/framework/core/src/Install/Steps/CreateAdminUser.php +++ b/framework/core/src/Install/Steps/CreateAdminUser.php @@ -16,7 +16,6 @@ use Flarum\Group\Group; use Flarum\Install\Step; use Illuminate\Database\ConnectionInterface; use Illuminate\Hashing\BcryptHasher; -use UnexpectedValueException; class CreateAdminUser implements Step { @@ -43,10 +42,6 @@ class CreateAdminUser implements Step public function run() { - if ($this->admin['password'] !== $this->admin['password_confirmation']) { - throw new UnexpectedValueException('The password did not match its confirmation.'); - } - $uid = $this->database->table('users')->insertGetId([ 'username' => $this->admin['username'], 'email' => $this->admin['email'], diff --git a/framework/core/src/Install/ValidationFailed.php b/framework/core/src/Install/ValidationFailed.php new file mode 100644 index 000000000..d79206637 --- /dev/null +++ b/framework/core/src/Install/ValidationFailed.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Install; + +use Exception; + +class ValidationFailed extends Exception +{ +}