Skip to content

Commit ecd73a6

Browse files
committed
Adjust remember me functionality to work with 2fa
If 2fa is enable the remember me cookie only gets set if the 2fa authentication was successful. To log the user back in from the cookie the 2fa will be skipped.
1 parent eaabe91 commit ecd73a6

File tree

5 files changed

+56
-34
lines changed

5 files changed

+56
-34
lines changed

application/controllers/AuthenticationController.php

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function loginAction()
4444
if (($requiresSetup = $icinga->requiresSetup()) && $icinga->setupTokenExists()) {
4545
$this->redirectNow(Url::fromPath('setup'));
4646
}
47-
47+
$skip2fa = false;
4848
$user = $this->Auth()->getUser();
4949
if ($user
5050
&& $user->getTwoFactorEnabled()
@@ -55,30 +55,31 @@ public function loginAction()
5555
$cancel2faForm->handleRequest();
5656
} else {
5757
$form = new LoginForm();
58-
}
5958

60-
if (RememberMe::hasCookie() && $this->hasDb()) {
61-
$authenticated = false;
62-
try {
63-
$rememberMeOld = RememberMe::fromCookie();
64-
$authenticated = $rememberMeOld->authenticate();
65-
if ($authenticated) {
66-
$rememberMe = $rememberMeOld->renew();
67-
$this->getResponse()->setCookie($rememberMe->getCookie());
68-
$rememberMe->persist($rememberMeOld->getAesCrypt()->getIV());
59+
if (RememberMe::hasCookie() && $this->hasDb()) {
60+
$authenticated = false;
61+
try {
62+
$rememberMeOld = RememberMe::fromCookie();
63+
$authenticated = $rememberMeOld->authenticate();
64+
if ($authenticated) {
65+
$rememberMe = $rememberMeOld->renew();
66+
$this->getResponse()->setCookie($rememberMe->getCookie());
67+
$rememberMe->persist($rememberMeOld->getAesCrypt()->getIV());
68+
$skip2fa = true;
69+
}
70+
} catch (RuntimeException $e) {
71+
Logger::error("Can't authenticate user via remember me cookie: %s", $e->getMessage());
72+
} catch (AuthenticationException $e) {
73+
Logger::error($e);
6974
}
70-
} catch (RuntimeException $e) {
71-
Logger::error("Can't authenticate user via remember me cookie: %s", $e->getMessage());
72-
} catch (AuthenticationException $e) {
73-
Logger::error($e);
74-
}
7575

76-
if (! $authenticated) {
77-
$this->getResponse()->setCookie(RememberMe::forget());
76+
if (! $authenticated) {
77+
$this->getResponse()->setCookie(RememberMe::forget());
78+
}
7879
}
7980
}
8081

81-
if ($this->Auth()->isAuthenticated()) {
82+
if ($this->Auth()->isAuthenticated($skip2fa)) {
8283
// Call provided AuthenticationHook(s) when login action is called
8384
// but icinga web user is already authenticated
8485
AuthenticationHook::triggerLogin($this->Auth()->getUser());

application/forms/Authentication/Challenge2FAForm.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
namespace Icinga\Forms\Authentication;
44

5+
use Exception;
56
use Icinga\Application\Hook\AuthenticationHook;
7+
use Icinga\Application\Logger;
68
use Icinga\Authentication\Auth;
79
use Icinga\Authentication\IcingaTotp;
810
use Icinga\Web\Session;
@@ -55,8 +57,20 @@ public function onSuccess()
5557

5658
$auth->setAuthenticated($user);
5759

60+
if ($rememberMe = Session::getSession()->get('2fa_remember_me_cookie')) {
61+
try {
62+
$this->getResponse()->setCookie($rememberMe->getCookie());
63+
$rememberMe->persist();
64+
} catch (Exception $e) {
65+
Logger::error('Failed to let user "%s" stay logged in: %s', $user->getUsername(), $e);
66+
}
67+
}
68+
69+
// Call provided AuthenticationHook(s) after successful login
5870
AuthenticationHook::triggerLogin($user);
71+
5972
$this->getResponse()->setRerenderLayout(true);
73+
6074
return true;
6175
}
6276

application/forms/Authentication/LoginForm.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,18 +150,6 @@ public function onSuccess()
150150
$authenticated = $authChain->authenticate($user, $password);
151151
if ($authenticated) {
152152
$auth->setAuthenticated($user);
153-
if ($this->getElement('rememberme')->isChecked()) {
154-
try {
155-
$rememberMe = RememberMe::fromCredentials($user->getUsername(), $password);
156-
$this->getResponse()->setCookie($rememberMe->getCookie());
157-
$rememberMe->persist();
158-
} catch (Exception $e) {
159-
Logger::error('Failed to let user "%s" stay logged in: %s', $user->getUsername(), $e);
160-
}
161-
}
162-
163-
// Call provided AuthenticationHook(s) after successful login
164-
AuthenticationHook::triggerLogin($user);
165153

166154
// If user has 2FA enabled and the token hasn't been validated, redirect to login again, so that
167155
// the token is challenged.
@@ -172,9 +160,27 @@ public function onSuccess()
172160
);
173161
Session::getSession()->set('2fa_must_challenge_token', true);
174162

163+
if ($this->getElement('rememberme')->isChecked()) {
164+
$rememberMe = RememberMe::fromCredentials($user->getUsername(), $password);
165+
Session::getSession()->set('2fa_remember_me_cookie', $rememberMe);
166+
}
167+
175168
return true;
176169
}
177170

171+
if ($this->getElement('rememberme')->isChecked()) {
172+
try {
173+
$rememberMe = RememberMe::fromCredentials($user->getUsername(), $password);
174+
$this->getResponse()->setCookie($rememberMe->getCookie());
175+
$rememberMe->persist();
176+
} catch (Exception $e) {
177+
Logger::error('Failed to let user "%s" stay logged in: %s', $user->getUsername(), $e);
178+
}
179+
}
180+
181+
// Call provided AuthenticationHook(s) after successful login
182+
AuthenticationHook::triggerLogin($user);
183+
178184
$this->getResponse()->setRerenderLayout(true);
179185
return true;
180186
}

library/Icinga/Authentication/Auth.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,10 @@ public function getAuthChain()
8888
*
8989
* @return bool
9090
*/
91-
public function isAuthenticated()
91+
public function isAuthenticated(bool $skip2fa = false)
9292
{
9393
if ($this->user !== null) {
94-
if ($this->user->getTwoFactorEnabled() && ! $this->user->getTwoFactorSuccessful()) {
94+
if (! $skip2fa && $this->user->getTwoFactorEnabled() && ! $this->user->getTwoFactorSuccessful()) {
9595
return false;
9696
}
9797
return true;

library/Icinga/Web/RememberMe.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
use Icinga\Application\Config;
77
use Icinga\Authentication\Auth;
8-
use Icinga\Crypt\AesCrypt;
98
use Icinga\Common\Database;
9+
use Icinga\Crypt\AesCrypt;
1010
use Icinga\User;
1111
use ipl\Sql\Expression;
1212
use ipl\Sql\Select;
@@ -246,6 +246,7 @@ public function authenticate()
246246
);
247247

248248
if ($authenticated) {
249+
$user->setTwoFactorSuccessful(true);
249250
$auth->setAuthenticated($user);
250251
}
251252

0 commit comments

Comments
 (0)