First attempt to simplify IMP login tasks code.

Stupid framesets needlessly complicate everything.

diff --git a/imp/lib/IMP.php b/imp/lib/IMP.php
index 69d5324..dc081a1 100644 (file)
--- a/imp/lib/IMP.php
+++ b/imp/lib/IMP.php
@@ -881,27 +881,6 @@ class IMP
     }
 
     /**
-     * Either sets or checks the value of the logintasks flag.
-     *
-     * @param integer $set  The value of the flag.
-     *
-     * @return integer  The value of the flag.
-     *                  0 = No login tasks pending
-     *                  1 = Login tasks pending
-     *                  2 = Login tasks pending, previous tasks interrupted
-     */
-    static public function loginTasksFlag($set = null)
-    {
-        if (!is_null($set)) {
-            $_SESSION['imp']['logintasks'] = $set;
-        }
-
-        return isset($_SESSION['imp']['logintasks'])
-            ? $_SESSION['imp']['logintasks']
-            : 0;
-    }
-
-    /**
      * Convert a preference value to/from the value stored in the preferences.
      *
      * Preferences that need to call this function before storing/retrieving:

diff --git a/imp/lib/Session.php b/imp/lib/Session.php
index 35f6de5..c1c9f26 100644 (file)
--- a/imp/lib/Session.php
+++ b/imp/lib/Session.php
@@ -70,14 +70,12 @@ class IMP_Session
         $_SESSION['imp'] = array(
             'cache' => array(),
             'imap' => array(),
+            'logintasks' => false,
             'server_key' => $server,
             'showunsub' => false
         );
         $sess = &$_SESSION['imp'];
 
-        /* Set the logintasks flag. */
-        IMP::loginTasksFlag(1);
-
         /* Run the username through virtualhost expansion functions if
          * necessary. */
         if (!empty($conf['hooks']['vinfo'])) {
@@ -192,18 +190,17 @@ class IMP_Session
 
     /**
      * Perform IMP login tasks.
+     *
+     * @param string $url  The URL to use for the Horde_LoginTasks redirect.
      */
-    static public function loginTasks()
+    static public function loginTasks($url = null)
     {
-        if (!IMP::loginTasksFlag()) {
+        if (!empty($_SESSION['imp']['logintasks'])) {
             return;
         }
 
-        IMP::loginTasksFlag(2);
-        IMP::checkAuthentication(true);
-
         /* Do login tasks. */
-        $tasks = &Horde_LoginTasks::singleton('imp', Horde_Util::addParameter(Horde::selfUrl(true, true, true), array('logintasks_done' => true)));
+        $tasks = &Horde_LoginTasks::singleton('imp', is_null($url) ? Horde::selfUrl(true, true, true) : $url);
         $tasks->runTasks();
 
         /* If the user wants to run filters on login, make sure they get
@@ -218,7 +215,7 @@ class IMP_Session
         $imp_compose = &IMP_Compose::singleton();
         $imp_compose->recoverSessionExpireDraft();
 
-        IMP::loginTasksFlag(0);
+        $_SESSION['imp']['logintasks'] = true;
     }
 
     /**

diff --git a/imp/lib/api.php b/imp/lib/api.php
index a352140..58f98e2 100644 (file)
--- a/imp/lib/api.php
+++ b/imp/lib/api.php
@@ -181,11 +181,11 @@ function _imp_authCredentials()
 /**
  * Tries to authenticate with the mail server and create a mail session.
  *
- * @param string $userID The username of the user.
- * @param array $credentials Credentials of the user. Only allowed key:
- * 'password'.
- * @param array $params Additional parameters. Only allowed key:
- * 'server'.
+ * @param string $userID      The username of the user.
+ * @param array $credentials  Credentials of the user. Only allowed key:
+ *                            'password'.
+ * @param array $params       Additional parameters. Only allowed key:
+ *                            'server'.
  *
  * @return boolean True on success, false on failure.
  */
@@ -197,8 +197,8 @@ function _imp_authenticate($userID, $credentials, $params = array())
     require_once IMP_BASE . '/lib/Session.php';
 
     $server_key = empty($params['server'])
-    ? IMP_Session::getAutoLoginServer()
-    : $params['server'];
+        ? IMP_Session::getAutoLoginServer()
+        : $params['server'];
 
      return IMP_Session::createSession($userID, $credentials['password'], $server_key);
 }

diff --git a/imp/lib/base.php b/imp/lib/base.php
index 1085379..6d51b6e 100644 (file)
--- a/imp/lib/base.php
+++ b/imp/lib/base.php
@@ -183,4 +183,11 @@ if ($viewmode == 'mimp') {
     $debug = Horde_Util::nonInputVar('mimp_debug');
     $GLOBALS['mimp_render'] = new Horde_Mobile(null, $debug);
     $GLOBALS['mimp_render']->set('debug', !empty($debug));
+} elseif (empty($_SESSION['imp']['logintasks']) &&
+          ($authentication !== 'none') &&
+          !defined('AUTH_HANDLER')) {
+    /* This captures all login tasks requests other than the IMP
+     * authentication + frameset case which needs to be handled in
+     * redirect.php. */
+    IMP_Session::loginTasks();
 }

diff --git a/imp/redirect.php b/imp/redirect.php
index 0ea30e3..769b09c 100644 (file)
--- a/imp/redirect.php
+++ b/imp/redirect.php
@@ -12,8 +12,11 @@
 function _framesetUrl($url)
 {
     if (!$GLOBALS['noframeset'] && Horde_Util::getFormData('load_frameset')) {
-        $full_url = Horde::applicationUrl($GLOBALS['registry']->get('webroot', 'horde') . '/index.php', true);
-        $url = Horde_Util::addParameter($full_url, 'url', _addAnchor($url, 'param'), false);
+        $url = Horde_Util::addParameter(Horde::applicationUrl($GLOBALS['registry']->get('webroot', 'horde') . '/index.php', true), array('url' => _addAnchor($url, 'param')), null, false);
+
+        /* Need to do a loginTasks check here because we must display login
+         * tasks before frameset is loaded. */
+        IMP_Session::loginTasks($url);
     }
     return $url;
 }
@@ -89,7 +92,7 @@ if (!empty($autologin)) {
     $imapuser = IMP_Session::canAutoLogin();
     $pass = Auth::getCredential('password');
 }
-$isLogin = IMP::loginTasksFlag();
+$isLogin = empty($_SESSION['imp']['logintasks']);
 $noframeset = false;
 
 /* Get URL/Anchor strings now. */
@@ -100,14 +103,6 @@ if (($pos = strrpos($url_in, '#')) !== false) {
     $url_in = substr($url_in, 0, $pos);
 }
 
-/* If we are returning from LoginTasks processing. */
-if (Horde_Util::getFormData('logintasks_done')) {
-    /* Finish up any login tasks we haven't completed yet. */
-    IMP_Session::loginTasks();
-
-    _redirect(_framesetUrl(_newSessionUrl($actionID, $isLogin)));
-}
-
 /* If we already have a session: */
 if (isset($_SESSION['imp']) && is_array($_SESSION['imp'])) {
     /* Make sure that if a username was specified, it is the current
@@ -119,9 +114,6 @@ if (isset($_SESSION['imp']) && is_array($_SESSION['imp'])) {
         _redirect(IMP::getLogoutUrl(AUTH_REASON_FAILED, true));
     }
 
-    /* Finish up any login tasks we haven't completed yet. */
-    IMP_Session::loginTasks();
-
     $url = $url_in;
     if (empty($url_in)) {
         $url = IMP_Session::getInitialUrl($actionID, false);
@@ -168,8 +160,6 @@ if (!is_null($imapuser) && !is_null($pass)) {
             Horde::callHook('_imp_hook_postlogin', array($actionID, $isLogin), 'imp');
         }
 
-        IMP_Session::loginTasks();
-
         _redirect(_framesetUrl(_newSessionUrl($actionID, $isLogin)));
     }