fix selection of mode on login

[immerda]

When the User selects a mode on the login form (ie. setting horde_select_view),
it gets always overwritten by 'auto'. The reason is that during login there is
a cycle that leads to authentication being triggered twice.

The culprit is the call to setAuth at
https://github.com/horde/Core/blob/master/lib/Horde/Core/Auth/Application.php#L669
which ends up triggering a second authentication through

$this->loadPrefs($this->getApp());

which in turn does not have access to the original $credentials array and then
sets the view to the default value.

This patch breaks the cycle, by preventing the second auth when the first one
did not finish.

[mrubinsk]

I can't reproduce this. When selecting a specific view from the login screen, it's always honored. I.e., I always see the mobile view, or desktop view as expected. I'm not following what you mean by the original $credentials array is not available in loadPrefs...

[immerda]

it is been a while since I debugged this, so it is hard to remember all the details. but I can confirm that with the newest released version of horde we still have this issue, when we disable the patch from this PR.

the problem is probably related to the authentication method. we are using

$conf['auth']['params']['app'] = 'imp';
$conf['auth']['driver'] = 'application';

the problem as I understand it is:
0.

Core/lib/Horde/Core/Auth/Application.php

Line 125 in 122f267

public function authenticate($userId, $credentials, $login = true)

authenticate is called

1. at

   Core/lib/Horde/Core/Auth/Application.php

   Line 146 in 122f267

   if (!empty($credentials['mode'])) {

   the view is remembered from the $credentials array
2. at

   Core/lib/Horde/Core/Auth/Application.php

   Line 669 in 122f267

   $registry->setAuth($userId, $credentials, array(

   setAuth is called
3. at

   Core/lib/Horde/Registry.php

   Line 2614 in 122f267

   $this->loadPrefs($this->getApp());

   loadPrefs is called

now here i am not fully sure anymore, but I believe it was as follows:

4. Core/lib/Horde/Registry.php

   Line 1790 in 122f267

   $this->pushApp($app);

   pushApp

5. Core/lib/Horde/Registry.php

   Line 1566 in 122f267

   if ($this->isAuthenticated(array('app' => $app))) {

   isAuthenticated

6. Core/lib/Horde/Registry.php

   Line 2195 in 122f267

   $res = $injector->getInstance('Horde_Core_Factory_Auth')->create($app)->transparent();

   a new instance of Horde_Core_Auth_Application is created. this instance has no access to the original $credentials array. transparent() is called

7. Core/lib/Horde/Core/Auth/Application.php

   Line 404 in 122f267

   return $result && $this->_setAuth();

   setAuth_ is called on that new instance

8. Core/lib/Horde/Core/Auth/Application.php

   Line 677 in 122f267

   $this->_setView();

   view is set to 'auto' by the new instance

at this point $GLOBALS['session']->exists('horde', 'view') is set to 'auto' and the first instance of the authentication object does not set it the the user supplied value, that was passed in the $credentials

[immerda]

@mrubinsk is there anything we could provide on our end here? I can confirm that the issue is related to the application auth driver.

[mrubinsk]

Not really. I need to digest your code path. I'm using the exact same authentication driver(s) and am unable to reproduce.

[yunosh]

Looking at _setView(), the $mode is only overwritten if either force_view is true, or select_view is false. Otherwise $this->_view is used which is set earlier in authenticate().

[immerda]

I'll try to understand your comment tomorrow. But I just want to note again that we have since this ticket was opened the patch in this PR applied to our deployment and the mode selection really does not work without it!

[immerda]

@yunosh

Looking at _setView(), the $mode is only overwritten if either force_view is true, or select_view is false. Otherwise $this->_view is used which is set earlier in authenticate().

The problem is that authenticate stores the view to _view (2) on the initial Horde_Core_Auth_Application.

But then the call to setAuth internally creates a second Horde_Core_Auth_Application instance (6), on which authenticate is not called, and thus has _view unset.

This secondary instance beats the first one to call setView(). This in turn means that !$GLOBALS['session']->exists('horde', 'view') is false and the actual view is never set.

[immerda]

And just to tripple confirm again, that this is really what is going on, I logged in with the following patch applied:

-        if (!$GLOBALS['session']->exists('horde', 'view')) {
+            $GLOBALS['notification']->push($this->_view);
             $this->_setView();
-        }

i.e. I disable the check if the view is already set and instead always set it. And I log every time it is set.

Then I see first 8 'auto' notifications (for each horde app being initialized as described in my initial diagnosis of the issue). And then the final notification is 'dynamic', which correctly sets the view and I am logged in with the correct view.

I really don't know what else I can do to convince you the bug is real...

[immerda]

I have a theory why you can't reproduce it: In our case the normal horde login form is bypassed by a custom SSO mechanism. Thus, normally the horde session is created when the user loads the login form. In our case the horde session is created in the same request as the post to /login.php. And that is probably why apps are initialized during authentication, which triggers this issue.

[yunosh]

That's an important piece of information. Though, ff you use a SSO authentication, how are users selecting the view from the login screen?

[immerda]

Indeed, I did not realize that it plays a role here.

They select the view on a custom form and the SSO then logs them into horde, by relying them to horde's login.php, doing the exact same post request that the horde login form would do.