From b209f95479e5650ba461e29df387ccd5924e29f9 Mon Sep 17 00:00:00 2001 From: Bernard Date: Wed, 30 Oct 2024 20:00:50 +0000 Subject: [PATCH 1/5] 2FA for webtrees tested amended and fixed --- app/Contracts/UserInterface.php | 1 + app/Http/RequestHandlers/AccountEdit.php | 3 ++ app/Http/RequestHandlers/AccountUpdate.php | 9 ++++ app/Http/RequestHandlers/LoginAction.php | 18 +++++-- app/Http/RequestHandlers/RegisterAction.php | 6 ++- app/Http/RequestHandlers/SetupWizard.php | 2 + .../SiteRegistrationAction.php | 2 + app/Http/RequestHandlers/UserAddAction.php | 1 + app/Http/RequestHandlers/UserEditAction.php | 2 + app/Schema/Migration0.php | 1 + app/Schema/SeedUserTable.php | 1 + app/Services/UserService.php | 1 + app/User.php | 53 +++++++++++++++++++ composer.json | 2 + public/js/totp.js | 31 +++++++++++ resources/views/admin/site-registration.phtml | 15 ++++++ resources/views/admin/users-edit.phtml | 1 + resources/views/edit-account-page.phtml | 21 +++++++- resources/views/layouts/default.phtml | 1 + resources/views/login-page.phtml | 9 +++- .../views/setup/step-5-administrator.phtml | 2 + .../RequestHandlers/AccountUpdateTest.php | 7 +-- .../RequestHandlers/UserEditActionTest.php | 1 + 23 files changed, 180 insertions(+), 10 deletions(-) create mode 100644 public/js/totp.js diff --git a/app/Contracts/UserInterface.php b/app/Contracts/UserInterface.php index 8cc27b9f40a..63f47124bd3 100644 --- a/app/Contracts/UserInterface.php +++ b/app/Contracts/UserInterface.php @@ -31,6 +31,7 @@ interface UserInterface public const PREF_IS_ADMINISTRATOR = 'canadmin'; public const PREF_IS_EMAIL_VERIFIED = 'verified'; public const PREF_IS_VISIBLE_ONLINE = 'visibleonline'; + public const PREF_IS_STATUS_MFA = 'statusmfa'; public const PREF_LANGUAGE = 'language'; public const PREF_NEW_ACCOUNT_COMMENT = 'comment'; public const PREF_TIMESTAMP_REGISTERED = 'reg_timestamp'; diff --git a/app/Http/RequestHandlers/AccountEdit.php b/app/Http/RequestHandlers/AccountEdit.php index c69f0d0bdd8..f27e776d344 100644 --- a/app/Http/RequestHandlers/AccountEdit.php +++ b/app/Http/RequestHandlers/AccountEdit.php @@ -28,6 +28,7 @@ use Fisharebest\Webtrees\Registry; use Fisharebest\Webtrees\Services\MessageService; use Fisharebest\Webtrees\Services\ModuleService; +use Fisharebest\Webtrees\Site; use Fisharebest\Webtrees\Tree; use Fisharebest\Webtrees\Validator; use Psr\Http\Message\ResponseInterface; @@ -83,6 +84,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface }); $show_delete_option = $user->getPreference(UserInterface::PREF_IS_ADMINISTRATOR) !== '1'; + $show_2fa = Site::getPreference('SHOW_2FA_OPTION') === '1'; $timezone_ids = DateTimeZone::listIdentifiers(); $timezones = array_combine($timezone_ids, $timezone_ids); $title = I18N::translate('My account'); @@ -93,6 +95,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface 'languages' => $languages->all(), 'my_individual_record' => $my_individual_record, 'show_delete_option' => $show_delete_option, + 'show_2fa' => $show_2fa, 'timezones' => $timezones, 'title' => $title, 'tree' => $tree, diff --git a/app/Http/RequestHandlers/AccountUpdate.php b/app/Http/RequestHandlers/AccountUpdate.php index 3bfa21e05db..0804a88bd0a 100644 --- a/app/Http/RequestHandlers/AccountUpdate.php +++ b/app/Http/RequestHandlers/AccountUpdate.php @@ -67,15 +67,23 @@ public function handle(ServerRequestInterface $request): ResponseInterface $language = Validator::parsedBody($request)->string('language'); $real_name = Validator::parsedBody($request)->string('real_name'); $password = Validator::parsedBody($request)->string('password'); + $secret = Validator::parsedBody($request)->string('secret'); $time_zone = Validator::parsedBody($request)->string('timezone'); $user_name = Validator::parsedBody($request)->string('user_name'); $visible_online = Validator::parsedBody($request)->boolean('visible-online', false); + $status_mfa = Validator::parsedBody($request)->boolean('status-mfa', false); + // Change the password if ($password !== '') { $user->setPassword($password); } + // Change the secret + if ($secret !== '' || $status_mfa === false) { + $user->setSecret($secret); + } + // Change the username if ($user_name !== $user->userName()) { if ($this->user_service->findByUserName($user_name) === null) { @@ -99,6 +107,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $user->setPreference(UserInterface::PREF_LANGUAGE, $language); $user->setPreference(UserInterface::PREF_TIME_ZONE, $time_zone); $user->setPreference(UserInterface::PREF_IS_VISIBLE_ONLINE, (string) $visible_online); + $user->setPreference(UserInterface::PREF_IS_STATUS_MFA, (string) $status_mfa); if ($tree instanceof Tree) { $default_xref = Validator::parsedBody($request)->string('default-xref'); diff --git a/app/Http/RequestHandlers/LoginAction.php b/app/Http/RequestHandlers/LoginAction.php index 64744f3cdda..f22b522990c 100644 --- a/app/Http/RequestHandlers/LoginAction.php +++ b/app/Http/RequestHandlers/LoginAction.php @@ -69,10 +69,11 @@ public function handle(ServerRequestInterface $request): ResponseInterface $default_url = route(HomePage::class); $username = Validator::parsedBody($request)->string('username'); $password = Validator::parsedBody($request)->string('password'); + $code2fa = Validator::parsedBody($request)->string('code2fa'); $url = Validator::parsedBody($request)->isLocalUrl()->string('url', $default_url); try { - $this->doLogin($username, $password); + $this->doLogin($username, $password, $code2fa); if (Auth::isAdmin() && $this->upgrade_service->isUpgradeAvailable()) { FlashMessages::addMessage(I18N::translate('A new version of webtrees is available.') . ' ' . I18N::translate('Upgrade to webtrees %s.', '' . $this->upgrade_service->latestVersion() . '') . ''); @@ -97,11 +98,12 @@ public function handle(ServerRequestInterface $request): ResponseInterface * * @param string $username * @param string $password + * @param string $code2fa * * @return void * @throws Exception */ - private function doLogin(string $username, #[\SensitiveParameter] string $password): void + private function doLogin(string $username, #[\SensitiveParameter] string $password, string $code2fa): void { if ($_COOKIE === []) { Log::addAuthenticationLog('Login failed (no session cookies): ' . $username); @@ -129,7 +131,17 @@ private function doLogin(string $username, #[\SensitiveParameter] string $passwo Log::addAuthenticationLog('Login failed (not approved by admin): ' . $username); throw new Exception(I18N::translate('This account has not been approved. Please wait for an administrator to approve it.')); } - + if ($user->getPreference(UserInterface::PREF_IS_STATUS_MFA) !== '') { + # covers scenario where 2fa not enabled by user + if($code2fa != '') { + if (!$user->check2FAcode($code2fa)) { + throw new Exception(I18N::translate('2FA code does not match. Please try again.')); + } + } + else { + throw new Exception(I18N::translate('2FA code must be entered as you have 2FA authentication enabled. Please try again.')); + } + } Auth::login($user); Log::addAuthenticationLog('Login: ' . Auth::user()->userName() . '/' . Auth::user()->realName()); Auth::user()->setPreference(UserInterface::PREF_TIMESTAMP_ACTIVE, (string) time()); diff --git a/app/Http/RequestHandlers/RegisterAction.php b/app/Http/RequestHandlers/RegisterAction.php index 3fcc6e6caed..82d3c5c6afe 100644 --- a/app/Http/RequestHandlers/RegisterAction.php +++ b/app/Http/RequestHandlers/RegisterAction.php @@ -96,13 +96,14 @@ public function handle(ServerRequestInterface $request): ResponseInterface $password = Validator::parsedBody($request)->string('password'); $realname = Validator::parsedBody($request)->string('realname'); $username = Validator::parsedBody($request)->string('username'); + $secret = Validator::parsedBody($request)->string('secret'); try { if ($this->captcha_service->isRobot($request)) { throw new Exception(I18N::translate('Please try again.')); } - $this->doValidateRegistration($request, $username, $email, $realname, $comments, $password); + $this->doValidateRegistration($request, $username, $email, $realname, $comments, $password, $secret); Session::forget('register_comments'); Session::forget('register_email'); @@ -135,6 +136,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $user->setPreference(UserInterface::PREF_CONTACT_METHOD, MessageService::CONTACT_METHOD_INTERNAL_AND_EMAIL); $user->setPreference(UserInterface::PREF_NEW_ACCOUNT_COMMENT, $comments); $user->setPreference(UserInterface::PREF_IS_VISIBLE_ONLINE, '1'); + $user->setPreference(UserInterface::PREF_IS_STATUS_MFA, '0'); $user->setPreference(UserInterface::PREF_AUTO_ACCEPT_EDITS, ''); $user->setPreference(UserInterface::PREF_IS_ADMINISTRATOR, ''); $user->setPreference(UserInterface::PREF_TIMESTAMP_ACTIVE, '0'); @@ -245,7 +247,7 @@ private function doValidateRegistration( ServerRequestInterface $request, string $username, string $email, - string $realname, + string $realname, string $comments, #[\SensitiveParameter] string $password ): void { diff --git a/app/Http/RequestHandlers/SetupWizard.php b/app/Http/RequestHandlers/SetupWizard.php index b6e92b5a6df..48b4575c831 100644 --- a/app/Http/RequestHandlers/SetupWizard.php +++ b/app/Http/RequestHandlers/SetupWizard.php @@ -83,6 +83,7 @@ class SetupWizard implements RequestHandlerInterface 'wtuser' => '', 'wtpass' => '', 'wtemail' => '', + 'wtsecret' => '', ]; private const DEFAULT_PORTS = [ @@ -408,6 +409,7 @@ private function createConfigFile(array $data): void $admin = $this->user_service->create($data['wtuser'], $data['wtname'], $data['wtemail'], $data['wtpass']); $admin->setPreference(UserInterface::PREF_LANGUAGE, $data['lang']); $admin->setPreference(UserInterface::PREF_IS_VISIBLE_ONLINE, '1'); + $admin->setPreference(UserInterface::PREF_IS_STATUS_MFA, '0'); } else { $admin->setPassword($_POST['wtpass']); } diff --git a/app/Http/RequestHandlers/SiteRegistrationAction.php b/app/Http/RequestHandlers/SiteRegistrationAction.php index 6e58dfec0fa..b2c8cb93bc5 100644 --- a/app/Http/RequestHandlers/SiteRegistrationAction.php +++ b/app/Http/RequestHandlers/SiteRegistrationAction.php @@ -46,11 +46,13 @@ public function handle(ServerRequestInterface $request): ResponseInterface $text = Validator::parsedBody($request)->string('WELCOME_TEXT_AUTH_MODE_4'); $allow_registration = Validator::parsedBody($request)->boolean('USE_REGISTRATION_MODULE'); $show_caution = Validator::parsedBody($request)->boolean('SHOW_REGISTER_CAUTION'); + $show_2fa = Validator::parsedBody($request)->boolean('SHOW_2FA_OPTION'); Site::setPreference('WELCOME_TEXT_AUTH_MODE', $mode); Site::setPreference('WELCOME_TEXT_AUTH_MODE_' . I18N::languageTag(), $text); Site::setPreference('USE_REGISTRATION_MODULE', (string) $allow_registration); Site::setPreference('SHOW_REGISTER_CAUTION', (string) $show_caution); + Site::setPreference('SHOW_2FA_OPTION', (string) $show_2fa); FlashMessages::addMessage(I18N::translate('The website preferences have been updated.'), 'success'); diff --git a/app/Http/RequestHandlers/UserAddAction.php b/app/Http/RequestHandlers/UserAddAction.php index eb14172c116..a00f35b2eb1 100644 --- a/app/Http/RequestHandlers/UserAddAction.php +++ b/app/Http/RequestHandlers/UserAddAction.php @@ -59,6 +59,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $real_name = Validator::parsedBody($request)->string('real_name'); $email = Validator::parsedBody($request)->string('email'); $password = Validator::parsedBody($request)->string('password'); + $secret = ""; $errors = false; diff --git a/app/Http/RequestHandlers/UserEditAction.php b/app/Http/RequestHandlers/UserEditAction.php index 3d3f1191b74..aca117a1bf6 100644 --- a/app/Http/RequestHandlers/UserEditAction.php +++ b/app/Http/RequestHandlers/UserEditAction.php @@ -75,6 +75,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $real_name = Validator::parsedBody($request)->string('real_name'); $email = Validator::parsedBody($request)->string('email'); $password = Validator::parsedBody($request)->string('password'); + $secret = Validator::parsedBody($request)->string('secret'); $theme = Validator::parsedBody($request)->string('theme'); $language = Validator::parsedBody($request)->string('language'); $timezone = Validator::parsedBody($request)->string('timezone'); @@ -84,6 +85,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $canadmin = Validator::parsedBody($request)->boolean('canadmin', false); $visible_online = Validator::parsedBody($request)->boolean('visible-online', false); $verified = Validator::parsedBody($request)->boolean('verified', false); + $status_mfa = Validator::parsedBody($request)->boolean('status-mfa', false); $approved = Validator::parsedBody($request)->boolean('approved', false); $edit_user = $this->user_service->find($user_id); diff --git a/app/Schema/Migration0.php b/app/Schema/Migration0.php index e536dff521b..3773b896d2e 100644 --- a/app/Schema/Migration0.php +++ b/app/Schema/Migration0.php @@ -61,6 +61,7 @@ public function upgrade(): void $table->string('real_name', 64); $table->string('email', 64); $table->string('password', 128); + $table->string('secret', 128); $table->unique('user_name'); $table->unique('email'); diff --git a/app/Schema/SeedUserTable.php b/app/Schema/SeedUserTable.php index 17b147237d5..ed45a07f8a3 100644 --- a/app/Schema/SeedUserTable.php +++ b/app/Schema/SeedUserTable.php @@ -46,6 +46,7 @@ public function run(): void 'real_name' => 'DEFAULT_USER', 'email' => 'DEFAULT_USER', 'password' => 'DEFAULT_USER', + 'secret' => 'DEFAULT_USER', ]); if (DB::driverName() === DB::SQL_SERVER) { diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 5d821fd39fa..6bc49b02d30 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -319,6 +319,7 @@ public function create(string $user_name, string $real_name, string $email, #[\S 'real_name' => $real_name, 'email' => $email, 'password' => password_hash($password, PASSWORD_DEFAULT), + 'secret' => '', ]); $user_id = DB::lastInsertId(); diff --git a/app/User.php b/app/User.php index 7d879d225ae..8966b1a8a54 100644 --- a/app/User.php +++ b/app/User.php @@ -21,6 +21,8 @@ use Closure; use Fisharebest\Webtrees\Contracts\UserInterface; +use PragmaRX\Google2FA\Google2FA; +use chillerlan\QRCode\QRCode; use function is_string; @@ -110,7 +112,22 @@ public function realName(): string { return $this->real_name; } + /** + * Generate a QR code image based on 2FA secret and return both. + * + * @return array + */ + public function genQRcode(): array + { + $qrinfo = array(); + $google2fa = new Google2FA(); + $qrinfo['secret'] = $google2fa->generateSecretKey(); + $data = 'otpauth://totp/' . $this->user_id . '?secret=' . $qrinfo['secret'] . '&issuer=' . $_SERVER['SERVER_NAME']; + $qrinfo['qrcode'] = (new QRCode)->render($data); + return $qrinfo; + } + /** * Set the real name of this user. * @@ -243,6 +260,42 @@ public function checkPassword(#[\SensitiveParameter] string $password): bool return false; } + /** + * Set the Secret of this user. + * + * @param string $secret + * + * @return User + */ + public function setSecret(#[\SensitiveParameter] string $secret): User + { + DB::table('user') + ->where('user_id', '=', $this->user_id) + ->update([ + 'secret' => $secret, + ]); + + return $this; + } + + /** + * Validate a supplied 2fa code + * + * @param string $code2fa + * + * @return bool + */ + public function check2facode(string $code2fa): bool + { + $secret = DB::table('user') + ->where('user_id', '=', $this->id()) + ->value('secret'); + $google2fa = new Google2FA; + if($google2fa->verifyKey($secret, $code2fa)) { + return true; + } + return false; + } /** * A closure which will create an object from a database row. diff --git a/composer.json b/composer.json index 6e489a66855..9ee8e150e02 100644 --- a/composer.json +++ b/composer.json @@ -42,6 +42,7 @@ "ext-session": "*", "ext-xml": "*", "aura/router": "3.3.0", + "chillerlan/php-qrcode": "4.4.0", "ezyang/htmlpurifier": "4.17.0", "fig/http-message-util": "1.1.5", "fisharebest/algorithm": "1.6.0", @@ -62,6 +63,7 @@ "nyholm/psr7": "1.8.2", "nyholm/psr7-server": "1.1.0", "oscarotero/middleland": "1.0.1", + "pragmarx/google2fa": "^8.0", "psr/cache": "3.0.0", "psr/http-message": "1.1", "psr/http-server-handler": "1.0.2", diff --git a/public/js/totp.js b/public/js/totp.js new file mode 100644 index 00000000000..c94b53dad25 --- /dev/null +++ b/public/js/totp.js @@ -0,0 +1,31 @@ +$( document ).ready(function() { +// resize the qr code and hide as default in edit user page + $('div#qrcode').css('maxWidth', '300px'); + $('div#qrcode').hide(); + +// show a link to get another qr code if 2fa enabled + if($('input#status-mfa-1').is(':checked')) { + $('input#status-mfa-1').parent().append("  - (Click to generate new QR code) ") + } + +// click to get new qr code and secret +$("a#getnewqr").click(function(){ + $('div#qrcode').show(); + $("a#getnewqr").hide(); + $('input#secret').val($('input#newsecret').val()) +}); +// deal with toggling of 2fa setting to ensure no secret saved if no 2fa required but the secret associated with any generated qr code is saved. + $('input#status-mfa-1').change(function() { + if(this.checked) { + $(this).parent().append("  - (Click to generate new QR code) ") + $('div#qrcode').show(); + $('input#secret').val($('input#newsecret').val()) + } + else { + $('div#qrcode').hide(); + $("a#getnewqr").hide(); + $('input#secret').val(''); + } + }); +}); + diff --git a/resources/views/admin/site-registration.phtml b/resources/views/admin/site-registration.phtml index 930121f0ef7..c628ff7e366 100644 --- a/resources/views/admin/site-registration.phtml +++ b/resources/views/admin/site-registration.phtml @@ -71,6 +71,21 @@ use Fisharebest\Webtrees\Site; + + +
+ + + +
+ 'SHOW_2FA_OPTION', 'options' => [I18N::translate('no'), I18N::translate('yes')], 'selected' => (int) Site::getPreference('SHOW_2FA_OPTION')]) ?> +
+
+
+
+ +
diff --git a/resources/views/admin/users-edit.phtml b/resources/views/admin/users-edit.phtml index da9a2eb2217..f9dff5c343f 100644 --- a/resources/views/admin/users-edit.phtml +++ b/resources/views/admin/users-edit.phtml @@ -36,6 +36,7 @@ use Illuminate\Support\Collection;
+
diff --git a/resources/views/edit-account-page.phtml b/resources/views/edit-account-page.phtml index 991dde6344d..45bef4a9bcf 100644 --- a/resources/views/edit-account-page.phtml +++ b/resources/views/edit-account-page.phtml @@ -14,6 +14,7 @@ use Fisharebest\Webtrees\Tree; * @var array $languages * @var Individual|null $my_individual_record * @var bool $show_delete_option + * @var bool $show_2fa * @var array $timezones * @var string $title * @var Tree|null $tree @@ -153,7 +154,25 @@ use Fisharebest\Webtrees\Tree;
- + +
+ + + +
+ I18N::translate('Enable or disable 2FA status'), 'name' => 'status-mfa', 'checked' => (bool) $user->getPreference(UserInterface::PREF_IS_STATUS_MFA)]) ?> +
+ +
+
+
+ +
+ genQRcode(); ?> + + + QR Code +
diff --git a/resources/views/layouts/default.phtml b/resources/views/layouts/default.phtml index 006026025fa..5b1e7ad03bb 100644 --- a/resources/views/layouts/default.phtml +++ b/resources/views/layouts/default.phtml @@ -144,6 +144,7 @@ use Psr\Http\Message\ServerRequestInterface; +