Merge branch 'release/3.3.2'

getgrav · Jun 3, 2020 · a6f3e20 · a6f3e20
2 parents 67e26fa + b85117d
commit a6f3e20
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,14 @@
+# 3.3.2
+## 06/03/2020
+
+1. [](#new)
+    * Grav 1.7 only: Added `Sync User in Session` feature [#252](https://github.com/getgrav/grav-plugin-login/issues/252)
+1. [](#improved)
+    * Refactored code to use helper methods to find redirect routes
+    * Added error message when user cannot log in because of account has not been activated or it has been disabled
+    * Set better defaults for redirects on login and logout
+    * Fixed proper highlights and default states for admin option toggles
+
 # 3.3.1
 ## 05/07/2020
 
@@ -7,7 +18,7 @@
 # 3.3.0
 ## 04/30/2020
 
-* [](#new)
+1. [](#new)
     * 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
@@ -23,7 +34,7 @@
 
 1. [](#new)
     * CHANGE: `redirect_to_login` and `redirect_after_logout` are now boolean, with accompanying `route_after_login` and `route_after_logout` options.  NOTE: Compatibility is maintained with existing config.
-* [](#improved)
+1. [](#improved)
     * Improved configuration layout
     * Better handling of login route when that page doesn't exist 
 1. [](#bugfix)
@@ -46,7 +57,7 @@
 1. [](#new)
     * Pass phpstan level 1 tests
     * Updated 2FA library to v1.7.0
-* [](#improved)
+1. [](#improved)
     * Added some debugging messages (turned off by default)
 
 # 3.0.5

diff --git a/blueprints.yaml b/blueprints.yaml
@@ -1,7 +1,7 @@
 name: Login
 slug: login
 type: plugin
-version: 3.3.1
+version: 3.3.2
 testing: false
 description: Enables user authentication and login screen.
 icon: sign-in
@@ -62,7 +62,8 @@ form:
               type: toggle
               label: PLUGIN_LOGIN.REDIRECT_TO_LOGIN
               help: PLUGIN_LOGIN.REDIRECT_TO_LOGIN_HELP
-              default: 1
+              default: 0
+              highlight: 0
               options:
                 1: PLUGIN_ADMIN.ENABLED
                 0: PLUGIN_ADMIN.DISABLED
@@ -75,6 +76,7 @@ form:
               help: PLUGIN_LOGIN.REDIRECT_AFTER_LOGIN_HELP
               force_bool: true
               default: 0
+              highlight: 0
               options:
                 1: PLUGIN_ADMIN.ENABLED
                 0: PLUGIN_ADMIN.DISABLED
@@ -86,7 +88,8 @@ form:
               label: PLUGIN_LOGIN.REDIRECT_AFTER_LOGOUT
               help: PLUGIN_LOGIN.REDIRECT_AFTER_LOGOUT_HELP
               force_bool: true
-              default: 0
+              default: 1
+              highlight: 1
               options:
                 1: PLUGIN_ADMIN.ENABLED
                 0: PLUGIN_ADMIN.DISABLED
@@ -96,7 +99,7 @@ form:
             parent_acl:
               type: toggle
               label: PLUGIN_LOGIN.USE_PARENT_ACL_LABEL
-              highlight: 1
+              highlight: 0
               default: 0
               help: PLUGIN_LOGIN.USE_PARENT_ACL_HELP
               options:
@@ -132,7 +135,7 @@ form:
             protect_protected_page_media:
               type: toggle
               label: PLUGIN_LOGIN.PROTECT_PROTECTED_PAGE_MEDIA_LABEL
-              highlight: 1
+              highlight: 0
               default: 0
               help: PLUGIN_LOGIN.PROTECT_PROTECTED_PAGE_MEDIA_HELP
               options:
@@ -141,6 +144,18 @@ form:
               validate:
                 type: bool
 
+            session_user_sync:
+              type: toggle
+              label: PLUGIN_LOGIN.SESSION_USER_SYNC
+              highlight: 0
+              default: 0
+              help: PLUGIN_LOGIN.SESSION_USER_SYNC_HELP
+              options:
+                1: PLUGIN_ADMIN.ENABLED
+                0: PLUGIN_ADMIN.DISABLED
+              validate:
+                type: bool
+
             rememberme:
               type: section
               title: PLUGIN_LOGIN.REMEMBER_ME

diff --git a/classes/Controller.php b/classes/Controller.php
@@ -132,10 +132,7 @@ public function taskLogin()
         $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 */
-        $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;
+        $login_redirect = LoginPlugin::defaultRedirectAfterLogin();
 
         if ($user->authenticated) {
             if ($user->authorized) {
@@ -238,10 +235,7 @@ public function taskTwoFa()
             $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;
+                $login_redirect = LoginPlugin::defaultRedirectAfterLogin();
 
                 $event->setRedirect(
                     $this->grav['session']->redirect_after_login ?: $login_redirect ?: $this->grav['uri']->referrer('/'),
@@ -325,10 +319,7 @@ public function taskLogout()
             $messages->add($t->translate($message), $event->getMessageType());
         }
 
-        /* Support old string-based $redirect_after_logout + new bool approach */
-        $redirect_after_logout = $this->grav['config']->get('plugins.login.redirect_after_logout');
-        $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;
+        $logout_redirect = LoginPlugin::defaultRedirectAfterLogout();
 
         $redirect = $event->getRedirect() ?: $logout_redirect ?: $this->getCurrentRedirect();
         if ($redirect) {

diff --git a/languages/en.yaml b/languages/en.yaml
@@ -139,4 +139,7 @@ PLUGIN_LOGIN:
   MANUALLY_ENABLE: "Manually Enable"
   MANUALLY_ENABLE_HELP: "When using 'activation email' and 'notification email', you can ensure the user can self activate, but requires manually enabling the user to login"
   IPV6_SUBNET_SIZE: "IPv6 Subnet Size"
-  IPV6_SUBNET_SIZE_HELP: "The number of IPv6 addresses typically assigned to a machine"
+  IPV6_SUBNET_SIZE_HELP: "The number of IPv6 addresses typically assigned to a machine"
+  USER_ACCOUNT_DISABLED: "Your user account is disabled or it has not yet been activated."
+  SESSION_USER_SYNC: "Sync User in Session"
+  SESSION_USER_SYNC_HELP: "If Enabled, user in the session is kept in sync with the stored user file. WARNING: This feature may break some existing plugins which update user object in the session without saving it to the filesystem using `$user->save()` method."
diff --git a/login.php b/login.php
@@ -12,6 +12,7 @@
 use Composer\Autoload\ClassLoader;
 use Grav\Common\Data\Data;
 use Grav\Common\Debugger;
+use Grav\Common\Flex\Types\Users\UserObject;
 use Grav\Common\Grav;
 use Grav\Common\Language\Language;
 use Grav\Common\Page\Interfaces\PageInterface;
@@ -23,6 +24,8 @@
 use Grav\Common\User\Interfaces\UserInterface;
 use Grav\Common\Utils;
 use Grav\Common\Uri;
+use Grav\Events\SessionStartEvent;
+use Grav\Framework\Flex\Interfaces\FlexCollectionInterface;
 use Grav\Framework\Flex\Interfaces\FlexObjectInterface;
 use Grav\Framework\Session\SessionInterface;
 use Grav\Plugin\Form\Form;
@@ -59,6 +62,7 @@ class LoginPlugin extends Plugin
     public static function getSubscribedEvents()
     {
         return [
+            SessionStartEvent::class    => ['onSessionStart', 0],
             'onPluginsInitialized'      => [['autoload', 100000], ['initializeSession', 10000], ['initializeLogin', 1000]],
             'onTask.login.login'        => ['loginController', 0],
             'onTask.login.twofa'        => ['loginController', 0],
@@ -92,6 +96,45 @@ public function autoload() : ClassLoader
         return require __DIR__ . '/vendor/autoload.php';
     }
 
+
+    public function onSessionStart(SessionStartEvent $event)
+    {
+        $session = $event->session;
+
+        $user = $session->user ?? null;
+        if ($user && $user->exists() && ($this->config()['session_user_sync'] ?? false)) {
+            // User is stored into the filesystem.
+
+            /** @var UserCollectionInterface $accounts */
+            $accounts = $this->grav['accounts'];
+
+            /** @var UserObject $stored */
+            if ($accounts instanceof FlexCollectionInterface) {
+                $stored = $accounts[$user->username];
+            } else {
+                // TODO: remove when removing legacy support.
+                $stored = $accounts->load($user->username);
+            }
+
+            if ($stored && $stored->exists()) {
+                // User still exists, update user object in the session.
+                $user->update($stored->jsonSerialize());
+            } else {
+                // User doesn't exist anymore, prepare for session invalidation.
+                $user->state = 'disabled';
+            }
+
+            if ($user->state !== 'enabled') {
+                // If user isn't enabled, clear all session data and display error.
+                $session->invalidate()->start();
+
+                /** @var Message $messages */
+                $messages = $this->grav['messages'];
+                $messages->add($this->grav['language']->translate('PLUGIN_LOGIN.USER_ACCOUNT_DISABLED'), 'error');
+            }
+        }
+    }
+
     /**
      * [onPluginsInitialized] Initialize login plugin if path matches.
      * @throws \RuntimeException
@@ -238,10 +281,7 @@ public function storeReferrerPage()
         $uri = $this->grav['uri'];
         $current_route = $uri->route();
 
-        /* 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');
-        $redirect = is_bool($redirect_after_login) && $redirect_after_login == true ? $route_after_login : $redirect_after_login;
+        $redirect = static::defaultRedirectAfterLogin();
 
         if (!$redirect && !in_array($current_route, $invalid_redirect_routes, true)) {
             // No login redirect set in the configuration; can we redirect to the current page?
@@ -1094,6 +1134,9 @@ public function userLoginAuthorize(UserLoginEvent $event)
         $user = $event->getUser();
         foreach ($event->getAuthorize() as $authorize) {
             if (!$user->authorize($authorize)) {
+                if ($user->state !== 'enabled') {
+                    $event->setMessage($this->grav['language']->translate('PLUGIN_LOGIN.USER_ACCOUNT_DISABLED'), 'error');
+                }
                 $event->setStatus($event::AUTHORIZATION_DENIED);
                 $event->stopPropagation();
 
@@ -1173,24 +1216,19 @@ public function userLogout(UserLoginEvent $event)
 
     public static function defaultRedirectAfterLogin()
     {
-        $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;
+        $config = Grav::instance()['config'];
+        $redirect_after_login = $config->get('plugins.login.redirect_after_login');
+        $route_after_login = $config->get('plugins.login.route_after_login');
 
+        return is_bool($redirect_after_login) && $redirect_after_login == true ? $route_after_login : $redirect_after_login;
     }
 
     public static function defaultRedirectAfterLogout()
     {
-        $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;
+        $config = Grav::instance()['config'];
+        $redirect_after_logout = $config->get('plugins.logout.redirect_after_logout');
+        $route_after_logout = $config->get('plugins.logout.route_after_logout');
+
+        return is_bool($redirect_after_logout) && $redirect_after_logout == true ? $route_after_logout : $redirect_after_logout;
     }
 }
diff --git a/login.yaml b/login.yaml
@@ -1,11 +1,12 @@
 enabled: true                               # Enable the plugin
 built_in_css: true                          # Use built-in CSS
 redirect_to_login: false                    # If you try to access a page you don't have access to, should you redirect to login route
-redirect_after_login: true                  # Path to redirect to after a successful login
+redirect_after_login: false                 # Path to redirect to after a successful login
 redirect_after_logout: true                 # Path to redirect to after a successful logout
+session_user_sync: false                    # Sync session user with changes in stored user file
 
 route: '/login'                             # Specific route for Login page (default is '/login')
-route_after_login:                          # Route to go to after login if enabled
+route_after_login: '/'                      # Route to go to after login if enabled
 route_after_logout: '/'                     # Route to logout to if enabled
 route_activate: '/activate_user'            # Route for the user activation process
 route_forgot: '/forgot_password'            # Route for the forgot password process