From eb4f4963a897c01f33b81478d54bac06c3340d44 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Tue, 28 Apr 2020 22:10:15 +0300 Subject: [PATCH 1/9] Fixed logout not removing task if there was no redirect set --- CHANGELOG.md | 6 ++++++ classes/Controller.php | 21 +++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 216bac5..fdcfeb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# 3.2.1 +## mm/dd/2020 + +1. [](#bugfix) + * Fixed logout not removing task if there was no redirect set + # 3.2.0 ## 04/27/2020 diff --git a/classes/Controller.php b/classes/Controller.php index a1ea585..e7e3ad1 100644 --- a/classes/Controller.php +++ b/classes/Controller.php @@ -3,7 +3,7 @@ /** * @package Grav\Plugin\Login * - * @copyright Copyright (C) 2014 - 2017 RocketTheme, LLC. All rights reserved. + * @copyright Copyright (C) 2014 - 2020 RocketTheme, LLC. All rights reserved. * @license MIT License; see LICENSE file for details. */ @@ -268,7 +268,7 @@ public function taskLogout() $route_after_logout = $this->grav['config']->get('plugins.login.route_after_logout'); $logout_redirect = is_bool($redirect_after_logout) && $redirect_after_logout == true ? $route_after_logout : $redirect_after_logout; - $redirect = $event->getRedirect() ?: $logout_redirect; + $redirect = $event->getRedirect() ?: $logout_redirect ?: $this->getCurrentRedirect(); if ($redirect) { $this->setRedirect($redirect, $event->getRedirectCode()); } @@ -485,6 +485,23 @@ public function taskRegenerate2FASecret() exit; } + /** + * @return string + */ + protected function getCurrentRedirect() + { + /** @var Uri $uri */ + $uri = $this->grav['uri']; + $redirect = $uri->route(); + foreach ($uri->params(null, true) as $key => $value) { + if (!in_array($key, ['task', 'nonce', 'login-nonce', 'logout-nonce'], true)) { + $redirect .= $uri->params($key); + } + } + + return $redirect; + } + /** * Redirects an action */ From 0a0d1d903eed6a7b1e5485ebc443c0d8cc4d8033 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Tue, 28 Apr 2020 22:38:14 +0300 Subject: [PATCH 2/9] Fixed remember me triggering `onUserLoginFailure`, use `onUserLoginGuest` event instead --- CHANGELOG.md | 1 + classes/Login.php | 3 ++- login.php | 30 ++++++++++++++++-------------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdcfeb6..1ce5bbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ 1. [](#bugfix) * Fixed logout not removing task if there was no redirect set + * Fixed remember me triggering `onUserLoginFailure`, use `onUserLoginGuest` event instead # 3.2.0 ## 04/27/2020 diff --git a/classes/Login.php b/classes/Login.php index b29abd7..d04f484 100755 --- a/classes/Login.php +++ b/classes/Login.php @@ -137,8 +137,9 @@ public function login(array $credentials, array $options = [], array $extra = [] static::DEBUG && static::addDebugMessage('Login failed', $event); // Allow plugins to log errors or do other tasks on failure. + $eventName = $event->getOption('failureEvent') ?? 'onUserLoginFailure'; $event = new UserLoginEvent($event->toArray()); - $grav->fireEvent('onUserLoginFailure', $event); + $grav->fireEvent($eventName, $event); // Make sure that event didn't mess up with the user authorization. $user = $event->getUser(); diff --git a/login.php b/login.php index cea7641..e564b29 100755 --- a/login.php +++ b/login.php @@ -3,7 +3,7 @@ /** * @package Grav\Plugin\Login * - * @copyright Copyright (C) 2014 - 2017 RocketTheme, LLC. All rights reserved. + * @copyright Copyright (C) 2014 - 2020 RocketTheme, LLC. All rights reserved. * @license MIT License; see LICENSE file for details. */ @@ -44,18 +44,13 @@ class LoginPlugin extends Plugin /** @var string */ protected $route; - /** @var string */ - protected $route_register; - - /** @var string */ - protected $route_forgot; - /** @var bool */ protected $authenticated = true; /** @var Login */ protected $login; + /** @var bool */ protected $redirect_to_login; /** @@ -79,7 +74,8 @@ public static function getSubscribedEvents() 'onFormProcessed' => ['onFormProcessed', 0], 'onUserLoginAuthenticate' => [['userLoginAuthenticateByRegistration', 10002], ['userLoginAuthenticateByRememberMe', 10001], ['userLoginAuthenticateByEmail', 10000], ['userLoginAuthenticate', 0]], 'onUserLoginAuthorize' => ['userLoginAuthorize', 0], - 'onUserLoginFailure' => ['userLoginFailure', 0], + 'onUserLoginFailure' => ['userLoginGuest', 0], + 'onUserLoginGuest' => ['userLoginGuest', 0], 'onUserLogin' => ['userLogin', 0], 'onUserLogout' => ['userLogout', 0], ]; @@ -107,16 +103,20 @@ public function initializeSession() } // Define login service. - $this->grav['login'] = function (Grav $c) { + $this->grav['login'] = static function (Grav $c) { return new Login($c); }; // Define current user service. - $this->grav['user'] = function (Grav $c) { + $this->grav['user'] = static function (Grav $c) { $session = $c['session']; if (empty($session->user)) { - $session->user = $c['login']->login(['username' => ''], ['remember_me' => true, 'remember_me_login' => true]); + // Try remember me login. + $session->user = $c['login']->login( + ['username' => ''], + ['remember_me' => true, 'remember_me_login' => true, 'failureEvent' => 'onUserLoginGuest'] + ); } return $session->user; @@ -390,7 +390,7 @@ public function handleUserActivation() $message = $this->grav['language']->translate('PLUGIN_LOGIN.INVALID_REQUEST'); $messages->add($message, 'error'); } else { - list($good_token, $expire) = explode('::', $user->activation_token, 2); + [$good_token, $expire] = explode('::', $user->activation_token, 2); if ($good_token === $token) { if (time() > $expire) { @@ -1079,12 +1079,14 @@ public function userLoginAuthorize(UserLoginEvent $event) } } - public function userLoginFailure(UserLoginEvent $event) + public function userLoginGuest(UserLoginEvent $event) { /** @var UserCollectionInterface $users */ $users = $this->grav['accounts']; + $user = $users->load(''); - $this->grav['session']->user = $users->load(''); + $event->setUser($user); + $this->grav['session']->user = $user; } public function userLogin(UserLoginEvent $event) From efed7d256b6b2ae49d3139db923c763652e8f615 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 29 Apr 2020 11:38:04 +0300 Subject: [PATCH 3/9] Update rate limiter to login events --- CHANGELOG.md | 3 +++ classes/Controller.php | 31 +++------------------------- classes/Login.php | 47 ++++++++++++++++++++++++++++++++++++++++++ login.php | 44 +++++++++++++++++++++++++++++++++++---- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce5bbc..74a7741 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # 3.2.1 ## mm/dd/2020 +* [](#new) + * Rete limiter logic was moved to login events and can be turned on with `['rate_limit' => true]` option + * Rete limiter sets `UserLoginEvent::AUTHENTICATION_CANCELLED` and triggers `onUserLoginFailure` event 1. [](#bugfix) * Fixed logout not removing task if there was no redirect set * Fixed remember me triggering `onUserLoginFailure`, use `onUserLoginGuest` event instead diff --git a/classes/Controller.php b/classes/Controller.php index e7e3ad1..13b8924 100644 --- a/classes/Controller.php +++ b/classes/Controller.php @@ -120,37 +120,14 @@ public function taskLogin() /** @var Message $messages */ $messages = $this->grav['messages']; - $userKey = (string)($this->post['username'] ?? ''); - $ip = Uri::ip(); - $isIPv4 = filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4); - $ipKey = $isIPv4 ? $ip : Utils::getSubnet($ip, $this->grav['config']->get('plugins.login.ipv6_subnet_size')); + // Remove login nonce from the form. + $form = array_diff_key($this->post, ['login-form-nonce' => true]); // Is twofa enabled? $twofa = $this->grav['config']->get('plugins.login.twofa_enabled', false); - // Pseudonymization of the IP - $ipKey = sha1($ipKey . $this->grav['config']->get('security.salt')); - - $rateLimiter = $this->login->getRateLimiter('login_attempts'); - - // Check if the current IP has been used in failed login attempts. - $attempts = \count($rateLimiter->getAttempts($ipKey, 'ip')); - - $rateLimiter->registerRateLimitedAction($ipKey, 'ip')->registerRateLimitedAction($userKey); - - // Check rate limit for both IP and user, but allow each IP a single try even if user is already rate limited. - if ($rateLimiter->isRateLimited($ipKey, 'ip') || ($attempts && $rateLimiter->isRateLimited($userKey))) { - $messages->add($t->translate(['PLUGIN_LOGIN.TOO_MANY_LOGIN_ATTEMPTS', $rateLimiter->getInterval()]), 'error'); - $this->setRedirect($this->grav['config']->get('plugins.login.route', '/')); - - return true; - } - - // Remove login nonce from the form. - $form = array_diff_key($this->post, ['login-form-nonce' => true]); - // Fire Login process. - $event = $this->login->login($form, ['remember_me' => true, 'twofa' => $twofa], ['return_event' => true]); + $event = $this->login->login($form, ['rate_limit' => true, 'remember_me' => true, 'twofa' => $twofa], ['return_event' => true]); $user = $event->getUser(); /* Support old string-based $redirect_after_login + new bool approach */ @@ -158,9 +135,7 @@ public function taskLogin() $route_after_login = $this->grav['config']->get('plugins.login.route_after_login'); $login_redirect = is_bool($redirect_after_login) && $redirect_after_login == true ? $route_after_login : $redirect_after_login; - if ($user->authenticated) { - $rateLimiter->resetRateLimit($ipKey, 'ip')->resetRateLimit($userKey); if ($user->authorized) { $event->defMessage('PLUGIN_LOGIN.LOGIN_SUCCESSFUL', 'info'); diff --git a/classes/Login.php b/classes/Login.php index d04f484..cc8fe64 100755 --- a/classes/Login.php +++ b/classes/Login.php @@ -276,6 +276,53 @@ public function register(array $data, array $files = []) return $user; } + /** + * @param string $username + * @param string|null $ip + * @return int Return positive number if rate limited, otherwise return 0. + */ + public function checkLoginRateLimit(string $username, string $ip = null): int + { + $ipKey = $this->getIpKey($ip); + $rateLimiter = $this->getRateLimiter('login_attempts'); + $rateLimiter->registerRateLimitedAction($ipKey, 'ip')->registerRateLimitedAction($username); + + // Check rate limit for both IP and user, but allow each IP a single try even if user is already rate limited. + $attempts = \count($rateLimiter->getAttempts($ipKey, 'ip')); + if ($rateLimiter->isRateLimited($ipKey, 'ip') || ($attempts && $rateLimiter->isRateLimited($username))) { + return $rateLimiter->getInterval(); + } + + return 0; + } + + /** + * @param string $username + * @param string|null $ip + */ + public function resetLoginRateLimit(string $username, string $ip = null): void + { + $ipKey = $this->getIpKey($ip); + $rateLimiter = $this->getRateLimiter('login_attempts'); + $rateLimiter->resetRateLimit($ipKey, 'ip')->resetRateLimit($username); + } + + /** + * @param string|null $ip + * @return string + */ + public function getIpKey(string $ip = null): string + { + if (null === $ip) { + $ip = Uri::ip(); + } + $isIPv4 = filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4); + $ipKey = $isIPv4 ? $ip : Utils::getSubnet($ip, $this->grav['config']->get('plugins.login.ipv6_subnet_size')); + + // Pseudonymization of the IP + return sha1($ipKey . $this->grav['config']->get('security.salt')); + } + /** * @param string $type * @param mixed $value diff --git a/login.php b/login.php index e564b29..0cc380d 100755 --- a/login.php +++ b/login.php @@ -72,11 +72,11 @@ public static function getSubscribedEvents() 'onTwigTemplatePaths' => ['onTwigTemplatePaths', 0], 'onTwigSiteVariables' => ['onTwigSiteVariables', -100000], 'onFormProcessed' => ['onFormProcessed', 0], - 'onUserLoginAuthenticate' => [['userLoginAuthenticateByRegistration', 10002], ['userLoginAuthenticateByRememberMe', 10001], ['userLoginAuthenticateByEmail', 10000], ['userLoginAuthenticate', 0]], + 'onUserLoginAuthenticate' => [['userLoginAuthenticateRateLimit', 10003], ['userLoginAuthenticateByRegistration', 10002], ['userLoginAuthenticateByRememberMe', 10001], ['userLoginAuthenticateByEmail', 10000], ['userLoginAuthenticate', 0]], 'onUserLoginAuthorize' => ['userLoginAuthorize', 0], 'onUserLoginFailure' => ['userLoginGuest', 0], 'onUserLoginGuest' => ['userLoginGuest', 0], - 'onUserLogin' => ['userLogin', 0], + 'onUserLogin' => [['userLoginResetRateLimit', 1000], ['userLogin', 0]], 'onUserLogout' => ['userLogout', 0], ]; } @@ -939,6 +939,32 @@ public function onFormProcessed(Event $event) } } + /** + * @param UserLoginEvent $event + * @throws \RuntimeException + */ + public function userLoginAuthenticateRateLimit(UserLoginEvent $event) + { + // Check that we're logging in with rate limit turned on. + if (!$event->getOption('rate_limit')) { + return; + } + + $credentials = $event->getCredentials(); + $username = $credentials['username']; + + // Check rate limit for both IP and user, but allow each IP a single try even if user is already rate limited. + if ($interval = $this->login->checkLoginRateLimit($username)) { + /** @var Language $t */ + $t = $this->grav['language']; + + $event->setMessage($t->translate(['PLUGIN_LOGIN.TOO_MANY_LOGIN_ATTEMPTS', $interval]), 'error'); + $event->setRedirect($this->grav['config']->get('plugins.login.route', '/')); + $event->setStatus(UserLoginEvent::AUTHENTICATION_CANCELLED); + $event->stopPropagation(); + } + } + /** * @param UserLoginEvent $event * @throws \RuntimeException @@ -1089,6 +1115,15 @@ public function userLoginGuest(UserLoginEvent $event) $this->grav['session']->user = $user; } + public function userLoginResetRateLimit(UserLoginEvent $event) + { + if ($event->getOption('rate_limit')) { + // Reset user rate limit. + $user = $event->getUser(); + $this->login->resetLoginRateLimit($user->get('username')); + } + } + public function userLogin(UserLoginEvent $event) { /** @var SessionInterface $session */ @@ -1099,7 +1134,8 @@ public function userLogin(UserLoginEvent $event) if (method_exists($session, 'regenerateId')) { $session->regenerateId(); } - $session->user = $event->getUser(); + + $session->user = $user = $event->getUser(); if ($event->getOption('remember_me')) { /** @var Login $login */ @@ -1108,7 +1144,7 @@ public function userLogin(UserLoginEvent $event) $session->remember_me = (bool)$event->getOption('remember_me_login'); // If the user wants to be remembered, create Rememberme cookie. - $username = $event->getUser()->get('username'); + $username = $user->get('username'); if ($event->getCredential('rememberme')) { $login->rememberMe()->createCookie($username); } From e31f9f3a1948b0177d4c60ae05d441d952658ea0 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 29 Apr 2020 13:07:20 +0300 Subject: [PATCH 4/9] Improved 2FA with triggered events --- CHANGELOG.md | 6 +- classes/Controller.php | 125 +++++++++++++++++++++++++----- classes/Events/UserLoginEvent.php | 90 +++++++++++++-------- classes/Login.php | 5 +- 4 files changed, 170 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a7741..9eccac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,10 @@ ## mm/dd/2020 * [](#new) - * Rete limiter logic was moved to login events and can be turned on with `['rate_limit' => true]` option - * Rete limiter sets `UserLoginEvent::AUTHENTICATION_CANCELLED` and triggers `onUserLoginFailure` event + * Rate limiter logic was moved to login events and can be turned on with `['rate_limit' => true]` option + * Rate limiter sets `UserLoginEvent::AUTHENTICATION_CANCELLED` and triggers `onUserLoginFailure` event + * Login now triggers extra `onUserLoginAuthorized` event if user is authorized + * 2FA now triggers either `onUserLoginAuthorized` or `onUserLoginFailure` event with `AUTHORIZATION_CHALLENGE` state 1. [](#bugfix) * Fixed logout not removing task if there was no redirect set * Fixed remember me triggering `onUserLoginFailure`, use `onUserLoginGuest` event instead diff --git a/classes/Controller.php b/classes/Controller.php index 13b8924..35c9d2f 100644 --- a/classes/Controller.php +++ b/classes/Controller.php @@ -9,6 +9,7 @@ namespace Grav\Plugin\Login; +use Grav\Common\Config\Config; use Grav\Common\Grav; use Grav\Common\Language\Language; use Grav\Common\Uri; @@ -16,6 +17,7 @@ use Grav\Common\User\Interfaces\UserInterface; use Grav\Common\Utils; use Grav\Plugin\Email\Utils as EmailUtils; +use Grav\Plugin\Login\Events\UserLoginEvent; use Grav\Plugin\Login\TwoFactorAuth\TwoFactorAuth; use Grav\Plugin\LoginPlugin; use RocketTheme\Toolbox\Session\Message; @@ -174,48 +176,133 @@ public function taskLogin() public function taskTwoFa() { + /** @var Config $config */ + $config = $this->grav['config']; + /** @var Language $t */ $t = $this->grav['language']; /** @var Message $messages */ $messages = $this->grav['messages']; + if (!$config->get('plugins.login.twofa_enabled', false)) { + $messages->add($t->translate('PLUGIN_LOGIN.2FA_FAILED'), 'error'); - /** @var TwoFactorAuth $twoFa */ - $twoFa = $this->grav['login']->twoFactorAuth(); + return true; + } + + $twoFa = $this->login->twoFactorAuth(); $user = $this->grav['user']; $code = $this->post['2fa_code'] ?? null; $secret = $user->twofa_secret ?? null; + $eventOptions = [ + 'credentials' => ['username' => $user->get('username')], + 'options' => ['twofa' => true] + ]; + + // Attempt to authenticate the user. + $event = new UserLoginEvent($eventOptions); + $event->setUser($user); + if (!$code || !$secret || !$twoFa->verifyCode($secret, $code)) { - $messages->add($t->translate('PLUGIN_LOGIN.2FA_FAILED'), 'error'); + $event->setStatus(UserLoginEvent::AUTHENTICATION_FAILURE | UserLoginEvent::AUTHORIZATION_CHALLENGE); + $event->setMessage($t->translate('PLUGIN_LOGIN.2FA_FAILED'), 'error'); + + $this->grav->fireEvent('onUserLoginFailure', $event); + // Make sure that event didn't mess up with the user authorization. + $user = $event->getUser(); $user->authenticated = false; + $user->authorized = false; - $redirect_to_login = $this->grav['config']->get('plugins.login.route_to_login'); - $login_route = $this->grav['config']->get('plugins.login.route'); - $redirect_route = $redirect_to_login && $login_route ? $login_route : false; - if ($redirect_route) { - $this->setRedirect($redirect_route, 303); + if (!$event->getRedirect()) { + $redirect_to_login = $this->grav['config']->get('plugins.login.route_to_login'); + $login_route = $this->grav['config']->get('plugins.login.route'); + + $event->setRedirect( + $redirect_to_login && $login_route ? $login_route : $this->getCurrentRedirect(), + 303 + ); } + } else { + + $event->setStatus(UserLoginEvent::AUTHENTICATION_SUCCESS | UserLoginEvent::AUTHORIZATION_CHALLENGE); + $event->setMessage($t->translate('PLUGIN_LOGIN.LOGIN_SUCCESSFUL'), 'info'); + + $this->grav->fireEvent('onUserLoginAuthorized', $event); + + // Make sure that event didn't mess up with the user authorization. + $user = $event->getUser(); + $user->authenticated = $event->isSuccess(); + $user->authorized = !$event->isDelayed(); + + if (!$event->getRedirect()) { + /* Support old string-based $redirect_after_login + new bool approach */ + $redirect_after_login = $this->grav['config']->get('plugins.login.redirect_after_login'); + $route_after_login = $this->grav['config']->get('plugins.login.route_after_login'); + $login_redirect = is_bool($redirect_after_login) && $redirect_after_login === true ? $route_after_login : $redirect_after_login; + + $event->setRedirect( + $this->grav['session']->redirect_after_login ?: $login_redirect ?: $this->grav['uri']->referrer('/'), + 303 + ); + } + } + + /** @var Message $messages */ + $messages = $this->grav['messages']; + $messages->add($event->getMessage(), $event->getMessageType()); + + $redirect = $event->getRedirect() ?: $this->getCurrentRedirect(); + $this->setRedirect($redirect, $event->getRedirectCode()); + + return true; + } + + public function taskTwofa_cancel() + { + /** @var Config $config */ + $config = $this->grav['config']; + + /** @var Language $t */ + $t = $this->grav['language']; + + /** @var Message $messages */ + $messages = $this->grav['messages']; + if (!$config->get('plugins.login.twofa_enabled', false)) { + $messages->add($t->translate('PLUGIN_LOGIN.2FA_FAILED'), 'error'); return true; } - $messages->add($t->translate('PLUGIN_LOGIN.LOGIN_SUCCESSFUL'), 'info'); + $user = $this->grav['user']; + $eventOptions = [ + 'credentials' => ['username' => $user->get('username')], + 'options' => ['twofa' => true] + ]; - $user->authorized = true; + $event = new UserLoginEvent($eventOptions); - /* Support old string-based $redirect_after_login + new bool approach */ - $redirect_after_login = $this->grav['config']->get('plugins.login.redirect_after_login'); - $route_after_login = $this->grav['config']->get('plugins.login.route_after_login'); - $login_redirect = is_bool($redirect_after_login) && $redirect_after_login == true ? $route_after_login : $redirect_after_login; + $event->setStatus(UserLoginEvent::AUTHENTICATION_CANCELLED | UserLoginEvent::AUTHORIZATION_CHALLENGE); + $event->setMessage($t->translate('PLUGIN_LOGIN.2FA_FAILED'), 'error'); + + $this->grav->fireEvent('onUserLoginFailure', $event); - $this->setRedirect( - $this->grav['session']->redirect_after_login - ?: $login_redirect - ?: $this->grav['uri']->referrer('/') - ); + // Make sure that event didn't mess up with the user authorization. + $user = $event->getUser(); + $user->authenticated = false; + $user->authorized = false; + + if (!$event->getRedirect()) { + $redirect_to_login = $this->grav['config']->get('plugins.login.route_to_login'); + $login_route = $this->grav['config']->get('plugins.login.route'); + + $event->setRedirect( + $redirect_to_login && $login_route ? $login_route : $this->getCurrentRedirect(), + 303 + ); + } return true; } diff --git a/classes/Events/UserLoginEvent.php b/classes/Events/UserLoginEvent.php index e5a75e0..970f016 100644 --- a/classes/Events/UserLoginEvent.php +++ b/classes/Events/UserLoginEvent.php @@ -13,6 +13,7 @@ use Grav\Common\Session; use Grav\Common\User\Interfaces\UserCollectionInterface; use Grav\Common\User\Interfaces\UserInterface; +use Grav\Framework\Session\SessionInterface; use Grav\Plugin\Login\Login; use RocketTheme\Toolbox\Event\Event; @@ -34,37 +35,42 @@ class UserLoginEvent extends Event /** * Undefined event state. */ - const AUTHENTICATION_UNDEFINED = 0; + public const AUTHENTICATION_UNDEFINED = 0; /** * onUserAuthenticate success. */ - const AUTHENTICATION_SUCCESS = 1; + public const AUTHENTICATION_SUCCESS = 1; /** * onUserAuthenticate fails on bad username/password. */ - const AUTHENTICATION_FAILURE = 2; + public const AUTHENTICATION_FAILURE = 2; /** * onUserAuthenticate fails on auth cancellation. */ - const AUTHENTICATION_CANCELLED = 4; + public const AUTHENTICATION_CANCELLED = 4; /** * onUserAuthorizeLogin fails on expired account. */ - const AUTHORIZATION_EXPIRED = 8; + public const AUTHORIZATION_EXPIRED = 8; /** - * onUserAuthorizeLogin is delayed until user has performed extra action(s). + * onUserAuthorizeLogin is delayed until user has performed AUTHORIZATION_CHALLENGE. */ - const AUTHORIZATION_DELAYED = 16; + public const AUTHORIZATION_DELAYED = 16; /** * onUserAuthorizeLogin fails for other reasons. */ - const AUTHORIZATION_DENIED = 32; + public const AUTHORIZATION_DENIED = 32; + + /** + * onUserAuthorizeLogin was challenged, combine with AUTHENTICATION_SUCCESS, AUTHENTICATION_FAILURE or AUTHENTICATION_CANCELLED. + */ + public const AUTHORIZATION_CHALLENGE = 64; /** * UserLoginEvent constructor. @@ -107,7 +113,10 @@ public function __construct(array $items = []) } } - public function isSuccess() + /** + * @return bool + */ + public function isSuccess(): bool { $status = $this->offsetGet('status'); $failure = static::AUTHENTICATION_FAILURE | static::AUTHENTICATION_CANCELLED | static::AUTHORIZATION_EXPIRED @@ -116,15 +125,28 @@ public function isSuccess() return ($status & static::AUTHENTICATION_SUCCESS) && !($status & $failure); } - public function isDelayed() + /** + * @return bool + */ + public function isDelayed(): bool { return $this->isSuccess() && ($this->offsetGet('status') & static::AUTHORIZATION_DELAYED); } + /** + * @return bool + */ + public function isChallenged(): bool + { + $status = $this->offsetGet('status'); + + return (bool)($status & static::AUTHORIZATION_CHALLENGE); + } + /** * @return int */ - public function getStatus() + public function getStatus(): int { return (int)$this->offsetGet('status'); } @@ -133,7 +155,7 @@ public function getStatus() * @param int $status * @return $this */ - public function setStatus($status) + public function setStatus($status): self { $this->offsetSet('status', $this->offsetGet('status') | (int)$status); @@ -143,7 +165,7 @@ public function setStatus($status) /** * @return array */ - public function getCredentials() + public function getCredentials(): array { return $this->offsetGet('credentials') + ['username' => '', 'password' => '']; } @@ -162,7 +184,7 @@ public function getCredential($name) * @param mixed $value * @return $this */ - public function setCredential($name, $value) + public function setCredential($name, $value): self { $this->items['credentials'][$name] = $value; @@ -172,7 +194,7 @@ public function setCredential($name, $value) /** * @return array */ - public function getOptions() + public function getOptions(): array { return $this->offsetGet('options'); } @@ -191,7 +213,7 @@ public function getOption($name) * @param mixed $value * @return $this */ - public function setOption($name, $value) + public function setOption($name, $value): self { $this->items['options'][$name] = $value; @@ -199,9 +221,9 @@ public function setOption($name, $value) } /** - * @return Session|null + * @return SessionInterface|Session|null */ - public function getSession() + public function getSession(): ?SessionInterface { return $this->offsetGet('session'); } @@ -209,7 +231,7 @@ public function getSession() /** * @return UserInterface */ - public function getUser() + public function getUser(): UserInterface { return $this->offsetGet('user'); } @@ -218,7 +240,7 @@ public function getUser() * @param UserInterface $user * @return $this */ - public function setUser(UserInterface $user) + public function setUser(UserInterface $user): self { $this->offsetSet('user', $user); @@ -228,7 +250,7 @@ public function setUser(UserInterface $user) /** * @return array */ - public function getAuthorize() + public function getAuthorize(): array { return (array)$this->offsetGet('authorize'); } @@ -236,15 +258,15 @@ public function getAuthorize() /** * @return string|null */ - public function getMessage() + public function getMessage(): ?string { return !empty($this->items['message'][0]) ? (string)$this->items['message'][0] : null; } /** - * @return string|null + * @return string */ - public function getMessageType() + public function getMessageType(): string { return !empty($this->items['message'][1]) ? (string)$this->items['message'][1] : 'info'; } @@ -254,7 +276,7 @@ public function getMessageType() * @param string|null $type * @return $this */ - public function setMessage($message, $type = null) + public function setMessage($message, $type = null): self { $this->items['message'] = $message ? [$message, $type] : null; @@ -266,7 +288,7 @@ public function setMessage($message, $type = null) * @param string|null $type * @return $this */ - public function defMessage($message, $type = null) + public function defMessage($message, $type = null): self { if ($message && !isset($this->items['message'])) { $this->setMessage($message, $type); @@ -278,7 +300,7 @@ public function defMessage($message, $type = null) /** * @return string|null */ - public function getRedirect() + public function getRedirect(): ?string { return $this->items['redirect'] ?? null; } @@ -286,9 +308,9 @@ public function getRedirect() /** * @return int */ - public function getRedirectCode() + public function getRedirectCode(): int { - return $this->items['redirect_code'] ?? 303; + return (int)($this->items['redirect_code'] ?? 303); } /** @@ -296,7 +318,7 @@ public function getRedirectCode() * @param int $code * @return $this */ - public function setRedirect($path, $code = 303) + public function setRedirect($path, $code = 303): self { $this->items['redirect'] = $path ?: null; $this->items['redirect_code'] = (int)$code; @@ -309,7 +331,7 @@ public function setRedirect($path, $code = 303) * @param int $code * @return $this */ - public function defRedirect($path, $code = 303) + public function defRedirect($path, $code = 303): self { if ($path && !isset($this->items['redirect'])) { $this->setRedirect($path, $code); @@ -324,7 +346,7 @@ public function defRedirect($path, $code = 303) * @param mixed $offset Asset name value * @param mixed $value Asset value */ - public function __set($offset, $value) + public function __set($offset, $value): void { $this->offsetSet($offset, $value); } @@ -346,7 +368,7 @@ public function __get($offset) * @param mixed $offset Asset name value * @return boolean True if the value is set */ - public function __isset($offset) + public function __isset($offset): bool { return $this->offsetExists($offset); } @@ -356,7 +378,7 @@ public function __isset($offset) * * @param mixed $offset The name value to unset */ - public function __unset($offset) + public function __unset($offset): void { $this->offsetUnset($offset); } diff --git a/classes/Login.php b/classes/Login.php index cc8fe64..3956a21 100755 --- a/classes/Login.php +++ b/classes/Login.php @@ -132,7 +132,10 @@ public function login(array $credentials, array $options = [], array $extra = [] $user = $event->getUser(); $user->authenticated = true; $user->authorized = !$event->isDelayed(); - + if ($user->authorized) { + $event = new UserLoginEvent($event->toArray()); + $this->grav->fireEvent('onUserLoginAuthorized', $event); + } } else { static::DEBUG && static::addDebugMessage('Login failed', $event); From 66e86cb80880955b533ff5feaae63472ad9b99f5 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 29 Apr 2020 13:21:06 +0300 Subject: [PATCH 5/9] Readme update --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index fcdf0a8..fd37db2 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,16 @@ These are available via GPM, and because the plugin has dependencies you just ne $ bin/gpm install login ``` +# Changes in version 3.2 + +New events: + +* `onUserLoginAuthorized` Allows plugins to include their own logic when user gets authorized (usually after 2FA challenge). + # Changes in version 3.1 +New events: + * `onUserActivated` Allows plugins to hook into user activation, when user has clicked on confirmation email. # Changes in version 2.6 @@ -41,6 +49,8 @@ They use following events which can be hooked by plugins: * `onUserLoginRegisterData` Allows plugins to include their own data to be added to the user object during registration. * `onUserLoginRegistered` Allows plugins to hook into user registration just before the redirect. +All the events use `UserLoginEvent` with some useful methods to see what is going on. + New Plugin options have been added for: * `dynamic_page_visibility` - Integrate access into page visibility so things can be shown or hidden in the menu From b83d342e5182d1fe06a3b846b52dccdde3b0053e Mon Sep 17 00:00:00 2001 From: Djamil Legato Date: Wed, 29 Apr 2020 11:46:12 -0700 Subject: [PATCH 6/9] Subscribed login to twofa_cancel task --- login.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/login.php b/login.php index 0cc380d..b3b76e2 100755 --- a/login.php +++ b/login.php @@ -62,6 +62,7 @@ public static function getSubscribedEvents() 'onPluginsInitialized' => [['autoload', 100000], ['initializeSession', 10000], ['initializeLogin', 1000]], 'onTask.login.login' => ['loginController', 0], 'onTask.login.twofa' => ['loginController', 0], + 'onTask.login.twofa_cancel' => ['loginController', 0], 'onTask.login.forgot' => ['loginController', 0], 'onTask.login.logout' => ['loginController', 0], 'onTask.login.reset' => ['loginController', 0], @@ -215,7 +216,7 @@ public function pageVisibility(Event $e) $page->visible(false); } } - } + } } } } From 4d400c94ee31d29e1815a914032c4281964ef761 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 30 Apr 2020 15:55:53 -0600 Subject: [PATCH 7/9] backwards compatibility issues --- CHANGELOG.md | 7 +++++++ login.php | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 216bac5..c1996d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# 3.2.1 +## 04/30/2020 + +1. [](#bugfix) + * Fixed issue with backwards compatibility for `route_after_login` and `route_after_logout` + * Removed duplicate entries in `blueprint.yaml` causing YAML errors + # 3.2.0 ## 04/27/2020 diff --git a/login.php b/login.php index cea7641..675e2ec 100755 --- a/login.php +++ b/login.php @@ -1134,12 +1134,24 @@ public function userLogout(UserLoginEvent $event) public static function defaultRedirectAfterLogin() { - return Grav::instance()['config']->get('plugins.login.redirect_after_login'); + $legacy_option = Grav::instance()['config']->get('plugins.login.redirect_after_login'); + if (is_bool($legacy_option)) { + $default = Grav::instance()['config']->get('plugins.login.route_after_login'); + } else { + $default = $legacy_option; + } + return $default; } public static function defaultRedirectAfterLogout() { - return Grav::instance()['config']->get('plugins.login.redirect_after_logout'); + $legacy_option = Grav::instance()['config']->get('plugins.login.redirect_after_logout'); + if (is_bool($legacy_option)) { + $default = Grav::instance()['config']->get('plugins.login.route_after_logout'); + } else { + $default = $legacy_option; + } + return $default; } } From b8a4a326cac7aa273d6b5faeecac018c96b22d92 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 30 Apr 2020 15:56:00 -0600 Subject: [PATCH 8/9] remove duplicate entries --- blueprints.yaml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/blueprints.yaml b/blueprints.yaml index 642807b..cfe1b44 100644 --- a/blueprints.yaml +++ b/blueprints.yaml @@ -233,24 +233,6 @@ form: help: PLUGIN_LOGIN.REDIRECT_AFTER_ACTIVATION_HELP placeholder: "/page-to-show-after-activation" - route_forgot: - type: text - size: medium - label: PLUGIN_LOGIN.ROUTE_FORGOT - placeholder: '/forgot_password' - - route_reset: - type: text - size: medium - label: PLUGIN_LOGIN.ROUTE_RESET - placeholder: '/reset_password' - - route_profile: - type: text - size: medium - label: PLUGIN_LOGIN.ROUTE_PROFILE - placeholder: '/user_profile' - route_register: type: text size: medium From 65db1d3f8c20f2d38e1c2665feb31ef9db2980ab Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 30 Apr 2020 15:58:03 -0600 Subject: [PATCH 9/9] prepare for release --- blueprints.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blueprints.yaml b/blueprints.yaml index cfe1b44..3da19e5 100644 --- a/blueprints.yaml +++ b/blueprints.yaml @@ -1,7 +1,7 @@ name: Login slug: login type: plugin -version: 3.2.0 +version: 3.3.0 testing: false description: Enables user authentication and login screen. icon: sign-in