diff --git a/application/controllers/AuthenticationController.php b/application/controllers/AuthenticationController.php index 1ce954877..6ea836331 100644 --- a/application/controllers/AuthenticationController.php +++ b/application/controllers/AuthenticationController.php @@ -203,6 +203,10 @@ class AuthenticationController extends CompatController $form->getPressedSubmitElement()?->getName() === TwoFactorChallengeForm::SUBMIT_CANCEL; if ($csrfValid && $cancelPressed) { + // The login flow may have persisted a remember-me record before issuing the + // challenge. Remove it now since the challenge was canceled and the cookie + // was never delivered to the browser. + RememberMe::removeAllByUsername((new TwoFactorState())->getChallengedUser()->getUsername()); Session::getSession()->purge(); $this->redirectNow($this->withRedirect('authentication/login')); } diff --git a/application/forms/Authentication/LoginForm.php b/application/forms/Authentication/LoginForm.php index ae41a0062..415f376e0 100644 --- a/application/forms/Authentication/LoginForm.php +++ b/application/forms/Authentication/LoginForm.php @@ -138,7 +138,8 @@ class LoginForm extends CompatForm if ($this->getElement('rememberme')->isChecked()) { $rememberMe = RememberMe::fromCredentials($user->getUsername(), $password); - $twoFactorState->setRememberMeCookie($rememberMe); + $twoFactorState->setRememberMeCookieData($rememberMe->getCookie()->getValue()); + $rememberMe->persist(); } $redirectUrl = Url::fromPath('authentication/twofactor'); diff --git a/application/forms/Authentication/TwoFactorChallengeForm.php b/application/forms/Authentication/TwoFactorChallengeForm.php index 06cb2986f..1c45bcb0d 100644 --- a/application/forms/Authentication/TwoFactorChallengeForm.php +++ b/application/forms/Authentication/TwoFactorChallengeForm.php @@ -13,6 +13,7 @@ use Icinga\Application\Logger; use Icinga\Authentication\Auth; use Icinga\Authentication\TwoFactorState; use Icinga\Web\Form\Element\LoginRedirect; +use Icinga\Web\RememberMe; use Icinga\Web\Session; use Icinga\Web\Url; use ipl\Html\Attributes; @@ -126,10 +127,10 @@ class TwoFactorChallengeForm extends CompatForm $response = Icinga::app()->getResponse(); - if ($rememberMe = $twoFactorState->getRememberMeCookie()) { + if ($cookieData = $twoFactorState->getRememberMeCookieData()) { try { + $rememberMe = RememberMe::fromCookieData($cookieData); $response->setCookie($rememberMe->getCookie()); - $rememberMe->persist(); } catch (Exception $e) { Logger::error('Failed to let user "%s" stay logged in: %s', $user->getUsername(), $e); } diff --git a/library/Icinga/Authentication/TwoFactorState.php b/library/Icinga/Authentication/TwoFactorState.php index 0f3bee18e..4d8f9671b 100644 --- a/library/Icinga/Authentication/TwoFactorState.php +++ b/library/Icinga/Authentication/TwoFactorState.php @@ -6,7 +6,6 @@ namespace Icinga\Authentication; use Icinga\User; -use Icinga\Web\RememberMe; use Icinga\Web\Session; use Icinga\Web\Session\SessionNamespace; @@ -21,8 +20,8 @@ class TwoFactorState /** @var string Session key storing the challenged User */ protected const SESSION_CHALLENGED_USER = 'challenged_user'; - /** @var string Session key storing the temporary {@see RememberMe} instance */ - protected const SESSION_REMEMBER_ME_COOKIE = 'remember_me_cookie'; + /** @var string Session key storing the raw remember-me cookie */ + protected const SESSION_REMEMBER_ME_COOKIE_DATA = 'remember_me_cookie_data'; /** @var SessionNamespace Session namespace scoping the challenge state */ protected SessionNamespace $session; @@ -57,7 +56,7 @@ class TwoFactorState public function completeChallenge(): void { $this->session->delete(static::SESSION_CHALLENGED_USER); - $this->session->delete(static::SESSION_REMEMBER_ME_COOKIE); + $this->session->delete(static::SESSION_REMEMBER_ME_COOKIE_DATA); } /** @@ -71,25 +70,25 @@ class TwoFactorState } /** - * Set the remember-me instance to issue after a successful challenge + * Set the remember-me cookie value to issue after a successful challenge * - * @param RememberMe $cookie Temporary remember-me instance + * @param string $cookieData * * @return void */ - public function setRememberMeCookie(RememberMe $cookie): void + public function setRememberMeCookieData(string $cookieData): void { - $this->session->set(static::SESSION_REMEMBER_ME_COOKIE, $cookie); + $this->session->set(static::SESSION_REMEMBER_ME_COOKIE_DATA, $cookieData); } /** - * Get the stored remember-me instance + * Get the stored remember-me cookie value * - * @return ?RememberMe + * @return ?string null if no value was set */ - public function getRememberMeCookie(): ?RememberMe + public function getRememberMeCookieData(): ?string { - return $this->session->get(static::SESSION_REMEMBER_ME_COOKIE); + return $this->session->get(static::SESSION_REMEMBER_ME_COOKIE_DATA); } /** diff --git a/library/Icinga/Web/RememberMe.php b/library/Icinga/Web/RememberMe.php index b425b906f..095bd9745 100644 --- a/library/Icinga/Web/RememberMe.php +++ b/library/Icinga/Web/RememberMe.php @@ -98,9 +98,21 @@ class RememberMe * * @return static */ - public static function fromCookie() + public static function fromCookie(): static { - $data = explode('|', $_COOKIE[static::COOKIE]); + return static::fromCookieData($_COOKIE[static::COOKIE]); + } + + /** + * Create the remember me component from cookie data + * + * @param string $data The cookie data + * + * @return static + */ + public static function fromCookieData(string $data): static + { + $data = explode('|', $data); $iv = base64_decode(array_pop($data)); $select = (new Select()) @@ -135,6 +147,7 @@ class RememberMe $rememberMe->username = $rs->username; $rememberMe->encryptedPassword = $data[0]; + $rememberMe->expiresAt = strtotime($rs->expires_at); return $rememberMe; }