RFC: Identify and fix Authentication issue(s).

src/Backend/Core/Engine/Authentication.php

@@ -427,6 +427,9 @@ public static function loginUser($login, $password)
             \SpoonSession::set('backend_logged_in', true);
             \SpoonSession::set('backend_secret_key', $session['secret_key']);
 
+            // update/instantiate the value for the logged_in container.
+            BackendModel::getContainer()->set('logged_in', true);
+
             // return result
             return true;
         } else {
@@ -436,6 +439,9 @@ public static function loginUser($login, $password)
             \SpoonSession::set('backend_logged_in', false);
             \SpoonSession::set('backend_secret_key', '');
 
+            // update/instantiate the value for the logged_in container.
+            BackendModel::getContainer()->set('logged_in', false);
+
             // return result
             return false;
         }
@@ -454,4 +460,17 @@ public static function logout()
         \SpoonSession::set('backend_secret_key', '');
         \SpoonSession::set('csrf_token', '');
     }
+
+    /**
+     * Reset our class to make sure no contamination from previous
+     * authentications persists. This signifies a deeper issue with
+     * this class. Solving the issue would be preferable to introducting
+     * another method. This currently only exists to serve the test.
+     */
+    public static function tearDown()
+    {
+        self::$allowedActions = array();
+        self::$allowedModules = array();
+        self::$user = null;
+    }
 }

src/Backend/Modules/Authentication/Actions/Index.php

@@ -9,6 +9,8 @@
  * file that was distributed with this source code.
  */
 
+use Symfony\Component\Finder\Finder;
+
 use Backend\Core\Engine\Base\ActionIndex as BackendBaseActionIndex;
 use Backend\Core\Engine\Authentication as BackendAuthentication;
 use Backend\Core\Engine\Form as BackendForm;
@@ -40,7 +42,11 @@ public function execute()
     {
         // check if the user is really logged on
         if (BackendAuthentication::getUser()->isAuthenticated()) {
-            $this->redirect($this->getParameter('querystring', 'string', BackendModel::createUrlForAction(null, 'Dashboard')));
+            $userEmail = BackendAuthentication::getUser()->getEmail();
+            $this->getContainer()->get('logger')->info(
+                "User '{$userEmail}' is already authenticated."
+            );
+            $this->redirectToAllowedModuleAndAction();
         }
 
         parent::execute();
@@ -184,30 +190,15 @@ private function validateForm()
                     BackendUsersModel::setSetting($userId, 'last_login', $lastLogin);
                 }
 
-                // create filter with modules which may not be displayed
-                $filter = array('Authentication', 'Error', 'Core');
-
-                // get all modules
-                $modules = array_diff(BackendModel::getModules(), $filter);
-
-                // redirect to the dashboard module if possible
-                if (BackendAuthentication::isAllowedModule('Dashboard')) {
-                    $module = 'Dashboard';
-                } else {
-                    // if not allowed in the dashboard, redirect to the first allowed module
-                    foreach ($modules as $module) {
-                        if (BackendAuthentication::isAllowedModule($module)) {
-                            break;
-                        }
-                    }
-                }
+                $allowedModule = $this->getAllowedModule();
+                $allowedAction = $this->getAllowedAction($allowedModule);
 
                 $this->getContainer()->get('logger')->info(
                     "Successfully authenticated user '{$txtEmail->getValue()}'."
                 );
 
                 // redirect to the correct URL (URL the user was looking for or fallback)
-                $this->redirect($this->getParameter('querystring', 'string', BackendModel::createUrlForAction(null, $module)));
+                $this->redirectToAllowedModuleAndAction();
             }
         }
 
@@ -268,4 +259,83 @@ private function validateForm()
             }
         }
     }
+
+    /*
+     * Find out which module and action are allowed
+     * and send the user on his way.
+     */
+    private function redirectToAllowedModuleAndAction()
+    {
+        $allowedModule = $this->getAllowedModule();
+        $allowedAction = $this->getAllowedAction($allowedModule);
+        $allowedModuleActionUrl = $allowedModule ?
+            BackendModel::createUrlForAction($allowedAction, $allowedModule) :
+            BackendModel::createUrlForAction('Index', 'Authentication');
+
+        $userEmail = BackendAuthentication::getUser()->getEmail();
+        $this->getContainer()->get('logger')->info(
+            "Redirecting user '{$userEmail}' to {$allowedModuleActionUrl}."
+        );
+
+        $this->redirect(
+            $this->getParameter('querystring', 'string', $allowedModuleActionUrl)
+        );
+    }
+
+    /*
+     * Run through the action of a certain module
+     * and find us an action(name) this user is
+     * allowed to access.
+     */
+    private function getAllowedAction($module)
+    {
+        if(BackendAuthentication::isAllowedAction('Index', $module)) {
+            return 'Index';
+        }
+        $allowedAction = false;
+
+        $groupsRightsActions = BackendUsersModel::getModuleGroupsRightsActions(
+            $module
+        );
+
+        foreach ($groupsRightsActions as $groupsRightsAction) {
+            $isAllowedAction = BackendAuthentication::isAllowedAction(
+                $groupsRightsAction['action'],
+                $module
+            );
+            if($isAllowedAction) {
+                $allowedAction = $groupsRightsAction['action'];
+                break;
+            }
+        }
+
+        return $allowedAction;
+    }
+
+    /*
+     * Run through the modules and find us a module(name)
+     * this user is allowed to access.
+     */
+    private function getAllowedModule()
+    {
+        // create filter with modules which may not be displayed
+        $filter = array('Authentication', 'Error', 'Core');
+
+        // get all modules
+        $modules = array_diff(BackendModel::getModules(), $filter);
+        $allowedModule = false;
+
+        if (BackendAuthentication::isAllowedModule('Dashboard')) {
+            $allowedModule = 'Dashboard';
+        } else {
+            foreach ($modules as $module) {
+                if (BackendAuthentication::isAllowedModule($module)) {
+                    $allowedModule = $module;
+                    break;
+                }
+            }
+        }
+
+        return $allowedModule;
+    }
 }

src/Backend/Modules/Authentication/Tests/Actions/IndexTest.php

@@ -3,9 +3,24 @@
 namespace Backend\Modules\Authentication\Tests\Action;
 
 use Common\WebTestCase;
+use Backend\Core\Engine\Authentication as Authentication;
 
 class IndexTest extends WebTestCase
 {
+    /**
+     * The authentication class persist the previous user.
+     * In practice this situation will almost never occur:
+     * Login with one user, log out and subsequently log in
+     * with another user without a page reload to reinitialize
+     * the application.
+     * If the clients could be insulated from eachother, this
+     * would not be an issue.
+     */
+    protected function tearDown()
+    {
+        Authentication::tearDown();
+    }
+
     public function testPrivateRedirectsToAuthentication()
     {
         $client = static::createClient();
@@ -57,7 +72,7 @@ public function testAuthenticationWithWrongCredentials()
     public function testAuthenticationWithCorrectCredentials()
     {
         $client = static::createClient();
-        $client->followRedirects();
+        $client->setMaxRedirects(2);
 
         $crawler = $client->request('GET', '/private/en/authentication');
         $this->assertEquals(
@@ -77,5 +92,76 @@ public function testAuthenticationWithCorrectCredentials()
             'now editing:',
             $client->getResponse()->getContent()
         );
+
+        // logout to get rid of this session
+        $client->followRedirects(false);
+        $client->request('GET', '/private/en/authentication/logout');
+    }
+
+    /**
+     * Login as a pages user.
+     * This user has the rights to access only the pages module.
+     */
+    public function testPagesUserWithCorrectCredentials()
+    {
+        $client = static::createClient();
+        $client->setMaxRedirects(2);
+
+        $crawler = $client->request('GET', '/private/en/authentication');
+        $this->assertEquals(
+            200,
+            $client->getResponse()->getStatusCode()
+        );
+
+        $form = $crawler->selectButton('login')->form();
+        $this->submitForm($client, $form, array(
+            'form' => 'authenticationIndex',
+            'backend_email' => 'pages-user@fork-cms.com',
+            'backend_password' => 'fork',
+            'form_token' => $form['form_token']->getValue(),
+        ));
+
+        $this->assertContains(
+            'Recently edited',
+            $client->getResponse()->getContent()
+        );
+
+        // logout to get rid of this session
+        $client->followRedirects(false);
+        $client->request('GET', '/private/en/authentication/logout');
+    }
+
+    /**
+     * Login as a users user.
+     * This user only has the rights to access the users edit action.
+     * It should enable the user to edit his own user-account.
+     */
+    public function testUsersUserWithCorrectCredentials()
+    {
+        $client = static::createClient();
+        $client->setMaxRedirects(3);
+
+        $crawler = $client->request('GET', '/private/en/authentication');
+        $this->assertEquals(
+            200,
+            $client->getResponse()->getStatusCode()
+        );
+
+        $form = $crawler->selectButton('login')->form();
+        $this->submitForm($client, $form, array(
+            'form' => 'authenticationIndex',
+            'backend_email' => 'users-edit-user@fork-cms.com',
+            'backend_password' => 'fork',
+            'form_token' => $form['form_token']->getValue(),
+        ));
+
+        $this->assertContains(
+            'Users: edit user "Users User"',
+            $client->getResponse()->getContent()
+        );
+
+        // logout to get rid of this session
+        $client->followRedirects(false);
+        $client->request('GET', '/private/en/authentication/logout');
     }
 }

src/Backend/Modules/Groups/Engine/Model.php

@@ -290,12 +290,16 @@ public static function getModulePermissions($id)
     public static function getSetting($groupId, $name)
     {
         $setting = (array) BackendModel::getContainer()->get('database')->getRecord(
-            'SELECT i.*
+            'SELECT i.value
              FROM groups_settings AS i
              WHERE i.group_id = ? AND i.name = ?',
             array((int) $groupId, (string) $name)
         );
 
+        if (!$setting) {
+            return array();
+        }
+
         if (isset($setting['value'])) {
             return unserialize($setting['value']);
         }

src/Backend/Modules/Users/Actions/Edit.php

@@ -57,6 +57,25 @@ class Edit extends BackendBaseActionEdit
     public function execute()
     {
         $this->id = $this->getParameter('id', 'int');
+        $this->error = $this->getParameter('error', 'string');
+        $this->loadAuthenticatedUser();
+
+        // If id and error parameters are not set we'll assume the user logged in
+        // and has been redirected to this action by the authentication index action.
+        // When this is the case the user will be redirected to the index action of this module.
+        // An action to which he may not have any user rights.
+        // Redirect to the user's own profile instead to avoid unnessary words.
+        if (
+            $this->id === null &&
+            $this->error === null &&
+            $this->authenticatedUser->getUserId()
+        ) {
+            $this->redirect(
+                BackendModel::createURLForAction(
+                    'Edit'
+                ) . '&id=' . $this->authenticatedUser->getUserId()
+            );
+        }
 
         // does the user exists
         if ($this->id !== null && BackendUsersModel::exists($this->id)) {
@@ -71,14 +90,22 @@ public function execute()
         }
     }
 
+    /*
+     * Load the authenticated user in a seperate method
+     * so we can load it before the form starts loading.
+     */
+    private function loadAuthenticatedUser()
+    {
+        $this->authenticatedUser = BackendAuthentication::getUser();
+    }
+
     /**
      * Load the form
      */
     private function loadForm()
     {
         // create user objects
         $this->user = new BackendUser($this->id);
-        $this->authenticatedUser = BackendAuthentication::getUser();
         $this->allowUserRights = (
             (BackendAuthentication::isAllowedAction('Add') || $this->authenticatedUser->getUserId() != $this->id) ||
             $this->authenticatedUser->isGod()

src/Backend/Modules/Users/Engine/Model.php

@@ -252,6 +252,28 @@ public static function getGroups()
         );
     }
 
+    /**
+     * Get all relevant actions for a certain module.
+     *
+     * @param string $module The module name.
+     * @return array
+     */
+    public static function getModuleGroupsRightsActions($userId)
+    {
+        return (array) BackendModel::getContainer()->get('database')
+            ->getRecords(
+                'SELECT a.module, a.action
+                FROM groups AS g
+                    INNER JOIN users_groups AS u ON u.group_id = g.id
+                    INNER JOIN groups_rights_modules AS m ON m.group_id = g.id
+                    INNER JOIN groups_rights_actions AS a ON a.group_id = g.id
+                        AND m.module = a.module
+                WHERE m.module = ?
+                GROUP BY a.module, a.action',
+                $userId
+            );
+    }
+
     /**
      * Get the user ID linked to a given email
      *