00100118: Refactoring of login methods
authorFlávio Gomes da Silva Lisboa <flavio.lisboa@serpro.gov.br>
Mon, 4 Aug 2014 17:58:35 +0000 (14:58 -0300)
committerPhilipp Schüle <p.schuele@metaways.de>
Tue, 12 Aug 2014 15:21:45 +0000 (17:21 +0200)
 - Applied refactoring technique "Extract Method" for methods
   login() of Tinebase_Controller and Tinebase_Frontend_Json
 - Aim is reducing complexity and avoid conflicts for changes
   made into login implementation.

Change-Id: I71eb921cd3186486476b59711875fb73063b3d7d
Reviewed-on: https://gerrit.tine20.org/tine20/2858
Tested-by: jenkins user
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
Reviewed-by: Stefanie Stamer <s.stamer@metaways.de>
tine20/Tinebase/Controller.php
tine20/Tinebase/Frontend/Json.php

index 528f0b1..2ec6834 100644 (file)
@@ -1,18 +1,18 @@
 <?php
 /**
  * Tine 2.0
- * 
+ *
  * @package     Tinebase
  * @subpackage  Server
  * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
  * @copyright   Copyright (c) 2007-2011 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Lars Kneschke <l.kneschke@metaways.de>
- * 
+ *
  */
 
 /**
  * the class provides functions to handle applications
- * 
+ *
  * @package     Tinebase
  * @subpackage  Server
  */
@@ -36,7 +36,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
      * the constructor
      *
      */
-    private function __construct() 
+    private function __construct()
     {
     }
 
@@ -51,7 +51,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
      *
      * @return Tinebase_Controller
      */
-    public static function getInstance() 
+    public static function getInstance()
     {
         if (self::$_instance === NULL) {
             self::$_instance = new Tinebase_Controller;
@@ -73,22 +73,49 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     public function login($_loginname, $_password, $_ipAddress, $_clientIdString = NULL, $securitycode = NULL)
     {
         $authResult = Tinebase_Auth::getInstance()->authenticate($_loginname, $_password);
-        $authResultCode = $authResult->getCode();
-        $authResultIdentity = $authResult->getIdentity();
         
         Tinebase_Core::set(Tinebase_Core::SESSIONID, Zend_Session::isStarted() ? session_id() : Tinebase_Record_Abstract::generateUID());
         
-        $accessLog = new Tinebase_Model_AccessLog(array(
+        $accessLog = $this->_getModelAccessLog($authResult, $_clientIdString, $_ipAddress);
+        
+        $result = $this->_getResultFromAccessLog($accessLog, $authResult, $_loginname, $_password, $_ipAddress);
+        
+        $this->_updateAccessLogInstance($accessLog);
+
+        return $result;
+    }
+    
+    /**
+     *
+     * @param Zend_Auth_Result  $authResult
+     * @param string            $_clientIdString
+     * @param string            $_ipAddress
+     * @return Tinebase_Model_AccessLog
+     */
+    protected function _getModelAccessLog(Zend_Auth_Result $authResult, $_clientIdString, $_ipAddress)
+    {
+        return new Tinebase_Model_AccessLog(array(
             'sessionid'     => Tinebase_Core::get(Tinebase_Core::SESSIONID),
             'ip'            => $_ipAddress,
             'li'            => Tinebase_DateTime::now()->get(Tinebase_Record_Abstract::ISO8601LONG),
-            'result'        => $authResultCode,
+            'result'        => $authResult->getCode(),
             'clienttype'    => $_clientIdString,
         ), TRUE);
-        
+    }
+    
+    /**
+     *
+     * @param Tinebase_Model_AccessLog          $accessLog
+     * @param Zend_Auth_Result                  $authResult
+     * @param string                            $_loginname
+     * @param string                            $_password
+     * @param string                            $_ipAddress
+     */
+    protected function _getResultFromAccessLog(Tinebase_Model_AccessLog $accessLog, Zend_Auth_Result $authResult, $_loginname, $_password, $_ipAddress)
+    {
         $user = NULL;
         if ($accessLog->result == Tinebase_Auth::SUCCESS) {
-            $user = $this->_getLoginUser($authResultIdentity, $accessLog);
+            $user = $this->_getLoginUser($authResult->getIdentity(), $accessLog);
             if ($user !== NULL) {
                 $this->_checkUserStatus($user, $accessLog);
             }
@@ -96,32 +123,39 @@ class Tinebase_Controller extends Tinebase_Controller_Event
         
         if ($accessLog->result === Tinebase_Auth::SUCCESS && $user !== NULL && $user->accountStatus === Tinebase_User::STATUS_ENABLED) {
             if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info(
-                __METHOD__ . '::' . __LINE__ . " Login with username $_loginname from $_ipAddress succeeded.");
-
+                    __METHOD__ . '::' . __LINE__ . " Login with username $_loginname from $_ipAddress succeeded.");
+        
             $this->_initUser($user, $accessLog, $_password);
-            
-            $result = true;
+        
+            $result = TRUE;
         } else {
             if (Tinebase_Core::isLogLevel(Zend_Log::WARN)) Tinebase_Core::getLogger()->warn(
-                __METHOD__ . '::' . __LINE__ . " Login with username $_loginname from $_ipAddress failed ($authResultCode)!");
+                    __METHOD__ . '::' . __LINE__ . " Login with username $_loginname from $_ipAddress failed ({$authResult->getCode()})!");
             if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(
-                __METHOD__ . '::' . __LINE__ . ' Failure messages: ' . print_r($authResult->getMessages(), TRUE));
-
-            $this->_loginFailed($_loginname ? $_loginname : $authResultIdentity, $accessLog);
-            
-            $result = false;
-        } 
-
+                    __METHOD__ . '::' . __LINE__ . ' Failure messages: ' . print_r($authResult->getMessages(), TRUE));
+        
+            $this->_loginFailed($_loginname ? $_loginname : $authResult->getIdentity(), $accessLog);
+        
+            $result = FALSE;
+        }
+        return $result;
+    }
+    
+    
+    /**
+     *
+     * @param Tinebase_Model_AccessLog $accessLog
+     */
+    protected function _updateAccessLogInstance(Tinebase_Model_AccessLog $accessLog)
+    {
         if (Tinebase_Core::get('serverclassname') !== 'ActiveSync_Server_Http' || !(ActiveSync_Config::getInstance()->get(ActiveSync_Config::DISABLE_ACCESS_LOG))) {
             Tinebase_AccessLog::getInstance()->create($accessLog);
         }
-
-        return $result;
     }
     
     /**
      * get login user
-     * 
+     *
      * @param string $_username
      * @param Tinebase_Model_AccessLog $_accessLog
      * @return Tinebase_Model_FullUser|NULL
@@ -138,7 +172,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
                  * catch all exceptions during user data sync
                  * either it's the first sync and no user data get synchronized or
                  * we can work with the data synced during previous login
-                 */ 
+                 */
                 try {
                     Tinebase_User::syncUser($_username,array('syncContactData' => TRUE));
                 } catch (Exception $e) {
@@ -161,7 +195,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     
     /**
      * check user status
-     * 
+     *
      * @param Tinebase_Model_FullUser $_user
      * @param Tinebase_Model_AccessLog $_accessLog
      */
@@ -185,13 +219,13 @@ class Tinebase_Controller extends Tinebase_Controller_Event
             else if ($_user->accountStatus == Tinebase_User::STATUS_BLOCKED) {
                 if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__ . ' Account: '. $_user->accountLoginName . ' is blocked');
                 $_accessLog->result = Tinebase_Auth::FAILURE_BLOCKED;
-            } 
+            }
         }
     }
     
     /**
      * init user session
-     * 
+     *
      * @param Tinebase_Model_FullUser $_user
      * @param Tinebase_Model_AccessLog $_accessLog
      * @param string $_password
@@ -224,7 +258,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     
     /**
      * init session after successful login
-     * 
+     *
      * @param Tinebase_Model_FullUser $_user
      */
     protected function _initUserSession(Tinebase_Model_FullUser $_user)
@@ -247,8 +281,8 @@ class Tinebase_Controller extends Tinebase_Controller_Event
             Zend_Session::regenerateId();
             Tinebase_Core::set(Tinebase_Core::SESSIONID, session_id());
             
-            /** 
-             * fix php session header handling http://forge.tine20.org/mantisbt/view.php?id=4918 
+            /**
+             * fix php session header handling http://forge.tine20.org/mantisbt/view.php?id=4918
              * -> search all Set-Cookie: headers and replace them with the last one!
              **/
             $cookieHeaders = array();
@@ -267,7 +301,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     
     /**
      * login failed
-     * 
+     *
      * @param  string                    $loginName
      * @param  Tinebase_Model_AccessLog  $accessLog
      */
@@ -362,7 +396,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
          * we unset the Zend_Auth session variable. This way we keep the session,
          * but the user is not logged into Tine 2.0
          * we use this to validate passwords for OpenId for example
-         */ 
+         */
         unset(Tinebase_Core::getSession()->Zend_Auth);
         unset(Tinebase_Core::getSession()->currentAccount);
         
@@ -408,7 +442,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     
     /**
      * gets image info and data
-     * 
+     *
      * @param   string $_application application which manages the image
      * @param   string $_identifier identifier of image/record
      * @param   string $_location optional additional identifier
@@ -434,7 +468,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
      * remove obsolete/outdated stuff from cache
      * notes: CLEANING_MODE_OLD -> removes obsolete cache entries (files for file cache)
      *        CLEANING_MODE_ALL -> removes complete cache structure (directories for file cache) + cache entries
-     * 
+     *
      * @param string $_mode
      */
     public function cleanupCache($_mode = Zend_Cache::CLEANING_MODE_OLD)
@@ -485,10 +519,10 @@ class Tinebase_Controller extends Tinebase_Controller_Event
     
     /**
      * spy function for unittesting of queue workers
-     * 
-     * this function writes the number of executions of itself in the given 
+     *
+     * this function writes the number of executions of itself in the given
      * file and optionally sleeps a given time
-     * 
+     *
      * @param string  $filename
      * @param int     $sleep
      * @param int     $fail
@@ -513,7 +547,7 @@ class Tinebase_Controller extends Tinebase_Controller_Event
 
     /**
      * handle events for Tinebase
-     * 
+     *
      * @param Tinebase_Event_Abstract $_eventObject
      */
     protected function _handleEvent(Tinebase_Event_Abstract $_eventObject)
index 5e8b0f6..0310bbd 100644 (file)
@@ -1,26 +1,32 @@
 <?php
 /**
  * Tine 2.0
- * 
+ *
  * @package     Tinebase
  * @subpackage  Server
  * @license     http://www.gnu.org/licenses/agpl.html AGPL Version 3
  * @copyright   Copyright (c) 2007-2012 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Lars Kneschke <l.kneschke@metaways.de>
- * 
+ *
  */
 
 /**
  * Json interface to Tinebase
- * 
+ *
  * @package     Tinebase
  * @subpackage  Server
  */
 class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
 {
     /**
+     *
+     * @var boolean
+     */
+    protected $_hasCaptcha = FALSE;
+
+    /**
      * wait for changes
-     * 
+     *
      * @todo do we still need this?
      */
     public function ping()
@@ -32,9 +38,9 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
 
     /**
      * get list of translated country names
-     * 
+     *
      * Wrapper for {@see Tinebase_Core::getCountrylist}
-     * 
+     *
      * @return array list of countrys
      */
     public function getCountryList()
@@ -233,7 +239,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
 
     /**
      * Used for updating multiple records
-     * 
+     *
      * @param string $appName
      * @param string $modelName
      * @param array $changes
@@ -444,70 +450,115 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
     {
         Tinebase_Core::startSession('tinebase');
         
-        $captcha = (isset(Tinebase_Core::getConfig()->captcha->count) && Tinebase_Core::getConfig()->captcha->count != 0) ? TRUE : FALSE; 
-        if ($captcha) {
-            $config_count = Tinebase_Core::getConfig()->captcha->count;
-            $count = (isset($_SESSION['captcha']['count']) ? $_SESSION['captcha']['count'] : 1);
-            if ($count >= $config_count) {
-                $aux = isset($_SESSION['captcha']['code']) ? $_SESSION['captcha']['code'] : NULL;
-                if ($aux != $securitycode) {
-                    $rets = Tinebase_Controller::getInstance()->makeCaptcha(); 
-                    $response = array(
-                        'success'      => FALSE,
-                        'errorMessage' => "Wrong username or password!",
-                        'c1' => $rets['1']
-                    );
-                    return $response;
-                   }
-            }
+        if (is_array(($response = $this->_getCaptchaResponse($securitycode)))) {
+            return $response;
         }
+        
         // try to login user
         $success = (Tinebase_Controller::getInstance()->login($username, $password, $_SERVER['REMOTE_ADDR'], 'TineJson', $securitycode) === TRUE);
         
-        if ($success == true) {
-            $response = array(
+        if($success === TRUE) {
+            return $this->_getLoginSuccessResponse($username);
+        } else {
+            return $this->_getLoginFailedResponse();
+        }
+    }
+    
+    /**
+     * Returns TRUE if there is a captcha
+     * @return boolean
+     */
+    protected function _hasCaptcha()
+    {
+        if ($this->_hasCaptcha == NULL){
+            $this->_hasCaptcha = (isset(Tinebase_Core::getConfig()->captcha->count) && Tinebase_Core::getConfig()->captcha->count != 0) ? TRUE : FALSE;
+        }
+        return $this->_hasCaptcha;
+    }
+    
+    /**
+     *
+     * @param string $securitycode
+     * @return array | NULL
+     */
+    protected function _getCaptchaResponse($securitycode)
+    {
+        if ($this->_hasCaptcha()) {
+            if ($captcha) {
+                $config_count = Tinebase_Core::getConfig()->captcha->count;
+                $count = (isset($_SESSION['captcha']['count']) ? $_SESSION['captcha']['count'] : 1);
+                if ($count >= $config_count) {
+                    $aux = isset($_SESSION['captcha']['code']) ? $_SESSION['captcha']['code'] : NULL;
+                    if ($aux != $securitycode) {
+                        $rets = Tinebase_Controller::getInstance()->makeCaptcha();
+                        $response = array(
+                            'success'      => FALSE,
+                            'errorMessage' => "Wrong username or password!",
+                            'c1' => $rets['1']
+                        );
+                        return $response;
+                    }
+                }
+            }
+        }
+        return NULL;
+    }
+    
+    /**
+     *
+     * @param string $username
+     * @return array
+     */
+    protected function _getLoginSuccessResponse($username)
+    {
+        $response = array(
                 'success'        => TRUE,
                 'account'        => Tinebase_Core::getUser()->getPublicUser()->toArray(),
                 'jsonKey'        => Tinebase_Core::get('jsonKey'),
                 'welcomeMessage' => "Welcome to Tine 2.0!"
-            );
-             
-            if (Tinebase_Config::getInstance()->get(Tinebase_Config::REUSEUSERNAME_SAVEUSERNAME, 0)) {
-                // save in cookie (expires in 2 weeks)
-                setcookie('TINE20LASTUSERID', $username, time()+60*60*24*14);
-            } else {
-                setcookie('TINE20LASTUSERID', '', 0);
-            }
-
-            $this->_setCredentialCacheCookie();
-            
+        );
+         
+        if (Tinebase_Config::getInstance()->get(Tinebase_Config::REUSEUSERNAME_SAVEUSERNAME, 0)) {
+            // save in cookie (expires in 2 weeks)
+            setcookie('TINE20LASTUSERID', $username, time()+60*60*24*14);
         } else {
-            $response = array(
+            setcookie('TINE20LASTUSERID', '', 0);
+        }
+        
+        $this->_setCredentialCacheCookie();
+
+        return $response;
+    }
+    
+    /**
+     *
+     * @return array
+     */
+    protected function _getLoginFailedResponse()
+    {
+        $response = array(
                 'success'      => FALSE,
                 'errorMessage' => "Wrong username or password!",
-            );
-            Tinebase_Auth_CredentialCache::getInstance()->getCacheAdapter()->resetCache();
-            if ($captcha) {
-                $config_count = Tinebase_Core::getConfig()->captcha->count;
-                if (!isset($_SESSION['captcha']['count'])) {
-                    $_SESSION['captcha']['count'] = 1;
-                } else {
-                    $_SESSION['captcha']['count'] = $_SESSION['captcha']['count'] + 1;
-                }
-                if ($_SESSION['captcha']['count'] >= $config_count) {
-                    $rets = Tinebase_Controller::getInstance()->makeCaptcha(); 
-                    $response = array(
+        );
+        Tinebase_Auth_CredentialCache::getInstance()->getCacheAdapter()->resetCache();
+        if ($captcha) {
+            $config_count = Tinebase_Core::getConfig()->captcha->count;
+            if (!isset($_SESSION['captcha']['count'])) {
+                $_SESSION['captcha']['count'] = 1;
+            } else {
+                $_SESSION['captcha']['count'] = $_SESSION['captcha']['count'] + 1;
+            }
+            if ($_SESSION['captcha']['count'] >= $config_count) {
+                $rets = Tinebase_Controller::getInstance()->makeCaptcha();
+                $response = array(
                         'success'      => FALSE,
                         'errorMessage' => "Wrong username or password!",
                         'c1' => $rets['1']
-                    );
-                }
-            } else {
-                Zend_Session::destroy(false,true);
+                );
             }
-            
+        } else {
+            Zend_Session::destroy(false,true);
         }
-
         return $response;
     }
 
@@ -600,7 +651,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
     
     /**
      * get anonymous registry
-     * 
+     *
      * @return array
      */
     protected function _getAnonymousRegistryData()
@@ -657,7 +708,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
     
     /**
      * get user registry
-     * 
+     *
      * @return array
      */
     protected function _getUserRegistryData()
@@ -734,7 +785,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
                     try {
                         $applicationJson = new $jsonAppName();
                         $registryData[$application->name] = ((isset($registryData[$application->name]) || array_key_exists($application->name, $registryData)))
-                            ? array_merge_recursive($registryData[$application->name], $applicationJson->getRegistryData()) 
+                            ? array_merge_recursive($registryData[$application->name], $applicationJson->getRegistryData())
                             : $applicationJson->getRegistryData();
                     
                     } catch (Exception $e) {
@@ -771,7 +822,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
                     $appPrefs = Tinebase_Core::getPreference($application->name);
                     if ($appPrefs !== NULL) {
                         $allPrefs = $appPrefs->getAllApplicationPreferences();
-                        if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ 
+                        if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
                             . ' ' . print_r($allPrefs, TRUE));
                         
                         foreach ($allPrefs as $pref) {
@@ -828,9 +879,9 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
         if ($backend) {
             $records = $backend->search($filter);
             
-            if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ 
+            if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__
                 . ' Got ' . count($records) . ' preferences for app ' . $applicationName);
-            if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ 
+            if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
                 . ' ' . print_r($records->toArray(), TRUE));
             
             $result = $this->_multipleRecordsToJson($records, $filter);
@@ -1105,7 +1156,7 @@ class Tinebase_Frontend_Json extends Tinebase_Frontend_Json_Abstract
      * @param string $modelName
      * @param string $property
      * @param string $startswith
-     * 
+     *
      * @return array
      */
     public function autoComplete($appName, $modelName, $property, $startswith)