From f92dc3a44575e9f3fe663b4d1c5def67fa24212e Mon Sep 17 00:00:00 2001 From: Jan Schuppik Date: Sun, 27 Jul 2025 23:11:57 +0200 Subject: [PATCH] WIP: Authentication Working --- application/controllers/AccountController.php | 4 +- application/forms/Account/TotpForm.php | 27 ++- .../forms/Authentication/Challenge2FAForm.php | 6 +- library/Icinga/Authentication/Totp.php | 226 +++++++++++++----- 4 files changed, 191 insertions(+), 72 deletions(-) diff --git a/application/controllers/AccountController.php b/application/controllers/AccountController.php index 38f5abfd4..cad2804a2 100644 --- a/application/controllers/AccountController.php +++ b/application/controllers/AccountController.php @@ -14,6 +14,7 @@ use Icinga\Forms\PreferenceForm; use Icinga\Authentication\Totp; use Icinga\User\Preferences\PreferencesStore; use Icinga\Web\Controller; +use Icinga\Web\Session; /** * My Account @@ -73,7 +74,8 @@ class AccountController extends Controller // create a form to add and enable 2FA via TOTP if ( $user->can('user/two-factor-authentication') ) { - $totp = new Totp($user->getUsername()); + + $totp = Session::getSession()->get('icingaweb_totp', null) ?? new Totp($user->getUsername()); $totpForm = (new TotpForm()) ->setPreferences($user->getPreferences()) ->setTotp($totp); diff --git a/application/forms/Account/TotpForm.php b/application/forms/Account/TotpForm.php index 6a5ed1da5..a34afa15e 100644 --- a/application/forms/Account/TotpForm.php +++ b/application/forms/Account/TotpForm.php @@ -20,6 +20,9 @@ use Icinga\Web\Session; */ class TotpForm extends PreferenceForm { + const PREFERENCE_KEYS = [ + 'enabled_2fa', + ]; protected Totp $totp; /** * {@inheritdoc} @@ -112,7 +115,7 @@ class TotpForm extends PreferenceForm 'btn_submit', array( 'ignore' => true, - 'label' => $this->translate('Save to the Preferences'), + 'label' => $this->translate('Save Change'), 'decorators' => array('ViewHelper'), 'class' => 'btn-primary' ) @@ -135,24 +138,19 @@ class TotpForm extends PreferenceForm */ public function onSuccess() { - - try { if ($this->getElement('btn_submit') && $this->getElement('btn_submit')->isChecked()) { $this->preferences = new Preferences($this->store ? $this->store->load() : array()); $webPreferences = $this->preferences->get('icingaweb'); foreach ($this->getValues() as $key => $value) { - if ($value === '' - || $value === null - || $value === 'autodetect' - ) { - if (isset($webPreferences[$key])) { - unset($webPreferences[$key]); - } - } else { + if (in_array($key, self::PREFERENCE_KEYS, true)) { $webPreferences[$key] = $value; } } + $this->totp->makeStatePersistent(); + if ($webPreferences['enabled_2fa'] == 1) { + $webPreferences['enabled_2fa'] = $this->totp->userHasSecret() ? '1' : '0'; + } $this->preferences->icingaweb = $webPreferences; Session::getSession()->user->setPreferences($this->preferences); $this->save(); @@ -160,17 +158,20 @@ class TotpForm extends PreferenceForm return true; } elseif ($this->getElement('btn_generate_totp') && $this->getElement('btn_generate_totp')->isChecked()) { + $this->totp->generateSecret()->saveTemporaryInSession(); Notification::success($this->translate('Submitted btn_generate_totp')); return true; } elseif ($this->getElement('btn_renew_totp') && $this->getElement('btn_renew_totp')->isChecked()) { + $this->totp->renewSecret()->saveTemporaryInSession(); Notification::success($this->translate('Submitted btn_renew_totp')); return true; } elseif ($this->getElement('btn_delete_totp') && $this->getElement('btn_delete_totp')->isChecked()) { - Notification::info($this->translate('Submitted btn_delete_totp')); + $this->totp->deleteSecret()->saveTemporaryInSession(); + Notification::success($this->translate('Submitted btn_delete_totp')); - return false; + return true; } } catch (Exception $e) { Logger::error($e); diff --git a/application/forms/Authentication/Challenge2FAForm.php b/application/forms/Authentication/Challenge2FAForm.php index 3bb99820e..fd5c59eae 100644 --- a/application/forms/Authentication/Challenge2FAForm.php +++ b/application/forms/Authentication/Challenge2FAForm.php @@ -4,6 +4,7 @@ namespace Icinga\Forms\Authentication; use Icinga\Application\Hook\AuthenticationHook; use Icinga\Authentication\Auth; +use Icinga\Authentication\Totp; use Icinga\Web\Session; use Icinga\Web\Url; @@ -50,7 +51,10 @@ class Challenge2FAForm extends LoginForm public function onSuccess() { // TODO: Implement proper 2FA code validation - if ($_POST['code'] == 666) { + $user = Auth::getInstance()->getUser(); + $totp = new Totp($user->getUsername()); + if ($totp->verify($_POST['code'])) { +// if ($_POST['code'] == 666) { $auth = Auth::getInstance(); $user = $auth->getUser(); diff --git a/library/Icinga/Authentication/Totp.php b/library/Icinga/Authentication/Totp.php index 8dd679215..f6b507673 100644 --- a/library/Icinga/Authentication/Totp.php +++ b/library/Icinga/Authentication/Totp.php @@ -5,13 +5,17 @@ namespace Icinga\Authentication; use Icinga\Clock\PsrClock; use Icinga\Common\Database; use Icinga\Exception\ConfigurationError; +use Icinga\Web\Session; use ipl\Orm\Model; use ipl\Orm\Query; use Icinga\Model\Totp as TotpModel; +use ipl\Sql\Delete; use ipl\Sql\Insert; +use ipl\Sql\Select; use ipl\Sql\Update; use ipl\Stdlib\Filter; use OTPHP\TOTP as extTOTP; +use Zend_Db_Expr; class Totp { @@ -19,6 +23,29 @@ class Totp getDb as private getWebDb; } + /** + * Table name for TOTP records + */ + const TABLE_NAME = 'icingaweb_totp'; + /** + * Column name for secret + */ + const COLUMN_USERNAME = 'username'; + /** + * Column name for secret + */ + const COLUMN_SECRET = 'secret'; + + /** + * Column name for created time + */ + const COLUMN_CREATED_TIME = 'ctime'; + + /** + * Column name for modified time + */ + const COLUMN_MODIFIED_TIME = 'mtime'; + protected string $username; protected PsrClock $clock; protected extTOTP $totpObject; @@ -64,41 +91,134 @@ class Totp public function generateSecret(): self { - $this->temporarySecret = $this->totpObject->getSecret(); + $newSecret = $this->totpObject->getSecret(); + if ($newSecret !== $this->secret && $newSecret !== $this->temporarySecret) { + $this->temporarySecret = $this->totpObject->getSecret(); + } return $this; } - public function setSecretForUser(): self + public function renewSecret(): self { - if ($this->temporarySecret === null) { - throw new ConfigurationError('No temporary secret set to apply to user'); + if (!($this->secret === null && $this->temporarySecret === null)) { + if ( + ($currentSecret = $this->totpObject->getSecret()) + && $currentSecret !== $this->temporarySecret + && $currentSecret !== $this->secret + ) { + $this->temporarySecret = $this->totpObject->getSecret(); + } else { + $this->setTotpObject(true)->generateSecret(); + } } - if ($this->secret === null) { - $this->getWebDb()->prepexec( - (new Insert())->into('icingaweb_totp')->values( - [ - 'username' => $this->username, - 'secret' => $this->temporarySecret, - 'created_at' => $this->clock->now(), - ] - ) - ); - } else { - $this->getWebDb()->prepexec( - (new Update())->table('icingaweb_totp') - ->set([ - 'secret' => $this->temporarySecret, - 'updated_at' => $this->clock->now(), - ]) - ->where(['username' => $this->username]) - ); - } - $this->secret = $this->temporarySecret; return $this; } + public function deleteSecret(): self + { + $this->secret = null; + $this->temporarySecret = null; + + return $this; + } + + public function saveTemporaryInSession(): self + { + Session::getSession()->set( + 'icingaweb_totp', + $this + ); + + return $this; + } + public function makeStatePersistent(): self + { + $db = $this->getWebDb(); + $db->beginTransaction(); + + $dbEntry = $db->prepexec( + (new Select()) + ->columns(['*']) + ->from('icingaweb_totp') + ->where(['username = ?' => $this->username]) + )->getIterator()->current(); + + try { + if ($this->temporarySecret !== null) { + if (!$dbEntry) { + $db->prepexec( + (new Insert()) + ->into(self::TABLE_NAME) + ->values( + [ + self::COLUMN_USERNAME => $this->username, + self::COLUMN_SECRET => $this->temporarySecret, + self::COLUMN_CREATED_TIME => date('Y-m-d H:i:s'), + self::COLUMN_MODIFIED_TIME => date('Y-m-d H:i:s'), + ] + ) + ); + } else { + $db->prepexec( + (new Update()) + ->table(self::TABLE_NAME) + ->set([ + self::COLUMN_SECRET => $this->temporarySecret, + self::COLUMN_MODIFIED_TIME => date('Y-m-d H:i:s'), + ]) + ->where([self::COLUMN_USERNAME . ' = ?' => $this->username]) + ); + } + $this->secret = $this->temporarySecret; + $this->temporarySecret = null; + } elseif ($this->secret === null && $dbEntry->secret !== null) { + $db->prepexec( + (new Delete()) + ->from(self::TABLE_NAME) + ->where([self::COLUMN_USERNAME . ' = ?' => $this->username]) + ); + $this->setTotpObject(true); + } + + $db->commitTransaction(); + } catch (\Exception $e) { + $db->rollBackTransaction(); + throw new ConfigurationError(sprintf( + 'Failed to persist TOTP state for user %s: %s', + $this->username, + $e->getMessage() + ), 0, $e); + } + + + return $this; + } + + /** + * Retrieves the TOTP secret for the current user. + * + * @return string|null The TOTP secret or null if not found + */ + + + public function getCurrentCode(): string + { + return $this->totpObject->now(); + } + public function getSecret(): ?string + { + + return $this->temporarySecret ?? $this->secret; + } + + public function getTemporarySecret(): ?string + { + return $this->temporarySecret; + } + + /** * Returns a query for the TOTP model. * This method is used to fetch TOTP records from the database. @@ -163,45 +283,37 @@ class Totp return $totp; } - /** - * Retrieves the TOTP secret for the current user. - * - * @return string|null The TOTP secret or null if not found - */ - public function getSecret(): ?string - { - - return $this->secret; - } - - public function getTemporarySecret(): ?string - { - return $this->temporarySecret; - } - - /** * Sets the TOTP object based on the user's secret. * If the secret is not set, a new TOTP object is generated. + * + * @param bool $new If true, a new TOTP object is created regardless of the existing secret + * @return self Returns the current instance for method chaining */ - private function setTotpObject(bool $new = false): void + private function setTotpObject(bool $new = false): self { - if (isset($this->totpObject)) { - return; - } - - if (!$new && ($totpModel = $this->getTotpModel()) !== null) { + $totpModel = null; + if ($this->secret === null && ($totpModel = $this->getTotpModel()) !== null) { $this->secret = $totpModel->secret; - $this->totpObject = extTOTP::createFromSecret($this->secret, $this->clock); - } elseif (!$new && $this->temporarySecret !== null) { - $this->totpObject = extTOTP::createFromSecret($this->temporarySecret, $this->clock); - } else { - $this->totpObject = extTOTP::generate($this->clock); } - } - public function getCurrentCode(): string - { - return $this->totpObject->now(); + if (!$new) { + if (isset($this->totpObject)) { + + return $this; + } elseif ($this->temporarySecret !== null) { + $this->totpObject = extTOTP::createFromSecret($this->temporarySecret, $this->clock); + + return $this; + } elseif ($totpModel !== null) { + $this->totpObject = extTOTP::createFromSecret($this->secret, $this->clock); + + return $this; + } + } + + $this->totpObject = extTOTP::generate($this->clock); + + return $this; } }